diff options
| -rw-r--r-- | app/models/ci/trigger_schedule.rb | 5 | ||||
| -rw-r--r-- | app/workers/trigger_schedule_worker.rb | 2 | ||||
| -rw-r--r-- | lib/ci/cron_parser.rb | 2 | ||||
| -rw-r--r-- | spec/lib/ci/cron_parser_spec.rb | 25 | ||||
| -rw-r--r-- | spec/models/ci/trigger_schedule_spec.rb | 93 |
5 files changed, 55 insertions, 72 deletions
diff --git a/app/models/ci/trigger_schedule.rb b/app/models/ci/trigger_schedule.rb index fc144d3958d..b861f41fed1 100644 --- a/app/models/ci/trigger_schedule.rb +++ b/app/models/ci/trigger_schedule.rb @@ -18,7 +18,6 @@ module Ci after_create :schedule_next_run! def schedule_next_run! - # puts "cron: #{cron.inspect} | cron_time_zone: #{cron_time_zone.inspect}" next_time = Ci::CronParser.new(cron, cron_time_zone).next_time_from(Time.now) if next_time.present? && !less_than_1_hour_from_now?(next_time) @@ -29,11 +28,9 @@ module Ci def real_next_run(worker_cron: nil, worker_time_zone: nil) worker_cron = Settings.cron_jobs['trigger_schedule_worker']['cron'] unless worker_cron.present? worker_time_zone = Time.zone.name unless worker_time_zone.present? - # puts "real_next_run: worker_cron: #{worker_cron.inspect} | worker_time_zone: #{worker_time_zone.inspect}" worker_next_time = Ci::CronParser.new(worker_cron, worker_time_zone).next_time_from(Time.now) - # puts "real_next_run: next_run_at: #{next_run_at.inspect} | worker_next_time: #{worker_next_time.inspect}" if next_run_at > worker_next_time next_run_at else @@ -64,7 +61,7 @@ module Ci def check_ref if !ref.present? self.errors.add(:ref, " is empty") - elsif project.repository.ref_exists?(ref) + elsif !project.repository.branch_exists?(ref) self.errors.add(:ref, " does not exist") end end diff --git a/app/workers/trigger_schedule_worker.rb b/app/workers/trigger_schedule_worker.rb index 9216103b8da..36e640592a7 100644 --- a/app/workers/trigger_schedule_worker.rb +++ b/app/workers/trigger_schedule_worker.rb @@ -4,8 +4,6 @@ class TriggerScheduleWorker def perform Ci::TriggerSchedule.where("next_run_at < ?", Time.now).find_each do |trigger_schedule| - next if Ci::Pipeline.where(project: trigger_schedule.project, ref: trigger_schedule.ref).running_or_pending.count > 0 - begin Ci::CreateTriggerRequestService.new.execute(trigger_schedule.project, trigger_schedule.trigger, diff --git a/lib/ci/cron_parser.rb b/lib/ci/cron_parser.rb index 2919543bbef..eb348306436 100644 --- a/lib/ci/cron_parser.rb +++ b/lib/ci/cron_parser.rb @@ -11,7 +11,7 @@ module Ci def next_time_from(time) cronLine = try_parse_cron(@cron, @cron_time_zone) if cronLine.present? - cronLine.next_time(time) + cronLine.next_time(time).in_time_zone(Time.zone) else nil end diff --git a/spec/lib/ci/cron_parser_spec.rb b/spec/lib/ci/cron_parser_spec.rb index dd1449b5b02..11d1e1c8a78 100644 --- a/spec/lib/ci/cron_parser_spec.rb +++ b/spec/lib/ci/cron_parser_spec.rb @@ -8,10 +8,10 @@ module Ci context 'when cron and cron_time_zone are valid' do context 'when specific time' do let(:cron) { '3 4 5 6 *' } - let(:cron_time_zone) { 'Europe/London' } + let(:cron_time_zone) { 'UTC' } it 'returns exact time in the future' do - expect(subject).to be > Time.now.in_time_zone(cron_time_zone) + expect(subject).to be > Time.now expect(subject.min).to eq(3) expect(subject.hour).to eq(4) expect(subject.day).to eq(5) @@ -21,20 +21,20 @@ module Ci context 'when specific day of week' do let(:cron) { '* * * * 0' } - let(:cron_time_zone) { 'Europe/London' } + let(:cron_time_zone) { 'UTC' } it 'returns exact day of week in the future' do - expect(subject).to be > Time.now.in_time_zone(cron_time_zone) + expect(subject).to be > Time.now expect(subject.wday).to eq(0) end end context 'when slash used' do let(:cron) { '*/10 */6 */10 */10 *' } - let(:cron_time_zone) { 'US/Pacific' } + let(:cron_time_zone) { 'UTC' } it 'returns exact minute' do - expect(subject).to be > Time.now.in_time_zone(cron_time_zone) + expect(subject).to be > Time.now expect(subject.min).to be_in([0, 10, 20, 30, 40, 50]) expect(subject.hour).to be_in([0, 6, 12, 18]) expect(subject.day).to be_in([1, 11, 21, 31]) @@ -44,22 +44,25 @@ module Ci context 'when range used' do let(:cron) { '0,20,40 * 1-5 * *' } - let(:cron_time_zone) { 'US/Pacific' } + let(:cron_time_zone) { 'UTC' } it 'returns next time from now' do - expect(subject).to be > Time.now.in_time_zone(cron_time_zone) + expect(subject).to be > Time.now expect(subject.min).to be_in([0, 20, 40]) expect(subject.day).to be_in((1..5).to_a) end end context 'when cron_time_zone is US/Pacific' do - let(:cron) { '* * * * *' } + let(:cron) { '0 0 * * *' } let(:cron_time_zone) { 'US/Pacific' } it 'returns next time from now' do - expect(subject).to be > Time.now.in_time_zone(cron_time_zone) - expect(subject.utc_offset/60/60).to eq(-7) + expect(subject).to be > Time.now + end + + it 'converts time in server time zone' do + expect(subject.hour).to eq(7) end end end diff --git a/spec/models/ci/trigger_schedule_spec.rb b/spec/models/ci/trigger_schedule_spec.rb index d47ab529bc0..da23611f1a0 100644 --- a/spec/models/ci/trigger_schedule_spec.rb +++ b/spec/models/ci/trigger_schedule_spec.rb @@ -8,11 +8,36 @@ describe Ci::TriggerSchedule, models: true do # it { is_expected.to validate_presence_of :cron_time_zone } it { is_expected.to respond_to :ref } - it 'should validate less_than_1_hour_from_now' do + it 'should validate ref existence' do trigger_schedule = create(:ci_trigger_schedule, :cron_nightly_build) - trigger_schedule.cron = '* * * * *' + trigger_schedule.trigger.ref = 'invalid-ref' trigger_schedule.valid? - expect(trigger_schedule.errors[:cron].first).to include('can not be less than 1 hour') + expect(trigger_schedule.errors[:ref].first).to include('does not exist') + end + + describe 'cron limitation' do + let(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build) } + + before do + trigger_schedule.cron = cron + trigger_schedule.valid? + end + + context 'when every hour' do + let(:cron) { '0 * * * *' } # 00:00, 01:00, 02:00, ..., 23:00 + + it 'fails' do + expect(trigger_schedule.errors[:cron].first).to include('can not be less than 1 hour') + end + end + + context 'when each six hours' do + let(:cron) { '0 */6 * * *' } # 00:00, 06:00, 12:00, 18:00 + + it 'succeeds' do + expect(trigger_schedule.errors[:cron]).to be_empty + end + end end describe '#schedule_next_run!' do @@ -31,65 +56,25 @@ describe Ci::TriggerSchedule, models: true do end describe '#real_next_run' do - subject { trigger_schedule.real_next_run(worker_cron: worker_cron, worker_time_zone: worker_time_zone) } - - context 'when next_run_at > worker_next_time' do - let(:worker_cron) { '0 */12 * * *' } # each 00:00, 12:00 - let(:worker_time_zone) { 'UTC' } - let(:trigger_schedule) { create(:ci_trigger_schedule, :cron_weekly_build, cron_time_zone: user_time_zone, trigger: trigger) } + let(:trigger_schedule) { create(:ci_trigger_schedule, cron: user_cron, cron_time_zone: 'UTC') } - context 'when user is in Europe/London(+00:00)' do - let(:user_time_zone) { 'Europe/London' } + subject { trigger_schedule.real_next_run(worker_cron: worker_cron, worker_time_zone: 'UTC') } - it 'returns next_run_at' do - is_expected.to eq(trigger_schedule.next_run_at) - end - end - - context 'when user is in Asia/Hong_Kong(+08:00)' do - let(:user_time_zone) { 'Asia/Hong_Kong' } - - it 'returns next_run_at' do - is_expected.to eq(trigger_schedule.next_run_at) - end - end - - context 'when user is in Canada/Pacific(-08:00)' do - let(:user_time_zone) { 'Canada/Pacific' } + context 'when next_run_at > worker_next_time' do + let(:worker_cron) { '* * * * *' } # every minutes + let(:user_cron) { '0 0 1 1 *' } # every 00:00, January 1st - it 'returns next_run_at' do - is_expected.to eq(trigger_schedule.next_run_at) - end + it 'returns next_run_at' do + is_expected.to eq(trigger_schedule.next_run_at) end end context 'when worker_next_time > next_run_at' do - let(:worker_cron) { '0 0 */2 * *' } # every 2 days - let(:worker_time_zone) { 'UTC' } - let(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build, cron_time_zone: user_time_zone, trigger: trigger) } - - context 'when user is in Europe/London(+00:00)' do - let(:user_time_zone) { 'Europe/London' } - - it 'returns worker_next_time' do - is_expected.to eq(Ci::CronParser.new(worker_cron, worker_time_zone).next_time_from_now) - end - end - - context 'when user is in Asia/Hong_Kong(+08:00)' do - let(:user_time_zone) { 'Asia/Hong_Kong' } - - it 'returns worker_next_time' do - is_expected.to eq(Ci::CronParser.new(worker_cron, worker_time_zone).next_time_from_now) - end - end - - context 'when user is in Canada/Pacific(-08:00)' do - let(:user_time_zone) { 'Canada/Pacific' } + let(:worker_cron) { '0 0 1 1 *' } # every 00:00, January 1st + let(:user_cron) { '0 */6 * * *' } # each six hours - it 'returns worker_next_time' do - is_expected.to eq(Ci::CronParser.new(worker_cron, worker_time_zone).next_time_from_now) - end + it 'returns worker_next_time' do + is_expected.to eq(Ci::CronParser.new(worker_cron, 'UTC').next_time_from(Time.now)) end end end |