Merge branch...

Merge branch '300619-stop-updating-pages-configuration-on-disk-if-feature-flag-is-disabled' into 'master'

Stop updating pages configuration on disk if feature flag is disabled

See merge request gitlab-org/gitlab!54086

app/services/projects/update_pages_configuration_service.rb

@@ -11,6 +11,8 @@ module Projects
     end
 
     def execute
+      return success unless Feature.enabled?(:pages_update_legacy_storage, default_enabled: true)
+
       # If the pages were never deployed, we can't write out the config, as the
       # directory would not exist.
       # https://gitlab.com/gitlab-org/gitlab/-/issues/235139

app/workers/pages_update_configuration_worker.rb

@@ -6,6 +6,12 @@ class PagesUpdateConfigurationWorker
   idempotent!
   feature_category :pages
 
+  def self.perform_async(*args)
+    return unless Feature.enabled?(:pages_update_legacy_storage, default_enabled: true)
+
+    super(*args)
+  end
+
   def perform(project_id)
     project = Project.find_by_id(project_id)
     return unless project

spec/services/projects/update_pages_configuration_service_spec.rb

@@ -26,11 +26,18 @@ RSpec.describe Projects::UpdatePagesConfigurationService do
 
       context 'when configuration changes' do
         it 'updates the config and reloads the daemon' do
-          allow(service).to receive(:update_file).and_call_original
-
           expect(service).to receive(:update_file).with(file.path, an_instance_of(String))
             .and_call_original
-          expect(service).to receive(:reload_daemon).and_call_original
+          allow(service).to receive(:update_file).with(File.join(::Settings.pages.path, '.update'),
+                                                       an_instance_of(String)).and_call_original
+
+          expect(subject).to include(status: :success)
+        end
+
+        it "doesn't update configuration files if updates on legacy storage are disabled" do
+          stub_feature_flags(pages_update_legacy_storage: false)
+
+          expect(service).not_to receive(:update_file)
 
           expect(subject).to include(status: :success)
         end
@@ -42,8 +49,8 @@ RSpec.describe Projects::UpdatePagesConfigurationService do
           service.execute
         end
 
-        it 'does not update the .update file' do
-          expect(service).not_to receive(:reload_daemon)
+        it 'does not update anything' do
+          expect(service).not_to receive(:update_file)
 
           expect(subject).to include(status: :success)
         end

spec/workers/pages_update_configuration_worker_spec.rb

@@ -2,9 +2,9 @@
 require "spec_helper"
 
 RSpec.describe PagesUpdateConfigurationWorker do
-  describe "#perform" do
-    let_it_be(:project) { create(:project) }
+  let_it_be(:project) { create(:project) }
 
+  describe "#perform" do
     it "does not break if the project doesn't exist" do
       expect { subject.perform(-1) }.not_to raise_error
     end
@@ -42,4 +42,22 @@ RSpec.describe PagesUpdateConfigurationWorker do
       end
     end
   end
+
+  describe '#perform_async' do
+    it "calls the correct service", :sidekiq_inline do
+      expect_next_instance_of(Projects::UpdatePagesConfigurationService, project) do |service|
+        expect(service).to receive(:execute).and_return(status: :success)
+      end
+
+      described_class.perform_async(project.id)
+    end
+
+    it "doesn't schedule a worker if updates on legacy storage are disabled", :sidekiq_inline do
+      stub_feature_flags(pages_update_legacy_storage: false)
+
+      expect(Projects::UpdatePagesConfigurationService).not_to receive(:new)
+
+      described_class.perform_async(project.id)
+    end
+  end
 end