diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index f50be1957505dd09c0a86e89d185473ad32be67a..b099d4788fad1616fc87fca6ca04112018b931e7 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -96,7 +96,7 @@ def schedule def schedule_params params.require(:schedule) - .permit(:description, :cron, :cron_timezone, :ref, :active, + .permit(:description, :cron, :cron_timezone, :ref, :active, :scheduled_pipeline_skip_window, variables_attributes: [:id, :variable_type, :key, :secret_value, :_destroy]) end diff --git a/app/graphql/mutations/ci/pipeline_schedule/create.rb b/app/graphql/mutations/ci/pipeline_schedule/create.rb index 6849719598b36c375828e11d7c87398563af0c00..0022478637d74598befd541498161fe00de418ce 100644 --- a/app/graphql/mutations/ci/pipeline_schedule/create.rb +++ b/app/graphql/mutations/ci/pipeline_schedule/create.rb @@ -47,6 +47,9 @@ class Create < BaseMutation description: 'Inputs for the pipeline schedule.', experiment: { milestone: '17.10' } + argument :scheduled_pipeline_skip_window, GraphQL::Types::Int, required: false, + description: 'Scheduled pipeline skip window (in seconds).' + field :pipeline_schedule, Types::Ci::PipelineScheduleType, description: 'Created pipeline schedule.' diff --git a/app/graphql/mutations/ci/pipeline_schedule/update.rb b/app/graphql/mutations/ci/pipeline_schedule/update.rb index b738dd464ff6163ed08855d02e8f868d2b311dbd..25e8652e52a20ab21e18d47646fb027f9a4c6fca 100644 --- a/app/graphql/mutations/ci/pipeline_schedule/update.rb +++ b/app/graphql/mutations/ci/pipeline_schedule/update.rb @@ -24,6 +24,9 @@ class Update < Base For example: "Pacific Time (US & Canada)" (default: "UTC"). STR + argument :scheduled_pipeline_skip_window, GraphQL::Types::Int, required: false, + description: 'Scheduled pipeline skip window (in seconds).' + argument :ref, GraphQL::Types::String, required: false, description: 'Ref of the pipeline schedule.' diff --git a/app/graphql/types/ci/pipeline_schedule_type.rb b/app/graphql/types/ci/pipeline_schedule_type.rb index 0cdbf47df467a7efcb49eaece04a65e5cd634999..f0228c2967460497b214996da5b0ec024dfa34f4 100644 --- a/app/graphql/types/ci/pipeline_schedule_type.rb +++ b/app/graphql/types/ci/pipeline_schedule_type.rb @@ -63,6 +63,9 @@ class PipelineScheduleType < BaseObject null: false, description: 'Cron notation for the schedule.' + field :scheduled_pipeline_skip_window, GraphQL::Types::Int, null: true, + description: 'Scheduled pipeline skip window (in seconds).' + field :cron_timezone, GraphQL::Types::String, null: false, description: 'Timezone for the pipeline schedule.' diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 8a4e6649e68f2dbcd514b811363a9416d55f82c4..bbf6ab99e21f3858e629c11539a6d5d3409795c2 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -897,6 +897,14 @@ def needs_processing? .exists? end + def within_scheduled_window_after_creation? + return true unless pipeline_schedule + return true unless scheduled? + return true if pipeline_schedule.scheduled_pipeline_skip_window.to_i == 0 + + (created_at + pipeline_schedule.scheduled_pipeline_skip_window.to_i).future? + end + def has_yaml_errors? yaml_errors.present? end diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index db945919734de42f26370fe6e6509341685cec09..c4eddab430058fc2ea04f3e1f2f69f1069f40726 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -44,6 +44,7 @@ class PipelineSchedule < Ci::ApplicationRecord validates :variables, nested_attributes_duplicates: true validates :inputs, nested_attributes_duplicates: { child_attributes: %i[name] } validates :project, presence: true + validates :scheduled_pipeline_skip_window, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true validates :inputs, length: { maximum: Ci::Pipeline::INPUTS_LIMIT, @@ -156,6 +157,13 @@ def last_pipeline pipelines.last end + def within_window?(old_next_run_at) + return true if scheduled_pipeline_skip_window.to_i == 0 + + scheduled_pipeline_skip_window.to_i > 0 && + (old_next_run_at + scheduled_pipeline_skip_window.to_i).future? + end + private def ambiguous_ref? diff --git a/app/services/ci/pipeline_processing/atomic_processing_service.rb b/app/services/ci/pipeline_processing/atomic_processing_service.rb index 9484c5b0ffe818afd05c0255e8fd862f7136693a..a0f4474f57c82f1909cdec0b0c1ef0bc54de11ab 100644 --- a/app/services/ci/pipeline_processing/atomic_processing_service.rb +++ b/app/services/ci/pipeline_processing/atomic_processing_service.rb @@ -19,6 +19,11 @@ def initialize(pipeline) def execute return unless pipeline.needs_processing? + if Feature.enabled?(:scheduled_pipeline_skip_window, + pipeline.project) && !pipeline.within_scheduled_window_after_creation? + pipeline.skip + return + end # Run the process only if we can obtain an exclusive lease; returns nil if lease is unavailable success = try_obtain_lease { process! } diff --git a/app/workers/run_pipeline_schedule_worker.rb b/app/workers/run_pipeline_schedule_worker.rb index f0bd5a262cd77d7ee1b7e9cf42187086dd3bce18..0bb1392e625b0c51b095c15b0a1ce3c17d7dbad2 100644 --- a/app/workers/run_pipeline_schedule_worker.rb +++ b/app/workers/run_pipeline_schedule_worker.rb @@ -19,8 +19,18 @@ def perform(schedule_id, user_id, options = {}) return unless schedule_valid?(schedule, schedule_id, user, options) + old_next_run_at = schedule.next_run_at if Feature.enabled?(:scheduled_pipeline_skip_window, schedule.project) + update_next_run_at_for(schedule) if options['scheduling'] + if Feature.enabled?(:scheduled_pipeline_skip_window, schedule.project) && !schedule.within_window?(old_next_run_at) + message = 'Pipeline schedule runnable window exceeded.' + response = ServiceResponse.error(message: message) + log_error(schedule.id, message) + + return response + end + response = run_pipeline_schedule(schedule, user) log_error(schedule.id, response.message) if response&.error? diff --git a/config/feature_flags/gitlab_com_derisk/scheduled_pipeline_skip_window.yml b/config/feature_flags/gitlab_com_derisk/scheduled_pipeline_skip_window.yml new file mode 100644 index 0000000000000000000000000000000000000000..9c54831b4486e8d40c8e8435453e18b2305cf8c2 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/scheduled_pipeline_skip_window.yml @@ -0,0 +1,10 @@ +--- +name: scheduled_pipeline_skip_window +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/539393 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/202610 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/571092 +milestone: '18.4' +group: group::pipeline execution +type: gitlab_com_derisk +default_enabled: false diff --git a/db/migrate/20250924135718_add_scheduled_pipeline_skip_window_to_ci_pipeline_schedules.rb b/db/migrate/20250924135718_add_scheduled_pipeline_skip_window_to_ci_pipeline_schedules.rb new file mode 100644 index 0000000000000000000000000000000000000000..3d10114fc702a0ca61e632a359ab2952afb1a14d --- /dev/null +++ b/db/migrate/20250924135718_add_scheduled_pipeline_skip_window_to_ci_pipeline_schedules.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddScheduledPipelineSkipWindowToCiPipelineSchedules < Gitlab::Database::Migration[2.3] + milestone '18.5' + + def change + add_column :ci_pipeline_schedules, :scheduled_pipeline_skip_window, :integer, default: nil + end +end diff --git a/db/schema_migrations/20250924135718 b/db/schema_migrations/20250924135718 new file mode 100644 index 0000000000000000000000000000000000000000..9990d17dddd434cb86ef454a4c0272fa42be1781 --- /dev/null +++ b/db/schema_migrations/20250924135718 @@ -0,0 +1 @@ +dbc5e414d0594374ef3c62698fa0e94385bbfa01915f0a8c7c2de170cda52e45 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 73340652aedb470b4445c2974b78903e3a5a1499..169a860b7f273d441546ff661eca8980149dd486 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13773,6 +13773,7 @@ CREATE TABLE ci_pipeline_schedules ( active boolean DEFAULT true, created_at timestamp without time zone, updated_at timestamp without time zone, + scheduled_pipeline_skip_window integer, CONSTRAINT check_4a0f7b994d CHECK ((project_id IS NOT NULL)) ); diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 2ea8358d291a624d5fb3ce86805e2a01078fa7d9..cbdeb20d3ec3f9c6cde68fe84d11000aa81f0a20 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -10101,6 +10101,7 @@ Input type: `PipelineScheduleCreateInput` | `inputs` {{< icon name="warning-solid" >}} | [`[CiInputsInput!]`](#ciinputsinput) | **Deprecated**: **Status**: Experiment. Introduced in GitLab 17.10. | | `projectPath` | [`ID!`](#id) | Full path of the project the pipeline schedule is associated with. | | `ref` | [`String!`](#string) | Ref of the pipeline schedule. | +| `scheduledPipelineSkipWindow` | [`Int`](#int) | Scheduled pipeline skip window (in seconds). | | `variables` | [`[PipelineScheduleVariableInput!]`](#pipelineschedulevariableinput) | Variables for the pipeline schedule. | #### Fields @@ -10183,6 +10184,7 @@ Input type: `PipelineScheduleUpdateInput` | `id` | [`CiPipelineScheduleID!`](#cipipelinescheduleid) | ID of the pipeline schedule to mutate. | | `inputs` {{< icon name="warning-solid" >}} | [`[CiInputsInput!]`](#ciinputsinput) | **Deprecated**: **Status**: Experiment. Introduced in GitLab 17.11. | | `ref` | [`String`](#string) | Ref of the pipeline schedule. | +| `scheduledPipelineSkipWindow` | [`Int`](#int) | Scheduled pipeline skip window (in seconds). | | `variables` | [`[PipelineScheduleVariableInput!]`](#pipelineschedulevariableinput) | Variables for the pipeline schedule. | #### Fields @@ -38193,6 +38195,7 @@ Represents a pipeline schedule. | `ref` | [`String`](#string) | Ref of the pipeline schedule. | | `refForDisplay` | [`String`](#string) | Git ref for the pipeline schedule. | | `refPath` | [`String`](#string) | Path to the ref that triggered the pipeline. | +| `scheduledPipelineSkipWindow` | [`Int`](#int) | Scheduled pipeline skip window (in seconds). | | `updatedAt` | [`Time!`](#time) | Timestamp of when the pipeline schedule was last updated. | | `userPermissions` | [`PipelineSchedulePermissions!`](#pipelineschedulepermissions) | Permissions for the current user on the resource. | | `variables` | [`PipelineScheduleVariableConnection`](#pipelineschedulevariableconnection) | Pipeline schedule variables. (see [Connections](#connections)) | diff --git a/spec/fixtures/api/schemas/pipeline_schedule.json b/spec/fixtures/api/schemas/pipeline_schedule.json index 1a05770aacd1318ed49d00b99d13867791f5410e..1388adf8b2c55493932c3cd7b80f2792e83316d9 100644 --- a/spec/fixtures/api/schemas/pipeline_schedule.json +++ b/spec/fixtures/api/schemas/pipeline_schedule.json @@ -96,7 +96,10 @@ "type": "string" }, "public_email": { - "type": ["string", "null"] + "type": [ + "string", + "null" + ] }, "id": { "type": "integer" diff --git a/spec/graphql/types/ci/pipeline_schedule_type_spec.rb b/spec/graphql/types/ci/pipeline_schedule_type_spec.rb index f5c6c7ada730d76006e9955f99240848054d4617..a4ab26f99e2b9ed9640835b07a6b1c5aeec2b210 100644 --- a/spec/graphql/types/ci/pipeline_schedule_type_spec.rb +++ b/spec/graphql/types/ci/pipeline_schedule_type_spec.rb @@ -21,6 +21,7 @@ forTag nextRunAt realNextRun + scheduledPipelineSkipWindow cron cronTimezone userPermissions diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index bfc1248817f455dd6b27c68c9b2c4dd04c7bb638..db2712dc14877a8f3df69c73a4f157726e26558a 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -10,6 +10,7 @@ it { is_expected.to belong_to(:project) } it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_numericality_of(:scheduled_pipeline_skip_window) } it { is_expected.to belong_to(:owner) } it { is_expected.to have_many(:pipelines).dependent(:nullify) } @@ -32,6 +33,30 @@ end describe 'validations' do + it 'does allow nil value scheduled_pipeline_skip_window' do + pipeline_schedule = build(:ci_pipeline_schedule, scheduled_pipeline_skip_window: nil, project: project) + + expect(pipeline_schedule).to be_valid + end + + it 'does allow value greater 0 for scheduled_pipeline_skip_window' do + pipeline_schedule = build(:ci_pipeline_schedule, scheduled_pipeline_skip_window: 3600, project: project) + + expect(pipeline_schedule).to be_valid + end + + it 'does allow 0 for scheduled_pipeline_skip_window' do + pipeline_schedule = build(:ci_pipeline_schedule, scheduled_pipeline_skip_window: 0, project: project) + + expect(pipeline_schedule).to be_valid + end + + it 'does not allow negative values for scheduled_pipeline_skip_window' do + pipeline_schedule = build(:ci_pipeline_schedule, scheduled_pipeline_skip_window: -1, project: project) + + expect(pipeline_schedule).not_to be_valid + end + it 'does not allow invalid cron patterns' do pipeline_schedule = build(:ci_pipeline_schedule, cron: '0 0 0 * *', project: project) diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 2d7e97f77b7f5832338ebed2e38d5d59bfcb37de..67b40cfcf8e721458058254e62de9199a74c30eb 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -319,6 +319,60 @@ end end + describe '#within_scheduled_window_after_creation?' do + let_it_be(:project) { create(:project, :repository) } + let(:pipeline) { build(:ci_pipeline, project: project, created_at: created_at, pipeline_schedule: pipeline_schedule) } + let(:created_at) { Time.current } + + subject { pipeline.within_scheduled_window_after_creation? } + + context 'when pipeline has no schedule' do + let(:pipeline_schedule) { nil } + + it { is_expected.to be true } + end + + context 'when pipeline is not scheduled' do + let(:pipeline_schedule) { build(:ci_pipeline_schedule) } + + before do + allow(pipeline).to receive(:scheduled?).and_return(false) + end + + it { is_expected.to be true } + end + + context 'when schedule skip window is 0' do + let(:pipeline_schedule) { build(:ci_pipeline_schedule, scheduled_pipeline_skip_window: 0) } + + before do + allow(pipeline).to receive(:scheduled?).and_return(true) + end + + it { is_expected.to be true } + end + + context 'when schedule skip window is > 0' do + let(:pipeline_schedule) { build(:ci_pipeline_schedule, scheduled_pipeline_skip_window: 10.minutes) } + + before do + allow(pipeline).to receive(:scheduled?).and_return(true) + end + + context 'and pipeline is within the window' do + let(:created_at) { 5.minutes.ago } + + it { is_expected.to be true } + end + + context 'and pipeline is outside the window' do + let(:created_at) { 15.minutes.ago } + + it { is_expected.to be false } + end + end + end + describe '.trigger_status_change_subscriptions' do let(:pipeline) { build(:ci_pipeline, user: user) } diff --git a/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb b/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb index fef18cd2cdb7cb19cd18c50b95b5a1c81bb19e79..fee70153c1e652cff07663cceb0e82be74e8c1e3 100644 --- a/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb +++ b/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb @@ -505,6 +505,51 @@ def event_on_pipeline(event) expect(pipeline.reload.status).to eq 'pending' end end + + context 'when a delayed job is allowed to be automatically skipped' do + let!(:project_pipeline_schedule) { create(:ci_pipeline_schedule, project: project, scheduled_pipeline_skip_window: 2.days) } + + before do + pipeline.update!(pipeline_schedule: project_pipeline_schedule) + create_build('delayed', **delayed_options, allow_failure: true, stage_idx: 0) + create_build('job', stage_idx: 1) + + allow(Ci::BuildScheduleWorker).to receive(:perform_at) + end + + it 'skips the pipeline when it is out of the window' do + travel_to 3.days.from_now do + expect(process_pipeline).to be_truthy + expect(builds_names_and_statuses).to eq({ delayed: 'scheduled' }) + enqueue_scheduled('delayed') + end + expect(pipeline.reload.status).to eq 'skipped' + end + + it 'does not skip the pipeline when it within the window' do + travel_to 1.day.from_now do + expect(process_pipeline).to be_truthy + expect(builds_names_and_statuses).to eq({ delayed: 'scheduled' }) + enqueue_scheduled('delayed') + end + expect(pipeline.reload.status).to eq 'pending' + end + + context 'when derisk feature flag is disabled' do + before do + stub_feature_flags(scheduled_pipeline_skip_window: false) + end + + it 'does not skip the pipeline when it is out of the window' do + travel_to 3.days.from_now do + expect(process_pipeline).to be_truthy + expect(builds_names_and_statuses).to eq({ delayed: 'scheduled' }) + enqueue_scheduled('delayed') + end + expect(pipeline.reload.status).to eq 'pending' + end + end + end end context 'when an exception is raised during a persistent ref creation' do diff --git a/spec/workers/run_pipeline_schedule_worker_spec.rb b/spec/workers/run_pipeline_schedule_worker_spec.rb index cbf837ac5dda34739aa1eaf2cee4551b8469dd85..146811740e0f47d68af5a56246cb168c164a2945 100644 --- a/spec/workers/run_pipeline_schedule_worker_spec.rb +++ b/spec/workers/run_pipeline_schedule_worker_spec.rb @@ -136,6 +136,29 @@ end end + context "when pipeline is outside of allowed scheduled window" do + let(:error_response) do + instance_double(ServiceResponse, + error?: true, + success?: false, + message: 'Pipeline schedule runnable window exceeded.' + ) + end + + before do + pipeline_schedule.update!(scheduled_pipeline_skip_window: 1) + allow(ServiceResponse).to receive(:error) + .with(message: 'Pipeline schedule runnable window exceeded.') + .and_return(error_response) + allow(Time).to receive(:now).and_return(Time.now.utc + 30.days) + end + + it "returns the a timeout response" do + expect(worker.perform(pipeline_schedule.id, + user.id)).to eq(error_response) + end + end + describe "#run_pipeline_schedule" do let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService, execute: service_response) } let(:service_response) { instance_double(ServiceResponse, payload: pipeline, error?: false) }