Merge branch 'remove-transactionless-destroy-feature-flag' into 'master'

Remove project_transactionless_destroy flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55795

app/services/projects/destroy_service.rb

@@ -107,12 +107,7 @@ module Projects
       end
 
       project.leave_pool_repository
-
-      if Gitlab::Ci::Features.project_transactionless_destroy?(project)
-        destroy_project_related_records(project)
-      else
-        Project.transaction { destroy_project_related_records(project) }
-      end
+      destroy_project_related_records(project)
     end
 
     def destroy_project_related_records(project)

changelogs/unreleased/remove-transactionless-destroy-feature-flag.yml

+---
+title: Remove project_transactionless_destroy feature flag
+merge_request: 55795
+author:
+type: changed

config/feature_flags/development/project_transactionless_destroy.yml

----
-name: project_transactionless_destroy
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39367
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/255972
-milestone: '13.4'
-type: development
-group: group::continuous integration
-default_enabled: false

ee/spec/services/projects/destroy_service_spec.rb

@@ -20,93 +20,79 @@ RSpec.describe Projects::DestroyService do
     stub_container_registry_tags(repository: :any, tags: [])
   end
 
-  shared_examples 'project destroy ee' do
-    context 'when project is a mirror' do
-      let(:max_capacity) { Gitlab::CurrentSettings.mirror_max_capacity }
-      let_it_be(:project_mirror) { create(:project, :mirror, :repository, :import_scheduled) }
-      let(:result) { described_class.new(project_mirror, project_mirror.owner, {}).execute }
-
-      before do
-        Gitlab::Mirror.increment_capacity(project_mirror.id)
-      end
+  context 'when project is a mirror' do
+    let(:max_capacity) { Gitlab::CurrentSettings.mirror_max_capacity }
+    let_it_be(:project_mirror) { create(:project, :mirror, :repository, :import_scheduled) }
+    let(:result) { described_class.new(project_mirror, project_mirror.owner, {}).execute }
 
-      it 'decrements capacity if mirror was scheduled' do
-        expect {result}.to change { Gitlab::Mirror.available_capacity }.from(max_capacity - 1).to(max_capacity)
-      end
+    before do
+      Gitlab::Mirror.increment_capacity(project_mirror.id)
     end
 
-    context 'when running on a primary node' do
-      let_it_be(:primary) { create(:geo_node, :primary) }
-      let_it_be(:secondary) { create(:geo_node) }
-
-      before do
-        stub_current_geo_node(primary)
-      end
+    it 'decrements capacity if mirror was scheduled' do
+      expect {result}.to change { Gitlab::Mirror.available_capacity }.from(max_capacity - 1).to(max_capacity)
+    end
+  end
 
-      it 'logs an event to the Geo event log' do
-        # Run Sidekiq immediately to check that renamed repository will be removed
-        Sidekiq::Testing.inline! do
-          expect(subject).to receive(:log_destroy_events).and_call_original
-          expect { subject.execute }.to change(Geo::RepositoryDeletedEvent, :count).by(1)
-        end
-      end
+  context 'when running on a primary node' do
+    let_it_be(:primary) { create(:geo_node, :primary) }
+    let_it_be(:secondary) { create(:geo_node) }
 
-      it 'does not log event to the Geo log if project deletion fails' do
-        expect(subject).to receive(:log_destroy_event).and_call_original
-        expect(project).to receive(:destroy!).and_raise(StandardError.new('Other error message'))
+    before do
+      stub_current_geo_node(primary)
+    end
 
-        Sidekiq::Testing.inline! do
-          expect { subject.execute }.not_to change(Geo::RepositoryDeletedEvent, :count)
-        end
+    it 'logs an event to the Geo event log' do
+      # Run Sidekiq immediately to check that renamed repository will be removed
+      Sidekiq::Testing.inline! do
+        expect(subject).to receive(:log_destroy_events).and_call_original
+        expect { subject.execute }.to change(Geo::RepositoryDeletedEvent, :count).by(1)
       end
     end
 
-    context 'audit events' do
-      include_examples 'audit event logging' do
-        let(:operation) { subject.execute }
-
-        let(:fail_condition!) do
-          expect(project).to receive(:destroy!).and_raise(StandardError.new('Other error message'))
-        end
-
-        let(:attributes) do
-          {
-            author_id: user.id,
-            entity_id: project.id,
-            entity_type: 'Project',
-            details: {
-              remove: 'project',
-              author_name: user.name,
-              target_id: project.id,
-              target_type: 'Project',
-              target_details: project.full_path
-            }
-          }
-        end
+    it 'does not log event to the Geo log if project deletion fails' do
+      expect(subject).to receive(:log_destroy_event).and_call_original
+      expect(project).to receive(:destroy!).and_raise(StandardError.new('Other error message'))
+
+      Sidekiq::Testing.inline! do
+        expect { subject.execute }.not_to change(Geo::RepositoryDeletedEvent, :count)
       end
     end
+  end
+
+  context 'audit events' do
+    include_examples 'audit event logging' do
+      let(:operation) { subject.execute }
 
-    context 'system hooks exception' do
-      before do
-        allow_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).and_raise('something went wrong')
+      let(:fail_condition!) do
+        expect(project).to receive(:destroy!).and_raise(StandardError.new('Other error message'))
       end
 
-      it 'logs an audit event' do
-        expect(subject).to receive(:log_destroy_event).and_call_original
-        expect { subject.execute }.to change(AuditEvent, :count)
+      let(:attributes) do
+        {
+          author_id: user.id,
+          entity_id: project.id,
+          entity_type: 'Project',
+          details: {
+            remove: 'project',
+            author_name: user.name,
+            target_id: project.id,
+            target_type: 'Project',
+            target_details: project.full_path
+          }
+        }
       end
     end
   end
 
-  context 'when project_transactionless_destroy enabled' do
-    it_behaves_like 'project destroy ee'
-  end
-
-  context 'when project_transactionless_destroy disabled', :sidekiq_inline do
+  context 'system hooks exception' do
     before do
-      stub_feature_flags(project_transactionless_destroy: false)
+      allow_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).and_raise('something went wrong')
     end
 
-    it_behaves_like 'project destroy ee'
+    it 'logs an audit event' do
+      expect(subject).to receive(:log_destroy_event).and_call_original
+      expect { subject.execute }.to change(AuditEvent, :count)
+    end
   end
 end

lib/gitlab/ci/features.rb

@@ -38,10 +38,6 @@ module Gitlab
         ::Feature.enabled?(:ci_disallow_to_create_merge_request_pipelines_in_target_project, target_project)
       end
 
-      def self.project_transactionless_destroy?(project)
-        Feature.enabled?(:project_transactionless_destroy, project, default_enabled: false)
-      end
-
       def self.trace_overwrite?
         ::Feature.enabled?(:ci_trace_overwrite, type: :ops, default_enabled: false)
       end