Merge branch 'fix/gb/pipeline-retry-skipped-jobs' into 'master'

Fix reprocessing skipped jobs when retrying pipeline

See merge request !9436

4 files changed, 48 insertions, 6 deletions

diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb
index 2c5e130e5aa..574561adc4c 100644
--- a/app/services/ci/retry_pipeline_service.rb
+++ b/app/services/ci/retry_pipeline_service.rb
@@ -1,5 +1,7 @@
 module Ci
   class RetryPipelineService < ::BaseService
+    include Gitlab::OptimisticLocking
+
     def execute(pipeline)
       unless can?(current_user, :update_pipeline, pipeline)
         raise Gitlab::Access::AccessDeniedError
@@ -12,6 +14,10 @@ module Ci
           .reprocess(build)
       end
 
+      pipeline.builds.skipped.find_each do |skipped|
+        retry_optimistic_lock(skipped) { |build| build.process }
+      end
+
       MergeRequests::AddTodoWhenBuildFailsService
         .new(project, current_user)
         .close_all(pipeline)
diff --git a/lib/gitlab/optimistic_locking.rb b/lib/gitlab/optimistic_locking.rb
index 879d46446b3..8c47c807bf9 100644
--- a/lib/gitlab/optimistic_locking.rb
+++ b/lib/gitlab/optimistic_locking.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module OptimisticLocking
-    extend self
+    module_function
 
     def retry_lock(subject, retries = 100, &block)
       loop do
@@ -15,5 +15,7 @@ module Gitlab
         end
       end
     end
+
+    alias_method :retry_optimistic_lock, :retry_lock
   end
 end
diff --git a/spec/lib/gitlab/optimistic_locking_spec.rb b/spec/lib/gitlab/optimistic_locking_spec.rb
index 498dc514c8c..acce2be93f2 100644
--- a/spec/lib/gitlab/optimistic_locking_spec.rb
+++ b/spec/lib/gitlab/optimistic_locking_spec.rb
@@ -1,10 +1,10 @@
 require 'spec_helper'
 
 describe Gitlab::OptimisticLocking, lib: true do
-  describe '#retry_lock' do
-    let!(:pipeline) { create(:ci_pipeline) }
-    let!(:pipeline2) { Ci::Pipeline.find(pipeline.id) }
+  let!(:pipeline) { create(:ci_pipeline) }
+  let!(:pipeline2) { Ci::Pipeline.find(pipeline.id) }
 
+  describe '#retry_lock' do
     it 'does not reload object if state changes' do
       expect(pipeline).not_to receive(:reload)
       expect(pipeline).to receive(:succeed).and_call_original
@@ -36,4 +36,17 @@ describe Gitlab::OptimisticLocking, lib: true do
       end.to raise_error(ActiveRecord::StaleObjectError)
     end
   end
+
+  describe '#retry_optimistic_lock' do
+    context 'when locking module is mixed in' do
+      let(:unlockable) do
+        Class.new.include(described_class).new
+      end
+
+      it 'is an alias for retry_lock' do
+        expect(unlockable.method(:retry_optimistic_lock))
+          .to eq unlockable.method(:retry_lock)
+      end
+    end
+  end
 end
diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb
index c0af8b8450a..8b1ed6470e4 100644
--- a/spec/services/ci/retry_pipeline_service_spec.rb
+++ b/spec/services/ci/retry_pipeline_service_spec.rb
@@ -69,6 +69,25 @@ describe Ci::RetryPipelineService, '#execute', :services do
       end
     end
 
+    context 'when the last stage was skipepd' do
+      before do
+        create_build('build 1', :success, 0)
+        create_build('test 2', :failed, 1)
+        create_build('report 3', :skipped, 2)
+        create_build('report 4', :skipped, 2)
+      end
+
+      it 'retries builds only in the first stage' do
+        service.execute(pipeline)
+
+        expect(build('build 1')).to be_success
+        expect(build('test 2')).to be_pending
+        expect(build('report 3')).to be_created
+        expect(build('report 4')).to be_created
+        expect(pipeline.reload).to be_running
+      end
+    end
+
     context 'when pipeline contains manual actions' do
       context 'when there is a canceled manual action in first stage' do
         before do
@@ -90,14 +109,16 @@ describe Ci::RetryPipelineService, '#execute', :services do
       context 'when there is a skipped manual action in last stage' do
         before do
           create_build('rspec 1', :canceled, 0)
+          create_build('rspec 2', :skipped, 0, :manual)
           create_build('staging', :skipped, 1, :manual)
         end
 
-        it 'retries canceled job and skips manual action' do
+        it 'retries canceled job and reprocesses manual actions' do
           service.execute(pipeline)
 
           expect(build('rspec 1')).to be_pending
-          expect(build('staging')).to be_skipped
+          expect(build('rspec 2')).to be_skipped
+          expect(build('staging')).to be_created
           expect(pipeline.reload).to be_running
         end
       end