Merge branch 'remove-create_deployment_in_separate_transaction-feature-flag' into 'master'

Cleanup `create_deployment_in_separate_transaction` feature flag

See merge request gitlab-org/gitlab!77734

app/models/ci/pipeline.rb

@@ -1284,12 +1284,6 @@ module Ci
       end
     end
 
-    def create_deployment_in_separate_transaction?
-      strong_memoize(:create_deployment_in_separate_transaction) do
-        ::Feature.enabled?(:create_deployment_in_separate_transaction, project, default_enabled: :yaml)
-      end
-    end
-
     def use_variables_builder_definitions?
       strong_memoize(:use_variables_builder_definitions) do
         ::Feature.enabled?(:ci_use_variables_builder_definitions, project, default_enabled: :yaml)

app/services/ci/retry_build_service.rb

@@ -40,17 +40,13 @@ module Ci
       new_build = clone_build(build)
 
       new_build.run_after_commit do
+        ::Deployments::CreateForBuildService.new.execute(new_build)
+
         ::MergeRequests::AddTodoWhenBuildFailsService
           .new(project: project)
           .close(new_build)
       end
 
-      if create_deployment_in_separate_transaction?
-        new_build.run_after_commit do |new_build|
-          ::Deployments::CreateForBuildService.new.execute(new_build)
-        end
-      end
-
       ::Ci::Pipelines::AddJobService.new(build.pipeline).execute!(new_build) do |job|
         BulkInsertableAssociations.with_bulk_insert do
           job.save!
@@ -74,11 +70,7 @@ module Ci
     def check_assignable_runners!(build); end
 
     def clone_build(build)
-      project.builds.new(build_attributes(build)).tap do |new_build|
-        unless create_deployment_in_separate_transaction?
-          new_build.assign_attributes(deployment_attributes_for(new_build, build))
-        end
-      end
+      project.builds.new(build_attributes(build))
     end
 
     def build_attributes(build)
@@ -86,7 +78,7 @@ module Ci
         [attribute, build.public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend
       end
 
-      if create_deployment_in_separate_transaction? && build.persisted_environment.present?
+      if build.persisted_environment.present?
         attributes[:metadata_attributes] ||= {}
         attributes[:metadata_attributes][:expanded_environment_name] = build.expanded_environment_name
       end
@@ -94,17 +86,6 @@ module Ci
       attributes[:user] = current_user
       attributes
     end
-
-    def deployment_attributes_for(new_build, old_build)
-      ::Gitlab::Ci::Pipeline::Seed::Build
-        .deployment_attributes_for(new_build, old_build.persisted_environment)
-    end
-
-    def create_deployment_in_separate_transaction?
-      strong_memoize(:create_deployment_in_separate_transaction) do
-        ::Feature.enabled?(:create_deployment_in_separate_transaction, project, default_enabled: :yaml)
-      end
-    end
   end
 end
 

config/feature_flags/development/create_deployment_in_separate_transaction.yml

----
-name: create_deployment_in_separate_transaction
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75604
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/346879
-milestone: '14.6'
-type: development
-group: group::release
-default_enabled: true

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

@@ -21,10 +21,6 @@ module Gitlab
               merge_request: @command.merge_request,
               external_pull_request: @command.external_pull_request,
               locked: @command.project.default_pipeline_lock)
-
-            # Initialize the feature flag at the beginning of the pipeline creation process
-            # so that the flag references in the latter chains return the same value.
-            @pipeline.create_deployment_in_separate_transaction?
           end
 
           def break?

lib/gitlab/ci/pipeline/chain/create_deployments.rb

@@ -6,8 +6,6 @@ module Gitlab
       module Chain
         class CreateDeployments < Chain::Base
           def perform!
-            return unless pipeline.create_deployment_in_separate_transaction?
-
             create_deployments!
           end
 

lib/gitlab/ci/pipeline/chain/ensure_environments.rb

@@ -6,8 +6,6 @@ module Gitlab
       module Chain
         class EnsureEnvironments < Chain::Base
           def perform!
-            return unless pipeline.create_deployment_in_separate_transaction?
-
             pipeline.stages.map(&:statuses).flatten.each(&method(:ensure_environment))
           end
 

lib/gitlab/ci/pipeline/chain/ensure_resource_groups.rb

@@ -6,8 +6,6 @@ module Gitlab
       module Chain
         class EnsureResourceGroups < Chain::Base
           def perform!
-            return unless pipeline.create_deployment_in_separate_transaction?
-
             pipeline.stages.map(&:statuses).flatten.each(&method(:ensure_resource_group))
           end
 

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

@@ -79,9 +79,7 @@ module Gitlab
 
           def to_resource
             strong_memoize(:resource) do
-              processable = initialize_processable
-              assign_resource_group(processable) unless @pipeline.create_deployment_in_separate_transaction?
-              processable
+              initialize_processable
             end
           end
 
@@ -89,39 +87,10 @@ module Gitlab
             if bridge?
               ::Ci::Bridge.new(attributes)
             else
-              ::Ci::Build.new(attributes).tap do |build|
-                unless @pipeline.create_deployment_in_separate_transaction?
-                  build.assign_attributes(self.class.deployment_attributes_for(build))
-                end
-              end
+              ::Ci::Build.new(attributes)
             end
           end
 
-          def assign_resource_group(processable)
-            processable.resource_group =
-              Seed::Processable::ResourceGroup.new(processable, @resource_group_key)
-                                              .to_resource
-          end
-
-          def self.deployment_attributes_for(build, environment = nil)
-            return {} unless build.has_environment?
-
-            environment = Seed::Environment.new(build).to_resource if environment.nil?
-
-            unless environment.persisted?
-              return { status: :failed, failure_reason: :environment_creation_failure }
-            end
-
-            build.persisted_environment = environment
-
-            {
-              deployment: Seed::Deployment.new(build, environment).to_resource,
-              metadata_attributes: {
-                expanded_environment_name: environment.name
-              }
-            }
-          end
-
           private
 
           delegate :logger, to: :@context

spec/lib/gitlab/ci/pipeline/chain/create_deployments_spec.rb

@@ -47,18 +47,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::CreateDeployments do
           expect(job.deployment).to be_nil
         end
       end
-
-      context 'when create_deployment_in_separate_transaction feature flag is disabled' do
-        before do
-          stub_feature_flags(create_deployment_in_separate_transaction: false)
-        end
-
-        it 'does not create a deployment record' do
-          expect { subject }.not_to change { Deployment.count }
-
-          expect(job.deployment).to be_nil
-        end
-      end
     end
 
     context 'when a pipeline contains a teardown job' do

spec/lib/gitlab/ci/pipeline/chain/ensure_environments_spec.rb

@@ -57,18 +57,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::EnsureEnvironments do
           expect(job.persisted_environment).to be_nil
         end
       end
-
-      context 'when create_deployment_in_separate_transaction feature flag is disabled' do
-        before do
-          stub_feature_flags(create_deployment_in_separate_transaction: false)
-        end
-
-        it 'does not create any environments' do
-          expect { subject }.not_to change { Environment.count }
-
-          expect(job.persisted_environment).to be_nil
-        end
-      end
     end
 
     context 'when a pipeline contains a teardown job' do

spec/lib/gitlab/ci/pipeline/chain/ensure_resource_groups_spec.rb

@@ -60,18 +60,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::EnsureResourceGroups do
           expect(job.resource_group).to be_nil
         end
       end
-
-      context 'when create_deployment_in_separate_transaction feature flag is disabled' do
-        before do
-          stub_feature_flags(create_deployment_in_separate_transaction: false)
-        end
-
-        it 'does not create any resource groups' do
-          expect { subject }.not_to change { Ci::ResourceGroup.count }
-
-          expect(job.resource_group).to be_nil
-        end
-      end
     end
 
     context 'when a pipeline does not contain a job that requires a resource group' do

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

@@ -411,171 +411,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
   describe '#to_resource' do
     subject { seed_build.to_resource }
 
-    before do
-      stub_feature_flags(create_deployment_in_separate_transaction: false)
-    end
-
-    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)
-        end
-      end
-
-      shared_examples_for 'non-deployment job' do
-        it 'returns a job without deployment' do
-          expect(subject.deployment).to be_nil
-        end
-      end
-
-      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)
-          expect(Environment.exists?(name: expected_environment_name)).to eq(true)
-        end
-      end
-
-      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
-          expect(Environment.exists?(name: expected_environment_name)).to eq(false)
-        end
-      end
-
-      context 'when job deploys to production' do
-        let(:environment_name) { 'production' }
-        let(:expected_environment_name) { 'production' }
-        let(:attributes) { { name: 'deploy', ref: 'master', environment: 'production' } }
-
-        it_behaves_like 'deployment job'
-        it_behaves_like 'ensures environment existence'
-
-        context 'when create_deployment_in_separate_transaction feature flag is enabled' do
-          before do
-            stub_feature_flags(create_deployment_in_separate_transaction: true)
-          end
-
-          it 'does not create any deployments nor environments' do
-            expect(subject.deployment).to be_nil
-            expect(Environment.count).to eq(0)
-            expect(Deployment.count).to eq(0)
-          end
-        end
-
-        context 'when the environment name is invalid' do
-          let(:attributes) { { name: 'deploy', ref: 'master', environment: '!!!' } }
-
-          it 'fails the job with a failure reason and does not create an environment' do
-            expect(subject).to be_failed
-            expect(subject).to be_environment_creation_failure
-            expect(subject.metadata.expanded_environment_name).to be_nil
-            expect(Environment.exists?(name: expected_environment_name)).to eq(false)
-          end
-        end
-      end
-
-      context 'when job starts a review app' do
-        let(:environment_name) { 'review/$CI_COMMIT_REF_NAME' }
-        let(:expected_environment_name) { "review/#{pipeline.ref}" }
-
-        let(:attributes) do
-          {
-            name: 'deploy', ref: 'master', environment: environment_name,
-            options: { environment: { name: environment_name } }
-          }
-        end
-
-        it_behaves_like 'deployment job'
-        it_behaves_like 'ensures environment existence'
-      end
-
-      context 'when job stops a review app' do
-        let(:environment_name) { 'review/$CI_COMMIT_REF_NAME' }
-        let(:expected_environment_name) { "review/#{pipeline.ref}" }
-
-        let(:attributes) do
-          {
-            name: 'deploy', ref: 'master', environment: environment_name,
-            options: { environment: { name: environment_name, action: 'stop' } }
-          }
-        end
-
-        it 'returns a job without deployment' do
-          expect(subject.deployment).to be_nil
-        end
-
-        it_behaves_like 'non-deployment job'
-        it_behaves_like 'ensures environment existence'
-      end
-
-      context 'when job belongs to a resource group' do
-        let(:resource_group) { 'iOS' }
-        let(:attributes) { { name: 'rspec', ref: 'master', resource_group_key: resource_group, environment: 'production' }}
-
-        it 'returns a job with resource group' do
-          expect(subject.resource_group).not_to be_nil
-          expect(subject.resource_group.key).to eq('iOS')
-          expect(Ci::ResourceGroup.count).to eq(1)
-        end
-
-        context 'when create_deployment_in_separate_transaction feature flag is enabled' do
-          before do
-            stub_feature_flags(create_deployment_in_separate_transaction: true)
-          end
-
-          it 'does not create any resource groups' do
-            expect(subject.resource_group).to be_nil
-            expect(Ci::ResourceGroup.count).to eq(0)
-          end
-        end
-
-        context 'when resource group has $CI_ENVIRONMENT_NAME in it' do
-          let(:resource_group) { 'test/$CI_ENVIRONMENT_NAME' }
-
-          it 'expands environment name' do
-            expect(subject.resource_group.key).to eq('test/production')
-          end
-        end
-      end
-    end
-
-    context 'when job is a bridge' do
-      let(:base_attributes) do
-        {
-          name: 'rspec', ref: 'master', options: { trigger: 'my/project' }, scheduling_type: :stage
-        }
-      end
-
-      let(:attributes) { base_attributes }
-
-      it { is_expected.to be_a(::Ci::Bridge) }
-      it { is_expected.to be_valid }
-
-      context 'when job belongs to a resource group' do
-        let(:attributes) { base_attributes.merge(resource_group_key: 'iOS') }
-
-        it 'returns a job with resource group' do
-          expect(subject.resource_group).not_to be_nil
-          expect(subject.resource_group.key).to eq('iOS')
-        end
-      end
-    end
-
     it 'memoizes a resource object' do
       expect(subject.object_id).to eq seed_build.to_resource.object_id
     end

spec/services/ci/retry_build_service_spec.rb

@@ -370,23 +370,6 @@ RSpec.describe Ci::RetryBuildService do
       it_behaves_like 'when build with deployment is retried'
       it_behaves_like 'when build with dynamic environment is retried'
 
-      context 'when create_deployment_in_separate_transaction feature flag is disabled' do
-        let(:new_build) do
-          travel_to(1.second.from_now) do
-            ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/345668') do
-              service.clone!(build)
-            end
-          end
-        end
-
-        before do
-          stub_feature_flags(create_deployment_in_separate_transaction: false)
-        end
-
-        it_behaves_like 'when build with deployment is retried'
-        it_behaves_like 'when build with dynamic environment is retried'
-      end
-
       context 'when build has needs' do
         before do
           create(:ci_build_need, build: build, name: 'build1')