Merge branch 'ali/prevent-retries-of-non-retryable-jobs' into 'master'

Prevent non-retryable jobs from being retried

See merge request gitlab-org/gitlab!78135

app/services/ci/play_build_service.rb

@@ -14,7 +14,10 @@ module Ci
           AfterRequeueJobService.new(project, current_user).execute(build)
         end
       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
 

spec/factories/ci/builds.rb

@@ -597,6 +597,11 @@ FactoryBot.define do
       failure_reason { 13 }
     end
 
+    trait :deployment_rejected do
+      failed
+      failure_reason { 22 }
+    end
+
     trait :with_runner_session do
       after(:build) do |build|
         build.build_runner_session(url: 'https://localhost')

spec/models/ci/build_spec.rb

@@ -2002,6 +2002,16 @@ RSpec.describe Ci::Build do
 
           it { is_expected.not_to be_retryable }
         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
 

spec/services/ci/play_build_service_spec.rb

@@ -122,7 +122,7 @@ RSpec.describe Ci::PlayBuildService, '#execute' do
   end
 
   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) }
 
     it 'duplicates the build' do
@@ -138,6 +138,18 @@ RSpec.describe Ci::PlayBuildService, '#execute' do
       expect(build.user).not_to eq user
       expect(duplicate.user).to eq user
     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
 
   context 'when build is not action' do