diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index 6d7cc83971e..cf6e53c4ca4 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -28,10 +28,18 @@ module Ci !active? end + def deactivate! + update_attribute(:active, false) + end + def importing_or_inactive? importing? || inactive? end + def runnable_by_owner? + Ability.allowed?(owner, :create_pipeline, project) + end + def set_next_run_at self.next_run_at = Gitlab::Ci::CronParser.new(cron, cron_timezone).next_time_from(Time.now) end diff --git a/app/workers/pipeline_schedule_worker.rb b/app/workers/pipeline_schedule_worker.rb index a449a765f7b..7eb0e84acb2 100644 --- a/app/workers/pipeline_schedule_worker.rb +++ b/app/workers/pipeline_schedule_worker.rb @@ -3,8 +3,14 @@ class PipelineScheduleWorker include CronjobQueue def perform - Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now).find_each do |schedule| + Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now) + .preload(:owner, :project).find_each do |schedule| begin + unless schedule.runnable_by_owner? + schedule.deactivate! + next + end + Ci::CreatePipelineService.new(schedule.project, schedule.owner, ref: schedule.ref) diff --git a/changelogs/unreleased/zj-pipeline-schedule-owner.yml b/changelogs/unreleased/zj-pipeline-schedule-owner.yml new file mode 100644 index 00000000000..be704e173ab --- /dev/null +++ b/changelogs/unreleased/zj-pipeline-schedule-owner.yml @@ -0,0 +1,4 @@ +--- +title: Add foreign key for pipeline schedule owner +merge_request: 11233 +author: diff --git a/doc/ci/pipeline_schedules.md b/doc/ci/pipeline_schedules.md index 0a9b0e7173f..73451da6c0c 100644 --- a/doc/ci/pipeline_schedules.md +++ b/doc/ci/pipeline_schedules.md @@ -35,6 +35,10 @@ To change the Sidekiq worker's frequency, you have to edit the `trigger_schedule value in your `gitlab.rb` and restart GitLab. The Sidekiq worker's configuration on GiLab.com is able to be looked up at [here](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/gitlab.yml.example#L185). - Cron notation is parsed by [Rufus-Scheduler](https://github.com/jmettraux/rufus-scheduler). +- When the owner of the schedule does not have the ability to create pipelines +anymore, due to e.g. being blocked or removed from the project, the schedule is +deactivated. Another user can take ownership and activate it, so the schedule is +run again. [ce-10533]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10533 [ce-10853]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10853 diff --git a/spec/workers/pipeline_schedule_worker_spec.rb b/spec/workers/pipeline_schedule_worker_spec.rb index 91d5a16993f..9c650354d72 100644 --- a/spec/workers/pipeline_schedule_worker_spec.rb +++ b/spec/workers/pipeline_schedule_worker_spec.rb @@ -11,40 +11,53 @@ describe PipelineScheduleWorker do end before do - project.add_master(user) - stub_ci_pipeline_to_return_yaml_file + + pipeline_schedule.update_column(:next_run_at, 1.day.ago) end - context 'when there is a scheduled pipeline within next_run_at' do - let(:next_run_at) { 2.days.ago } - + context 'when the schedule is runnable by the user' do before do - pipeline_schedule.update_column(:next_run_at, next_run_at) + project.add_master(user) end - it 'creates a new pipeline' do - expect { subject }.to change { project.pipelines.count }.by(1) + context 'when there is a scheduled pipeline within next_run_at' do + it 'creates a new pipeline' do + expect { subject }.to change { project.pipelines.count }.by(1) + end + + it 'updates the next_run_at field' do + subject + + expect(pipeline_schedule.reload.next_run_at).to be > Time.now + end + + it 'sets the schedule on the pipeline' do + subject + + expect(project.pipelines.last.pipeline_schedule).to eq(pipeline_schedule) + end end - it 'updates the next_run_at field' do - subject + context 'inactive schedule' do + before do + pipeline_schedule.deactivate! + end - expect(pipeline_schedule.reload.next_run_at).to be > Time.now - end - - it 'sets the schedule on the pipeline' do - subject - expect(project.pipelines.last.pipeline_schedule).to eq(pipeline_schedule) + it 'does not creates a new pipeline' do + expect { subject }.not_to change { project.pipelines.count } + end end end - context 'inactive schedule' do - before do - pipeline_schedule.update(active: false) + context 'when the schedule is not runnable by the user' do + it 'deactivates the schedule' do + subject + + expect(pipeline_schedule.reload.active).to be_falsy end - it 'does not creates a new pipeline' do + it 'does not schedule a pipeline' do expect { subject }.not_to change { project.pipelines.count } end end