Merge branch 'zj-pipeline-schedule-owner' into 'master'

Foreign key for Pipeline schedule owner

Closes #31932

See merge request !11233

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

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)

changelogs/unreleased/zj-pipeline-schedule-owner.yml

+---
+title: Add foreign key for pipeline schedule owner
+merge_request: 11233
+author:

db/post_migrate/20170510101043_add_foreign_key_on_pipeline_schedule_owner.rb

+class AddForeignKeyOnPipelineScheduleOwner < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    execute <<-SQL
+      UPDATE ci_pipeline_schedules
+      SET owner_id = NULL
+      WHERE NOT EXISTS (
+        SELECT true
+        FROM users
+        WHERE ci_pipeline_schedules.owner_id = users.id
+      )
+    SQL
+
+    add_concurrent_foreign_key(:ci_pipeline_schedules, :users, column: :owner_id, on_delete: on_delete)
+  end
+
+  def down
+    remove_foreign_key(:ci_pipeline_schedules, column: :owner_id)
+  end
+
+  private
+
+  def on_delete
+    if Gitlab::Database.mysql?
+      :nullify
+    else
+      'SET NULL'
+    end
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170508190732) do
+ActiveRecord::Schema.define(version: 20170510101043) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1416,6 +1416,7 @@ ActiveRecord::Schema.define(version: 20170508190732) do
   add_foreign_key "chat_teams", "namespaces", on_delete: :cascade
   add_foreign_key "ci_builds", "ci_pipelines", column: "auto_canceled_by_id", name: "fk_a2141b1522", on_delete: :nullify
   add_foreign_key "ci_pipeline_schedules", "projects", name: "fk_8ead60fcc4", on_delete: :cascade
+  add_foreign_key "ci_pipeline_schedules", "users", column: "owner_id", name: "fk_9ea99f58d2", on_delete: :nullify
   add_foreign_key "ci_pipelines", "ci_pipeline_schedules", column: "pipeline_schedule_id", name: "fk_3d34ab2e06", on_delete: :nullify
   add_foreign_key "ci_pipelines", "ci_pipelines", column: "auto_canceled_by_id", name: "fk_262d4c2d19", on_delete: :nullify
   add_foreign_key "ci_trigger_requests", "ci_triggers", column: "trigger_id", name: "fk_b8ec8b7245", on_delete: :cascade

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

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
-  end
 
-  context 'when there is a scheduled pipeline within next_run_at' do
-    let(:next_run_at) { 2.days.ago }
+    pipeline_schedule.update_column(:next_run_at, 1.day.ago)
+  end
 
+  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)
-    end
+    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
+      it 'updates the next_run_at field' do
+        subject
+
+        expect(pipeline_schedule.reload.next_run_at).to be > Time.now
+      end
 
-      expect(pipeline_schedule.reload.next_run_at).to be > Time.now
+      it 'sets the schedule on the pipeline' do
+        subject
+
+        expect(project.pipelines.last.pipeline_schedule).to eq(pipeline_schedule)
+      end
     end
 
-    it 'sets the schedule on the pipeline' do
-      subject
-      expect(project.pipelines.last.pipeline_schedule).to eq(pipeline_schedule)
+    context 'inactive schedule' do
+      before do
+        pipeline_schedule.deactivate!
+      end
+
+      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