diff --git a/app/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb index 263dac17c2458234e3e35345ee5098f108aabd5a..a3a96d816d2cf377eba0eb25aebc7ccd40479a4b 100644 --- a/app/models/concerns/ci/metadatable.rb +++ b/app/models/concerns/ci/metadatable.rb @@ -20,7 +20,6 @@ module Metadatable accepts_nested_attributes_for :metadata - delegate :interruptible, to: :metadata, prefix: false, allow_nil: true delegate :exit_code, to: :metadata, allow_nil: true before_validation :ensure_metadata, on: :create @@ -101,11 +100,13 @@ def yaml_variables=(value) end def interruptible - metadata&.interruptible + return job_definition.interruptible if read_from_new_destination? && job_definition + + metadata&.read_attribute(:interruptible) end def interruptible=(value) - ensure_metadata.interruptible = value + ensure_metadata.interruptible = value if can_write_metadata? end def id_tokens diff --git a/spec/factories/ci/processable.rb b/spec/factories/ci/processable.rb index dc482b7a44291cf70d9a79a0a0f3bf6514f2adce..470c51d19448aa4ec599b128bd7d92555b8f1e68 100644 --- a/spec/factories/ci/processable.rb +++ b/spec/factories/ci/processable.rb @@ -124,6 +124,12 @@ trait :interruptible do after(:build) do |processable| processable.metadata.interruptible = true + + next unless processable.job_definition + + updated_config = processable.job_definition.config.merge(interruptible: true) + processable.job_definition.write_attribute(:config, updated_config) + processable.job_definition.write_attribute(:interruptible, true) end end end diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 603ec92f649c1bfdb2292807762949da7789ee1c..78d5b831b27da6b7e8f735cbbf6663e08410c953 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -77,7 +77,7 @@ %i[pipeline project ref tag options name allow_failure stage_idx yaml_variables when environment coverage_regex description tag_list protected needs_attributes job_variables_attributes timeout timeout_source debug_trace_enabled - resource_group scheduling_type ci_stage partition_id id_tokens interruptible] + resource_group scheduling_type ci_stage partition_id id_tokens] end let(:reject_accessors) do @@ -116,7 +116,7 @@ dast_site_profile dast_scanner_profile stage_id dast_site_profiles_build dast_scanner_profiles_build auto_canceled_by_partition_id execution_config_id execution_config build_source id_value inputs error_job_messages - job_definition job_definition_instance job_messages temp_job_definition].freeze + job_definition job_definition_instance job_messages temp_job_definition interruptible].freeze end before_all do @@ -137,7 +137,8 @@ shared_examples_for 'clones the processable' do before_all do - processable.assign_attributes(stage_id: stage.id, interruptible: true) + processable.assign_attributes(stage_id: stage.id) + processable.metadata.interruptible = true processable.save! create(:ci_build_need, build: processable) @@ -187,6 +188,27 @@ expect(new_processable.protected).to eq processable.protected end end + + # Remove with stop_writing_builds_metadata + context 'when writing to builds metadata' do + before do + # Ci::Bridge doesn't implement interruptible + skip unless processable.class.clone_accessors.include?(:interruptible) + + processable.clear_memoization(:read_from_new_destination?) + processable.clear_memoization(:can_write_metadata?) + + stub_feature_flags( + stop_writing_builds_metadata: false, + read_from_new_ci_destinations: false + ) + end + + it 'clones the interruptible job attribute' do + expect(new_processable.interruptible).not_to be_nil + expect(new_processable.interruptible).to eq processable.interruptible + end + end end describe 'reject accessors' do diff --git a/spec/models/concerns/ci/metadatable_spec.rb b/spec/models/concerns/ci/metadatable_spec.rb index 535b3857b9c9edc656df910adaf136a06b5cb53f..a2192ebaa3fbe372a9ce3cc4ce262c4dc0192ec4 100644 --- a/spec/models/concerns/ci/metadatable_spec.rb +++ b/spec/models/concerns/ci/metadatable_spec.rb @@ -245,4 +245,60 @@ end end end + + describe '#interruptible' do + subject(:interruptible) { processable.interruptible } + + context 'when metadata interruptible is present' do + before do + processable.job_definition = nil + processable.ensure_metadata.write_attribute(:interruptible, true) + end + + it 'returns metadata interruptible' do + expect(interruptible).to be_truthy + end + + context 'when job definition interruptible is present' do + before do + processable.ensure_metadata.write_attribute(:interruptible, nil) + processable.build_job_definition.write_attribute(:interruptible, false) + end + + it 'returns job definition interruptible' do + expect(interruptible).to be false + end + + context 'when FF `read_from_new_ci_destinations` is disabled' do + before do + stub_feature_flags(read_from_new_ci_destinations: false) + processable.ensure_metadata.write_attribute(:interruptible, true) + processable.job_definition.interruptible = false + end + + it 'returns metadata interruptible' do + expect(interruptible).to be_truthy + end + end + end + end + end + + describe '#interruptible=' do + it 'does not change metadata.interruptible' do + expect { processable.interruptible = true } + .to not_change { processable.metadata.interruptible } + end + + context 'when FF `stop_writing_builds_metadata` is disabled' do + before do + stub_feature_flags(stop_writing_builds_metadata: false) + end + + it 'sets the value into metadata.interruptible' do + expect { processable.interruptible = true } + .to change { processable.metadata.interruptible } + end + end + end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 0438754bb94038a94f8b3b31055554cb9d0d5495..3046dbd00b9350ccc3c3a11151596d2826392662 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -229,10 +229,10 @@ def execute_service( stub_ci_pipeline_yaml_file(config) end - it 'is cancelable' do + it 'is not cancelable' do pipeline = execute_service.payload - expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_nil + expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_falsy end end @@ -265,6 +265,7 @@ def execute_service( context 'interruptible builds' do before do + stub_feature_flags(stop_writing_builds_metadata: false) stub_ci_pipeline_yaml_file(YAML.dump(config)) end @@ -301,19 +302,21 @@ def execute_service( } end + # Remove metadata interruptible with stop_writing_builds_metadata it 'properly configures interruptible status' do interruptible_status = pipeline_on_previous_commit .builds .joins(:metadata) - .pluck(:name, "#{Ci::BuildMetadata.quoted_table_name}.interruptible") + .joins(:job_definition) + .pluck(:name, "#{Ci::BuildMetadata.quoted_table_name}.interruptible", "#{Ci::JobDefinition.quoted_table_name}.interruptible") expect(interruptible_status).to contain_exactly( - ['build_1_1', true], - ['build_1_2', true], - ['build_2_1', true], - ['build_3_1', false], - ['build_4_1', nil] + ['build_1_1', true, true], + ['build_1_2', true, true], + ['build_2_1', true, true], + ['build_3_1', false, false], + ['build_4_1', nil, false] ) end