Commit 7679bbd6 authored by Amy Troschinetz's avatar Amy Troschinetz

Cancel review app deployment when MR is merged

**app/models/environment.rb:**
Adds a `cancel_deployment_jobs!` method which goes through and cancels
deployment jobs associated with that environment.

**app/services/merge_requests/base_service.rb:**
Adds a `cancel_deployment_jobs!` method that takes a merge request
and cancels deployment jobs for associated environments.

**app/services/merge_requests/post_merge_service.rb:**
Post merge we cancel all deployment jobs.
parent 2e51be25
...@@ -226,6 +226,11 @@ class Environment < ApplicationRecord ...@@ -226,6 +226,11 @@ class Environment < ApplicationRecord
available? && stop_action.present? available? && stop_action.present?
end end
def cancel_deployment_jobs!
jobs = all_deployments.active.with_deployable
jobs.each { |deployment| deployment.deployable.cancel! }
end
def stop_with_action!(current_user) def stop_with_action!(current_user)
return unless available? return unless available?
......
...@@ -29,6 +29,11 @@ module MergeRequests ...@@ -29,6 +29,11 @@ module MergeRequests
.execute_for_merge_request(merge_request) .execute_for_merge_request(merge_request)
end end
def cancel_review_app_jobs!(merge_request)
environments = merge_request.environments.in_review_folder.available
environments.each { |environment| environment.cancel_deployment_jobs! }
end
def source_project def source_project
@source_project ||= merge_request.source_project @source_project ||= merge_request.source_project
end end
......
...@@ -18,6 +18,7 @@ module MergeRequests ...@@ -18,6 +18,7 @@ module MergeRequests
invalidate_cache_counts(merge_request, users: merge_request.assignees) invalidate_cache_counts(merge_request, users: merge_request.assignees)
merge_request.update_project_counter_caches merge_request.update_project_counter_caches
delete_non_latest_diffs(merge_request) delete_non_latest_diffs(merge_request)
cancel_review_app_jobs!(merge_request)
cleanup_environments(merge_request) cleanup_environments(merge_request)
end end
......
---
title: Cancel review app deployment when MR is merged
merge_request: 34960
author:
type: fixed
...@@ -7,6 +7,8 @@ describe MergeRequests::PostMergeService do ...@@ -7,6 +7,8 @@ describe MergeRequests::PostMergeService do
let(:merge_request) { create(:merge_request, assignees: [user]) } let(:merge_request) { create(:merge_request, assignees: [user]) }
let(:project) { merge_request.project } let(:project) { merge_request.project }
subject { described_class.new(project, user).execute(merge_request) }
before do before do
project.add_maintainer(user) project.add_maintainer(user)
end end
...@@ -19,10 +21,7 @@ describe MergeRequests::PostMergeService do ...@@ -19,10 +21,7 @@ describe MergeRequests::PostMergeService do
project.open_merge_requests_count project.open_merge_requests_count
merge_request.update!(state: 'merged') merge_request.update!(state: 'merged')
service = described_class.new(project, user, {}) expect { subject }.to change { project.open_merge_requests_count }.from(1).to(0)
expect { service.execute(merge_request) }
.to change { project.open_merge_requests_count }.from(1).to(0)
end end
it 'updates metrics' do it 'updates metrics' do
...@@ -35,7 +34,7 @@ describe MergeRequests::PostMergeService do ...@@ -35,7 +34,7 @@ describe MergeRequests::PostMergeService do
expect(metrics_service).to receive(:merge) expect(metrics_service).to receive(:merge)
described_class.new(project, user, {}).execute(merge_request) subject
end end
it 'deletes non-latest diffs' do it 'deletes non-latest diffs' do
...@@ -45,7 +44,7 @@ describe MergeRequests::PostMergeService do ...@@ -45,7 +44,7 @@ describe MergeRequests::PostMergeService do
.to receive(:new).with(merge_request) .to receive(:new).with(merge_request)
.and_return(diff_removal_service) .and_return(diff_removal_service)
described_class.new(project, user, {}).execute(merge_request) subject
expect(diff_removal_service).to have_received(:execute) expect(diff_removal_service).to have_received(:execute)
end end
...@@ -56,21 +55,59 @@ describe MergeRequests::PostMergeService do ...@@ -56,21 +55,59 @@ describe MergeRequests::PostMergeService do
issue = create(:issue, project: project) issue = create(:issue, project: project)
allow(merge_request).to receive(:visible_closing_issues_for).and_return([issue]) allow(merge_request).to receive(:visible_closing_issues_for).and_return([issue])
expect_next_instance_of(Issues::CloseService) do |service| expect_next_instance_of(Issues::CloseService) do |close_service|
allow(service).to receive(:execute).with(issue, commit: merge_request).and_raise(RuntimeError) allow(close_service).to receive(:execute).with(issue, commit: merge_request).and_raise(RuntimeError)
end end
expect { described_class.new(project, user).execute(merge_request) }.to raise_error(RuntimeError) expect { subject }.to raise_error(RuntimeError)
expect(merge_request.reload).to be_merged expect(merge_request.reload).to be_merged
end end
it 'clean up environments for the merge request' do it 'clean up environments for the merge request' do
expect_next_instance_of(Ci::StopEnvironmentsService) do |service| expect_next_instance_of(Ci::StopEnvironmentsService) do |stop_environment_service|
expect(service).to receive(:execute_for_merge_request).with(merge_request) expect(stop_environment_service).to receive(:execute_for_merge_request).with(merge_request)
end end
described_class.new(project, user).execute(merge_request) subject
end
context 'when the merge request has review apps' do
it 'cancels all review app deployments' do
pipeline = create(:ci_pipeline,
source: :merge_request_event,
merge_request: merge_request,
project: project,
sha: merge_request.diff_head_sha,
merge_requests_as_head_pipeline: [merge_request])
review_env_a = create(:environment, project: project, state: :available, name: 'review/a')
review_env_b = create(:environment, project: project, state: :available, name: 'review/b')
review_env_c = create(:environment, project: project, state: :stopped, name: 'review/c')
deploy_env = create(:environment, project: project, state: :available, name: 'deploy')
review_job_a1 = create(:ci_build, :with_deployment, :start_review_app,
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)
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,
pipeline: pipeline, project: project, environment: review_env_b.name)
review_job_c1 = create(:ci_build, :with_deployment, :start_review_app,
pipeline: pipeline, project: project, environment: review_env_c.name)
deploy_job = create(:ci_build, :with_deployment, :deploy_to_production,
pipeline: pipeline, project: project, environment: deploy_env.name)
subject
expect(review_job_a1.reload.canceled?).to be true
expect(review_job_a2.reload.canceled?).to be true
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
expect(deploy_job.reload.canceled?).to be false
end
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment