Commit 192d4e52 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Only update pages config if pages are deployed

The pages config is a file inside the path on the shared NFS. The
directory into which this file is written is created when pages are
deployed and the artifacts are extracted.

If this path has been removed, or hasn't been created yet, we don't
need to update configuration.

Updating the configuration is also a step in the deployment of
pages. So the first deploy, the configuration would be written.
parent e1724883
......@@ -11,6 +11,11 @@ module Projects
end
def execute
# 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
return success unless project.pages_deployed?
unless file_equals?(pages_config_file, pages_config_json)
update_file(pages_config_file, pages_config_json)
reload_daemon
......
......@@ -142,6 +142,8 @@ module Projects
end
def update_pages_config
return unless project.pages_deployed?
if Feature.enabled?(:async_update_pages_config, project)
PagesUpdateConfigurationWorker.perform_async(project.id)
else
......
......@@ -3,50 +3,75 @@
require 'spec_helper'
RSpec.describe Projects::UpdatePagesConfigurationService do
let(:project) { create(:project) }
let(:service) { described_class.new(project) }
describe "#execute" do
let(:file) { Tempfile.new('pages-test') }
subject { service.execute }
after do
file.close
file.unlink
end
context 'when pages are deployed' do
let_it_be(:project) do
create(:project).tap(&:mark_pages_as_deployed)
end
before do
allow(service).to receive(:pages_config_file).and_return(file.path)
end
let(:file) { Tempfile.new('pages-test') }
context 'when configuration changes' do
it 'updates the .update file' do
expect(service).to receive(:reload_daemon).and_call_original
before do
allow(service).to receive(:pages_config_file).and_return(file.path)
end
expect(subject).to include(status: :success)
after do
file.close
file.unlink
end
end
context 'when configuration does not change' do
before do
# we set the configuration
service.execute
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
expect(subject).to include(status: :success)
end
end
context 'when configuration does not change' do
before do
# we set the configuration
service.execute
end
it 'does not update the .update file' do
expect(service).not_to receive(:reload_daemon)
expect(subject).to include(status: :success)
end
end
it 'does not update the .update file' do
expect(service).not_to receive(:reload_daemon)
context 'when an error occurs' do
it 'returns an error object' do
e = StandardError.new("Failure")
allow(service).to receive(:reload_daemon).and_raise(e)
expect(subject).to include(status: :success)
expect(subject).to eq(status: :error, message: "Failure", exception: e)
end
end
end
context 'when an error occurs' do
it 'returns an error object' do
e = StandardError.new("Failure")
allow(service).to receive(:reload_daemon).and_raise(e)
context 'when pages are not deployed' do
let_it_be(:project) do
create(:project).tap(&:mark_pages_as_not_deployed)
end
it 'returns successfully' do
expect(subject).to eq(status: :success)
end
it 'does not update the config' do
expect(service).not_to receive(:update_file)
expect(subject).to eq(status: :error, message: "Failure", exception: e)
subject
end
end
end
......
......@@ -397,18 +397,30 @@ RSpec.describe Projects::UpdateService do
end
shared_examples 'updating pages configuration' do
it 'schedules the `PagesUpdateConfigurationWorker`' do
it 'schedules the `PagesUpdateConfigurationWorker` when pages are deployed' do
project.mark_pages_as_deployed
expect(PagesUpdateConfigurationWorker).to receive(:perform_async).with(project.id)
subject
end
it "does not schedule a job when pages aren't deployed" do
project.mark_pages_as_not_deployed
expect(PagesUpdateConfigurationWorker).not_to receive(:perform_async).with(project.id)
subject
end
context 'when `async_update_pages_config` is disabled' do
before do
stub_feature_flags(async_update_pages_config: false)
end
it 'calls Projects::UpdatePagesConfigurationService' do
it 'calls Projects::UpdatePagesConfigurationService when pages are deployed' do
project.mark_pages_as_deployed
expect(Projects::UpdatePagesConfigurationService)
.to receive(:new)
.with(project)
......@@ -416,6 +428,15 @@ RSpec.describe Projects::UpdateService do
subject
end
it "does not update pages config when pages aren't deployed" do
project.mark_pages_as_not_deployed
expect(Projects::UpdatePagesConfigurationService)
.not_to receive(:new)
subject
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment