diff options
-rw-r--r-- | app/models/ci/pipeline_schedule.rb | 23 | ||||
-rw-r--r-- | changelogs/unreleased/fix-pipeline-schedule-edge-case.yml | 6 | ||||
-rw-r--r-- | spec/models/ci/pipeline_schedule_spec.rb | 51 |
3 files changed, 42 insertions, 38 deletions
diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index 6a4241c94bc..ba8cea0cea9 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -55,15 +55,20 @@ module Ci # This way, a schedule like `*/1 * * * *` won't be triggered in a short interval # when PipelineScheduleWorker runs irregularly by Sidekiq Memory Killer. def set_next_run_at - self.next_run_at = Gitlab::Ci::CronParser.new(Settings.cron_jobs['pipeline_schedule_worker']['cron'], - Time.zone.name) - .next_time_from(ideal_next_run_at) + now = Time.zone.now + ideal_next_run = ideal_next_run_from(now) + + self.next_run_at = if ideal_next_run == cron_worker_next_run_from(now) + ideal_next_run + else + cron_worker_next_run_from(ideal_next_run) + end end def schedule_next_run! save! # with set_next_run_at rescue ActiveRecord::RecordInvalid - update_attribute(:next_run_at, nil) # update without validation + update_column(:next_run_at, nil) # update without validation end def job_variables @@ -72,9 +77,15 @@ module Ci private - def ideal_next_run_at + def ideal_next_run_from(start_time) Gitlab::Ci::CronParser.new(cron, cron_timezone) - .next_time_from(Time.zone.now) + .next_time_from(start_time) + end + + def cron_worker_next_run_from(start_time) + Gitlab::Ci::CronParser.new(Settings.cron_jobs['pipeline_schedule_worker']['cron'], + Time.zone.name) + .next_time_from(start_time) end end end diff --git a/changelogs/unreleased/fix-pipeline-schedule-edge-case.yml b/changelogs/unreleased/fix-pipeline-schedule-edge-case.yml new file mode 100644 index 00000000000..2b7e3611567 --- /dev/null +++ b/changelogs/unreleased/fix-pipeline-schedule-edge-case.yml @@ -0,0 +1,6 @@ +--- +title: Fix pipeline schedule does not run correctly when it's scheduled at the same + time with the cron worker +merge_request: 29848 +author: +type: fixed diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index d7b81caddf5..aee43025288 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -88,23 +88,8 @@ describe Ci::PipelineSchedule do describe '#set_next_run_at' do let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly) } - let(:ideal_next_run_at) { pipeline_schedule.send(:ideal_next_run_at) } - - let(:expected_next_run_at) do - Gitlab::Ci::CronParser.new(Settings.cron_jobs['pipeline_schedule_worker']['cron'], Time.zone.name) - .next_time_from(ideal_next_run_at) - end - - let(:cron_worker_next_run_at) do - Gitlab::Ci::CronParser.new(Settings.cron_jobs['pipeline_schedule_worker']['cron'], Time.zone.name) - .next_time_from(Time.zone.now) - end - - context 'when creates new pipeline schedule' do - it 'updates next_run_at automatically' do - expect(pipeline_schedule.next_run_at).to eq(expected_next_run_at) - end - end + 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) } context 'when PipelineScheduleWorker runs at a specific interval' do before do @@ -129,7 +114,7 @@ describe Ci::PipelineSchedule do let(:pipeline_schedule) { create(:ci_pipeline_schedule, :every_minute) } it "updates next_run_at to the sidekiq worker's execution time" do - Timecop.freeze(2019, 06, 19, 12, 00) do + Timecop.freeze(Time.parse("2019-06-01 12:18:00+0000")) do expect(pipeline_schedule.next_run_at).to eq(cron_worker_next_run_at) end end @@ -157,9 +142,8 @@ describe Ci::PipelineSchedule do let(:new_cron) { '0 0 1 1 *' } it 'updates next_run_at automatically' do - pipeline_schedule.update!(cron: new_cron) - - expect(pipeline_schedule.next_run_at).to eq(expected_next_run_at) + expect { pipeline_schedule.update!(cron: new_cron) } + .to change { pipeline_schedule.next_run_at } end end end @@ -167,21 +151,24 @@ describe Ci::PipelineSchedule do describe '#schedule_next_run!' do let!(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly) } - context 'when reschedules after 10 days from now' do - let(:future_time) { 10.days.from_now } - let(:ideal_next_run_at) { pipeline_schedule.send(:ideal_next_run_at) } + before do + pipeline_schedule.update_column(:next_run_at, nil) + end - let(:expected_next_run_at) do - Gitlab::Ci::CronParser.new(Settings.cron_jobs['pipeline_schedule_worker']['cron'], Time.zone.name) - .next_time_from(ideal_next_run_at) + it 'updates next_run_at' do + expect { pipeline_schedule.schedule_next_run! } + .to change { pipeline_schedule.next_run_at } + end + + context 'when record is invalid' do + before do + allow(pipeline_schedule).to receive(:save!) { raise ActiveRecord::RecordInvalid.new(pipeline_schedule) } end - it 'points to proper next_run_at' do - Timecop.freeze(future_time) do - pipeline_schedule.schedule_next_run! + it 'nullifies the next run at' do + pipeline_schedule.schedule_next_run! - expect(pipeline_schedule.next_run_at).to eq(expected_next_run_at) - end + expect(pipeline_schedule.next_run_at).to be_nil end end end |