Merge branch '12996-cancel-redundant-pipelines' into 'master'

Cancel redundant pipelines in merge train when reconstruction happens

Closes #12996

See merge request gitlab-org/gitlab-ee!15450

app/models/ci/pipeline.rb

@@ -460,8 +460,8 @@ module Ci
       canceled? && auto_canceled_by_id?
     end
 
-    def cancel_running
-      retry_optimistic_lock(cancelable_statuses) do |cancelable|
+    def cancel_running(retries: nil)
+      retry_optimistic_lock(cancelable_statuses, retries) do |cancelable|
         cancelable.find_each do |job|
           yield(job) if block_given?
           job.cancel
@@ -469,10 +469,10 @@ module Ci
       end
     end
 
-    def auto_cancel_running(pipeline)
+    def auto_cancel_running(pipeline, retries: nil)
       update(auto_canceled_by: pipeline)
 
-      cancel_running do |job|
+      cancel_running(retries: retries) do |job|
         job.auto_canceled_by = pipeline
       end
     end

ee/app/services/merge_trains/refresh_merge_request_service.rb

@@ -14,7 +14,6 @@ module MergeTrains
       @merge_request = merge_request
 
       validate!
-
       pipeline_created = create_pipeline! if should_create_pipeline?
       merge! if should_merge?
 
@@ -66,7 +65,8 @@ module MergeTrains
 
       raise ProcessError, result[:message] unless result[:status] == :success
 
-      merge_train.update!(pipeline: result[:pipeline])
+      cancel_pipeline_for_merge_train(result[:pipeline])
+      update_pipeline_for_merge_train(result[:pipeline])
     end
 
     def should_merge?
@@ -113,6 +113,18 @@ module MergeTrains
       merge_train.pipeline
     end
 
+    def cancel_pipeline_for_merge_train(new_pipeline)
+      pipeline_for_merge_train&.auto_cancel_running(new_pipeline, retries: 1)
+    rescue ActiveRecord::StaleObjectError
+      # Often the pipeline has already been canceled by the default cancelaltion
+      # mechanizm `Ci::CreatePipelineService#cancel_pending_pipelines`. In this
+      # case, we can ignore the exception as it's already canceled.
+    end
+
+    def update_pipeline_for_merge_train(pipeline)
+      merge_train.update!(pipeline: pipeline)
+    end
+
     def merge_user
       merge_request.merge_user
     end

ee/changelogs/unreleased/12996-cancel-redundant-pipelines.yml

+---
+title: Cancel redundant merge train pipelines
+merge_request: 15450
+author:
+type: added

ee/spec/services/merge_trains/refresh_merge_request_service_spec.rb

@@ -50,6 +50,26 @@ describe MergeTrains::RefreshMergeRequestService do
       end
     end
 
+    shared_examples_for 'cancels and recreates a pipeline for the merge train' do
+      let(:previous_ref) { 'refs/heads/master' }
+
+      it do
+        expect_next_instance_of(MergeTrains::CreatePipelineService, project, maintainer) do |pipeline_service|
+          allow(pipeline_service).to receive(:execute) { { status: :success, pipeline: create(:ci_pipeline) } }
+          expect(pipeline_service).to receive(:execute).with(merge_request, previous_ref)
+        end
+
+        result = subject
+        new_pipeline = merge_request.merge_train.pipeline
+        pipeline.reset
+
+        expect(result[:status]).to eq(:success)
+        expect(result[:pipeline_created]).to eq(true)
+        expect(pipeline.status).to eq('canceled')
+        expect(pipeline.auto_canceled_by_id).to eq(new_pipeline.id)
+      end
+    end
+
     shared_examples_for 'does not create a pipeline' do
       it do
         expect(service).not_to receive(:create_pipeline!)
@@ -137,7 +157,7 @@ describe MergeTrains::RefreshMergeRequestService do
     end
 
     context 'when pipeline for merge train is running' do
-      let(:pipeline) { create(:ci_pipeline, :running, target_sha: previous_ref_sha, source_sha: merge_request.diff_head_sha) }
+      let(:pipeline) { create(:ci_pipeline, :running, :with_job, project: project, target_sha: previous_ref_sha, source_sha: merge_request.diff_head_sha) }
       let(:previous_ref_sha) { project.repository.commit('refs/heads/master').sha }
 
       before do
@@ -151,13 +171,13 @@ describe MergeTrains::RefreshMergeRequestService do
       context 'when the pipeline is stale' do
         let(:previous_ref_sha) { project.repository.commits('refs/heads/master', limit: 2).last.sha }
 
-        it_behaves_like 'creates a pipeline for merge train'
+        it_behaves_like 'cancels and recreates a pipeline for the merge train'
       end
 
-      context 'when the pipeline is reuired to be recreated' do
+      context 'when the pipeline is required to be recreated' do
         let(:require_recreate) { true }
 
-        it_behaves_like 'creates a pipeline for merge train'
+        it_behaves_like 'cancels and recreates a pipeline for the merge train'
       end
     end
 

lib/gitlab/optimistic_locking.rb

@@ -4,7 +4,8 @@ module Gitlab
   module OptimisticLocking
     module_function
 
-    def retry_lock(subject, retries = 100, &block)
+    def retry_lock(subject, retries = nil, &block)
+      retries ||= 100
       # TODO(Observability): We should be recording details of the number of retries and the duration of the total execution here
       ActiveRecord::Base.transaction do
         yield(subject)