Commit 87275bd2 authored by Alishan Ladhani's avatar Alishan Ladhani Committed by Shinya Maeda

Prevent non-retryable jobs from being retried

parent d679861f
...@@ -14,7 +14,10 @@ module Ci ...@@ -14,7 +14,10 @@ module Ci
AfterRequeueJobService.new(project, current_user).execute(build) AfterRequeueJobService.new(project, current_user).execute(build)
end end
else else
Ci::Build.retry(build, current_user) # Retrying in Ci::PlayBuildService is a legacy process that should be removed.
# Instead, callers should explicitly execute Ci::RetryBuildService.
# See https://gitlab.com/gitlab-org/gitlab/-/issues/347493.
build.retryable? ? Ci::Build.retry(build, current_user) : build
end end
end end
......
...@@ -597,6 +597,11 @@ FactoryBot.define do ...@@ -597,6 +597,11 @@ FactoryBot.define do
failure_reason { 13 } failure_reason { 13 }
end end
trait :deployment_rejected do
failed
failure_reason { 22 }
end
trait :with_runner_session do trait :with_runner_session do
after(:build) do |build| after(:build) do |build|
build.build_runner_session(url: 'https://localhost') build.build_runner_session(url: 'https://localhost')
......
...@@ -2002,6 +2002,16 @@ RSpec.describe Ci::Build do ...@@ -2002,6 +2002,16 @@ RSpec.describe Ci::Build do
it { is_expected.not_to be_retryable } it { is_expected.not_to be_retryable }
end end
context 'when build is waiting for deployment approval' do
subject { build_stubbed(:ci_build, :manual, environment: 'production') }
before do
create(:deployment, :blocked, deployable: subject)
end
it { is_expected.not_to be_retryable }
end
end end
end end
......
...@@ -122,7 +122,7 @@ RSpec.describe Ci::PlayBuildService, '#execute' do ...@@ -122,7 +122,7 @@ RSpec.describe Ci::PlayBuildService, '#execute' do
end end
context 'when build is not a playable manual action' do context 'when build is not a playable manual action' do
let(:build) { create(:ci_build, when: :manual, pipeline: pipeline) } let(:build) { create(:ci_build, :success, pipeline: pipeline) }
let!(:branch) { create(:protected_branch, :developers_can_merge, name: build.ref, project: project) } let!(:branch) { create(:protected_branch, :developers_can_merge, name: build.ref, project: project) }
it 'duplicates the build' do it 'duplicates the build' do
...@@ -138,6 +138,18 @@ RSpec.describe Ci::PlayBuildService, '#execute' do ...@@ -138,6 +138,18 @@ RSpec.describe Ci::PlayBuildService, '#execute' do
expect(build.user).not_to eq user expect(build.user).not_to eq user
expect(duplicate.user).to eq user expect(duplicate.user).to eq user
end end
context 'and is not retryable' do
let(:build) { create(:ci_build, :deployment_rejected, pipeline: pipeline) }
it 'does not duplicate the build' do
expect { service.execute(build) }.not_to change { Ci::Build.count }
end
it 'does not enqueue the build' do
expect { service.execute(build) }.not_to change { build.status }
end
end
end end
context 'when build is not action' do context 'when build is not action' do
......
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