Commit c2763b62 authored by Shinya Maeda's avatar Shinya Maeda

Fix Merge Request accidentally stops unrelated environments

This commit fixes the regression that unrelated environments
are teardown when a MR has been merged/closed.

This fix is still behind the feature flag.
parent 61e0ea10
......@@ -959,7 +959,7 @@ module Ci
Ci::Build.latest.where(pipeline: self_and_descendants)
end
def environments_in_self_and_descendants
def environments_in_self_and_descendants(deployment_status: nil)
# We limit to 100 unique environments for application safety.
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/340781#note_699114700
expanded_environment_names =
......@@ -969,7 +969,7 @@ module Ci
.limit(100)
.pluck(:expanded_environment_name)
Environment.where(project: project, name: expanded_environment_names).with_deployment(sha)
Environment.where(project: project, name: expanded_environment_names).with_deployment(sha, status: deployment_status)
end
# With multi-project and parent-child pipelines
......
......@@ -89,13 +89,19 @@ class Environment < ApplicationRecord
scope :for_project, -> (project) { where(project_id: project) }
scope :for_tier, -> (tier) { where(tier: tier).where.not(tier: nil) }
scope :with_deployment, -> (sha) { where('EXISTS (?)', Deployment.select(1).where('deployments.environment_id = environments.id').where(sha: sha)) }
scope :unfoldered, -> { where(environment_type: nil) }
scope :with_rank, -> do
select('environments.*, rank() OVER (PARTITION BY project_id ORDER BY id DESC)')
end
scope :for_id, -> (id) { where(id: id) }
scope :with_deployment, -> (sha, status: nil) do
deployments = Deployment.select(1).where('deployments.environment_id = environments.id').where(sha: sha)
deployments = deployments.where(status: status) if status
where('EXISTS (?)', deployments)
end
scope :stopped_review_apps, -> (before, limit) do
stopped
.in_review_folder
......
......@@ -1456,9 +1456,9 @@ class MergeRequest < ApplicationRecord
Environment.where(project: project, name: environments)
end
def environments_in_head_pipeline
def environments_in_head_pipeline(deployment_status: nil)
if ::Feature.enabled?(:fix_related_environments_for_merge_requests, target_project, default_enabled: :yaml)
actual_head_pipeline&.environments_in_self_and_descendants || Environment.none
actual_head_pipeline&.environments_in_self_and_descendants(deployment_status: deployment_status) || Environment.none
else
legacy_environments
end
......
......@@ -19,7 +19,9 @@ module Environments
end
def execute_for_merge_request(merge_request)
merge_request.environments_in_head_pipeline.each { |environment| execute(environment) }
merge_request.environments_in_head_pipeline(deployment_status: :success).each do |environment|
execute(environment)
end
end
private
......
......@@ -189,6 +189,20 @@ FactoryBot.define do
set_expanded_environment_name
end
trait :start_staging do
name { 'start staging' }
environment { 'staging' }
options do
{
script: %w(ls),
environment: { name: 'staging', action: 'start' }
}
end
set_expanded_environment_name
end
trait :stop_staging do
name { 'stop staging' }
environment { 'staging' }
......
......@@ -349,15 +349,28 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
end
describe '.with_deployment' do
subject { described_class.with_deployment(sha) }
subject { described_class.with_deployment(sha, status: status) }
let(:environment) { create(:environment, project: project) }
let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' }
let(:status) { nil }
context 'when deployment has the specified sha' do
let!(:deployment) { create(:deployment, environment: environment, sha: sha) }
it { is_expected.to eq([environment]) }
context 'with success status filter' do
let(:status) { :success }
it { is_expected.to be_empty }
end
context 'with created status filter' do
let(:status) { :created }
it { is_expected.to contain_exactly(environment) }
end
end
context 'when deployment does not have the specified sha' do
......
......@@ -202,6 +202,7 @@ RSpec.describe Environments::StopService do
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!(:start_staging_job) { create(:ci_build, :start_staging, :with_deployment, :manual, 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
......
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