Commit 9c5c893b authored by Furkan Ayhan's avatar Furkan Ayhan Committed by Fabio Pitino

Add limit for daily scheduled pipelines for projects [RUN ALL RSPEC] [RUN AS-IF-FOSS]

parent ff4a37dc
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
module Ci module Ci
class PipelineSchedule < ApplicationRecord class PipelineSchedule < ApplicationRecord
extend Gitlab::Ci::Model extend Gitlab::Ci::Model
extend ::Gitlab::Utils::Override
include Importable include Importable
include StripAttribute include StripAttribute
include CronSchedulable include CronSchedulable
...@@ -55,6 +56,13 @@ module Ci ...@@ -55,6 +56,13 @@ module Ci
variables&.map(&:to_runner_variable) || [] variables&.map(&:to_runner_variable) || []
end end
override :set_next_run_at
def set_next_run_at
self.next_run_at = ::Ci::PipelineSchedules::CalculateNextRunService # rubocop: disable CodeReuse/ServiceClass
.new(project)
.execute(self, fallback_method: method(:calculate_next_run_at))
end
private private
def worker_cron_expression def worker_cron_expression
......
...@@ -4,23 +4,28 @@ module CronSchedulable ...@@ -4,23 +4,28 @@ module CronSchedulable
extend ActiveSupport::Concern extend ActiveSupport::Concern
include Schedulable include Schedulable
def set_next_run_at
self.next_run_at = calculate_next_run_at
end
private
## ##
# The `next_run_at` column is set to the actual execution date of worker that # The `next_run_at` column is set to the actual execution date of worker that
# triggers the schedule. This way, a schedule like `*/1 * * * *` won't be triggered # triggers the schedule. This way, a schedule like `*/1 * * * *` won't be triggered
# in a short interval when the worker runs irregularly by Sidekiq Memory Killer. # in a short interval when the worker runs irregularly by Sidekiq Memory Killer.
def set_next_run_at def calculate_next_run_at
now = Time.zone.now now = Time.zone.now
ideal_next_run = ideal_next_run_from(now) ideal_next_run = ideal_next_run_from(now)
self.next_run_at = if ideal_next_run == cron_worker_next_run_from(now) if ideal_next_run == cron_worker_next_run_from(now)
ideal_next_run ideal_next_run
else else
cron_worker_next_run_from(ideal_next_run) cron_worker_next_run_from(ideal_next_run)
end end
end end
private
def ideal_next_run_from(start_time) def ideal_next_run_from(start_time)
next_time_from(start_time, cron, cron_timezone) next_time_from(start_time, cron, cron_timezone)
end end
......
# frozen_string_literal: true
module Ci
module PipelineSchedules
class CalculateNextRunService < BaseService
include Gitlab::Utils::StrongMemoize
def execute(schedule, fallback_method:)
@schedule = schedule
return fallback_method.call unless ::Feature.enabled?(:ci_daily_limit_for_pipeline_schedules, project, default_enabled: :yaml)
return fallback_method.call unless plan_cron&.cron_valid?
now = Time.zone.now
schedule_next_run = schedule_cron.next_time_from(now)
return schedule_next_run if worker_cron.match?(schedule_next_run) && plan_cron.match?(schedule_next_run)
plan_next_run = plan_cron.next_time_from(now)
return plan_next_run if worker_cron.match?(plan_next_run)
worker_next_run = worker_cron.next_time_from(now)
return worker_next_run if plan_cron.match?(worker_next_run)
worker_cron.next_time_from(plan_next_run)
end
private
def schedule_cron
strong_memoize(:schedule_cron) do
Gitlab::Ci::CronParser.new(@schedule.cron, @schedule.cron_timezone)
end
end
def worker_cron
strong_memoize(:worker_cron) do
Gitlab::Ci::CronParser.new(worker_cron_expression, Time.zone.name)
end
end
def plan_cron
strong_memoize(:plan_cron) do
daily_scheduled_pipeline_limit = project.actual_limits.limit_for(:ci_daily_pipeline_schedule_triggers)
next unless daily_scheduled_pipeline_limit
every_x_minutes = (1.day.in_minutes / daily_scheduled_pipeline_limit).to_i
Gitlab::Ci::CronParser.parse_natural("every #{every_x_minutes} minutes", Time.zone.name)
end
end
def worker_cron_expression
Settings.cron_jobs['pipeline_schedule_worker']['cron']
end
end
end
end
---
name: ci_daily_limit_for_pipeline_schedules
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62826
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/332333
milestone: '14.0'
type: development
group: group::pipeline authoring
default_enabled: false
# frozen_string_literal: true
class AddCiDailyPipelineScheduleTriggersToPlanLimits < ActiveRecord::Migration[6.0]
def change
add_column(:plan_limits, :ci_daily_pipeline_schedule_triggers, :integer, default: 0, null: false)
end
end
# frozen_string_literal: true
class InsertCiDailyPipelineScheduleTriggersPlanLimits < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
EVERY_5_MINUTES = (1.day.in_minutes / 5).to_i
EVERY_HOUR = 1.day.in_hours.to_i
def up
return unless Gitlab.com?
create_or_update_plan_limit('ci_daily_pipeline_schedule_triggers', 'free', EVERY_HOUR)
create_or_update_plan_limit('ci_daily_pipeline_schedule_triggers', 'bronze', EVERY_5_MINUTES)
create_or_update_plan_limit('ci_daily_pipeline_schedule_triggers', 'silver', EVERY_5_MINUTES)
create_or_update_plan_limit('ci_daily_pipeline_schedule_triggers', 'gold', EVERY_5_MINUTES)
end
def down
return unless Gitlab.com?
create_or_update_plan_limit('ci_daily_pipeline_schedule_triggers', 'free', 0)
create_or_update_plan_limit('ci_daily_pipeline_schedule_triggers', 'bronze', 0)
create_or_update_plan_limit('ci_daily_pipeline_schedule_triggers', 'silver', 0)
create_or_update_plan_limit('ci_daily_pipeline_schedule_triggers', 'gold', 0)
end
end
ae2829a06f02ff3e1adc977f5e789b17d1f760e6aaa40be44586cc6a90870c4a
\ No newline at end of file
824e0930de14587f6ccaeb6b5fbec16676d243550a2dfd3a5999b67dfc16d4c8
\ No newline at end of file
...@@ -16242,7 +16242,8 @@ CREATE TABLE plan_limits ( ...@@ -16242,7 +16242,8 @@ CREATE TABLE plan_limits (
helm_max_file_size bigint DEFAULT 5242880 NOT NULL, helm_max_file_size bigint DEFAULT 5242880 NOT NULL,
ci_registered_group_runners integer DEFAULT 1000 NOT NULL, ci_registered_group_runners integer DEFAULT 1000 NOT NULL,
ci_registered_project_runners integer DEFAULT 1000 NOT NULL, ci_registered_project_runners integer DEFAULT 1000 NOT NULL,
web_hook_calls integer DEFAULT 0 NOT NULL web_hook_calls integer DEFAULT 0 NOT NULL,
ci_daily_pipeline_schedule_triggers integer DEFAULT 0 NOT NULL
); );
CREATE SEQUENCE plan_limits_id_seq CREATE SEQUENCE plan_limits_id_seq
...@@ -6,6 +6,10 @@ module Gitlab ...@@ -6,6 +6,10 @@ module Gitlab
VALID_SYNTAX_SAMPLE_TIME_ZONE = 'UTC' VALID_SYNTAX_SAMPLE_TIME_ZONE = 'UTC'
VALID_SYNTAX_SAMPLE_CRON = '* * * * *' VALID_SYNTAX_SAMPLE_CRON = '* * * * *'
def self.parse_natural(expression, cron_timezone = 'UTC')
new(Fugit::Nat.parse(expression)&.original, cron_timezone)
end
def initialize(cron, cron_timezone = 'UTC') def initialize(cron, cron_timezone = 'UTC')
@cron = cron @cron = cron
@cron_timezone = timezone_name(cron_timezone) @cron_timezone = timezone_name(cron_timezone)
...@@ -27,6 +31,10 @@ module Gitlab ...@@ -27,6 +31,10 @@ module Gitlab
try_parse_cron(VALID_SYNTAX_SAMPLE_CRON, @cron_timezone).present? try_parse_cron(VALID_SYNTAX_SAMPLE_CRON, @cron_timezone).present?
end end
def match?(time)
cron_line.match?(time)
end
private private
def timezone_name(timezone) def timezone_name(timezone)
......
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'migrate', '20210526190553_insert_ci_daily_pipeline_schedule_triggers_plan_limits.rb')
RSpec.describe InsertCiDailyPipelineScheduleTriggersPlanLimits do
let_it_be(:plans) { table(:plans) }
let_it_be(:plan_limits) { table(:plan_limits) }
context 'when on Gitlab.com' do
let(:free_plan) { plans.create!(name: 'free') }
let(:bronze_plan) { plans.create!(name: 'bronze') }
let(:silver_plan) { plans.create!(name: 'silver') }
let(:gold_plan) { plans.create!(name: 'gold') }
before do
allow(Gitlab).to receive(:com?).and_return(true)
plan_limits.create!(plan_id: free_plan.id)
plan_limits.create!(plan_id: bronze_plan.id)
plan_limits.create!(plan_id: silver_plan.id)
plan_limits.create!(plan_id: gold_plan.id)
end
it 'correctly migrates up and down' do
reversible_migration do |migration|
migration.before -> {
expect(plan_limits.pluck(:plan_id, :ci_daily_pipeline_schedule_triggers)).to contain_exactly(
[free_plan.id, 0],
[bronze_plan.id, 0],
[silver_plan.id, 0],
[gold_plan.id, 0]
)
}
migration.after -> {
expect(plan_limits.pluck(:plan_id, :ci_daily_pipeline_schedule_triggers)).to contain_exactly(
[free_plan.id, 24],
[bronze_plan.id, 288],
[silver_plan.id, 288],
[gold_plan.id, 288]
)
}
end
end
end
context 'when on self hosted' do
let(:free_plan) { plans.create!(name: 'free') }
before do
allow(Gitlab).to receive(:com?).and_return(false)
plan_limits.create!(plan_id: free_plan.id)
end
it 'does nothing' do
reversible_migration do |migration|
migration.before -> {
expect(plan_limits.pluck(:plan_id, :ci_daily_pipeline_schedule_triggers)).to contain_exactly(
[free_plan.id, 0]
)
}
migration.after -> {
expect(plan_limits.pluck(:plan_id, :ci_daily_pipeline_schedule_triggers)).to contain_exactly(
[free_plan.id, 0]
)
}
end
end
end
end
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Ci::PipelineSchedule do RSpec.describe Ci::PipelineSchedule do
let_it_be(:project) { create_default(:project) }
subject { build(:ci_pipeline_schedule) } subject { build(:ci_pipeline_schedule) }
it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:project) }
...@@ -18,7 +20,7 @@ RSpec.describe Ci::PipelineSchedule do ...@@ -18,7 +20,7 @@ RSpec.describe Ci::PipelineSchedule do
it { is_expected.to respond_to(:next_run_at) } it { is_expected.to respond_to(:next_run_at) }
it_behaves_like 'includes Limitable concern' do it_behaves_like 'includes Limitable concern' do
subject { build(:ci_pipeline_schedule) } subject { build(:ci_pipeline_schedule, project: project) }
end end
describe 'validations' do describe 'validations' do
...@@ -103,26 +105,46 @@ RSpec.describe Ci::PipelineSchedule do ...@@ -103,26 +105,46 @@ RSpec.describe Ci::PipelineSchedule do
end end
describe '#set_next_run_at' do describe '#set_next_run_at' do
let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly) } using RSpec::Parameterized::TableSyntax
let(:ideal_next_run_at) { pipeline_schedule.send(:ideal_next_run_from, Time.zone.now) }
let(:cron_worker_next_run_at) { pipeline_schedule.send(:cron_worker_next_run_from, Time.zone.now) } where(:worker_cron, :schedule_cron, :plan_limit, :ff_enabled, :now, :result) do
'0 1 2 3 *' | '0 1 * * *' | nil | true | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0)
'0 1 2 3 *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0)
'0 1 2 3 *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | false | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0)
'*/5 * * * *' | '*/1 * * * *' | nil | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 5)
'*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0)
'*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | false | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 5)
'*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 10).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 10)
'*/5 * * * *' | '*/1 * * * *' | 200 | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 10)
'*/5 * * * *' | '*/1 * * * *' | 200 | false | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 5)
'*/5 * * * *' | '0 * * * *' | nil | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5)
'*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 10).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0)
'*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 10).to_i | false | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5)
'*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0)
'*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 2.hours.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5)
end
with_them do
let(:pipeline_schedule) { create(:ci_pipeline_schedule, cron: schedule_cron) }
context 'when PipelineScheduleWorker runs at a specific interval' do
before do before do
allow(Settings).to receive(:cron_jobs) do allow(Settings).to receive(:cron_jobs) do
{ { 'pipeline_schedule_worker' => { 'cron' => worker_cron } }
'pipeline_schedule_worker' => {
'cron' => '0 1 2 3 *'
}
}
end end
create(:plan_limits, :default_plan, ci_daily_pipeline_schedule_triggers: plan_limit) if plan_limit
stub_feature_flags(ci_daily_limit_for_pipeline_schedules: false) unless ff_enabled
# Setting this here to override initial save with the current time
pipeline_schedule.next_run_at = now
end end
it "updates next_run_at to the sidekiq worker's execution time" do it 'updates next_run_at' do
expect(pipeline_schedule.next_run_at.min).to eq(0) travel_to(now) do
expect(pipeline_schedule.next_run_at.hour).to eq(1) pipeline_schedule.set_next_run_at
expect(pipeline_schedule.next_run_at.day).to eq(2)
expect(pipeline_schedule.next_run_at.month).to eq(3) expect(pipeline_schedule.next_run_at).to eq(result)
end
end end
end end
......
...@@ -211,6 +211,7 @@ RSpec.describe PlanLimits do ...@@ -211,6 +211,7 @@ RSpec.describe PlanLimits do
storage_size_limit storage_size_limit
daily_invites daily_invites
web_hook_calls web_hook_calls
ci_daily_pipeline_schedule_triggers
] + disabled_max_artifact_size_columns ] + disabled_max_artifact_size_columns
end end
......
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