Merge branch 'jv-pages-remove-worker' into 'master'

Remove pages sites asynchronously

See merge request gitlab-org/gitlab!38313

app/controllers/projects/pages_controller.rb

@@ -21,7 +21,7 @@ class Projects::PagesController < Projects::ApplicationController
       format.html do
         redirect_to project_pages_path(@project),
                     status: :found,
-                    notice: 'Pages were removed'
+                    notice: 'Pages were scheduled for removal'
       end
     end
   end

app/services/pages/delete_service.rb

@@ -3,8 +3,11 @@
 module Pages
   class DeleteService < BaseService
     def execute
-      project.remove_pages
-      project.pages_domains.destroy_all # rubocop: disable Cop/DestroyAll
+      if Feature.enabled?(:async_pages_removal, project)
+        PagesRemoveWorker.perform_async(project.id)
+      else
+        PagesRemoveWorker.new.perform(project.id)
+      end
     end
   end
 end

app/workers/all_queues.yml

@@ -1358,7 +1358,7 @@
   :tags: []
 - :name: flush_counter_increments
   :feature_category: :not_owned
-  :has_external_dependencies: 
+  :has_external_dependencies:
   :urgency: :low
   :resource_boundary: :unknown
   :weight: 1
@@ -1450,7 +1450,7 @@
   :urgency: :high
   :resource_boundary: :unknown
   :weight: 5
-  :idempotent: 
+  :idempotent:
   :tags: []
 - :name: merge_request_mergeability_check
   :feature_category: :source_code_management
@@ -1532,6 +1532,14 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: pages_remove
+  :feature_category: :pages
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent:
+  :tags: []
 - :name: pages_update_configuration
   :feature_category: :pages
   :has_external_dependencies:

app/workers/pages_remove_worker.rb

+# frozen_string_literal: true
+
+class PagesRemoveWorker # rubocop:disable Scalability/IdempotentWorker
+  include ApplicationWorker
+
+  sidekiq_options retry: 3
+  feature_category :pages
+  loggable_arguments 0
+
+  def perform(project_id)
+    project = Project.find_by_id(project_id)
+    return unless project
+
+    project.remove_pages
+    project.pages_domains.delete_all
+  end
+end

config/feature_flags/development/async_pages_removal.yml

+---
+name: async_pages_removal
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38313
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/239318
+group: team::Scalability
+type: development
+default_enabled: false
\ No newline at end of file

config/sidekiq_queues.yml

@@ -178,6 +178,8 @@
   - 1
 - - pages_domain_verification
   - 1
+- - pages_remove
+  - 1
 - - pages_update_configuration
   - 1
 - - personal_access_tokens

lib/tasks/gitlab/sidekiq.rake

@@ -5,7 +5,7 @@ return if Rails.env.production?
 namespace :gitlab do
   namespace :sidekiq do
     def write_yaml(path, banner, object)
-      File.write(path, banner + YAML.dump(object))
+      File.write(path, banner + YAML.dump(object).gsub(/ *$/m, ''))
     end
 
     namespace :all_queues_yml do

spec/features/projects/pages_spec.rb

@@ -380,14 +380,14 @@ RSpec.shared_examples 'pages settings editing' do
         expect(project).to be_pages_deployed
       end
 
-      it 'removes the pages' do
+      it 'removes the pages', :sidekiq_inline do
         visit project_pages_path(project)
 
         expect(page).to have_link('Remove pages')
 
         accept_confirm { click_link 'Remove pages' }
 
-        expect(page).to have_content('Pages were removed')
+        expect(page).to have_content('Pages were scheduled for removal')
         expect(project.reload.pages_deployed?).to be_falsey
       end
     end

spec/requests/api/pages/pages_spec.rb

@@ -39,7 +39,9 @@ RSpec.describe API::Pages do
           expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return true
           expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, project.namespace.full_path, anything)
 
-          delete api("/projects/#{project.id}/pages", admin )
+          Sidekiq::Testing.inline! do
+            delete api("/projects/#{project.id}/pages", admin )
+          end
 
           expect(project.reload.pages_metadatum.deployed?).to be(false)
         end

spec/services/pages/delete_services_spec.rb

@@ -3,25 +3,45 @@
 require 'spec_helper'
 
 RSpec.describe Pages::DeleteService do
-  let_it_be(:project) { create(:project, path: "my.project")}
-  let_it_be(:admin) { create(:admin) }
-  let_it_be(:domain) { create(:pages_domain, project: project) }
-  let_it_be(:service) { described_class.new(project, admin)}
+  shared_examples 'remove pages' do
+    let_it_be(:project) { create(:project, path: "my.project")}
+    let_it_be(:admin) { create(:admin) }
+    let_it_be(:domain) { create(:pages_domain, project: project) }
+    let_it_be(:service) { described_class.new(project, admin)}
 
-  it 'deletes published pages' do
-    expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return true
-    expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, project.namespace.full_path, anything)
+    it 'deletes published pages' do
+      expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return true
+      expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, project.namespace.full_path, anything)
 
-    service.execute
+      Sidekiq::Testing.inline! { service.execute }
 
-    expect(project.reload.pages_metadatum.deployed?).to be(false)
+      expect(project.reload.pages_metadatum.deployed?).to be(false)
+    end
+
+    it 'deletes all domains' do
+      expect(project.pages_domains.count).to be 1
+
+      Sidekiq::Testing.inline! { service.execute }
+
+      expect(project.reload.pages_domains.count).to be 0
+    end
   end
 
-  it 'deletes all domains' do
-    expect(project.pages_domains.count).to be 1
+  context 'with feature flag disabled' do
+    before do
+      stub_feature_flags(async_pages_removal: false)
+
+      expect(PagesRemoveWorker).not_to receive(:perform_async)
+    end
+
+    it_behaves_like 'remove pages'
+  end
 
-    service.execute
+  context 'with feature flag enabled' do
+    before do
+      expect(PagesRemoveWorker).to receive(:perform_async).and_call_original
+    end
 
-    expect(project.reload.pages_domains.count).to be 0
+    it_behaves_like 'remove pages'
   end
 end

spec/workers/pages_remove_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe PagesRemoveWorker do
+  let_it_be(:project) { create(:project, path: "my.project")}
+  let_it_be(:domain) { create(:pages_domain, project: project) }
+  subject { described_class.new.perform(project.id) }
+
+  it 'deletes published pages' do
+    expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return true
+    expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, project.namespace.full_path, anything)
+
+    subject
+
+    expect(project.reload.pages_metadatum.deployed?).to be(false)
+  end
+
+  it 'deletes all domains' do
+    expect(project.pages_domains.count).to be 1
+
+    subject
+
+    expect(project.reload.pages_domains.count).to be 0
+  end
+end