Merge branch 'merge-cancel-deploy-followup' into 'master'

Follow-up on cancel review app deployments MR

See merge request gitlab-org/gitlab!35555

app/models/environment.rb

@@ -228,8 +228,18 @@ class Environment < ApplicationRecord
   end
 
   def cancel_deployment_jobs!
-    jobs = all_deployments.active.with_deployable
-    jobs.each { |deployment| deployment.deployable.cancel! }
+    jobs = active_deployments.with_deployable
+    jobs.each do |deployment|
+      # guard against data integrity issues,
+      # for example https://gitlab.com/gitlab-org/gitlab/-/issues/218659#note_348823660
+      next unless deployment.deployable
+
+      Gitlab::OptimisticLocking.retry_lock(deployment.deployable) do |deployable|
+        deployable.cancel! if deployable&.cancelable?
+      end
+    rescue => e
+      Gitlab::ErrorTracking.track_exception(e, environment_id: id, deployment_id: deployment.id)
+    end
   end
 
   def stop_with_action!(current_user)

changelogs/unreleased/merge-cancel-deploy-followup.yml

+---
+title: Guard against data integrity issues when canceling review app jobs
+merge_request: 35555
+author:
+type: fixed

spec/services/merge_requests/post_merge_service_spec.rb

@@ -90,6 +90,8 @@ RSpec.describe MergeRequests::PostMergeService do
           pipeline: pipeline, project: project, environment: review_env_a.name)
         review_job_a2 = create(:ci_build, :with_deployment, :start_review_app,
           pipeline: pipeline, project: project, environment: review_env_a.name)
+        finished_review_job_a = create(:ci_build, :with_deployment, :start_review_app,
+          pipeline: pipeline, project: project, status: :success, environment: review_env_a.name)
         review_job_b1 = create(:ci_build, :with_deployment, :start_review_app,
           pipeline: pipeline, project: project, environment: review_env_b.name)
         review_job_b2 = create(:ci_build, :start_review_app,
@@ -103,6 +105,8 @@ RSpec.describe MergeRequests::PostMergeService do
 
         expect(review_job_a1.reload.canceled?).to be true
         expect(review_job_a2.reload.canceled?).to be true
+        expect(finished_review_job_a.reload.status).to eq "success"
+        expect(finished_review_job_a.reload.canceled?).to be false
         expect(review_job_b1.reload.canceled?).to be true
         expect(review_job_b2.reload.canceled?).to be false
         expect(review_job_c1.reload.canceled?).to be false