Simplify implementation of pipeline retry service

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