Commit 2bed0a1a authored by Shinya Maeda's avatar Shinya Maeda

Fix environments are stopped incorrectly in merge requests

This commit fixes the regression that the environments
are stopped incorrectly in merge requests.

This fix is still behind the feature flag.
parent eca1751b
...@@ -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
......
...@@ -3550,8 +3550,8 @@ RSpec.describe MergeRequest, factory_default: :keep do ...@@ -3550,8 +3550,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