Commit 594f9389 authored by Robert Speicher's avatar Robert Speicher Committed by Ruben Davila

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
parent 54823279
...@@ -62,6 +62,7 @@ module Ci ...@@ -62,6 +62,7 @@ module Ci
status_event: 'enqueue' status_event: 'enqueue'
) )
MergeRequests::AddTodoWhenBuildFailsService.new(build.project, nil).close(new_build) MergeRequests::AddTodoWhenBuildFailsService.new(build.project, nil).close(new_build)
build.pipeline.mark_as_processable_after_stage(build.stage_idx)
new_build new_build
end end
end end
......
...@@ -140,6 +140,10 @@ module Ci ...@@ -140,6 +140,10 @@ module Ci
end end
end end
def mark_as_processable_after_stage(stage_idx)
builds.skipped.where('stage_idx > ?', stage_idx).find_each(&:process)
end
def latest? def latest?
return false unless ref return false unless ref
commit = project.commit(ref) commit = project.commit(ref)
......
...@@ -30,6 +30,10 @@ class CommitStatus < ActiveRecord::Base ...@@ -30,6 +30,10 @@ class CommitStatus < ActiveRecord::Base
transition [:created, :skipped] => :pending transition [:created, :skipped] => :pending
end end
event :process do
transition skipped: :created
end
event :run do event :run do
transition pending: :running transition pending: :running
end end
......
...@@ -3,8 +3,6 @@ require 'spec_helper' ...@@ -3,8 +3,6 @@ require 'spec_helper'
describe Ci::ProcessPipelineService, services: true do describe Ci::ProcessPipelineService, services: true do
let(:pipeline) { create(:ci_pipeline, ref: 'master') } let(:pipeline) { create(:ci_pipeline, ref: 'master') }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:all_builds) { pipeline.builds }
let(:builds) { all_builds.where.not(status: [:created, :skipped]) }
let(:config) { nil } let(:config) { nil }
before do before do
...@@ -12,6 +10,14 @@ describe Ci::ProcessPipelineService, services: true do ...@@ -12,6 +10,14 @@ describe Ci::ProcessPipelineService, services: true do
end end
describe '#execute' do describe '#execute' do
def all_builds
pipeline.builds
end
def builds
all_builds.where.not(status: [:created, :skipped])
end
def create_builds def create_builds
described_class.new(pipeline.project, user).execute(pipeline) described_class.new(pipeline.project, user).execute(pipeline)
end end
...@@ -48,7 +54,7 @@ describe Ci::ProcessPipelineService, services: true do ...@@ -48,7 +54,7 @@ describe Ci::ProcessPipelineService, services: true do
it 'does not process pipeline if existing stage is running' do it 'does not process pipeline if existing stage is running' do
expect(create_builds).to be_truthy expect(create_builds).to be_truthy
expect(builds.pending.count).to eq(2) expect(builds.pending.count).to eq(2)
expect(create_builds).to be_falsey expect(create_builds).to be_falsey
expect(builds.pending.count).to eq(2) expect(builds.pending.count).to eq(2)
end end
...@@ -224,6 +230,40 @@ describe Ci::ProcessPipelineService, services: true do ...@@ -224,6 +230,40 @@ describe Ci::ProcessPipelineService, services: true do
end end
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 context 'creates a builds from .gitlab-ci.yml' do
let(:config) do let(:config) do
YAML.dump({ YAML.dump({
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment