Commit f2335af2 authored by Vladimir Shushlin's avatar Vladimir Shushlin Committed by Etienne Baqué

Remove legacy storage pages removal logic

parent 81ba6674
...@@ -120,7 +120,6 @@ class Project < ApplicationRecord ...@@ -120,7 +120,6 @@ class Project < ApplicationRecord
use_fast_destroy :build_trace_chunks use_fast_destroy :build_trace_chunks
after_destroy -> { run_after_commit { legacy_remove_pages } }
after_destroy :remove_exports after_destroy :remove_exports
after_validation :check_pending_delete after_validation :check_pending_delete
...@@ -1903,27 +1902,6 @@ class Project < ApplicationRecord ...@@ -1903,27 +1902,6 @@ class Project < ApplicationRecord
.delete_all .delete_all
end end
# TODO: remove this method https://gitlab.com/gitlab-org/gitlab/-/issues/320775
# rubocop: disable CodeReuse/ServiceClass
def legacy_remove_pages
return unless ::Settings.pages.local_store.enabled
# Projects with a missing namespace cannot have their pages removed
return unless namespace
mark_pages_as_not_deployed unless destroyed?
# 1. We rename pages to temporary directory
# 2. We wait 5 minutes, due to NFS caching
# 3. We asynchronously remove pages with force
temp_path = "#{path}.#{SecureRandom.hex}.deleted"
if Gitlab::PagesTransfer.new.rename_project(path, temp_path, namespace.full_path)
PagesWorker.perform_in(5.minutes, :remove, namespace.full_path, temp_path)
end
end
# rubocop: enable CodeReuse/ServiceClass
def mark_pages_as_deployed(artifacts_archive: nil) def mark_pages_as_deployed(artifacts_archive: nil)
ensure_pages_metadatum.update!(deployed: true, artifacts_archive: artifacts_archive) ensure_pages_metadatum.update!(deployed: true, artifacts_archive: artifacts_archive)
end end
......
...@@ -12,9 +12,6 @@ module Pages ...@@ -12,9 +12,6 @@ module Pages
PagesDomain.for_project(project).delete_all PagesDomain.for_project(project).delete_all
DestroyPagesDeploymentsWorker.perform_async(project.id) DestroyPagesDeploymentsWorker.perform_async(project.id)
# TODO: remove this call https://gitlab.com/gitlab-org/gitlab/-/issues/320775
PagesRemoveWorker.perform_async(project.id) if ::Settings.pages.local_store.enabled
end end
end end
end end
# frozen_string_literal: true # frozen_string_literal: true
# TODO: remove this worker https://gitlab.com/gitlab-org/gitlab/-/issues/320775 # TODO: remove this worker https://gitlab.com/gitlab-org/gitlab/-/issues/340641
class PagesRemoveWorker # rubocop:disable Scalability/IdempotentWorker class PagesRemoveWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker include ApplicationWorker
...@@ -11,9 +11,6 @@ class PagesRemoveWorker # rubocop:disable Scalability/IdempotentWorker ...@@ -11,9 +11,6 @@ class PagesRemoveWorker # rubocop:disable Scalability/IdempotentWorker
loggable_arguments 0 loggable_arguments 0
def perform(project_id) def perform(project_id)
project = Project.find_by_id(project_id) # no-op
return unless project
project.legacy_remove_pages
end end
end end
...@@ -4556,44 +4556,6 @@ RSpec.describe Project, factory_default: :keep do ...@@ -4556,44 +4556,6 @@ RSpec.describe Project, factory_default: :keep do
end end
end end
describe '#legacy_remove_pages' do
let(:project) { create(:project).tap { |project| project.mark_pages_as_deployed } }
let(:pages_metadatum) { project.pages_metadatum }
let(:namespace) { project.namespace }
let(:pages_path) { project.pages_path }
around do |example|
FileUtils.mkdir_p(pages_path)
begin
example.run
ensure
FileUtils.rm_rf(pages_path)
end
end
it 'removes the pages directory and marks the project as not having pages deployed' do
expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return(true)
expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, namespace.full_path, anything)
expect { project.legacy_remove_pages }.to change { pages_metadatum.reload.deployed }.from(true).to(false)
end
it 'does nothing if updates on legacy storage are disabled' do
allow(Settings.pages.local_store).to receive(:enabled).and_return(false)
expect(Gitlab::PagesTransfer).not_to receive(:new)
expect(PagesWorker).not_to receive(:perform_in)
project.legacy_remove_pages
end
it 'is run when the project is destroyed' do
expect(project).to receive(:legacy_remove_pages).and_call_original
expect { project.destroy! }.not_to raise_error
end
end
describe '#remove_export' do describe '#remove_export' do
let(:project) { create(:project, :with_export) } let(:project) { create(:project, :with_export) }
......
...@@ -36,12 +36,7 @@ RSpec.describe API::Pages do ...@@ -36,12 +36,7 @@ RSpec.describe API::Pages do
end end
it 'removes the pages' do it 'removes the pages' do
expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return true delete api("/projects/#{project.id}/pages", admin )
expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, project.namespace.full_path, anything)
Sidekiq::Testing.inline! do
delete api("/projects/#{project.id}/pages", admin )
end
expect(project.reload.pages_metadatum.deployed?).to be(false) expect(project.reload.pages_metadatum.deployed?).to be(false)
end end
......
...@@ -12,27 +12,6 @@ RSpec.describe Pages::DeleteService do ...@@ -12,27 +12,6 @@ RSpec.describe Pages::DeleteService do
project.mark_pages_as_deployed project.mark_pages_as_deployed
end end
it 'deletes published pages', :sidekiq_inline do
expect_next_instance_of(Gitlab::PagesTransfer) do |pages_transfer|
expect(pages_transfer).to receive(:rename_project).and_return true
end
expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, project.namespace.full_path, anything)
service.execute
end
it "doesn't remove anything from the legacy storage if local_store is disabled", :sidekiq_inline do
allow(Settings.pages.local_store).to receive(:enabled).and_return(false)
expect(project.pages_deployed?).to be(true)
expect(PagesWorker).not_to receive(:perform_in)
service.execute
expect(project.pages_deployed?).to be(false)
end
it 'marks pages as not deployed' do it 'marks pages as not deployed' do
expect do expect do
service.execute service.execute
......
...@@ -18,10 +18,6 @@ RSpec.describe Projects::UpdatePagesService do ...@@ -18,10 +18,6 @@ RSpec.describe Projects::UpdatePagesService do
subject { described_class.new(project, build) } subject { described_class.new(project, build) }
before do
project.legacy_remove_pages
end
context 'for new artifacts' do context 'for new artifacts' do
context "for a valid job" do context "for a valid job" do
let!(:artifacts_archive) { create(:ci_job_artifact, :correct_checksum, file: file, job: build) } let!(:artifacts_archive) { create(:ci_job_artifact, :correct_checksum, file: file, job: build) }
......
...@@ -48,12 +48,6 @@ RSpec.describe NamespacelessProjectDestroyWorker do ...@@ -48,12 +48,6 @@ RSpec.describe NamespacelessProjectDestroyWorker do
subject.perform(project.id) subject.perform(project.id)
end end
it 'does not do anything in Project#legacy_remove_pages method' do
expect(Gitlab::PagesTransfer).not_to receive(:new)
subject.perform(project.id)
end
end end
context 'project forked from another' do context 'project forked from another' do
......
...@@ -3,23 +3,9 @@ ...@@ -3,23 +3,9 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe PagesRemoveWorker do RSpec.describe PagesRemoveWorker do
let(:project) { create(:project, path: "my.project")} it 'does not raise error' do
let!(:domain) { create(:pages_domain, project: project) } expect do
described_class.new.perform(create(:project).id)
subject { described_class.new.perform(project.id) } end.not_to raise_error
before do
project.mark_pages_as_deployed
end
it 'deletes published pages' do
expect(project.pages_deployed?).to be(true)
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_deployed?).to be(false)
end 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