Merge branch 'ci-implement-exit-code-functionality' into 'master'

Implement exit code functionality for allow_failure

See merge request gitlab-org/gitlab!49462

app/models/ci/build.rb

@@ -1031,6 +1031,13 @@ module Ci
       variables.any? { |variable| variable[:key] == 'CI_DEBUG_TRACE' && variable[:value].casecmp('true') == 0 }
     end
 
+    def drop_with_exit_code!(failure_reason, exit_code)
+      transaction do
+        conditionally_allow_failure!(exit_code)
+        drop!(failure_reason)
+      end
+    end
+
     protected
 
     def run_status_commit_hooks!
@@ -1114,6 +1121,22 @@ module Ci
         Gitlab::ErrorTracking.track_exception(e)
       end
     end
+
+    def conditionally_allow_failure!(exit_code)
+      return unless ::Gitlab::Ci::Features.allow_failure_with_exit_codes_enabled?
+      return unless exit_code
+
+      if allowed_to_fail_with_code?(exit_code)
+        update_columns(allow_failure: true)
+      end
+    end
+
+    def allowed_to_fail_with_code?(exit_code)
+      options
+        .dig(:allow_failure_criteria, :exit_codes)
+        .to_a
+        .include?(exit_code)
+    end
   end
 end
 

app/services/ci/update_build_state_service.rb

@@ -111,7 +111,7 @@ module Ci
 
         Result.new(status: 200)
       when 'failed'
-        build.drop!(params[:failure_reason] || :unknown_failure)
+        build.drop_with_exit_code!(params[:failure_reason] || :unknown_failure, params[:exit_code])
 
         Result.new(status: 200)
       else

lib/api/ci/runner.rb

@@ -180,6 +180,7 @@ module API
             optional :checksum, type: String, desc: %q(Job's trace CRC32 checksum)
             optional :bytesize, type: Integer, desc: %q(Job's trace size in bytes)
           end
+          optional :exit_code, type: Integer, desc: %q(Job's exit code)
         end
         put '/:id' do
           job = authenticate_job!

spec/models/ci/build_spec.rb

@@ -4824,4 +4824,107 @@ RSpec.describe Ci::Build do
       it { is_expected.to eq false }
     end
   end
+
+  describe '#drop_with_exit_code!' do
+    let(:exit_code) { 1 }
+    let(:options) { {} }
+
+    before do
+      build.options.merge!(options)
+      build.save!
+    end
+
+    subject(:drop_with_exit_code) do
+      build.drop_with_exit_code!(:unknown_failure, exit_code)
+    end
+
+    shared_examples 'drops the build without changing allow_failure' do
+      it 'does not change allow_failure' do
+        expect { drop_with_exit_code }
+          .not_to change { build.reload.allow_failure }
+      end
+
+      it 'drops the build' do
+        expect { drop_with_exit_code }
+          .to change { build.reload.failed? }
+      end
+    end
+
+    context 'when exit_codes are not defined' do
+      it_behaves_like 'drops the build without changing allow_failure'
+    end
+
+    context 'when allow_failure_criteria is nil' do
+      let(:options) { { allow_failure_criteria: nil } }
+
+      it_behaves_like 'drops the build without changing allow_failure'
+    end
+
+    context 'when exit_codes is nil' do
+      let(:options) do
+        {
+          allow_failure_criteria: {
+            exit_codes: nil
+          }
+        }
+      end
+
+      it_behaves_like 'drops the build without changing allow_failure'
+    end
+
+    context 'when exit_codes do not match' do
+      let(:options) do
+        {
+          allow_failure_criteria: {
+            exit_codes: [2, 3, 4]
+          }
+        }
+      end
+
+      it_behaves_like 'drops the build without changing allow_failure'
+    end
+
+    context 'with matching exit codes' do
+      let(:options) do
+        { allow_failure_criteria: { exit_codes: [1, 2, 3] } }
+      end
+
+      it 'changes allow_failure' do
+        expect { drop_with_exit_code }
+          .to change { build.reload.allow_failure }
+      end
+
+      it 'drops the build' do
+        expect { drop_with_exit_code }
+          .to change { build.reload.failed? }
+      end
+
+      it 'is executed inside a transaction' do
+        expect(build).to receive(:drop!)
+          .with(:unknown_failure)
+          .and_raise(ActiveRecord::Rollback)
+
+        expect(build).to receive(:conditionally_allow_failure!)
+          .with(1)
+          .and_call_original
+
+        expect { drop_with_exit_code }
+          .not_to change { build.reload.allow_failure }
+      end
+
+      context 'when exit_code is nil' do
+        let(:exit_code) {}
+
+        it_behaves_like 'drops the build without changing allow_failure'
+      end
+
+      context 'when ci_allow_failure_with_exit_codes is disabled' do
+        before do
+          stub_feature_flags(ci_allow_failure_with_exit_codes: false)
+        end
+
+        it_behaves_like 'drops the build without changing allow_failure'
+      end
+    end
+  end
 end

spec/requests/api/ci/runner/jobs_put_spec.rb

@@ -78,6 +78,33 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
           end
         end
 
+        context 'when an exit_code is provided' do
+          context 'when the exit_codes are acceptable' do
+            before do
+              job.options[:allow_failure_criteria] = { exit_codes: [1] }
+              job.save!
+            end
+
+            it 'accepts an exit code' do
+              update_job(state: 'failed', exit_code: 1)
+
+              expect(job.reload).to be_failed
+              expect(job.allow_failure).to be_truthy
+              expect(job).to be_unknown_failure
+            end
+          end
+
+          context 'when the exit_codes are not defined' do
+            it 'ignore the exit code' do
+              update_job(state: 'failed', exit_code: 1)
+
+              expect(job.reload).to be_failed
+              expect(job.allow_failure).to be_falsy
+              expect(job).to be_unknown_failure
+            end
+          end
+        end
+
         context 'when failure_reason is script_failure' do
           before do
             update_job(state: 'failed', failure_reason: 'script_failure')

spec/services/ci/update_build_state_service_spec.rb

@@ -82,8 +82,9 @@ RSpec.describe Ci::UpdateBuildStateService do
     let(:params) do
       {
         output: { checksum: 'crc32:12345678', bytesize: 123 },
+        state: 'failed',
         failure_reason: 'script_failure',
-        state: 'failed'
+        exit_code: 42
       }
     end
 
@@ -95,6 +96,15 @@ RSpec.describe Ci::UpdateBuildStateService do
         expect(result.status).to eq 200
       end
 
+      it 'updates the allow_failure flag' do
+        expect(build)
+          .to receive(:drop_with_exit_code!)
+          .with('script_failure', 42)
+          .and_call_original
+
+        subject.execute
+      end
+
       it 'does not increment invalid trace metric' do
         execute_with_stubbed_metrics!
 
@@ -115,6 +125,15 @@ RSpec.describe Ci::UpdateBuildStateService do
         expect(build).to be_failed
       end
 
+      it 'updates the allow_failure flag' do
+        expect(build)
+          .to receive(:drop_with_exit_code!)
+          .with('script_failure', 42)
+          .and_call_original
+
+        subject.execute
+      end
+
       it 'responds with 200 OK status' do
         result = subject.execute