Commit 27e1e3da authored by Sean McGivern's avatar Sean McGivern Committed by GitLab Release Tools Bot

Merge branch 'fix-environment-on-stop-not-work' into 'master'

`on_stop` is not automatically triggered with pipelines for merge requests

Closes #60885

See merge request gitlab-org/gitlab-ce!27618

(cherry picked from commit 2432a540)

daa8f784 Fix environment automatic on_stop trigger
parent 6c07afab
......@@ -1097,6 +1097,16 @@ class MergeRequest < ApplicationRecord
@environments[current_user]
end
##
# This method is for looking for active environments which created via pipelines for merge requests.
# Since deployments run on a merge request ref (e.g. `refs/merge-requests/:iid/head`),
# we cannot look up environments with source branch name.
def environments
return Environment.none unless actual_head_pipeline&.triggered_by_merge_request?
actual_head_pipeline.environments
end
def state_human_name
if merged?
"Merged"
......
......@@ -9,12 +9,11 @@ module Ci
return unless @ref.present?
environments.each do |environment|
next unless environment.stop_action_available?
next unless can?(current_user, :stop_environment, environment)
environments.each { |environment| stop(environment) }
end
environment.stop_with_action!(current_user)
end
def execute_for_merge_request(merge_request)
merge_request.environments.each { |environment| stop(environment) }
end
private
......@@ -24,5 +23,12 @@ module Ci
.new(project, current_user, ref: @ref, recently_updated: true)
.execute
end
def stop(environment)
return unless environment.stop_action_available?
return unless can?(current_user, :stop_environment, environment)
environment.stop_with_action!(current_user)
end
end
end
......@@ -24,6 +24,11 @@ module MergeRequests
end
end
def cleanup_environments(merge_request)
Ci::StopEnvironmentsService.new(merge_request.source_project, current_user)
.execute_for_merge_request(merge_request)
end
private
def handle_wip_event(merge_request)
......
......@@ -17,6 +17,7 @@ module MergeRequests
execute_hooks(merge_request, 'close')
invalidate_cache_counts(merge_request, users: merge_request.assignees)
merge_request.update_project_counter_caches
cleanup_environments(merge_request)
end
merge_request
......
......@@ -18,6 +18,7 @@ module MergeRequests
invalidate_cache_counts(merge_request, users: merge_request.assignees)
merge_request.update_project_counter_caches
delete_non_latest_diffs(merge_request)
cleanup_environments(merge_request)
end
private
......
---
title: "`on_stop` is not automatically triggered with pipelines for merge requests"
merge_request: 27618
author:
type: fixed
......@@ -2304,6 +2304,50 @@ describe MergeRequest do
end
end
describe "#environments" do
subject { merge_request.environments }
let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') }
let(:project) { merge_request.project }
let(:pipeline) do
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])
end
let!(:job) { create(:ci_build, :start_review_app, pipeline: pipeline, project: project) }
it 'returns environments' do
is_expected.to eq(pipeline.environments)
expect(subject.count).to be(1)
end
context 'when pipeline is not associated with environments' do
let!(:job) { create(:ci_build, pipeline: pipeline, project: project) }
it 'returns empty array' do
is_expected.to be_empty
end
end
context 'when pipeline is not a pipeline for merge request' do
let(:pipeline) do
create(:ci_pipeline,
project: project,
ref: 'feature',
sha: merge_request.diff_head_sha,
merge_requests_as_head_pipeline: [merge_request])
end
it 'returns empty relation' do
is_expected.to be_empty
end
end
end
describe "#reload_diff" do
it 'calls MergeRequests::ReloadDiffsService#execute with correct params' do
user = create(:user)
......
......@@ -103,6 +103,82 @@ describe Ci::StopEnvironmentsService do
end
end
describe '#execute_for_merge_request' do
subject { service.execute_for_merge_request(merge_request) }
let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') }
let(:project) { merge_request.project }
let(:user) { create(:user) }
let(:pipeline) do
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])
end
let!(:review_job) { create(:ci_build, :start_review_app, pipeline: pipeline, project: project) }
let!(:stop_review_job) { create(:ci_build, :stop_review_app, :manual, pipeline: pipeline, project: project) }
before do
review_job.deployment.success!
end
it 'has active environment at first' do
expect(pipeline.environments.first).to be_available
end
context 'when user is a developer' do
before do
project.add_developer(user)
end
it 'stops the active environment' do
subject
expect(pipeline.environments.first).to be_stopped
end
end
context 'when user is a reporter' do
before do
project.add_reporter(user)
end
it 'does not stop the active environment' do
subject
expect(pipeline.environments.first).to be_available
end
end
context 'when pipeline is not associated with environments' do
let!(:job) { create(:ci_build, pipeline: pipeline, project: project) }
it 'does not raise exception' do
expect { subject }.not_to raise_exception
end
end
context 'when pipeline is not a pipeline for merge request' do
let(:pipeline) do
create(:ci_pipeline,
project: project,
ref: 'feature',
sha: merge_request.diff_head_sha,
merge_requests_as_head_pipeline: [merge_request])
end
it 'does not stop the active environment' do
subject
expect(pipeline.environments.first).to be_available
end
end
end
def expect_environment_stopped_on(branch)
expect_any_instance_of(Environment)
.to receive(:stop!)
......
......@@ -72,6 +72,14 @@ describe MergeRequests::CloseService do
.to change { project.open_merge_requests_count }.from(1).to(0)
end
it 'clean up environments for the merge request' do
expect_next_instance_of(Ci::StopEnvironmentsService) do |service|
expect(service).to receive(:execute_for_merge_request).with(merge_request)
end
described_class.new(project, user).execute(merge_request)
end
context 'current user is not authorized to close merge request' do
before do
perform_enqueued_jobs do
......
......@@ -60,5 +60,13 @@ describe MergeRequests::PostMergeService do
expect(merge_request.reload).to be_merged
end
it 'clean up environments for the merge request' do
expect_next_instance_of(Ci::StopEnvironmentsService) do |service|
expect(service).to receive(:execute_for_merge_request).with(merge_request)
end
described_class.new(project, user).execute(merge_request)
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