Automatic merge of gitlab-org/gitlab-ce master

Gemfile

@@ -324,7 +324,7 @@ group :development do
   gem 'foreman', '~> 0.84.0'
   gem 'brakeman', '~> 4.2', require: false
 
-  gem 'letter_opener_web', '~> 1.3.0'
+  gem 'letter_opener_web', '~> 1.3.4'
   gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false
 
   # Better errors handler

Gemfile.lock

@@ -473,9 +473,9 @@ GEM
       rest-client (~> 2.0)
     launchy (2.4.3)
       addressable (~> 2.3)
-    letter_opener (1.4.1)
+    letter_opener (1.7.0)
       launchy (~> 2.2)
-    letter_opener_web (1.3.0)
+    letter_opener_web (1.3.4)
       actionmailer (>= 3.2)
       letter_opener (~> 1.0)
       railties (>= 3.2)
@@ -1133,7 +1133,7 @@ DEPENDENCIES
   kaminari (~> 1.0)
   knapsack (~> 1.17)
   kubeclient (~> 4.2.2)
-  letter_opener_web (~> 1.3.0)
+  letter_opener_web (~> 1.3.4)
   license_finder (~> 5.4)
   licensee (~> 8.9)
   lograge (~> 0.5)

app/workers/pipeline_schedule_worker.rb

@@ -3,20 +3,26 @@
 class PipelineScheduleWorker
   include ApplicationWorker
   include CronjobQueue
+  include ::Gitlab::ExclusiveLeaseHelpers
+
+  EXCLUSIVE_LOCK_KEY = 'pipeline_schedules:run:lock'
+  LOCK_TIMEOUT = 50.minutes
 
   # rubocop: disable CodeReuse/ActiveRecord
   def perform
-    Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now)
-      .preload(:owner, :project).find_each do |schedule|
-
-      Ci::CreatePipelineService.new(schedule.project,
-                                    schedule.owner,
-                                    ref: schedule.ref)
-        .execute!(:schedule, ignore_skip_ci: true, save_on_errors: true, schedule: schedule)
-    rescue => e
-      error(schedule, e)
-    ensure
-      schedule.schedule_next_run!
+    in_lock(EXCLUSIVE_LOCK_KEY, ttl: LOCK_TIMEOUT, retries: 1) do
+      Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now)
+        .preload(:owner, :project).find_each do |schedule|
+
+        schedule.schedule_next_run!
+
+        Ci::CreatePipelineService.new(schedule.project,
+                                      schedule.owner,
+                                      ref: schedule.ref)
+          .execute!(:schedule, ignore_skip_ci: true, save_on_errors: true, schedule: schedule)
+      rescue => e
+        error(schedule, e)
+      end
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord

changelogs/unreleased/lock-pipeline-schedule-worker.yml

+---
+title: Prevent concurrent execution of PipelineScheduleWorker
+merge_request: 27781
+author:
+type: performance

changelogs/unreleased/pl-upgrade-letter_opener_web.yml

+---
+title: Upgrade letter_opener_web to support Rails 5.1
+merge_request: 27829
+author:
+type: fixed

spec/workers/pipeline_schedule_worker_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe PipelineScheduleWorker do
+  include ExclusiveLeaseHelpers
+
   subject { described_class.new.perform }
 
   set(:project) { create(:project, :repository) }
@@ -39,6 +41,16 @@ describe PipelineScheduleWorker do
 
       it_behaves_like 'successful scheduling'
 
+      context 'when exclusive lease has already been taken by the other instance' do
+        before do
+          stub_exclusive_lease_taken(described_class::EXCLUSIVE_LOCK_KEY, timeout: described_class::LOCK_TIMEOUT)
+        end
+
+        it 'raises an error and does not start creating pipelines' do
+          expect { subject }.to raise_error(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError)
+        end
+      end
+
       context 'when the latest commit contains [ci skip]' do
         before do
           allow_any_instance_of(Ci::Pipeline)