From 056f1b702042382c99dcfdaf1e292bf68b8f55ba Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 14 Feb 2017 13:29:58 +0100 Subject: Simplify implementation of pipeline retry service --- app/models/ci/pipeline.rb | 8 +---- app/services/ci/retry_pipeline_service.rb | 42 +++--------------------- spec/models/ci/pipeline_spec.rb | 8 ++--- spec/services/ci/retry_pipeline_service_spec.rb | 43 ++++++++++++++++++------- 4 files changed, 41 insertions(+), 60 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 49bd2e1d7f0..dc4590a9923 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -214,13 +214,7 @@ module Ci def cancel_running Gitlab::OptimisticLocking.retry_lock( statuses.cancelable) do |cancelable| - cancelable.each do |status| - if status.created? - status.skip - elsif status.active? - status.cancel - end - end + cancelable.find_each(&:cancel) end end diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb index 1b931455fb8..8a1a9191478 100644 --- a/app/services/ci/retry_pipeline_service.rb +++ b/app/services/ci/retry_pipeline_service.rb @@ -1,57 +1,25 @@ module Ci class RetryPipelineService < ::BaseService def execute(pipeline) - @pipeline = pipeline - unless can?(current_user, :update_pipeline, pipeline) raise Gitlab::Access::AccessDeniedError end - pipeline.mark_as_processable_after_stage(resume_stage.index) + each_build(pipeline.builds.failed_or_canceled) do |build| + next unless build.retryable? - retryable_builds_in_subsequent_stages do |build| Ci::RetryBuildService.new(project, current_user) .reprocess(build) end - retryable_builds_in_first_unsuccessful_stage do |build| - Ci::RetryBuildService.new(project, current_user) - .retry(build) - end + pipeline.process! end private - def retryable_builds_in_subsequent_stages - relation = @pipeline.builds - .after_stage(resume_stage.index) - .failed_or_canceled - - each_retryable_build_with_locking(relation) do |build| - yield build - end - end - - def retryable_builds_in_first_unsuccessful_stage - relation = resume_stage.builds.failed_or_canceled - - each_retryable_build_with_locking(relation) do |build| - yield build - end - end - - def each_retryable_build_with_locking(relation) + def each_build(relation) Gitlab::OptimisticLocking.retry_lock(relation) do |builds| - builds.find_each do |build| - next unless build.retryable? - yield build - end - end - end - - def resume_stage - @resume_stage ||= @pipeline.stages.find do |stage| - stage.failed? || stage.canceled? + builds.find_each { |build| yield build } end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 199e09e2bb8..45c710b2381 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -766,8 +766,8 @@ describe Ci::Pipeline, models: true do pipeline.cancel_running end - it 'skips created builds' do - expect(latest_status).to eq ['canceled', 'skipped'] + it 'cancels created builds' do + expect(latest_status).to eq ['canceled', 'canceled'] end end end @@ -801,7 +801,7 @@ describe Ci::Pipeline, models: true do end it 'retries both builds' do - expect(latest_status).to contain_exactly('pending', 'pending') + expect(latest_status).to contain_exactly('pending', 'created') end end @@ -814,7 +814,7 @@ describe Ci::Pipeline, models: true do end it 'retries both builds' do - expect(latest_status).to contain_exactly('pending', 'pending') + expect(latest_status).to contain_exactly('pending', 'created') end end end diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index d7afca538aa..cee9792243c 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -11,9 +11,9 @@ describe Ci::RetryPipelineService, '#execute', :services do context 'when there are failed builds in the last stage' do before do - create_build(name: 'rspec 1', status: :success, stage_num: 0) - create_build(name: 'rspec 2', status: :failed, stage_num: 1) - create_build(name: 'rspec 3', status: :canceled, stage_num: 1) + create_build('rspec 1', :success, 0) + create_build('rspec 2', :failed, 1) + create_build('rspec 3', :canceled, 1) end it 'enqueues all builds in the last stage' do @@ -27,10 +27,10 @@ describe Ci::RetryPipelineService, '#execute', :services do context 'when there are failed or canceled builds in the first stage' do before do - create_build(name: 'rspec 1', status: :failed, stage_num: 0) - create_build(name: 'rspec 2', status: :canceled, stage_num: 0) - create_build(name: 'rspec 3', status: :skipped, stage_num: 1) - create_build(name: 'deploy 1', status: :skipped, stage_num: 2) + create_build('rspec 1', :failed, 0) + create_build('rspec 2', :canceled, 0) + create_build('rspec 3', :canceled, 1) + create_build('deploy 1', :canceled, 2) end it 'retries builds failed builds and marks subsequent for processing' do @@ -46,10 +46,10 @@ describe Ci::RetryPipelineService, '#execute', :services do context 'when there is failed build present which was run on failure' do before do - create_build(name: 'rspec 1', status: :failed, stage_num: 0) - create_build(name: 'rspec 2', status: :canceled, stage_num: 0) - create_build(name: 'rspec 3', status: :skipped, stage_num: 1) - create_build(name: 'report 1', status: :failed, stage_num: 2) + create_build('rspec 1', :failed, 0) + create_build('rspec 2', :canceled, 0) + create_build('rspec 3', :canceled, 1) + create_build('report 1', :failed, 2) end it 'retries builds failed builds and marks subsequent for processing' do @@ -65,9 +65,27 @@ describe Ci::RetryPipelineService, '#execute', :services do it 'creates a new job for report job in this case' do service.execute(pipeline) + # TODO, expect to be_retried expect(statuses.where(name: 'report 1').count).to eq 2 end end + + context 'when there is canceled manual build in first stage' do + before do + create_build('rspec 1', :failed, 0) + create_build('staging', :canceled, 0, :manual) + create_build('rspec 2', :canceled, 1) + end + + it 'retries builds failed builds and marks subsequent for processing' do + service.execute(pipeline) + + expect(build('rspec 1')).to be_pending + expect(build('staging')).to be_skipped + expect(build('rspec 2')).to be_created + expect(pipeline.reload).to be_running + end + end end context 'when user is not allowed to retry pipeline' do @@ -85,11 +103,12 @@ describe Ci::RetryPipelineService, '#execute', :services do statuses.latest.find_by(name: name) end - def create_build(name:, status:, stage_num:) + def create_build(name, status, stage_num, on = 'on_success') create(:ci_build, name: name, status: status, stage: "stage_#{stage_num}", stage_idx: stage_num, + when: on, pipeline: pipeline) do |build| pipeline.update_status end -- cgit v1.2.1