Merge branch 'do-not-create-environment-in-job-retry' into 'master'

Disallow to create different environments per job retry

See merge request gitlab-org/gitlab!72970

app/services/ci/retry_build_service.rb

@@ -63,7 +63,7 @@ module Ci
 
     def clone_build(build)
       project.builds.new(build_attributes(build)).tap do |new_build|
-        new_build.assign_attributes(::Gitlab::Ci::Pipeline::Seed::Build.environment_attributes_for(new_build))
+        new_build.assign_attributes(deployment_attributes_for(new_build, build))
       end
     end
 
@@ -75,6 +75,16 @@ module Ci
       attributes[:user] = current_user
       attributes
     end
+
+    def deployment_attributes_for(new_build, old_build)
+      if Feature.enabled?(:sticky_environments_in_job_retry, project, default_enabled: :yaml)
+        ::Gitlab::Ci::Pipeline::Seed::Build
+          .deployment_attributes_for(new_build, old_build.persisted_environment)
+      else
+        ::Gitlab::Ci::Pipeline::Seed::Build
+          .deployment_attributes_for(new_build)
+      end
+    end
   end
 end
 

config/feature_flags/development/sticky_environments_in_job_retry.yml

+---
+name: sticky_environments_in_job_retry
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72970
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343874
+milestone: '14.5'
+type: development
+group: group::release
+default_enabled: false

lib/gitlab/ci/pipeline/seed/build.rb

@@ -90,7 +90,7 @@ module Gitlab
               ::Ci::Bridge.new(attributes)
             else
               ::Ci::Build.new(attributes).tap do |build|
-                build.assign_attributes(self.class.environment_attributes_for(build))
+                build.assign_attributes(self.class.deployment_attributes_for(build))
               end
             end
           end
@@ -101,10 +101,10 @@ module Gitlab
                                               .to_resource
           end
 
-          def self.environment_attributes_for(build)
+          def self.deployment_attributes_for(build, environment = nil)
             return {} unless build.has_environment?
 
-            environment = Seed::Environment.new(build).to_resource
+            environment = Seed::Environment.new(build).to_resource if environment.nil?
 
             unless environment.persisted?
               if Feature.enabled?(:surface_environment_creation_failure, build.project, default_enabled: :yaml) &&

spec/lib/gitlab/ci/pipeline/seed/build_spec.rb

@@ -393,12 +393,14 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
   describe '#to_resource' do
     subject { seed_build.to_resource }
 
-    context 'when job is not a bridge' do
+    context 'when job is Ci::Build' do
       it { is_expected.to be_a(::Ci::Build) }
       it { is_expected.to be_valid }
 
       shared_examples_for 'deployment job' do
         it 'returns a job with deployment' do
+          expect { subject }.to change { Environment.count }.by(1)
+
           expect(subject.deployment).not_to be_nil
           expect(subject.deployment.deployable).to eq(subject)
           expect(subject.deployment.environment.name).to eq(expected_environment_name)
@@ -413,6 +415,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
 
       shared_examples_for 'ensures environment existence' do
         it 'has environment' do
+          expect { subject }.to change { Environment.count }.by(1)
+
           expect(subject).to be_has_environment
           expect(subject.environment).to eq(environment_name)
           expect(subject.metadata.expanded_environment_name).to eq(expected_environment_name)
@@ -422,6 +426,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
 
       shared_examples_for 'ensures environment inexistence' do
         it 'does not have environment' do
+          expect { subject }.not_to change { Environment.count }
+
           expect(subject).not_to be_has_environment
           expect(subject.environment).to be_nil
           expect(subject.metadata&.expanded_environment_name).to be_nil

spec/services/ci/retry_build_service_spec.rb

@@ -323,6 +323,53 @@ RSpec.describe Ci::RetryBuildService do
         it 'persists expanded environment name' do
           expect(new_build.metadata.expanded_environment_name).to eq('production')
         end
+
+        it 'does not create a new environment' do
+          expect { new_build }.not_to change { Environment.count }
+        end
+      end
+
+      context 'when build with dynamic environment is retried' do
+        let_it_be(:other_developer) { create(:user).tap { |u| project.add_developer(other_developer) } }
+
+        let(:environment_name) { 'review/$CI_COMMIT_REF_SLUG-$GITLAB_USER_ID' }
+
+        let!(:build) do
+          create(:ci_build, :with_deployment, environment: environment_name,
+                options: { environment: { name: environment_name } },
+                pipeline: pipeline, stage_id: stage.id, project: project,
+                user: other_developer)
+        end
+
+        it 're-uses the previous persisted environment' do
+          expect(build.persisted_environment.name).to eq("review/#{build.ref}-#{other_developer.id}")
+
+          expect(new_build.persisted_environment.name).to eq("review/#{build.ref}-#{other_developer.id}")
+        end
+
+        it 'creates a new deployment' do
+          expect { new_build }.to change { Deployment.count }.by(1)
+        end
+
+        it 'does not create a new environment' do
+          expect { new_build }.not_to change { Environment.count }
+        end
+
+        context 'when sticky_environments_in_job_retry feature flag is disabled' do
+          before do
+            stub_feature_flags(sticky_environments_in_job_retry: false)
+          end
+
+          it 'creates a new environment' do
+            expect { new_build }.to change { Environment.count }
+          end
+
+          it 'ignores the previous persisted environment' do
+            expect(build.persisted_environment.name).to eq("review/#{build.ref}-#{other_developer.id}")
+
+            expect(new_build.persisted_environment.name).to eq("review/#{build.ref}-#{developer.id}")
+          end
+        end
       end
 
       context 'when build has needs' do