Commit 1412094f authored by Marius Bobin's avatar Marius Bobin

Add exit codes to allow failure syntax

Allow users to specify exit codes for allowed failures:

```yaml
test_job:
  script:
    - execute_script_that_will_fail
  allow_failure:
    exit_codes:
      - 137
      - 255
```
parent c2865147
---
name: ci_allow_failure_with_exit_codes
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49145
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/292024
milestone: '13.7'
type: development
group: group::pipeline authoring
default_enabled: false
# frozen_string_literal: true
module Gitlab
module Ci
class Config
module Entry
##
# Entry that represents allow_failure settings.
#
class AllowFailure < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Attributable
include ::Gitlab::Config::Entry::Validatable
ALLOWED_KEYS = %i[exit_codes].freeze
attributes ALLOWED_KEYS
validations do
validates :config, hash_or_boolean: true
validates :config, allowed_keys: ALLOWED_KEYS
validates :exit_codes, array_of_integers_or_integer: true, allow_nil: true
end
def value
@config[:exit_codes] = Array.wrap(exit_codes) if exit_codes.present?
@config
end
end
end
end
end
end
......@@ -22,6 +22,7 @@ module Gitlab
in: ALLOWED_WHEN,
message: "should be one of: #{ALLOWED_WHEN.join(', ')}"
}
validates :allow_failure, boolean: true
end
validate on: :composed do
......@@ -47,7 +48,7 @@ module Gitlab
inherit: false,
metadata: { allowed_needs: %i[job bridge] }
attributes :when
attributes :when, :allow_failure
def self.matching?(name, config)
!name.to_s.start_with?('.') &&
......@@ -72,6 +73,10 @@ module Gitlab
def bridge_needs
needs_value[:bridge] if needs_value
end
def ignored?
allow_failure.nil? ? manual_action? : allow_failure
end
end
end
end
......
......@@ -31,6 +31,7 @@ module Gitlab
validates :dependencies, array_of_strings: true
validates :resource_group, type: String
validates :allow_failure, hash_or_boolean: true
end
validates :start_in, duration: { limit: '1 week' }, if: :delayed?
......@@ -117,9 +118,14 @@ module Gitlab
description: 'Parallel configuration for this job.',
inherit: false
entry :allow_failure, ::Gitlab::Ci::Config::Entry::AllowFailure,
description: 'Indicates whether this job is allowed to fail or not.',
inherit: false
attributes :script, :tags, :when, :dependencies,
:needs, :retry, :parallel, :start_in,
:interruptible, :timeout, :resource_group, :release
:interruptible, :timeout, :resource_group,
:release, :allow_failure
def self.matching?(name, config)
!name.to_s.start_with?('.') &&
......@@ -166,11 +172,32 @@ module Gitlab
release: release_value,
after_script: after_script_value,
ignore: ignored?,
allow_failure_criteria: allow_failure_criteria,
needs: needs_defined? ? needs_value : nil,
resource_group: resource_group,
scheduling_type: needs_defined? ? :dag : :stage
).compact
end
def ignored?
allow_failure_defined? ? static_allow_failure : manual_action?
end
private
def allow_failure_criteria
return unless ::Gitlab::Ci::Features.allow_failure_with_exit_codes_enabled?
if allow_failure_defined? && allow_failure_value.is_a?(Hash)
allow_failure_value
end
end
def static_allow_failure
return false if allow_failure_value.is_a?(Hash)
allow_failure_value
end
end
end
end
......
......@@ -32,7 +32,6 @@ module Gitlab
with_options allow_nil: true do
validates :extends, array_of_strings_or_string: true
validates :rules, array_of_hashes: true
validates :allow_failure, boolean: true
end
end
......@@ -65,7 +64,7 @@ module Gitlab
inherit: false,
default: {}
attributes :extends, :rules, :allow_failure
attributes :extends, :rules
end
def compose!(deps = nil)
......@@ -141,10 +140,6 @@ module Gitlab
def manual_action?
self.when == 'manual'
end
def ignored?
allow_failure.nil? ? manual_action? : allow_failure
end
end
end
end
......
......@@ -62,6 +62,10 @@ module Gitlab
def self.ci_pipeline_editor_page_enabled?(project)
::Feature.enabled?(:ci_pipeline_editor_page, project, default_enabled: false)
end
def self.allow_failure_with_exit_codes_enabled?
::Feature.enabled?(:ci_allow_failure_with_exit_codes)
end
end
end
end
......@@ -60,6 +60,7 @@ module Gitlab
@seed_attributes
.deep_merge(pipeline_attributes)
.deep_merge(rules_attributes)
.deep_merge(allow_failure_criteria_attributes)
.deep_merge(cache_attributes)
end
......@@ -154,9 +155,13 @@ module Gitlab
end
def rules_attributes
return {} unless @using_rules
strong_memoize(:rules_attributes) do
if @using_rules
rules_result.build_attributes
else
{}
end
end
end
def rules_result
......@@ -176,6 +181,17 @@ module Gitlab
@cache.build_attributes
end
end
# If a job uses `allow_failure:exit_codes` and `rules:allow_failure`
# we need to prevent the exit codes from being persisted because they
# would break the behavior defined by `rules:allow_failure`.
def allow_failure_criteria_attributes
return {} unless ::Gitlab::Ci::Features.allow_failure_with_exit_codes_enabled?
return {} if rules_attributes[:allow_failure].nil?
return {} unless @seed_attributes.dig(:options, :allow_failure_criteria)
{ options: { allow_failure_criteria: nil } }
end
end
end
end
......
......@@ -77,6 +77,7 @@ module Gitlab
options: {
image: job[:image],
services: job[:services],
allow_failure_criteria: job[:allow_failure_criteria],
artifacts: job[:artifacts],
dependencies: job[:dependencies],
cross_dependencies: job.dig(:needs, :cross_dependency),
......
......@@ -134,6 +134,16 @@ module Gitlab
end
end
class HashOrBooleanValidator < ActiveModel::EachValidator
include LegacyValidationHelpers
def validate_each(record, attribute, value)
unless value.is_a?(Hash) || validate_boolean(value)
record.errors.add(attribute, 'should be a hash or a boolean value')
end
end
end
class KeyValidator < ActiveModel::EachValidator
include LegacyValidationHelpers
......@@ -158,6 +168,22 @@ module Gitlab
end
end
class ArrayOfIntegersOrIntegerValidator < ActiveModel::EachValidator
include LegacyValidationHelpers
def validate_each(record, attribute, value)
unless validate_integer(value) || validate_array_of_integers(value)
record.errors.add(attribute, 'should be an array of integers or an integer')
end
end
private
def validate_array_of_integers(values)
values.is_a?(Array) && values.all? { |value| validate_integer(value) }
end
end
class RegexpValidator < ActiveModel::EachValidator
include LegacyValidationHelpers
......
......@@ -187,11 +187,17 @@ RSpec.describe Gitlab::Ci::Build::Rules do
let(:start_in) { nil }
let(:allow_failure) { nil }
subject { Gitlab::Ci::Build::Rules::Result.new(when_value, start_in, allow_failure) }
subject(:result) do
Gitlab::Ci::Build::Rules::Result.new(when_value, start_in, allow_failure)
end
describe '#build_attributes' do
subject(:build_attributes) do
result.build_attributes
end
it 'compacts nil values' do
expect(subject.build_attributes).to eq(options: {}, when: 'on_success')
is_expected.to eq(options: {}, when: 'on_success')
end
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Ci::Config::Entry::AllowFailure do
let(:entry) { described_class.new(config.deep_dup) }
let(:expected_config) { config }
describe 'validations' do
context 'when entry config value is valid' do
shared_examples 'valid entry' do
describe '#value' do
it 'returns key value' do
expect(entry.value).to eq(expected_config)
end
end
describe '#valid?' do
it 'is valid' do
expect(entry).to be_valid
end
end
end
context 'with boolean values' do
it_behaves_like 'valid entry' do
let(:config) { true }
end
it_behaves_like 'valid entry' do
let(:config) { false }
end
end
context 'with hash values' do
it_behaves_like 'valid entry' do
let(:config) { { exit_codes: 137 } }
let(:expected_config) { { exit_codes: [137] } }
end
it_behaves_like 'valid entry' do
let(:config) { { exit_codes: [42, 137] } }
end
end
end
context 'when entry value is not valid' do
shared_examples 'invalid entry' do
describe '#valid?' do
it { expect(entry).not_to be_valid }
it { expect(entry.errors).to include(error_message) }
end
end
context 'when it has a wrong type' do
let(:config) { [1] }
let(:error_message) do
'allow failure config should be a hash or a boolean value'
end
it_behaves_like 'invalid entry'
end
context 'with string exit codes' do
let(:config) { { exit_codes: 'string' } }
let(:error_message) do
'allow failure exit codes should be an array of integers or an integer'
end
it_behaves_like 'invalid entry'
end
context 'with array of strings as exit codes' do
let(:config) { { exit_codes: ['string 1', 'string 2'] } }
let(:error_message) do
'allow failure exit codes should be an array of integers or an integer'
end
it_behaves_like 'invalid entry'
end
context 'when it has an extra keys' do
let(:config) { { extra: true } }
let(:error_message) do
'allow failure config contains unknown keys: extra'
end
it_behaves_like 'invalid entry'
end
end
end
end
......@@ -227,6 +227,23 @@ RSpec.describe Gitlab::Ci::Config::Entry::Bridge do
end
end
end
context 'when bridge config contains exit_codes' do
let(:config) do
{ script: 'rspec', allow_failure: { exit_codes: [42] } }
end
describe '#valid?' do
it { is_expected.not_to be_valid }
end
describe '#errors' do
it 'returns an error message' do
expect(subject.errors)
.to include(/allow failure should be a boolean value/)
end
end
end
end
describe '#manual_action?' do
......
......@@ -670,6 +670,10 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do
end
describe '#ignored?' do
before do
entry.compose!
end
context 'when job is a manual action' do
context 'when it is not specified if job is allowed to fail' do
let(:config) do
......@@ -700,6 +704,16 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do
expect(entry).not_to be_ignored
end
end
context 'when job is dynamically allowed to fail' do
let(:config) do
{ script: 'deploy', when: 'manual', allow_failure: { exit_codes: 42 } }
end
it 'is not an ignored job' do
expect(entry).not_to be_ignored
end
end
end
context 'when job is not a manual action' do
......@@ -709,6 +723,10 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do
it 'is not an ignored job' do
expect(entry).not_to be_ignored
end
it 'does not return allow_failure' do
expect(entry.value.key?(:allow_failure_criteria)).to be_falsey
end
end
context 'when job is allowed to fail' do
......@@ -717,6 +735,10 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do
it 'is an ignored job' do
expect(entry).to be_ignored
end
it 'does not return allow_failure_criteria' do
expect(entry.value.key?(:allow_failure_criteria)).to be_falsey
end
end
context 'when job is not allowed to fail' do
......@@ -725,6 +747,32 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do
it 'is not an ignored job' do
expect(entry).not_to be_ignored
end
it 'does not return allow_failure_criteria' do
expect(entry.value.key?(:allow_failure_criteria)).to be_falsey
end
end
context 'when job is dynamically allowed to fail' do
let(:config) { { script: 'deploy', allow_failure: { exit_codes: 42 } } }
it 'is not an ignored job' do
expect(entry).not_to be_ignored
end
it 'returns allow_failure_criteria' do
expect(entry.value[:allow_failure_criteria]).to match(exit_codes: [42])
end
context 'with ci_allow_failure_with_exit_codes disabled' do
before do
stub_feature_flags(ci_allow_failure_with_exit_codes: false)
end
it 'does not return allow_failure_criteria' do
expect(entry.value.key?(:allow_failure_criteria)).to be_falsey
end
end
end
end
end
......
......@@ -165,6 +165,45 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
it { is_expected.to include(options: {}) }
end
context 'with allow_failure' do
let(:options) do
{ allow_failure_criteria: { exit_codes: [42] } }
end
let(:rules) do
[{ if: '$VAR == null', when: 'always' }]
end
let(:attributes) do
{
name: 'rspec',
ref: 'master',
options: options,
rules: rules
}
end
context 'when rules does not override allow_failure' do
it { is_expected.to match a_hash_including(options: options) }
end
context 'when rules set allow_failure to true' do
let(:rules) do
[{ if: '$VAR == null', when: 'always', allow_failure: true }]
end
it { is_expected.to match a_hash_including(options: { allow_failure_criteria: nil }) }
end
context 'when rules set allow_failure to false' do
let(:rules) do
[{ if: '$VAR == null', when: 'always', allow_failure: false }]
end
it { is_expected.to match a_hash_including(options: { allow_failure_criteria: nil }) }
end
end
end
describe '#bridge?' do
......
......@@ -231,6 +231,23 @@ module Gitlab
expect(subject[:allow_failure]).to be true
end
end
context 'when allow_failure has exit_codes' do
let(:config) do
YAML.dump(rspec: { script: 'rspec',
when: 'manual',
allow_failure: { exit_codes: 1 } })
end
it 'is not allowed to fail' do
expect(subject[:allow_failure]).to be false
end
it 'saves allow_failure_criteria into options' do
expect(subject[:options]).to match(
a_hash_including(allow_failure_criteria: { exit_codes: [1] }))
end
end
end
context 'when job is not a manual action' do
......@@ -254,6 +271,22 @@ module Gitlab
expect(subject[:allow_failure]).to be false
end
end
context 'when allow_failure is dynamically specified' do
let(:config) do
YAML.dump(rspec: { script: 'rspec',
allow_failure: { exit_codes: 1 } })
end
it 'is not allowed to fail' do
expect(subject[:allow_failure]).to be false
end
it 'saves allow_failure_criteria into options' do
expect(subject[:options]).to match(
a_hash_including(allow_failure_criteria: { exit_codes: [1] }))
end
end
end
end
......@@ -2494,7 +2527,13 @@ module Gitlab
context 'returns errors if job allow_failure parameter is not an boolean' do
let(:config) { YAML.dump({ rspec: { script: "test", allow_failure: "string" } }) }
it_behaves_like 'returns errors', 'jobs:rspec allow failure should be a boolean value'
it_behaves_like 'returns errors', 'jobs:rspec allow failure should be a hash or a boolean value'
end
context 'returns errors if job exit_code parameter from allow_failure is not an integer' do
let(:config) { YAML.dump({ rspec: { script: "test", allow_failure: { exit_codes: 'string' } } }) }
it_behaves_like 'returns errors', 'jobs:rspec:allow_failure exit codes should be an array of integers or an integer'
end
context 'returns errors if job stage is not a string' do
......
......@@ -93,6 +93,73 @@ RSpec.describe Ci::CreatePipelineService do
end
end
end
context 'with allow_failure and exit_codes', :aggregate_failures do
def find_job(name)
pipeline.builds.find_by(name: name)
end
let(:config) do
<<-EOY
job-1:
script: exit 42
allow_failure:
exit_codes: 42
rules:
- if: $CI_COMMIT_REF_NAME == "master"
allow_failure: false
job-2:
script: exit 42
allow_failure:
exit_codes: 42
rules:
- if: $CI_COMMIT_REF_NAME == "master"
allow_failure: true
job-3:
script: exit 42
allow_failure:
exit_codes: 42
rules:
- if: $CI_COMMIT_REF_NAME == "master"
when: manual
EOY
end
it 'creates a pipeline' do
expect(pipeline).to be_persisted
expect(build_names).to contain_exactly(
'job-1', 'job-2', 'job-3'
)
end
it 'assigns job:allow_failure values to the builds' do
expect(find_job('job-1').allow_failure).to eq(false)
expect(find_job('job-2').allow_failure).to eq(true)
expect(find_job('job-3').allow_failure).to eq(false)
end
it 'removes exit_codes if allow_failure is specified' do
expect(find_job('job-1').options.dig(:allow_failure_criteria)).to be_nil
expect(find_job('job-2').options.dig(:allow_failure_criteria)).to be_nil
expect(find_job('job-3').options.dig(:allow_failure_criteria, :exit_codes)).to eq([42])
end
context 'with ci_allow_failure_with_exit_codes disabled' do
before do
stub_feature_flags(ci_allow_failure_with_exit_codes: false)
end
it 'does not persist allow_failure_criteria' do
expect(pipeline).to be_persisted
expect(find_job('job-1').options.key?(:allow_failure_criteria)).to be_falsey
expect(find_job('job-2').options.key?(:allow_failure_criteria)).to be_falsey
expect(find_job('job-3').options.key?(:allow_failure_criteria)).to be_falsey
end
end
end
end
context 'when workflow:rules are used' 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