Commit b3f56e02 authored by Matthias Käppler's avatar Matthias Käppler

Merge branch 'fix-regression-environment-mr-pipelines' into 'master'

Fix environments are stopped incorrectly in merge requests

See merge request gitlab-org/gitlab!83382
parents 2c68448c 2bed0a1a
...@@ -1444,7 +1444,7 @@ class MergeRequest < ApplicationRecord ...@@ -1444,7 +1444,7 @@ class MergeRequest < ApplicationRecord
# This method is for looking for active environments which created via pipelines for merge requests. # 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`), # Since deployments run on a merge request ref (e.g. `refs/merge-requests/:iid/head`),
# we cannot look up environments with source branch name. # we cannot look up environments with source branch name.
def environments def legacy_environments
return Environment.none unless actual_head_pipeline&.merge_request? return Environment.none unless actual_head_pipeline&.merge_request?
build_for_actual_head_pipeline = Ci::Build.latest.where(pipeline: actual_head_pipeline) build_for_actual_head_pipeline = Ci::Build.latest.where(pipeline: actual_head_pipeline)
...@@ -1458,6 +1458,14 @@ class MergeRequest < ApplicationRecord ...@@ -1458,6 +1458,14 @@ class MergeRequest < ApplicationRecord
Environment.where(project: project, name: environments) Environment.where(project: project, name: environments)
end end
def environments_in_head_pipeline
if ::Feature.enabled?(:fix_related_environments_for_merge_requests, target_project, default_enabled: :yaml)
actual_head_pipeline&.environments_in_self_and_descendants || Environment.none
else
legacy_environments
end
end
def fetch_ref! def fetch_ref!
target_project.repository.fetch_source_branch!(source_project.repository, source_branch, ref_path) target_project.repository.fetch_source_branch!(source_project.repository, source_branch, ref_path)
end end
......
...@@ -19,7 +19,7 @@ module Environments ...@@ -19,7 +19,7 @@ module Environments
end end
def execute_for_merge_request(merge_request) def execute_for_merge_request(merge_request)
merge_request.environments.each { |environment| execute(environment) } merge_request.environments_in_head_pipeline.each { |environment| execute(environment) }
end end
private private
......
...@@ -72,7 +72,7 @@ module MergeRequests ...@@ -72,7 +72,7 @@ module MergeRequests
end end
def cancel_review_app_jobs!(merge_request) def cancel_review_app_jobs!(merge_request)
environments = merge_request.environments.in_review_folder.available environments = merge_request.environments_in_head_pipeline.in_review_folder.available
environments.each { |environment| environment.cancel_deployment_jobs! } environments.each { |environment| environment.cancel_deployment_jobs! }
end end
......
---
name: fix_related_environments_for_merge_requests
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/83382
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/356642
milestone: '14.10'
type: development
group: group::release
default_enabled: false
...@@ -175,6 +175,44 @@ FactoryBot.define do ...@@ -175,6 +175,44 @@ FactoryBot.define do
end end
end end
trait :prepare_staging do
name { 'prepare staging' }
environment { 'staging' }
options do
{
script: %w(ls),
environment: { name: 'staging', action: 'prepare' }
}
end
set_expanded_environment_name
end
trait :stop_staging do
name { 'stop staging' }
environment { 'staging' }
options do
{
script: %w(ls),
environment: { name: 'staging', action: 'stop' }
}
end
set_expanded_environment_name
end
trait :set_expanded_environment_name do
after(:build) do |build, evaluator|
build.assign_attributes(
metadata_attributes: {
expanded_environment_name: build.expanded_environment_name
}
)
end
end
trait :allowed_to_fail do trait :allowed_to_fail do
allow_failure { true } allow_failure { true }
end end
......
...@@ -3538,8 +3538,8 @@ RSpec.describe MergeRequest, factory_default: :keep do ...@@ -3538,8 +3538,8 @@ RSpec.describe MergeRequest, factory_default: :keep do
end end
end end
describe "#environments" do describe "#legacy_environments" do
subject { merge_request.environments } subject { merge_request.legacy_environments }
let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') } let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') }
let(:project) { merge_request.project } let(:project) { merge_request.project }
......
...@@ -198,6 +198,30 @@ RSpec.describe Environments::StopService do ...@@ -198,6 +198,30 @@ RSpec.describe Environments::StopService do
expect(pipeline.environments_in_self_and_descendants.first).to be_stopped expect(pipeline.environments_in_self_and_descendants.first).to be_stopped
end end
context 'with environment related jobs ' do
let!(:environment) { create(:environment, :available, name: 'staging', project: project) }
let!(:prepare_staging_job) { create(:ci_build, :prepare_staging, pipeline: pipeline, project: project) }
let!(:stop_staging_job) { create(:ci_build, :stop_staging, :manual, pipeline: pipeline, project: project) }
it 'does not stop environments that was not started by the merge request' do
subject
expect(prepare_staging_job.persisted_environment.state).to eq('available')
end
context 'when fix_related_environments_for_merge_requests feature flag is disabled' do
before do
stub_feature_flags(fix_related_environments_for_merge_requests: false)
end
it 'stops unrelated environments too' do
subject
expect(prepare_staging_job.persisted_environment.state).to eq('stopped')
end
end
end
end end
context 'when user is a reporter' do context 'when user is a reporter' do
......
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