Merge branch 'mark-as-processable' into 'master'

Make all future skipped builds as processable when retrying a build

## What does this MR do?

Makes a builds that are marked as skipped when a pipeline is processed to be reprocessed by changing their's state to created.

## Why was this MR needed?

Currently retry is broken. When you retry a build of pipeline it will succeed and be marked as succeeded, when the next stages should be triggered.

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/21066

See merge request !5879

4 files changed, 52 insertions, 3 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index ed056a07a49..096b3b801af 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -62,6 +62,7 @@ module Ci
           status_event: 'enqueue'
         )
         MergeRequests::AddTodoWhenBuildFailsService.new(build.project, nil).close(new_build)
+        build.pipeline.mark_as_processable_after_stage(build.stage_idx)
         new_build
       end
     end
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 0f0bfd4ee31..087abe4cbb1 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -150,6 +150,10 @@ module Ci
       end
     end
 
+    def mark_as_processable_after_stage(stage_idx)
+      builds.skipped.where('stage_idx > ?', stage_idx).find_each(&:process)
+    end
+
     def latest?
       return false unless ref
       commit = project.commit(ref)
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 7542399169f..84ceeac7d3e 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -31,6 +31,10 @@ class CommitStatus < ActiveRecord::Base
       transition [:created, :skipped] => :pending
     end
 
+    event :process do
+      transition skipped: :created
+    end
+
     event :run do
       transition pending: :running
     end
diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb
index ad8c2485888..8326e5cd313 100644
--- a/spec/services/ci/process_pipeline_service_spec.rb
+++ b/spec/services/ci/process_pipeline_service_spec.rb
@@ -3,8 +3,6 @@ require 'spec_helper'
 describe Ci::ProcessPipelineService, services: true do
   let(:pipeline) { create(:ci_pipeline, ref: 'master') }
   let(:user) { create(:user) }
-  let(:all_builds) { pipeline.builds }
-  let(:builds) { all_builds.where.not(status: [:created, :skipped]) }
   let(:config) { nil }
 
   before do
@@ -12,6 +10,14 @@ describe Ci::ProcessPipelineService, services: true do
   end
 
   describe '#execute' do
+    def all_builds
+      pipeline.builds
+    end
+
+    def builds
+      all_builds.where.not(status: [:created, :skipped])
+    end
+
     def create_builds
       described_class.new(pipeline.project, user).execute(pipeline)
     end
@@ -48,7 +54,7 @@ describe Ci::ProcessPipelineService, services: true do
       it 'does not process pipeline if existing stage is running' do
         expect(create_builds).to be_truthy
         expect(builds.pending.count).to eq(2)
-        
+
         expect(create_builds).to be_falsey
         expect(builds.pending.count).to eq(2)
       end
@@ -224,6 +230,40 @@ describe Ci::ProcessPipelineService, services: true do
       end
     end
 
+    context 'when failed build in the middle stage is retried' do
+      context 'when failed build is the only unsuccessful build in the stage' do
+        before do
+          create(:ci_build, :created, pipeline: pipeline, name: 'build:1', stage_idx: 0)
+          create(:ci_build, :created, pipeline: pipeline, name: 'build:2', stage_idx: 0)
+          create(:ci_build, :created, pipeline: pipeline, name: 'test:1', stage_idx: 1)
+          create(:ci_build, :created, pipeline: pipeline, name: 'test:2', stage_idx: 1)
+          create(:ci_build, :created, pipeline: pipeline, name: 'deploy:1', stage_idx: 2)
+          create(:ci_build, :created, pipeline: pipeline, name: 'deploy:2', stage_idx: 2)
+        end
+
+        it 'does trigger builds in the next stage' do
+          expect(create_builds).to be_truthy
+          expect(builds.pluck(:name)).to contain_exactly('build:1', 'build:2')
+
+          pipeline.builds.running_or_pending.each(&:success)
+
+          expect(builds.pluck(:name))
+            .to contain_exactly('build:1', 'build:2', 'test:1', 'test:2')
+
+          pipeline.builds.find_by(name: 'test:1').success
+          pipeline.builds.find_by(name: 'test:2').drop
+
+          expect(builds.pluck(:name))
+            .to contain_exactly('build:1', 'build:2', 'test:1', 'test:2')
+
+          Ci::Build.retry(pipeline.builds.find_by(name: 'test:2')).success
+
+          expect(builds.pluck(:name)).to contain_exactly(
+            'build:1', 'build:2', 'test:1', 'test:2', 'test:2', 'deploy:1', 'deploy:2')
+        end
+      end
+    end
+
     context 'creates a builds from .gitlab-ci.yml' do
       let(:config) do
         YAML.dump({