Merge branch 'remove-abort-pipelines-feature-flag' into 'master'

[Feature Flag] Remove abort_deleted_project_pipelines

See merge request gitlab-org/gitlab!76794

app/services/projects/destroy_service.rb

@@ -28,9 +28,7 @@ module Projects
       # Git data (e.g. a list of branch names).
       flush_caches(project)
 
-      if Feature.enabled?(:abort_deleted_project_pipelines, default_enabled: :yaml)
-        ::Ci::AbortPipelinesService.new.execute(project.all_pipelines, :project_deleted)
-      end
+      ::Ci::AbortPipelinesService.new.execute(project.all_pipelines, :project_deleted)
 
       Projects::UnlinkForkService.new(project, current_user).execute
 
@@ -133,9 +131,7 @@ module Projects
       destroy_web_hooks!
       destroy_project_bots!
 
-      if ::Feature.enabled?(:ci_optimize_project_records_destruction, project, default_enabled: :yaml) &&
-        Feature.enabled?(:abort_deleted_project_pipelines, default_enabled: :yaml)
-
+      if ::Feature.enabled?(:ci_optimize_project_records_destruction, project, default_enabled: :yaml)
         destroy_ci_records!
       end
 

config/feature_flags/development/abort_deleted_project_pipelines.yml

----
-name: abort_deleted_project_pipelines
-introduced_by_url: https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1220
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/301106
-milestone: '13.9'
-type: development
-group: group::pipeline execution
-default_enabled: true

spec/services/projects/destroy_service_spec.rb

@@ -55,22 +55,6 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
           .and change { Ci::Pipeline.count }.by(-1)
       end
 
-      context 'with abort_deleted_project_pipelines disabled' do
-        stub_feature_flags(abort_deleted_project_pipelines: false)
-
-        it 'avoids N+1 queries' do
-          recorder = ActiveRecord::QueryRecorder.new { destroy_project(project, user, {}) }
-
-          project = create(:project, :repository, namespace: user.namespace)
-          pipeline = create(:ci_pipeline, project: project)
-          builds = create_list(:ci_build, 3, :artifacts, pipeline: pipeline)
-          create(:ci_pipeline_artifact, pipeline: pipeline)
-          create_list(:ci_build_trace_chunk, 3, build: builds[0])
-
-          expect { destroy_project(project, project.owner, {}) }.not_to exceed_query_limit(recorder)
-        end
-      end
-
       context 'with ci_optimize_project_records_destruction disabled' do
         stub_feature_flags(ci_optimize_project_records_destruction: false)
 
@@ -86,7 +70,7 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
         end
       end
 
-      context 'with ci_optimize_project_records_destruction and abort_deleted_project_pipelines enabled' do
+      context 'with ci_optimize_project_records_destruction enabled' do
         it 'avoids N+1 queries' do
           recorder = ActiveRecord::QueryRecorder.new { destroy_project(project, user, {}) }
 
@@ -132,27 +116,25 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
     destroy_project(project, user, {})
   end
 
-  context 'with abort_deleted_project_pipelines feature disabled' do
-    before do
-      stub_feature_flags(abort_deleted_project_pipelines: false)
-    end
+  context 'with running pipelines to be aborted' do
+    let!(:pipelines)               { create_list(:ci_pipeline, 3, :running, project: project) }
+    let(:destroy_pipeline_service) { double('DestroyPipelineService', execute: nil) }
 
-    it 'does not bulk-fail project ci pipelines' do
-      expect(::Ci::AbortPipelinesService).not_to receive(:new)
+    it 'executes DestroyPipelineService for project ci pipelines' do
+      allow(::Ci::DestroyPipelineService).to receive(:new).and_return(destroy_pipeline_service)
 
-      destroy_project(project, user, {})
-    end
+      expect(::Ci::AbortPipelinesService)
+        .to receive_message_chain(:new, :execute)
+        .with(project.all_pipelines, :project_deleted)
 
-    it 'does not destroy CI records via DestroyPipelineService' do
-      expect(::Ci::DestroyPipelineService).not_to receive(:new)
+      pipelines.each do |pipeline|
+        expect(destroy_pipeline_service)
+          .to receive(:execute)
+          .with(pipeline)
+      end
 
       destroy_project(project, user, {})
     end
-  end
-
-  context 'with abort_deleted_project_pipelines feature enabled' do
-    let!(:pipelines)               { create_list(:ci_pipeline, 3, :running, project: project) }
-    let(:destroy_pipeline_service) { double('DestroyPipelineService', execute: nil) }
 
     context 'with ci_optimize_project_records_destruction disabled' do
       before do
@@ -173,24 +155,6 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
         destroy_project(project, user, {})
       end
     end
-
-    context 'with ci_optimize_project_records_destruction enabled' do
-      it 'executes DestroyPipelineService for project ci pipelines' do
-        allow(::Ci::DestroyPipelineService).to receive(:new).and_return(destroy_pipeline_service)
-
-        expect(::Ci::AbortPipelinesService)
-          .to receive_message_chain(:new, :execute)
-          .with(project.all_pipelines, :project_deleted)
-
-        pipelines.each do |pipeline|
-          expect(destroy_pipeline_service)
-            .to receive(:execute)
-            .with(pipeline)
-        end
-
-        destroy_project(project, user, {})
-      end
-    end
   end
 
   context 'when project has remote mirrors' do