From a10f07292e49154fcfdae1eaf48a3576bf684a70 Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Wed, 4 Oct 2023 14:03:38 +0200 Subject: [PATCH 1/4] WIP POC auto cancel redundant pipelines with workflow --- app/models/ci/build.rb | 5 -- app/models/ci/pipeline.rb | 2 +- app/models/ci/pipeline_metadata.rb | 7 +- app/models/ci/processable.rb | 1 - app/models/commit_status.rb | 10 +++ app/services/ci/cancel_pipeline_service.rb | 13 +++- .../cancel_redundant_pipelines_service.rb | 67 ++++++++++++------- .../ci_pipeline_metadata_auto_cancel.json | 11 +++ .../projects/settings/ci_cd/_form.html.haml | 3 +- ...add_auto_cancel_to_ci_pipeline_metadata.rb | 7 ++ ...eline_metadata_name_not_null_constraint.rb | 15 +++++ db/schema_migrations/20231003083034 | 1 + db/schema_migrations/20231003090220 | 1 + db/structure.sql | 2 +- lib/gitlab/ci/config.rb | 4 ++ lib/gitlab/ci/config/entry/auto_cancel.rb | 31 +++++++++ lib/gitlab/ci/config/entry/workflow.rb | 5 +- .../ci/pipeline/chain/populate_metadata.rb | 17 ++++- lib/gitlab/ci/yaml_processor/result.rb | 14 +++- .../ci/create_pipeline_service_spec.rb | 39 +++++++++++ 20 files changed, 214 insertions(+), 41 deletions(-) create mode 100644 app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json create mode 100644 db/migrate/20231003083034_add_auto_cancel_to_ci_pipeline_metadata.rb create mode 100644 db/migrate/20231003090220_remove_ci_pipeline_metadata_name_not_null_constraint.rb create mode 100644 db/schema_migrations/20231003083034 create mode 100644 db/schema_migrations/20231003090220 create mode 100644 lib/gitlab/ci/config/entry/auto_cancel.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 05122c96948de5..7ada6f8b7b8306 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -89,11 +89,6 @@ class Build < Ci::Processable validates :coverage, numericality: true, allow_blank: true validates :ref, presence: true - scope :not_interruptible, -> do - joins(:metadata) - .where.not(Ci::BuildMetadata.table_name => { id: Ci::BuildMetadata.scoped_build.with_interruptible.select(:id) }) - end - scope :unstarted, -> { where(runner_id: nil) } scope :with_any_artifacts, -> do diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index cf3efc5998fe1a..10feef25aac5bc 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -152,7 +152,7 @@ class Pipeline < Ci::ApplicationRecord accepts_nested_attributes_for :variables, reject_if: :persisted? delegate :full_path, to: :project, prefix: true - delegate :name, to: :pipeline_metadata, allow_nil: true + delegate :name, :auto_cancel_on_new_pipeline, to: :pipeline_metadata, allow_nil: true validates :sha, presence: { unless: :importing? } validates :ref, presence: { unless: :importing? } diff --git a/app/models/ci/pipeline_metadata.rb b/app/models/ci/pipeline_metadata.rb index 2bd206c5ca5d87..27fde513645487 100644 --- a/app/models/ci/pipeline_metadata.rb +++ b/app/models/ci/pipeline_metadata.rb @@ -9,6 +9,11 @@ class PipelineMetadata < Ci::ApplicationRecord validates :pipeline, presence: true validates :project, presence: true - validates :name, presence: true, length: { minimum: 1, maximum: 255 } + validates :name, length: { minimum: 1, maximum: 255 }, allow_nil: true + validates :auto_cancel, json_schema: { filename: "ci_pipeline_metadata_auto_cancel" }, allow_nil: true + + def auto_cancel_on_new_pipeline + auto_cancel&.dig('on_new_pipeline') + end end end diff --git a/app/models/ci/processable.rb b/app/models/ci/processable.rb index 7ad1a727a0ef87..671316d7595c18 100644 --- a/app/models/ci/processable.rb +++ b/app/models/ci/processable.rb @@ -6,7 +6,6 @@ module Ci class Processable < ::CommitStatus include Gitlab::Utils::StrongMemoize include FromUnion - include Ci::Metadatable extend ::Gitlab::Utils::Override has_one :resource, class_name: 'Ci::Resource', foreign_key: 'build_id', inverse_of: :processable diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 9f77bd8ebe226e..1636fc9217703c 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -3,6 +3,7 @@ class CommitStatus < Ci::ApplicationRecord include Ci::Partitionable include Ci::HasStatus + include Ci::Metadatable include Importable include AfterCommitQueue include Presentable @@ -110,6 +111,15 @@ class CommitStatus < Ci::ApplicationRecord merge(or_conditions) end + scope :interruptible, -> do + joins(:metadata).merge(Ci::BuildMetadata.with_interruptible) + end + + scope :not_interruptible, -> do + joins(:metadata) + .where.not(Ci::BuildMetadata.table_name => { id: Ci::BuildMetadata.scoped_build.with_interruptible.select(:id) }) + end + ## # We still create some CommitStatuses outside of CreatePipelineService. # diff --git a/app/services/ci/cancel_pipeline_service.rb b/app/services/ci/cancel_pipeline_service.rb index 38053b1392110f..b563f0b2aebecb 100644 --- a/app/services/ci/cancel_pipeline_service.rb +++ b/app/services/ci/cancel_pipeline_service.rb @@ -10,17 +10,20 @@ class CancelPipelineService # @cascade_to_children - if true cancels all related child pipelines for parent child pipelines # @auto_canceled_by_pipeline - store the pipeline_id of the pipeline that triggered cancellation # @execute_async - if true cancel the children asyncronously + # @safe_cancellation - TODO: document this def initialize( pipeline:, current_user:, cascade_to_children: true, auto_canceled_by_pipeline: nil, - execute_async: true) + execute_async: true, + safe_cancellation: false) @pipeline = pipeline @current_user = current_user @cascade_to_children = cascade_to_children @auto_canceled_by_pipeline = auto_canceled_by_pipeline @execute_async = execute_async + @safe_cancellation = safe_cancellation end def execute @@ -42,7 +45,13 @@ def force_execute log_pipeline_being_canceled pipeline.update_column(:auto_canceled_by_id, @auto_canceled_by_pipeline.id) if @auto_canceled_by_pipeline - cancel_jobs(pipeline.cancelable_statuses) + + # TODO: refactoring + if @safe_cancellation + cancel_jobs(pipeline.cancelable_statuses.interruptible) + else + cancel_jobs(pipeline.cancelable_statuses) + end return ServiceResponse.success unless cascade_to_children? diff --git a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb index 224b2d96205362..412124e821bf58 100644 --- a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb +++ b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb @@ -17,13 +17,12 @@ def initialize(pipeline) def execute return if service_disabled? return if pipeline.parent_pipeline? # skip if child pipeline - return unless project.auto_cancel_pending_pipelines? paginator.each do |ids| pipelines = parent_and_child_pipelines(ids) Gitlab::OptimisticLocking.retry_lock(pipelines, name: 'cancel_pending_pipelines') do |cancelables| - auto_cancel_interruptible_pipelines(cancelables.ids) + auto_cancel_pipelines(cancelables.ids) end end end @@ -52,6 +51,12 @@ def paginator end end + def parent_and_child_pipelines(ids) + Ci::Pipeline.object_hierarchy(parent_auto_cancelable_pipelines(ids), project_condition: :same) + .base_and_descendants + .alive_or_scheduled + end + def parent_auto_cancelable_pipelines(ids) scope = project.all_pipelines .created_after(pipelines_created_after) @@ -63,37 +68,49 @@ def parent_auto_cancelable_pipelines(ids) scope.id_in(ids) end - - def parent_and_child_pipelines(ids) - Ci::Pipeline.object_hierarchy(parent_auto_cancelable_pipelines(ids), project_condition: :same) - .base_and_descendants - .alive_or_scheduled - end # rubocop: enable CodeReuse/ActiveRecord - def auto_cancel_interruptible_pipelines(pipeline_ids) + # TODO: This code can be better, ignore this for now :) + def auto_cancel_pipelines(pipeline_ids) + conservative_cancellable_pipeline_ids = ::Ci::Pipeline.id_in(pipeline_ids).with_only_interruptible_builds.ids + ::Ci::Pipeline .id_in(pipeline_ids) - .with_only_interruptible_builds .each do |cancelable_pipeline| - Gitlab::AppLogger.info( - class: self.class.name, - message: "Pipeline #{pipeline.id} auto-canceling pipeline #{cancelable_pipeline.id}", - canceled_pipeline_id: cancelable_pipeline.id, - canceled_by_pipeline_id: pipeline.id, - canceled_by_pipeline_source: pipeline.source - ) - - # cascade_to_children not needed because we iterate through descendants here - ::Ci::CancelPipelineService.new( - pipeline: cancelable_pipeline, - current_user: nil, - auto_canceled_by_pipeline: pipeline, - cascade_to_children: false - ).force_execute + if (cancelable_pipeline.auto_cancel_on_new_pipeline.nil? && project.auto_cancel_pending_pipelines?) || # legacy behavior + cancelable_pipeline.auto_cancel_on_new_pipeline == 'conservative' + next unless conservative_cancellable_pipeline_ids.include?(cancelable_pipeline.id) + + log_info(cancelable_pipeline) + cancel_pipeline(cancelable_pipeline, safe_cancellation: false) + elsif cancelable_pipeline.auto_cancel_on_new_pipeline == 'enabled' + log_info(cancelable_pipeline) + cancel_pipeline(cancelable_pipeline, safe_cancellation: true) + end end end + def log_info(cancelable_pipeline) + Gitlab::AppLogger.info( + class: self.class.name, + message: "Pipeline #{pipeline.id} auto-canceling pipeline #{cancelable_pipeline.id}", + canceled_pipeline_id: cancelable_pipeline.id, + canceled_by_pipeline_id: pipeline.id, + canceled_by_pipeline_source: pipeline.source + ) + end + + def cancel_pipeline(cancelable_pipeline, safe_cancellation:) + # cascade_to_children not needed because we iterate through descendants here + ::Ci::CancelPipelineService.new( + pipeline: cancelable_pipeline, + current_user: nil, + auto_canceled_by_pipeline: cancelable_pipeline, + cascade_to_children: false, + safe_cancellation: safe_cancellation + ).force_execute + end + def pipelines_created_after 3.days.ago end diff --git a/app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json b/app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json new file mode 100644 index 00000000000000..654fbc5cf00501 --- /dev/null +++ b/app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json @@ -0,0 +1,11 @@ +{ + "description": "CI Pipeline Metadata Auto Cancel", + "type": "object", + "properties": { + "on_new_pipeline": { + "type": "string", + "enum": ["enabled", "disabled", "conservative"] + } + }, + "additionalProperties": false +} diff --git a/app/views/projects/settings/ci_cd/_form.html.haml b/app/views/projects/settings/ci_cd/_form.html.haml index d51acc5e708587..587627790f93f6 100644 --- a/app/views/projects/settings/ci_cd/_form.html.haml +++ b/app/views/projects/settings/ci_cd/_form.html.haml @@ -16,10 +16,11 @@ .form-group = f.gitlab_ui_checkbox_component :auto_cancel_pending_pipelines, - _("Auto-cancel redundant pipelines"), + _("Auto-cancel redundant pipelines (DEPRECATED)"), checked_value: 'enabled', unchecked_value: 'disabled', help_text: (_('Pipelines for new changes cause older pending or running pipelines on the same branch to be cancelled.') + ' ' + help_link_auto_canceling).html_safe + -# TODO: add link for deprecated explanation. Use workflow:auto_cancel instead. .form-group = f.fields_for :ci_cd_settings_attributes, @project.ci_cd_settings do |form| diff --git a/db/migrate/20231003083034_add_auto_cancel_to_ci_pipeline_metadata.rb b/db/migrate/20231003083034_add_auto_cancel_to_ci_pipeline_metadata.rb new file mode 100644 index 00000000000000..fdefc9b9e1200f --- /dev/null +++ b/db/migrate/20231003083034_add_auto_cancel_to_ci_pipeline_metadata.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddAutoCancelToCiPipelineMetadata < Gitlab::Database::Migration[2.1] + def change + add_column :ci_pipeline_metadata, :auto_cancel, :jsonb + end +end diff --git a/db/migrate/20231003090220_remove_ci_pipeline_metadata_name_not_null_constraint.rb b/db/migrate/20231003090220_remove_ci_pipeline_metadata_name_not_null_constraint.rb new file mode 100644 index 00000000000000..90444966ba4041 --- /dev/null +++ b/db/migrate/20231003090220_remove_ci_pipeline_metadata_name_not_null_constraint.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class RemoveCiPipelineMetadataNameNotNullConstraint < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + CONSTRAINT_NAME = 'check_25d23931f1' + + def up + remove_not_null_constraint :ci_pipeline_metadata, :name, constraint_name: CONSTRAINT_NAME + end + + def down + add_not_null_constraint :ci_pipeline_metadata, :name, validate: false, constraint_name: CONSTRAINT_NAME + end +end diff --git a/db/schema_migrations/20231003083034 b/db/schema_migrations/20231003083034 new file mode 100644 index 00000000000000..84ebc936c2319d --- /dev/null +++ b/db/schema_migrations/20231003083034 @@ -0,0 +1 @@ +6742a67dcae4fbeb1d21f94273a810e01e9525a839373716bb1c5dc1d62c27be \ No newline at end of file diff --git a/db/schema_migrations/20231003090220 b/db/schema_migrations/20231003090220 new file mode 100644 index 00000000000000..d5202b7b759de6 --- /dev/null +++ b/db/schema_migrations/20231003090220 @@ -0,0 +1 @@ +f0917888dd246359a946e03ea8523cd330fb0e6577359d858feeb6b79011b983 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 8582b819febac1..8fc29deebf8c38 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14183,7 +14183,7 @@ CREATE TABLE ci_pipeline_metadata ( project_id bigint NOT NULL, pipeline_id bigint NOT NULL, name text, - CONSTRAINT check_25d23931f1 CHECK ((name IS NOT NULL)), + auto_cancel jsonb, CONSTRAINT check_9d3665463c CHECK ((char_length(name) <= 255)) ); diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 73d329930a52e5..beff10add71f7e 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -99,6 +99,10 @@ def workflow_name root.workflow_entry.name end + def workflow_auto_cancel + root.workflow_entry.auto_cancel_entry + end + def normalized_jobs @normalized_jobs ||= Ci::Config::Normalizer.new(jobs).normalize_jobs end diff --git a/lib/gitlab/ci/config/entry/auto_cancel.rb b/lib/gitlab/ci/config/entry/auto_cancel.rb new file mode 100644 index 00000000000000..2d76b1199feb2a --- /dev/null +++ b/lib/gitlab/ci/config/entry/auto_cancel.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module Entry + class AutoCancel < ::Gitlab::Config::Entry::Node + include ::Gitlab::Config::Entry::Attributable + include ::Gitlab::Config::Entry::Validatable + + ALLOWED_KEYS = %i[on_new_pipeline].freeze # TODO: maybe the name should be `on_new_commit` instead. + ALLOWED_OPTIONS = %w[enabled disabled conservative].freeze + + attributes ALLOWED_KEYS + + validations do + validates :config, type: Hash, allowed_keys: ALLOWED_KEYS + validates :on_new_pipeline, allow_nil: true, type: String, inclusion: { + in: ALLOWED_OPTIONS, + message: "must be one of: #{ALLOWED_OPTIONS.join(', ')}" + } + end + + def default_interruptible + on_new_pipeline == 'enabled' + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/entry/workflow.rb b/lib/gitlab/ci/config/entry/workflow.rb index 691d9e2d48b269..5b81c74fe4dd68 100644 --- a/lib/gitlab/ci/config/entry/workflow.rb +++ b/lib/gitlab/ci/config/entry/workflow.rb @@ -9,7 +9,7 @@ class Workflow < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Validatable include ::Gitlab::Config::Entry::Attributable - ALLOWED_KEYS = %i[rules name].freeze + ALLOWED_KEYS = %i[rules name auto_cancel].freeze attributes :name @@ -23,6 +23,9 @@ class Workflow < ::Gitlab::Config::Entry::Node description: 'List of evaluable Rules to determine Pipeline status.', metadata: { allowed_when: %w[always never] } + entry :auto_cancel, Entry::AutoCancel, + description: 'Auto-cancel configuration for this pipeline.' + def has_rules? @config.try(:key?, :rules) end diff --git a/lib/gitlab/ci/pipeline/chain/populate_metadata.rb b/lib/gitlab/ci/pipeline/chain/populate_metadata.rb index e7a9009f8f4348..68c643f94a3b7b 100644 --- a/lib/gitlab/ci/pipeline/chain/populate_metadata.rb +++ b/lib/gitlab/ci/pipeline/chain/populate_metadata.rb @@ -9,6 +9,8 @@ class PopulateMetadata < Chain::Base def perform! set_pipeline_name + set_auto_cancel + return if pipeline.pipeline_metadata.nil? || pipeline.pipeline_metadata.valid? message = pipeline.pipeline_metadata.errors.full_messages.join(', ') @@ -29,13 +31,26 @@ def set_pipeline_name return if name.blank? - pipeline.build_pipeline_metadata(project: pipeline.project, name: name.strip) + assign_to_metadata(name: name.strip) + end + + def set_auto_cancel + auto_cancel = @command.yaml_processor_result.workflow_auto_cancel.value + + return if auto_cancel.blank? + + assign_to_metadata(auto_cancel: auto_cancel) end def global_context Gitlab::Ci::Build::Context::Global.new( pipeline, yaml_variables: @command.pipeline_seed.root_variables) end + + def assign_to_metadata(attributes) + metadata = pipeline.build_pipeline_metadata(project: pipeline.project) unless pipeline.pipeline_metadata + metadata.assign_attributes(attributes) + end end end end diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb index 2435d128bf2c9c..05d1da81b02ffc 100644 --- a/lib/gitlab/ci/yaml_processor/result.rb +++ b/lib/gitlab/ci/yaml_processor/result.rb @@ -6,9 +6,12 @@ module Gitlab module Ci class YamlProcessor class Result + include Gitlab::Utils::StrongMemoize + attr_reader :errors, :warnings, :root_variables, :root_variables_with_prefill_data, - :stages, :jobs, :workflow_rules, :workflow_name + :stages, :jobs, + :workflow_rules, :workflow_name, :workflow_auto_cancel def initialize(ci_config: nil, errors: [], warnings: []) @ci_config = ci_config @@ -71,6 +74,7 @@ def assign_valid_attributes @workflow_rules = @ci_config.workflow_rules @workflow_name = @ci_config.workflow_name&.strip + @workflow_auto_cancel = @ci_config.workflow_auto_cancel end def stage_builds_attributes(stage) @@ -94,7 +98,7 @@ def build_attributes(name) job_variables: transform_to_array(job[:job_variables]), root_variables_inheritance: job[:root_variables_inheritance], needs_attributes: job.dig(:needs, :job), - interruptible: job[:interruptible], + interruptible: job.fetch(:interruptible, default_interruptible), only: job[:only], except: job[:except], rules: job[:rules], @@ -128,6 +132,12 @@ def build_attributes(name) }.compact }.compact end + def default_interruptible + # returns nil if workflow_auto_cancel is not defined + workflow_auto_cancel.try(:default_interruptible) + end + strong_memoize_attr :default_interruptible + def transform_to_array(variables) ::Gitlab::Ci::Variables::Helpers.transform_to_array(variables) end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 11f9708f9f3d53..701b7c197b8eaf 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -1731,6 +1731,45 @@ def previous_commit_sha_from_ref(ref) end end + context 'when workflow:auto_cancel is used' do + let(:response) { execute_service } + let(:pipeline) { response.payload } + + context 'when on_new_pipeline is enabled' do + let(:config) do + <<~YAML + workflow: + auto_cancel: + on_new_pipeline: enabled + + test1: + script: exit 0 + + test2: + script: exit 0 + interruptible: true + + test3: + script: exit 0 + interruptible: false + YAML + end + + before do + stub_ci_pipeline_yaml_file(config) + end + + it 'creates a pipeline with on_new_pipeline and builds.interruptible is true by default' do + expect(pipeline).to be_persisted + expect(pipeline.pipeline_metadata.auto_cancel).to eq('on_new_pipeline' => 'enabled') + + expect(pipeline.builds.find_by(name: 'test1').metadata[:interruptible]).to eq(true) + expect(pipeline.builds.find_by(name: 'test2').metadata[:interruptible]).to eq(true) + expect(pipeline.builds.find_by(name: 'test3').metadata[:interruptible]).to eq(false) + end + end + end + describe 'pipeline components' do let(:components_project) do create(:project, :repository, creator: user, namespace: user.namespace) -- GitLab From da2f08f1b29e4114da85950adc44207f76e925b1 Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Fri, 13 Oct 2023 13:07:08 +0200 Subject: [PATCH 2/4] interruptible conservative none --- .../cancel_redundant_pipelines_service.rb | 4 ++-- .../json_schemas/ci_pipeline_metadata_auto_cancel.json | 2 +- app/views/projects/settings/ci_cd/_form.html.haml | 3 +-- lib/gitlab/ci/config/entry/auto_cancel.rb | 8 +++++--- lib/gitlab/ci/yaml_processor/result.rb | 2 +- spec/services/ci/create_pipeline_service_spec.rb | 6 +++--- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb index 412124e821bf58..bdb2846e0f0c50 100644 --- a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb +++ b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb @@ -17,6 +17,7 @@ def initialize(pipeline) def execute return if service_disabled? return if pipeline.parent_pipeline? # skip if child pipeline + return unless project.auto_cancel_pending_pipelines? paginator.each do |ids| pipelines = parent_and_child_pipelines(ids) @@ -77,8 +78,7 @@ def auto_cancel_pipelines(pipeline_ids) ::Ci::Pipeline .id_in(pipeline_ids) .each do |cancelable_pipeline| - if (cancelable_pipeline.auto_cancel_on_new_pipeline.nil? && project.auto_cancel_pending_pipelines?) || # legacy behavior - cancelable_pipeline.auto_cancel_on_new_pipeline == 'conservative' + if cancelable_pipeline.auto_cancel_on_new_pipeline.nil? || cancelable_pipeline.auto_cancel_on_new_pipeline == 'conservative' # legacy behavior next unless conservative_cancellable_pipeline_ids.include?(cancelable_pipeline.id) log_info(cancelable_pipeline) diff --git a/app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json b/app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json index 654fbc5cf00501..55b4fd607090b9 100644 --- a/app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json +++ b/app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json @@ -4,7 +4,7 @@ "properties": { "on_new_pipeline": { "type": "string", - "enum": ["enabled", "disabled", "conservative"] + "enum": ["interruptible", "conservative", "none"] } }, "additionalProperties": false diff --git a/app/views/projects/settings/ci_cd/_form.html.haml b/app/views/projects/settings/ci_cd/_form.html.haml index 587627790f93f6..d51acc5e708587 100644 --- a/app/views/projects/settings/ci_cd/_form.html.haml +++ b/app/views/projects/settings/ci_cd/_form.html.haml @@ -16,11 +16,10 @@ .form-group = f.gitlab_ui_checkbox_component :auto_cancel_pending_pipelines, - _("Auto-cancel redundant pipelines (DEPRECATED)"), + _("Auto-cancel redundant pipelines"), checked_value: 'enabled', unchecked_value: 'disabled', help_text: (_('Pipelines for new changes cause older pending or running pipelines on the same branch to be cancelled.') + ' ' + help_link_auto_canceling).html_safe - -# TODO: add link for deprecated explanation. Use workflow:auto_cancel instead. .form-group = f.fields_for :ci_cd_settings_attributes, @project.ci_cd_settings do |form| diff --git a/lib/gitlab/ci/config/entry/auto_cancel.rb b/lib/gitlab/ci/config/entry/auto_cancel.rb index 2d76b1199feb2a..752440d056eb85 100644 --- a/lib/gitlab/ci/config/entry/auto_cancel.rb +++ b/lib/gitlab/ci/config/entry/auto_cancel.rb @@ -9,7 +9,7 @@ class AutoCancel < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Validatable ALLOWED_KEYS = %i[on_new_pipeline].freeze # TODO: maybe the name should be `on_new_commit` instead. - ALLOWED_OPTIONS = %w[enabled disabled conservative].freeze + ALLOWED_OPTIONS = %w[interruptible conservative none].freeze attributes ALLOWED_KEYS @@ -21,8 +21,10 @@ class AutoCancel < ::Gitlab::Config::Entry::Node } end - def default_interruptible - on_new_pipeline == 'enabled' + # Normally, `interruptible` is `false` by default for jobs. When `workflow:auto_cancel:on_new_pipeline` is + # set to `interruptible`, we change the default of `interruptible` attribute of jobs to `true`. + def default_interruptible_for_jobs + on_new_pipeline == 'interruptible' end end end diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb index 05d1da81b02ffc..c174372631c38d 100644 --- a/lib/gitlab/ci/yaml_processor/result.rb +++ b/lib/gitlab/ci/yaml_processor/result.rb @@ -134,7 +134,7 @@ def build_attributes(name) def default_interruptible # returns nil if workflow_auto_cancel is not defined - workflow_auto_cancel.try(:default_interruptible) + workflow_auto_cancel.try(:default_interruptible_for_jobs) end strong_memoize_attr :default_interruptible diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 701b7c197b8eaf..2a8225bfcc26db 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -1735,12 +1735,12 @@ def previous_commit_sha_from_ref(ref) let(:response) { execute_service } let(:pipeline) { response.payload } - context 'when on_new_pipeline is enabled' do + context 'when on_new_pipeline is set to interruptible' do let(:config) do <<~YAML workflow: auto_cancel: - on_new_pipeline: enabled + on_new_pipeline: interruptible test1: script: exit 0 @@ -1761,7 +1761,7 @@ def previous_commit_sha_from_ref(ref) it 'creates a pipeline with on_new_pipeline and builds.interruptible is true by default' do expect(pipeline).to be_persisted - expect(pipeline.pipeline_metadata.auto_cancel).to eq('on_new_pipeline' => 'enabled') + expect(pipeline.pipeline_metadata.auto_cancel).to eq('on_new_pipeline' => 'interruptible') expect(pipeline.builds.find_by(name: 'test1').metadata[:interruptible]).to eq(true) expect(pipeline.builds.find_by(name: 'test2').metadata[:interruptible]).to eq(true) -- GitLab From 5b1df451e8d0a01b22f0c366a0b65f8d4213dda8 Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Tue, 31 Oct 2023 13:29:12 +0100 Subject: [PATCH 3/4] Fix cancel pipeline condition --- .../ci/pipeline_creation/cancel_redundant_pipelines_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb index bdb2846e0f0c50..6b7c2d8b29e4aa 100644 --- a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb +++ b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb @@ -83,7 +83,7 @@ def auto_cancel_pipelines(pipeline_ids) log_info(cancelable_pipeline) cancel_pipeline(cancelable_pipeline, safe_cancellation: false) - elsif cancelable_pipeline.auto_cancel_on_new_pipeline == 'enabled' + elsif cancelable_pipeline.auto_cancel_on_new_pipeline == 'interruptible' log_info(cancelable_pipeline) cancel_pipeline(cancelable_pipeline, safe_cancellation: true) end -- GitLab From a8fff16c2a3d58cebf409bf0d4a522939bdbf326 Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Wed, 8 Nov 2023 14:23:54 +0100 Subject: [PATCH 4/4] Use auto_cancel_on_new_pipeline column --- app/models/ci/pipeline.rb | 6 +++++- app/models/ci/pipeline_metadata.rb | 9 ++++----- .../cancel_redundant_pipelines_service.rb | 2 +- .../ci_pipeline_metadata_auto_cancel.json | 11 ----------- ...3083034_add_auto_cancel_to_ci_pipeline_metadata.rb | 7 ------- ..._cancel_on_new_pipeline_to_ci_pipeline_metadata.rb | 9 +++++++++ db/schema_migrations/20231003083034 | 1 - db/schema_migrations/20231108122110 | 1 + db/structure.sql | 2 +- lib/gitlab/ci/config/entry/auto_cancel.rb | 2 +- 10 files changed, 22 insertions(+), 28 deletions(-) delete mode 100644 app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json delete mode 100644 db/migrate/20231003083034_add_auto_cancel_to_ci_pipeline_metadata.rb create mode 100644 db/migrate/20231108122110_add_auto_cancel_on_new_pipeline_to_ci_pipeline_metadata.rb delete mode 100644 db/schema_migrations/20231003083034 create mode 100644 db/schema_migrations/20231108122110 diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 10feef25aac5bc..2631d0c281059f 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -152,7 +152,7 @@ class Pipeline < Ci::ApplicationRecord accepts_nested_attributes_for :variables, reject_if: :persisted? delegate :full_path, to: :project, prefix: true - delegate :name, :auto_cancel_on_new_pipeline, to: :pipeline_metadata, allow_nil: true + delegate :name, to: :pipeline_metadata, allow_nil: true validates :sha, presence: { unless: :importing? } validates :ref, presence: { unless: :importing? } @@ -1401,6 +1401,10 @@ def merge_request_diff merge_request.merge_request_diff_for(merge_request_diff_sha) end + def auto_cancel_on_new_pipeline + pipeline_metadata&.auto_cancel_on_new_pipeline || Ci::PipelineMetadata::AUTO_CANCEL_ON_NEW_PIPELINE_DEFAULT + end + private def add_message(severity, content) diff --git a/app/models/ci/pipeline_metadata.rb b/app/models/ci/pipeline_metadata.rb index 27fde513645487..fa9c043a9b0758 100644 --- a/app/models/ci/pipeline_metadata.rb +++ b/app/models/ci/pipeline_metadata.rb @@ -4,16 +4,15 @@ module Ci class PipelineMetadata < Ci::ApplicationRecord self.primary_key = :pipeline_id + AUTO_CANCEL_ON_NEW_PIPELINE_OPTIONS = %w[interruptible conservative none].freeze + AUTO_CANCEL_ON_NEW_PIPELINE_DEFAULT = 'conservative' + belongs_to :pipeline, class_name: "Ci::Pipeline", inverse_of: :pipeline_metadata belongs_to :project, class_name: "Project", inverse_of: :pipeline_metadata validates :pipeline, presence: true validates :project, presence: true validates :name, length: { minimum: 1, maximum: 255 }, allow_nil: true - validates :auto_cancel, json_schema: { filename: "ci_pipeline_metadata_auto_cancel" }, allow_nil: true - - def auto_cancel_on_new_pipeline - auto_cancel&.dig('on_new_pipeline') - end + validates :auto_cancel_on_new_pipeline, inclusion: { in: AUTO_CANCEL_ON_NEW_PIPELINE_OPTIONS }, allow_nil: true end end diff --git a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb index 6b7c2d8b29e4aa..85c1bf6b72525f 100644 --- a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb +++ b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb @@ -78,7 +78,7 @@ def auto_cancel_pipelines(pipeline_ids) ::Ci::Pipeline .id_in(pipeline_ids) .each do |cancelable_pipeline| - if cancelable_pipeline.auto_cancel_on_new_pipeline.nil? || cancelable_pipeline.auto_cancel_on_new_pipeline == 'conservative' # legacy behavior + if cancelable_pipeline.auto_cancel_on_new_pipeline == 'conservative' next unless conservative_cancellable_pipeline_ids.include?(cancelable_pipeline.id) log_info(cancelable_pipeline) diff --git a/app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json b/app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json deleted file mode 100644 index 55b4fd607090b9..00000000000000 --- a/app/validators/json_schemas/ci_pipeline_metadata_auto_cancel.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "description": "CI Pipeline Metadata Auto Cancel", - "type": "object", - "properties": { - "on_new_pipeline": { - "type": "string", - "enum": ["interruptible", "conservative", "none"] - } - }, - "additionalProperties": false -} diff --git a/db/migrate/20231003083034_add_auto_cancel_to_ci_pipeline_metadata.rb b/db/migrate/20231003083034_add_auto_cancel_to_ci_pipeline_metadata.rb deleted file mode 100644 index fdefc9b9e1200f..00000000000000 --- a/db/migrate/20231003083034_add_auto_cancel_to_ci_pipeline_metadata.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -class AddAutoCancelToCiPipelineMetadata < Gitlab::Database::Migration[2.1] - def change - add_column :ci_pipeline_metadata, :auto_cancel, :jsonb - end -end diff --git a/db/migrate/20231108122110_add_auto_cancel_on_new_pipeline_to_ci_pipeline_metadata.rb b/db/migrate/20231108122110_add_auto_cancel_on_new_pipeline_to_ci_pipeline_metadata.rb new file mode 100644 index 00000000000000..0a9cd77bd65315 --- /dev/null +++ b/db/migrate/20231108122110_add_auto_cancel_on_new_pipeline_to_ci_pipeline_metadata.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddAutoCancelOnNewPipelineToCiPipelineMetadata < Gitlab::Database::Migration[2.2] + milestone '16.6' + + def change + add_column :ci_pipeline_metadata, :auto_cancel_on_new_pipeline, :smallint + end +end diff --git a/db/schema_migrations/20231003083034 b/db/schema_migrations/20231003083034 deleted file mode 100644 index 84ebc936c2319d..00000000000000 --- a/db/schema_migrations/20231003083034 +++ /dev/null @@ -1 +0,0 @@ -6742a67dcae4fbeb1d21f94273a810e01e9525a839373716bb1c5dc1d62c27be \ No newline at end of file diff --git a/db/schema_migrations/20231108122110 b/db/schema_migrations/20231108122110 new file mode 100644 index 00000000000000..daf9270da6dd36 --- /dev/null +++ b/db/schema_migrations/20231108122110 @@ -0,0 +1 @@ +b2e8eb4025e8c7b40d26b09c1941f501cd71699eb4b011add5db36e360f6f393 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 8fc29deebf8c38..88681901fc1eac 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14183,7 +14183,7 @@ CREATE TABLE ci_pipeline_metadata ( project_id bigint NOT NULL, pipeline_id bigint NOT NULL, name text, - auto_cancel jsonb, + auto_cancel_on_new_pipeline smallint, CONSTRAINT check_9d3665463c CHECK ((char_length(name) <= 255)) ); diff --git a/lib/gitlab/ci/config/entry/auto_cancel.rb b/lib/gitlab/ci/config/entry/auto_cancel.rb index 752440d056eb85..ce665af5c6a69e 100644 --- a/lib/gitlab/ci/config/entry/auto_cancel.rb +++ b/lib/gitlab/ci/config/entry/auto_cancel.rb @@ -9,7 +9,7 @@ class AutoCancel < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Validatable ALLOWED_KEYS = %i[on_new_pipeline].freeze # TODO: maybe the name should be `on_new_commit` instead. - ALLOWED_OPTIONS = %w[interruptible conservative none].freeze + ALLOWED_OPTIONS = ::Ci::PipelineMetadata::AUTO_CANCEL_ON_NEW_PIPELINE_OPTIONS attributes ALLOWED_KEYS -- GitLab