diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 9301f33e7c00812f04a386c65abe9731f480f5f3..515e7966cc3c4af35ec5653354928fc9a42c0403 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -410,6 +410,10 @@ def auto_retry_allowed?
auto_retry.allowed?
end
+ def exit_code=(value)
+ ensure_metadata.exit_code = value
+ end
+
def auto_retry_expected?
failed? && auto_retry_allowed?
end
@@ -1030,7 +1034,7 @@ def drop_with_exit_code!(failure_reason, exit_code)
end
def exit_codes_defined?
- options.dig(:allow_failure_criteria, :exit_codes).present?
+ options.dig(:allow_failure_criteria, :exit_codes).present? || options.dig(:retry, :exit_codes).present?
end
def create_queuing_entry!
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 3a9b1465682fbd602261c891dcbcb85836351ef6..7ec3210f703ebad06366e8875ed85cbc9a648699 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -194,6 +194,7 @@ class CommitStatus < Ci::ApplicationRecord
commit_status.failure_reason = reason.failure_reason_enum
commit_status.allow_failure = true if reason.force_allow_failure?
+ commit_status.exit_code = reason.exit_code if Feature.enabled?(:ci_retry_on_exit_codes, Feature.current_request)
end
before_transition [:skipped, :manual] => :created do |commit_status, transition|
@@ -338,6 +339,9 @@ def stage_name
ci_stage&.name
end
+ # Handled only by ci_build
+ def exit_code=(value); end
+
# For AiAction
def to_ability_name
'build'
diff --git a/app/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb
index 3f48df080169dcbed72824a2e96648a6395cbf38..6aef24c45d052776b9911f2ede2fa0614879935d 100644
--- a/app/models/concerns/ci/metadatable.rb
+++ b/app/models/concerns/ci/metadatable.rb
@@ -24,6 +24,7 @@ module Metadatable
delegate :environment_auto_stop_in, to: :metadata, prefix: false, allow_nil: true
delegate :set_cancel_gracefully, to: :metadata, prefix: false, allow_nil: false
delegate :id_tokens, to: :metadata, allow_nil: true
+ delegate :exit_code, to: :metadata, allow_nil: true
before_validation :ensure_metadata, on: :create
diff --git a/db/migrate/20240130201017_add_exit_code_to_builds_metadata.rb b/db/migrate/20240130201017_add_exit_code_to_builds_metadata.rb
new file mode 100644
index 0000000000000000000000000000000000000000..d6cb62a1863d41a0bd97b847215caa996ef21923
--- /dev/null
+++ b/db/migrate/20240130201017_add_exit_code_to_builds_metadata.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class AddExitCodeToBuildsMetadata < Gitlab::Database::Migration[2.2]
+ enable_lock_retries!
+
+ milestone '16.10'
+
+ def change
+ add_column :p_ci_builds_metadata, :exit_code, :smallint, null: true
+ end
+end
diff --git a/db/schema_migrations/20240130201017 b/db/schema_migrations/20240130201017
new file mode 100644
index 0000000000000000000000000000000000000000..b97d0b3d17f4aa7e070f8cbd59ac054fe9c0a24c
--- /dev/null
+++ b/db/schema_migrations/20240130201017
@@ -0,0 +1 @@
+580d6e209c374c25af9b2cde9792ed13c6f9a3538845236ea7b7387c4ce44469
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 8fa6662d49c0d53516b334e6e8f3e2f5c7a40111..ba0f8565f21a3b74ca2004e3cd6f9503d715b606 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -885,7 +885,8 @@ CREATE TABLE p_ci_builds_metadata (
runtime_runner_features jsonb DEFAULT '{}'::jsonb NOT NULL,
id_tokens jsonb DEFAULT '{}'::jsonb NOT NULL,
partition_id bigint NOT NULL,
- debug_trace_enabled boolean DEFAULT false NOT NULL
+ debug_trace_enabled boolean DEFAULT false NOT NULL,
+ exit_code smallint
)
PARTITION BY LIST (partition_id);
@@ -5780,7 +5781,8 @@ CREATE TABLE ci_builds_metadata (
runtime_runner_features jsonb DEFAULT '{}'::jsonb NOT NULL,
id_tokens jsonb DEFAULT '{}'::jsonb NOT NULL,
partition_id bigint NOT NULL,
- debug_trace_enabled boolean DEFAULT false NOT NULL
+ debug_trace_enabled boolean DEFAULT false NOT NULL,
+ exit_code smallint
);
CREATE TABLE ci_builds_runner_session (
diff --git a/lib/gitlab/ci/build/auto_retry.rb b/lib/gitlab/ci/build/auto_retry.rb
index 453c293f6cd45c679f1db2fa0081e26ae028c6ca..95469b05f5bae93b5b1755ad3bd54e9978000022 100644
--- a/lib/gitlab/ci/build/auto_retry.rb
+++ b/lib/gitlab/ci/build/auto_retry.rb
@@ -43,20 +43,31 @@ def max_allowed_retries
end
def options_retry_max
- Integer(options_retry[:max], exception: false) if retry_on_reason_or_always?
+ Integer(options_retry[:max], exception: false) if retry_on_reason_or_always? || retry_on_exit_code?
end
def options_retry_when
- default = ['always']
+ default = options_retry_exit_codes.nil? ? ['always'] : []
options_retry.fetch(:when, default) || default
end
+ def options_retry_exit_codes
+ options_retry.fetch(:exit_codes, nil)
+ end
+
def retry_on_reason_or_always?
options_retry_when.include?(failure_reason.to_s) ||
options_retry_when.include?('always')
end
+ def retry_on_exit_code?
+ return false unless Feature.enabled?(:ci_retry_on_exit_codes, Feature.current_request)
+ return false unless @build.exit_code
+
+ options_retry_exit_codes&.include?(@build.exit_code)
+ end
+
# The format of the retry option changed in GitLab 11.5: Before it was
# integer only, after it is a hash. New builds are created with the new
# format, but builds created before GitLab 11.5 and saved in database still
diff --git a/spec/lib/gitlab/ci/build/auto_retry_spec.rb b/spec/lib/gitlab/ci/build/auto_retry_spec.rb
index 0b275e7d5645267e986a0586593f621d94a90587..15b053c47b0e4b02b0b40ba4ce604001b88ec382 100644
--- a/spec/lib/gitlab/ci/build/auto_retry_spec.rb
+++ b/spec/lib/gitlab/ci/build/auto_retry_spec.rb
@@ -12,21 +12,36 @@
subject { auto_retry.allowed? }
- where(:description, :retry_count, :options, :failure_reason, :result) do
- "retries are disabled" | 0 | { max: 0 } | nil | false
- "max equals count" | 2 | { max: 2 } | nil | false
- "max is higher than count" | 1 | { max: 2 } | nil | true
- "max is a string" | 1 | { max: '2' } | nil | true
- "matching failure reason" | 0 | { when: %w[api_failure], max: 2 } | :api_failure | true
- "not matching with always" | 0 | { when: %w[always], max: 2 } | :api_failure | true
- "not matching reason" | 0 | { when: %w[script_error], max: 2 } | :api_failure | false
- "scheduler failure override" | 1 | { when: %w[scheduler_failure], max: 1 } | :scheduler_failure | false
- "default for scheduler failure" | 1 | {} | :scheduler_failure | true
- "quota is exceeded" | 0 | { max: 2 } | :ci_quota_exceeded | false
- "no matching runner" | 0 | { max: 2 } | :no_matching_runner | false
- "missing dependencies" | 0 | { max: 2 } | :missing_dependency_failure | false
- "forward deployment failure" | 0 | { max: 2 } | :forward_deployment_failure | false
- "environment creation failure" | 0 | { max: 2 } | :environment_creation_failure | false
+ where(:description, :retry_count, :options, :failure_reason, :exit_code, :result) do
+ # retry:max
+ "retries are disabled" | 0 | { max: 0 } | nil | nil | false
+ "max equals count" | 2 | { max: 2 } | nil | nil | false
+ "max is higher than count" | 1 | { max: 2 } | nil | nil | true
+ "max is a string" | 1 | { max: '2' } | nil | nil | true
+ # retry:when
+ "matching failure reason" | 0 | { when: %w[api_failure], max: 2 } | :api_failure | nil | true
+ "not matching with always" | 0 | { when: %w[always], max: 2 } | :api_failure | nil | true
+ "not matching reason" | 0 | { when: %w[script_error], max: 2 } | :api_failure | nil | false
+ "scheduler failure override" | 1 | { when: %w[scheduler_failure], max: 1 } | :scheduler_failure | nil | false
+ # retry:exit_codes
+ "matching exit code" | 0 | { exit_codes: [255, 137], max: 2 } | nil | 137 | true
+ "matching exit code simple" | 0 | { exit_codes: [255], max: 2 } | nil | 255 | true
+ "not matching exit code" | 0 | { exit_codes: [255], max: 2 } | nil | 1 | false
+ "exit_code is not an integer" | 0 | { exit_codes: ["137"], max: 2 } | nil | 137 | false
+ # retry:when && retry:exit_codes
+ # EC: exit_codes
+ # FR: failure_reason
+ "matching EC & FR" | 0 | { exit_codes: [3], when: %w[script_failure], max: 2 } | :script_failure | 3 | true
+ "matching EC only" | 0 | { exit_codes: [3], when: %w[script_failure], max: 2 } | :api_failure | 3 | true
+ "matching FR only" | 0 | { exit_codes: [1], when: %w[script_failure], max: 2 } | :script_failure | 137 | true
+ "not matching EC & FR" | 0 | { exit_codes: [1], when: %w[script_failure], max: 2 } | :api_failure | 137 | false
+ # other
+ "default for scheduler failure" | 1 | {} | :scheduler_failure | nil | true
+ "quota is exceeded" | 0 | { max: 2 } | :ci_quota_exceeded | nil | false
+ "no matching runner" | 0 | { max: 2 } | :no_matching_runner | nil | false
+ "missing dependencies" | 0 | { max: 2 } | :missing_dependency_failure | nil | false
+ "forward deployment failure" | 0 | { max: 2 } | :forward_deployment_failure | nil | false
+ "environment creation failure" | 0 | { max: 2 } | :environment_creation_failure | nil | false
end
with_them do
@@ -35,12 +50,44 @@
build.options[:retry] = options
build.failure_reason = failure_reason
+ build.exit_code = exit_code
allow(build).to receive(:retryable?).and_return(true)
end
it { is_expected.to eq(result) }
end
+ context 'when ci_retry_on_exit_codes feature flag is disabled' do
+ where(:description, :retry_count, :options, :failure_reason, :exit_code, :result) do
+ "matching exit code" | 0 | { exit_codes: [255, 137], max: 2 } | nil | 137 | false
+ "not matching exit code" | 0 | { exit_codes: [255], max: 2 } | nil | 1 | false
+ "exit_code is not an integer" | 0 | { exit_codes: ["137"], max: 2 } | nil | 137 | false
+ # retry:when && retry:exit_codes
+ # EC: exit_codes
+ # FR: failure_reason
+ "matching EC & FR" | 0 | { exit_codes: [3], when: %w[script_failure], max: 2 } | :script_failure | 3 | true
+ "matching EC only" | 0 | { exit_codes: [3], when: %w[script_failure], max: 2 } | :api_failure | 3 | false
+ "matching FR only" | 0 | { exit_codes: [1], when: %w[script_failure], max: 2 } | :script_failure | 137 | true
+ "not matching EC & FR" | 0 | { exit_codes: [1], when: %w[script_failure], max: 2 } | :api_failure | 137 | false
+ end
+
+ with_them do
+ before do
+ # shouldn't retry on matching exit code
+ stub_feature_flags(ci_retry_on_exit_codes: false)
+
+ allow(build).to receive(:retries_count) { retry_count }
+
+ build.options[:retry] = options
+ build.failure_reason = failure_reason
+ build.metadata.exit_code = exit_code
+ allow(build).to receive(:retryable?).and_return(true)
+ end
+
+ it { is_expected.to eq(result) }
+ end
+ end
+
context 'when build is not retryable' do
before do
allow(build).to receive(:retryable?).and_return(false)
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 8e21a1f7cb5b782c72981ecb31c95b7b42df45b7..7d1199dfc258e957a87e9b474594f1d2b2733e39 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -5075,6 +5075,11 @@ def run_job_without_exception
build.drop_with_exit_code!(:unknown_failure, exit_code)
end
+ it 'correctly sets the exit code' do
+ expect { drop_with_exit_code }
+ .to change { build.reload.metadata&.exit_code }.from(nil).to(1)
+ end
+
shared_examples 'drops the build without changing allow_failure' do
it 'does not change allow_failure' do
expect { drop_with_exit_code }
@@ -5175,38 +5180,93 @@ def run_job_without_exception
build.exit_codes_defined?
end
- context 'without allow_failure_criteria' do
+ context 'without allow_failure_criteria nor retry' do
it { is_expected.to be_falsey }
end
- context 'when exit_codes is nil' do
- let(:options) do
- {
- allow_failure_criteria: {
- exit_codes: nil
+ context 'with allow_failure_critera' do
+ context 'when exit_codes is nil' do
+ let(:options) do
+ {
+ allow_failure_criteria: {
+ exit_codes: nil
+ }
}
- }
+ end
+
+ it { is_expected.to be_falsey }
end
- it { is_expected.to be_falsey }
+ context 'when exit_codes is an empty array' do
+ let(:options) do
+ {
+ allow_failure_criteria: {
+ exit_codes: []
+ }
+ }
+ end
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'when exit_codes are defined' do
+ let(:options) do
+ {
+ allow_failure_criteria: {
+ exit_codes: [5, 6]
+ }
+ }
+ end
+
+ it { is_expected.to be_truthy }
+ end
end
- context 'when exit_codes is an empty array' do
- let(:options) do
- {
- allow_failure_criteria: {
- exit_codes: []
+ context 'with retry' do
+ context 'when exit_codes is nil' do
+ let(:options) do
+ {
+ retry: {
+ exit_codes: nil
+ }
}
- }
+ end
+
+ it { is_expected.to be_falsey }
end
- it { is_expected.to be_falsey }
+ context 'when exit_codes is an empty array' do
+ let(:options) do
+ {
+ retry: {
+ exit_codes: []
+ }
+ }
+ end
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'when exit_codes are defined' do
+ let(:options) do
+ {
+ retry: {
+ exit_codes: [5, 6]
+ }
+ }
+ end
+
+ it { is_expected.to be_truthy }
+ end
end
- context 'when exit_codes are defined' do
+ context "with exit_codes defined for retry and allow_failure_criteria" do
let(:options) do
{
allow_failure_criteria: {
+ exit_codes: [1]
+ },
+ retry: {
exit_codes: [5, 6]
}
}