From 06db6416ec19eaa63a4cea2ec48838873f466fe2 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre <dbalexandre@gmail.com> Date: Wed, 31 Jan 2024 20:20:09 -0300 Subject: [PATCH] Remove legacy hashed storage migration code This code is not being used since the hashed storage migration has already been enforced, and the Geo legacy replication code has been removed. Changelog: removed EE: true --- .rubocop_todo/layout/line_length.yml | 2 - .rubocop_todo/rails/time_zone.yml | 1 - .rubocop_todo/rspec/expect_change.yml | 2 - .rubocop_todo/rspec/feature_category.yml | 1 - .rubocop_todo/rspec/named_subject.yml | 3 - .../migrate_attachments_service.rb | 2 - ee/app/models/geo/event_log.rb | 18 ++-- .../geo/hashed_storage_attachments_event.rb | 12 --- .../geo/hashed_storage_migrated_event.rb | 14 --- .../migrate_attachments_service.rb | 24 ----- .../hashed_storage_attachments_event_store.rb | 25 ------ ...d_storage_attachments_migration_service.rb | 57 ------------ ...ed_storage_attachments_migration_worker.rb | 8 +- .../hashed_storage_attachments_event.rb | 23 +---- ee/spec/factories/geo/event_log.rb | 26 ------ .../lib/gitlab/geo/log_cursor/daemon_spec.rb | 74 ---------------- .../hashed_storage_attachments_event_spec.rb | 32 ------- ee/spec/models/geo/event_log_spec.rb | 16 ---- .../geo/hashed_storage_migrated_event_spec.rb | 19 ---- ...ed_storage_attachments_event_store_spec.rb | 39 --------- ...rage_attachments_migration_service_spec.rb | 87 ------------------- .../migrate_attachments_service_spec.rb | 60 ------------- spec/support/rspec_order_todo.yml | 5 -- 23 files changed, 9 insertions(+), 541 deletions(-) delete mode 100644 ee/app/models/geo/hashed_storage_attachments_event.rb delete mode 100644 ee/app/models/geo/hashed_storage_migrated_event.rb delete mode 100644 ee/app/services/ee/projects/hashed_storage/migrate_attachments_service.rb delete mode 100644 ee/app/services/geo/hashed_storage_attachments_event_store.rb delete mode 100644 ee/app/services/geo/hashed_storage_attachments_migration_service.rb delete mode 100644 ee/spec/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event_spec.rb delete mode 100644 ee/spec/models/geo/hashed_storage_migrated_event_spec.rb delete mode 100644 ee/spec/services/geo/hashed_storage_attachments_event_store_spec.rb delete mode 100644 ee/spec/services/geo/hashed_storage_attachments_migration_service_spec.rb delete mode 100644 ee/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index ea079e765f41..5f9a6d282c88 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -985,7 +985,6 @@ Layout/LineLength: - 'ee/app/services/geo/event_service.rb' - 'ee/app/services/geo/file_registry_removal_service.rb' - 'ee/app/services/geo/framework_repository_sync_service.rb' - - 'ee/app/services/geo/hashed_storage_attachments_migration_service.rb' - 'ee/app/services/geo/request_service.rb' - 'ee/app/services/geo/verification_state_backfill_service.rb' - 'ee/app/services/gitlab_subscriptions/plan_upgrade_service.rb' @@ -2061,7 +2060,6 @@ Layout/LineLength: - 'ee/spec/services/geo/blob_download_service_spec.rb' - 'ee/spec/services/geo/container_repository_sync_spec.rb' - 'ee/spec/services/geo/framework_repository_sync_service_spec.rb' - - 'ee/spec/services/geo/hashed_storage_attachments_event_store_spec.rb' - 'ee/spec/services/gitlab_subscriptions/check_future_renewal_service_spec.rb' - 'ee/spec/services/gitlab_subscriptions/create_service_spec.rb' - 'ee/spec/services/groups/memberships/export_service_spec.rb' diff --git a/.rubocop_todo/rails/time_zone.yml b/.rubocop_todo/rails/time_zone.yml index afe92cc4ad7f..85593dce598a 100644 --- a/.rubocop_todo/rails/time_zone.yml +++ b/.rubocop_todo/rails/time_zone.yml @@ -11,7 +11,6 @@ Rails/TimeZone: - 'ee/spec/lib/gitlab/geo/base_request_spec.rb' - 'ee/spec/lib/gitlab/geo/log_cursor/events/cache_invalidation_event_spec.rb' - 'ee/spec/lib/gitlab/geo/log_cursor/events/event_spec.rb' - - 'ee/spec/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event_spec.rb' - 'ee/spec/lib/gitlab/geo/log_cursor/logger_spec.rb' - 'lib/api/helpers.rb' - 'lib/api/sidekiq_metrics.rb' diff --git a/.rubocop_todo/rspec/expect_change.yml b/.rubocop_todo/rspec/expect_change.yml index c5f56901f5e7..0e83d5d76b78 100644 --- a/.rubocop_todo/rspec/expect_change.yml +++ b/.rubocop_todo/rspec/expect_change.yml @@ -22,7 +22,6 @@ RSpec/ExpectChange: - 'ee/spec/lib/gitlab/compliance_management/violations/approved_by_committer_spec.rb' - 'ee/spec/lib/gitlab/compliance_management/violations/approved_by_insufficient_users_spec.rb' - 'ee/spec/lib/gitlab/compliance_management/violations/approved_by_merge_request_author_spec.rb' - - 'ee/spec/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event_spec.rb' - 'ee/spec/lib/gitlab/instrumentation/elasticsearch_transport_spec.rb' - 'ee/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb' - 'ee/spec/lib/quality/seeders/vulnerabilities_spec.rb' @@ -134,7 +133,6 @@ RSpec/ExpectChange: - 'ee/spec/services/iterations/roll_over_issues_service_spec.rb' - 'ee/spec/services/projects/alerting/notify_service_spec.rb' - 'ee/spec/services/projects/destroy_service_spec.rb' - - 'ee/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb' - 'ee/spec/services/projects/transfer_service_spec.rb' - 'ee/spec/services/protected_environments/create_service_spec.rb' - 'ee/spec/services/quality_management/test_cases/create_service_spec.rb' diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index 826aee1d3ae0..d5b9845951e0 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -1378,7 +1378,6 @@ RSpec/FeatureCategory: - 'ee/spec/services/projects/gitlab_projects_import_service_spec.rb' - 'ee/spec/services/projects/group_links/destroy_service_spec.rb' - 'ee/spec/services/projects/group_links/update_service_spec.rb' - - 'ee/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb' - 'ee/spec/services/projects/import_export/export_service_spec.rb' - 'ee/spec/services/projects/import_service_spec.rb' - 'ee/spec/services/projects/mark_for_deletion_service_spec.rb' diff --git a/.rubocop_todo/rspec/named_subject.yml b/.rubocop_todo/rspec/named_subject.yml index ff8e1ef5391b..4e4aa25c4176 100644 --- a/.rubocop_todo/rspec/named_subject.yml +++ b/.rubocop_todo/rspec/named_subject.yml @@ -429,7 +429,6 @@ RSpec/NamedSubject: - 'ee/spec/lib/gitlab/geo/log_cursor/event_logs_spec.rb' - 'ee/spec/lib/gitlab/geo/log_cursor/events/cache_invalidation_event_spec.rb' - 'ee/spec/lib/gitlab/geo/log_cursor/events/event_spec.rb' - - 'ee/spec/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event_spec.rb' - 'ee/spec/lib/gitlab/geo/oauth/session_spec.rb' - 'ee/spec/lib/gitlab/geo/registry_batcher_spec.rb' - 'ee/spec/lib/gitlab/geo/replication/blob_retriever_spec.rb' @@ -1040,7 +1039,6 @@ RSpec/NamedSubject: - 'ee/spec/services/geo/file_registry_removal_service_spec.rb' - 'ee/spec/services/geo/framework_repository_sync_service_spec.rb' - 'ee/spec/services/geo/graphql_request_service_spec.rb' - - 'ee/spec/services/geo/hashed_storage_attachments_event_store_spec.rb' - 'ee/spec/services/geo/metrics_update_service_spec.rb' - 'ee/spec/services/geo/replication_toggle_request_service_spec.rb' - 'ee/spec/services/gitlab_subscriptions/activate_service_spec.rb' @@ -1098,7 +1096,6 @@ RSpec/NamedSubject: - 'ee/spec/services/projects/gitlab_projects_import_service_spec.rb' - 'ee/spec/services/projects/group_links/create_service_spec.rb' - 'ee/spec/services/projects/group_links/destroy_service_spec.rb' - - 'ee/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb' - 'ee/spec/services/projects/import_export/export_service_spec.rb' - 'ee/spec/services/projects/import_service_spec.rb' - 'ee/spec/services/projects/mark_for_deletion_service_spec.rb' diff --git a/app/services/projects/hashed_storage/migrate_attachments_service.rb b/app/services/projects/hashed_storage/migrate_attachments_service.rb index 40c4fd5376c3..a61ea4595332 100644 --- a/app/services/projects/hashed_storage/migrate_attachments_service.rb +++ b/app/services/projects/hashed_storage/migrate_attachments_service.rb @@ -63,5 +63,3 @@ def find_old_attachments_path(project) end end end - -Projects::HashedStorage::MigrateAttachmentsService.prepend_mod_with('Projects::HashedStorage::MigrateAttachmentsService') diff --git a/ee/app/models/geo/event_log.rb b/ee/app/models/geo/event_log.rb index 05876d557d08..676a381c9b03 100644 --- a/ee/app/models/geo/event_log.rb +++ b/ee/app/models/geo/event_log.rb @@ -3,8 +3,14 @@ module Geo class EventLog < ApplicationRecord include IgnorableColumns + ignore_column :geo_event_id_convert_to_bigint, remove_with: '16.11', remove_after: '2024-03-21' + ignore_columns %i[ + hashed_storage_migrated_event_id + hashed_storage_attachments_event_id + ], remove_with: '17.0', remove_after: '2024-05-16' + include Geo::Model include ::EachBatch @@ -15,8 +21,6 @@ class EventLog < ApplicationRecord Geo::RepositoryRenamedEvent Geo::RepositoriesChangedEvent Geo::ResetChecksumEvent - Geo::HashedStorageMigratedEvent - Geo::HashedStorageAttachmentsEvent Geo::Event].freeze belongs_to :cache_invalidation_event, @@ -43,14 +47,6 @@ class EventLog < ApplicationRecord class_name: 'Geo::RepositoriesChangedEvent', foreign_key: :repositories_changed_event_id - belongs_to :hashed_storage_migrated_event, - class_name: 'Geo::HashedStorageMigratedEvent', - foreign_key: :hashed_storage_migrated_event_id - - belongs_to :hashed_storage_attachments_event, - class_name: 'Geo::HashedStorageAttachmentsEvent', - foreign_key: :hashed_storage_attachments_event_id - belongs_to :reset_checksum_event, class_name: 'Geo::ResetChecksumEvent', foreign_key: :reset_checksum_event_id @@ -86,8 +82,6 @@ def event repository_deleted_event || repository_renamed_event || repositories_changed_event || - hashed_storage_migrated_event || - hashed_storage_attachments_event || reset_checksum_event || cache_invalidation_event || geo_event diff --git a/ee/app/models/geo/hashed_storage_attachments_event.rb b/ee/app/models/geo/hashed_storage_attachments_event.rb deleted file mode 100644 index b27534cb8d0a..000000000000 --- a/ee/app/models/geo/hashed_storage_attachments_event.rb +++ /dev/null @@ -1,12 +0,0 @@ -# frozen_string_literal: true - -module Geo - class HashedStorageAttachmentsEvent < ApplicationRecord - include Geo::Model - include Geo::Eventable - - belongs_to :project - - validates :project, :old_attachments_path, :new_attachments_path, presence: true - end -end diff --git a/ee/app/models/geo/hashed_storage_migrated_event.rb b/ee/app/models/geo/hashed_storage_migrated_event.rb deleted file mode 100644 index 24b85b2f2485..000000000000 --- a/ee/app/models/geo/hashed_storage_migrated_event.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -module Geo - class HashedStorageMigratedEvent < ApplicationRecord - include Geo::Model - include Geo::Eventable - - belongs_to :project - - validates :project, :repository_storage_name, - :old_disk_path, :new_disk_path, :old_wiki_disk_path, - :new_wiki_disk_path, :new_storage_version, presence: true - end -end diff --git a/ee/app/services/ee/projects/hashed_storage/migrate_attachments_service.rb b/ee/app/services/ee/projects/hashed_storage/migrate_attachments_service.rb deleted file mode 100644 index 0bbbb80d4b05..000000000000 --- a/ee/app/services/ee/projects/hashed_storage/migrate_attachments_service.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -module EE - module Projects - module HashedStorage - module MigrateAttachmentsService - extend ::Gitlab::Utils::Override - - override :execute - def execute - super do - break true if skipped? - - ::Geo::HashedStorageAttachmentsEventStore.new( - project, - old_attachments_path: old_disk_path, - new_attachments_path: new_disk_path - ).create! - end - end - end - end - end -end diff --git a/ee/app/services/geo/hashed_storage_attachments_event_store.rb b/ee/app/services/geo/hashed_storage_attachments_event_store.rb deleted file mode 100644 index 1d53bf56a5ef..000000000000 --- a/ee/app/services/geo/hashed_storage_attachments_event_store.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -module Geo - class HashedStorageAttachmentsEventStore < EventStore - self.event_type = :hashed_storage_attachments_event - - private - - def build_event - Geo::HashedStorageAttachmentsEvent.new( - project: project, - old_attachments_path: old_attachments_path, - new_attachments_path: new_attachments_path - ) - end - - def old_attachments_path - params.fetch(:old_attachments_path) - end - - def new_attachments_path - params.fetch(:new_attachments_path) - end - end -end diff --git a/ee/app/services/geo/hashed_storage_attachments_migration_service.rb b/ee/app/services/geo/hashed_storage_attachments_migration_service.rb deleted file mode 100644 index 064f3b551619..000000000000 --- a/ee/app/services/geo/hashed_storage_attachments_migration_service.rb +++ /dev/null @@ -1,57 +0,0 @@ -# frozen_string_literal: true - -module Geo - AttachmentMigrationError = Class.new(StandardError) - - class HashedStorageAttachmentsMigrationService - include ::Gitlab::Geo::LogHelpers - - attr_reader :project_id, :old_attachments_path, :new_attachments_path - - def initialize(project_id, old_attachments_path:, new_attachments_path:) - @project_id = project_id - @old_attachments_path = old_attachments_path - @new_attachments_path = new_attachments_path - end - - def async_execute - Geo::HashedStorageAttachmentsMigrationWorker.perform_async( - project_id, - old_attachments_path, - new_attachments_path - ) - end - - def execute - origin = File.join(FileUploader.root, old_attachments_path) - target = File.join(FileUploader.root, new_attachments_path) - move_folder!(origin, target) - end - - private - - def project - @project ||= Project.find(project_id) - end - - def move_folder!(old_path, new_path) - unless File.directory?(old_path) - log_info("Skipped attachments migration to Hashed Storage, source path doesn't exist or is not a directory", project_id: project.id, source: old_path, target: new_path) - return - end - - if File.exist?(new_path) - log_error("Cannot migrate attachments to Hashed Storage, target path already exist", project_id: project.id, source: old_path, target: new_path) - raise AttachmentMigrationError, "Target path '#{new_path}' already exist" - end - - # Create hashed storage base path folder - FileUtils.mkdir_p(File.dirname(new_path)) - - FileUtils.mv(old_path, new_path) - log_info("Migrated project attachments to Hashed Storage", project_id: project.id, source: old_path, target: new_path) - - true - end - end -end diff --git a/ee/app/workers/geo/hashed_storage_attachments_migration_worker.rb b/ee/app/workers/geo/hashed_storage_attachments_migration_worker.rb index 82dce390b2ee..cf18b7f6dc96 100644 --- a/ee/app/workers/geo/hashed_storage_attachments_migration_worker.rb +++ b/ee/app/workers/geo/hashed_storage_attachments_migration_worker.rb @@ -10,12 +10,8 @@ class HashedStorageAttachmentsMigrationWorker # rubocop:disable Scalability/Idem loggable_arguments 1, 2 - def perform(project_id, old_attachments_path, new_attachments_path) - Geo::HashedStorageAttachmentsMigrationService.new( - project_id, - old_attachments_path: old_attachments_path, - new_attachments_path: new_attachments_path - ).execute + def perform(_project_id, _old_attachments_path, _new_attachments_path) + # no-op end end end diff --git a/ee/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event.rb b/ee/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event.rb index bf77dc84af85..d31a4e20b981 100644 --- a/ee/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event.rb +++ b/ee/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event.rb @@ -8,28 +8,7 @@ class HashedStorageAttachmentsEvent include BaseEvent def process - job_id = hashed_storage_attachments_migrate - log_event(job_id) - end - - private - - def hashed_storage_attachments_migrate - # Must always schedule, regardless of shard health - ::Geo::HashedStorageAttachmentsMigrationService.new( - event.project_id, - old_attachments_path: event.old_attachments_path, - new_attachments_path: event.new_attachments_path - ).async_execute - end - - def log_event(job_id) - super( - 'Migrating attachments to hashed storage', - project_id: event.project_id, - old_attachments_path: event.old_attachments_path, - new_attachments_path: event.new_attachments_path, - job_id: job_id) + # no-op end end end diff --git a/ee/spec/factories/geo/event_log.rb b/ee/spec/factories/geo/event_log.rb index 27a5f49d7b90..1bb95ba17978 100644 --- a/ee/spec/factories/geo/event_log.rb +++ b/ee/spec/factories/geo/event_log.rb @@ -18,14 +18,6 @@ repository_renamed_event factory: :geo_repository_renamed_event end - trait :hashed_storage_migration_event do - hashed_storage_migrated_event factory: :geo_hashed_storage_migrated_event - end - - trait :hashed_storage_attachments_event do - hashed_storage_attachments_event factory: :geo_hashed_storage_attachments_event - end - trait :reset_checksum_event do reset_checksum_event factory: :geo_reset_checksum_event end @@ -80,24 +72,6 @@ new_path { "#{project.path}_new" } end - factory :geo_hashed_storage_migrated_event, class: 'Geo::HashedStorageMigratedEvent' do - project { association(:project, :repository) } - - repository_storage_name { project.repository_storage } - old_disk_path { project.path_with_namespace } - new_disk_path { "#{project.path_with_namespace}_new" } - old_wiki_disk_path { project.wiki.path_with_namespace } - new_wiki_disk_path { "#{project.wiki.path_with_namespace}_new" } - new_storage_version { Project::HASHED_STORAGE_FEATURES[:repository] } - end - - factory :geo_hashed_storage_attachments_event, class: 'Geo::HashedStorageAttachmentsEvent' do - project { association(:project, :repository) } - - old_attachments_path { Storage::LegacyProject.new(project).disk_path } - new_attachments_path { Storage::Hashed.new(project).disk_path } - end - factory :geo_reset_checksum_event, class: 'Geo::ResetChecksumEvent' do project end diff --git a/ee/spec/lib/gitlab/geo/log_cursor/daemon_spec.rb b/ee/spec/lib/gitlab/geo/log_cursor/daemon_spec.rb index bba07dc39e38..9098dac28f71 100644 --- a/ee/spec/lib/gitlab/geo/log_cursor/daemon_spec.rb +++ b/ee/spec/lib/gitlab/geo/log_cursor/daemon_spec.rb @@ -142,80 +142,6 @@ daemon.find_and_handle_events! end end - - context 'when node has namespace restrictions' do - let(:group_1) { create(:group) } - let(:group_2) { create(:group) } - let(:project) { create(:project, group: group_1) } - let(:hashed_storage_attachments_event) { create(:geo_hashed_storage_attachments_event, project: project) } - let(:event_log) { create(:geo_event_log, hashed_storage_attachments_event: hashed_storage_attachments_event) } - let!(:event_log_state) { create(:geo_event_log_state, event_id: event_log.id - 1) } - let!(:registry) { create(:geo_project_repository_registry, :synced, project: project) } - - before do - allow(Gitlab::ShardHealthCache).to receive(:healthy_shard?).with('default').and_return(true) - allow(Gitlab::Geo::Logger).to receive(:info).and_call_original - end - - it 'detects when an event was skipped' do - updated_event = create(:geo_hashed_storage_attachments_event, project: project) - new_event = create(:geo_event_log, id: event_log.id + 2, hashed_storage_attachments_event: updated_event) - - daemon.find_and_handle_events! - - create(:geo_event_log, id: event_log.id + 1) - - expect(read_gaps).to eq([event_log.id + 1]) - - expect(::Geo::EventLogState.last_processed.id).to eq(new_event.id) - end - - it 'detects when an event was skipped between batches' do - updated_event = create(:geo_hashed_storage_attachments_event, project: project) - new_event = create(:geo_event_log, hashed_storage_attachments_event: updated_event) - - daemon.find_and_handle_events! - - create(:geo_event_log, id: new_event.id + 3, hashed_storage_attachments_event: updated_event) - - daemon.find_and_handle_events! - - create(:geo_event_log, id: new_event.id + 1, hashed_storage_attachments_event: updated_event) - create(:geo_event_log, id: new_event.id + 2, hashed_storage_attachments_event: updated_event) - - expect(read_gaps).to eq([new_event.id + 1, new_event.id + 2]) - end - - it "logs a message if an associated event can't be found" do - new_event = create(:geo_event_log) - - expect(Gitlab::Geo::Logger).to receive(:warn) - .with(hash_including( - class: 'Gitlab::Geo::LogCursor::Daemon', - message: '#handle_single_event: unknown event', - event_log_id: new_event.id)) - - daemon.find_and_handle_events! - - expect(::Geo::EventLogState.last_processed.id).to eq(new_event.id) - end - - it 'logs a message for skipped events' do - secondary.update!(selective_sync_type: 'namespaces', namespaces: [group_2]) - - expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including( - :pid, - :cursor_delay_s, - message: 'Skipped event', - class: 'Gitlab::Geo::LogCursor::Daemon', - event_log_id: event_log.id, - event_id: hashed_storage_attachments_event.id, - event_type: 'Geo::HashedStorageAttachmentsEvent', - project_id: project.id)) - - daemon.find_and_handle_events! - end - end end describe '#handle_events' do diff --git a/ee/spec/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event_spec.rb b/ee/spec/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event_spec.rb deleted file mode 100644 index 74266a2275bb..000000000000 --- a/ee/spec/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Geo::LogCursor::Events::HashedStorageAttachmentsEvent, - :clean_gitlab_redis_shared_state, - feature_category: :geo_replication do - let(:logger) { Gitlab::Geo::LogCursor::Logger.new(described_class, Logger::INFO) } - let(:event_log) { create(:geo_event_log, :hashed_storage_attachments_event) } - let!(:event_log_state) { create(:geo_event_log_state, event_id: event_log.id - 1) } - let(:hashed_storage_attachments_event) { event_log.hashed_storage_attachments_event } - let(:project) { hashed_storage_attachments_event.project } - let(:old_attachments_path) { hashed_storage_attachments_event.old_attachments_path } - let(:new_attachments_path) { hashed_storage_attachments_event.new_attachments_path } - - subject { described_class.new(hashed_storage_attachments_event, Time.now, logger) } - - around do |example| - Sidekiq::Testing.fake! { example.run } - end - - describe '#process' do - it 'schedules a Geo::HashedStorageAttachmentsMigrationWorker' do - expect(::Geo::HashedStorageAttachmentsMigrationWorker).to receive(:perform_async) - .with(project.id, old_attachments_path, new_attachments_path) - - subject.process - end - - it_behaves_like 'logs event source info' - end -end diff --git a/ee/spec/models/geo/event_log_spec.rb b/ee/spec/models/geo/event_log_spec.rb index bb6ec34ec6c4..3dd3cdca5b21 100644 --- a/ee/spec/models/geo/event_log_spec.rb +++ b/ee/spec/models/geo/event_log_spec.rb @@ -11,8 +11,6 @@ it { is_expected.to belong_to(:repository_renamed_event).class_name('Geo::RepositoryRenamedEvent').with_foreign_key('repository_renamed_event_id') } it { is_expected.to belong_to(:repository_updated_event).class_name('Geo::RepositoryUpdatedEvent').with_foreign_key('repository_updated_event_id') } it { is_expected.to belong_to(:reset_checksum_event).class_name('Geo::ResetChecksumEvent').with_foreign_key('reset_checksum_event_id') } - it { is_expected.to belong_to(:hashed_storage_migrated_event).class_name('Geo::HashedStorageMigratedEvent').with_foreign_key('hashed_storage_migrated_event_id') } - it { is_expected.to belong_to(:hashed_storage_attachments_event).class_name('Geo::HashedStorageAttachmentsEvent').with_foreign_key('hashed_storage_attachments_event_id') } end describe '.next_unprocessed_event' do @@ -84,20 +82,6 @@ expect(subject.event).to eq repositories_changed_event end - it 'returns hashed_storage_migrated_event when set' do - hashed_storage_migrated_event = build(:geo_hashed_storage_migrated_event) - subject.hashed_storage_migrated_event = hashed_storage_migrated_event - - expect(subject.event).to eq hashed_storage_migrated_event - end - - it 'returns hashed_storage_attachments_event when set' do - hashed_storage_attachments_event = build(:geo_hashed_storage_attachments_event) - subject.hashed_storage_attachments_event = hashed_storage_attachments_event - - expect(subject.event).to eq hashed_storage_attachments_event - end - it 'returns reset_checksum_event when set' do reset_checksum_event = build(:geo_reset_checksum_event) subject.reset_checksum_event = reset_checksum_event diff --git a/ee/spec/models/geo/hashed_storage_migrated_event_spec.rb b/ee/spec/models/geo/hashed_storage_migrated_event_spec.rb deleted file mode 100644 index 5e1a5280303e..000000000000 --- a/ee/spec/models/geo/hashed_storage_migrated_event_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Geo::HashedStorageMigratedEvent, type: :model, feature_category: :geo_replication do - describe 'relationships' do - it { is_expected.to belong_to(:project) } - end - - describe 'validations' do - it { is_expected.to validate_presence_of(:project) } - it { is_expected.to validate_presence_of(:repository_storage_name) } - it { is_expected.to validate_presence_of(:old_disk_path) } - it { is_expected.to validate_presence_of(:new_disk_path) } - it { is_expected.to validate_presence_of(:old_wiki_disk_path) } - it { is_expected.to validate_presence_of(:new_wiki_disk_path) } - it { is_expected.to validate_presence_of(:new_storage_version) } - end -end diff --git a/ee/spec/services/geo/hashed_storage_attachments_event_store_spec.rb b/ee/spec/services/geo/hashed_storage_attachments_event_store_spec.rb deleted file mode 100644 index 9ce1cf7d36c8..000000000000 --- a/ee/spec/services/geo/hashed_storage_attachments_event_store_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Geo::HashedStorageAttachmentsEventStore, feature_category: :geo_replication do - include EE::GeoHelpers - - let_it_be(:secondary_node) { create(:geo_node) } - - let(:project) { create(:project, path: 'bar') } - let(:attachments_event) { build(:geo_hashed_storage_attachments_event, project: project) } - let(:old_attachments_path) { attachments_event.old_attachments_path } - let(:new_attachments_path) { attachments_event.new_attachments_path } - - subject { described_class.new(project, old_storage_version: 1, new_storage_version: 2, old_attachments_path: old_attachments_path, new_attachments_path: new_attachments_path) } - - before do - TestEnv.clean_test_path - end - - describe '#create!' do - it_behaves_like 'a Geo event store', Geo::HashedStorageAttachmentsEvent - - context 'when running on a primary node' do - before do - stub_primary_node - end - - it 'tracks project attributes' do - subject.create! - - expect(Geo::HashedStorageAttachmentsEvent.last).to have_attributes( - old_attachments_path: old_attachments_path, - new_attachments_path: new_attachments_path - ) - end - end - end -end diff --git a/ee/spec/services/geo/hashed_storage_attachments_migration_service_spec.rb b/ee/spec/services/geo/hashed_storage_attachments_migration_service_spec.rb deleted file mode 100644 index fb4d66b7318a..000000000000 --- a/ee/spec/services/geo/hashed_storage_attachments_migration_service_spec.rb +++ /dev/null @@ -1,87 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -def base_path(storage) - File.join(FileUploader.root, storage.disk_path) -end - -RSpec.describe Geo::HashedStorageAttachmentsMigrationService, feature_category: :geo_replication do - let!(:project) { create(:project, :legacy_storage) } - - let(:legacy_storage) { Storage::LegacyProject.new(project) } - let(:hashed_storage) { Storage::Hashed.new(project) } - - let!(:upload) { Upload.find_by(path: file_uploader.upload_path) } - let(:file_uploader) { build(:file_uploader, project: project) } - let(:old_path) { File.join(base_path(legacy_storage), upload.path) } - let(:new_path) { File.join(base_path(hashed_storage), upload.path) } - - subject(:service) do - described_class.new( - project.id, - old_attachments_path: legacy_storage.disk_path, - new_attachments_path: hashed_storage.disk_path - ) - end - - describe '#execute' do - context 'when succeeds' do - it 'moves attachments to hashed storage layout' do - expect(File.file?(old_path)).to be_truthy - expect(File.file?(new_path)).to be_falsey - expect(File.exist?(base_path(legacy_storage))).to be_truthy - expect(File.exist?(base_path(hashed_storage))).to be_falsey - expect(FileUtils).to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage)).and_call_original - - service.execute - - expect(File.exist?(base_path(hashed_storage))).to be_truthy - expect(File.exist?(base_path(legacy_storage))).to be_falsey - expect(File.file?(old_path)).to be_falsey - expect(File.file?(new_path)).to be_truthy - end - end - - context 'when original folder does not exist anymore' do - before do - FileUtils.rm_rf(base_path(legacy_storage)) - end - - it 'skips moving folders and go to next' do - expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage)) - - service.execute - - expect(File.exist?(base_path(hashed_storage))).to be_falsey - expect(File.file?(new_path)).to be_falsey - end - end - - context 'when target folder already exists' do - before do - FileUtils.mkdir_p(base_path(hashed_storage)) - end - - it 'raises AttachmentMigrationError' do - expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage)) - - expect { service.execute }.to raise_error(::Geo::AttachmentMigrationError) - end - end - end - - describe '#async_execute' do - it 'starts the worker' do - expect(Geo::HashedStorageAttachmentsMigrationWorker).to receive(:perform_async) - - service.async_execute - end - - it 'returns job id' do - allow(Geo::HashedStorageAttachmentsMigrationWorker).to receive(:perform_async).and_return('foo') - - expect(service.async_execute).to eq('foo') - end - end -end diff --git a/ee/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb b/ee/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb deleted file mode 100644 index c24751c8992d..000000000000 --- a/ee/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb +++ /dev/null @@ -1,60 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::HashedStorage::MigrateAttachmentsService do - include EE::GeoHelpers - - let(:project) { create(:project, storage_version: 1) } - let(:legacy_storage) { Storage::LegacyProject.new(project) } - let(:hashed_storage) { Storage::Hashed.new(project) } - let(:old_attachments_path) { legacy_storage.disk_path } - let(:new_attachments_path) { hashed_storage.disk_path } - - let_it_be(:primary) { create(:geo_node, :primary) } - let_it_be(:secondary) { create(:geo_node) } - - subject { described_class.new(project: project, old_disk_path: old_attachments_path) } - - before do - stub_current_geo_node(primary) - end - - describe '#execute' do - context 'on success' do - before do - TestEnv.clean_test_path - FileUtils.mkdir_p(File.join(FileUploader.root, old_attachments_path)) - end - - it 'returns true' do - expect(subject.execute).to be_truthy - end - - it 'creates a Geo::HashedStorageAttachmentsEvent' do - expect { subject.execute }.to change(Geo::EventLog, :count).by(1) - - event = Geo::EventLog.last.event - - expect(event).to be_a(Geo::HashedStorageAttachmentsEvent) - expect(event).to have_attributes( - old_attachments_path: old_attachments_path, - new_attachments_path: new_attachments_path - ) - end - end - - context 'on failure' do - it 'does not create a Geo event when skipped' do - expect { subject.execute } - .not_to change { Geo::HashedStorageAttachmentsEvent.count } - end - - it 'does not create a Geo event on failure' do - expect(subject).to receive(:move_folder!).and_raise(::Projects::HashedStorage::AttachmentMigrationError) - expect { subject.execute }.to raise_error(::Projects::HashedStorage::AttachmentMigrationError) - expect(Geo::HashedStorageAttachmentsEvent.count).to eq(0) - end - end - end -end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index a3e59c074a42..9917ba0998a3 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -1320,7 +1320,6 @@ - './ee/spec/lib/gitlab/geo/log_cursor/event_logs_spec.rb' - './ee/spec/lib/gitlab/geo/log_cursor/events/cache_invalidation_event_spec.rb' - './ee/spec/lib/gitlab/geo/log_cursor/events/event_spec.rb' -- './ee/spec/lib/gitlab/geo/log_cursor/events/hashed_storage_attachments_event_spec.rb' - './ee/spec/lib/gitlab/geo/log_cursor/lease_spec.rb' - './ee/spec/lib/gitlab/geo/log_cursor/logger_spec.rb' - './ee/spec/lib/gitlab/geo/logger_spec.rb' @@ -1659,7 +1658,6 @@ - './ee/spec/models/geo/event_log_state_spec.rb' - './ee/spec/models/geo/every_geo_event_spec.rb' - './ee/spec/models/geo/group_wiki_repository_registry_spec.rb' -- './ee/spec/models/geo/hashed_storage_migrated_event_spec.rb' - './ee/spec/models/geo/job_artifact_registry_spec.rb' - './ee/spec/models/geo/lfs_object_registry_spec.rb' - './ee/spec/models/geo/merge_request_diff_registry_spec.rb' @@ -2595,8 +2593,6 @@ - './ee/spec/services/geo/file_registry_removal_service_spec.rb' - './ee/spec/services/geo/framework_repository_sync_service_spec.rb' - './ee/spec/services/geo/graphql_request_service_spec.rb' -- './ee/spec/services/geo/hashed_storage_attachments_event_store_spec.rb' -- './ee/spec/services/geo/hashed_storage_attachments_migration_service_spec.rb' - './ee/spec/services/geo/metrics_update_service_spec.rb' - './ee/spec/services/geo/node_create_service_spec.rb' - './ee/spec/services/geo/node_status_request_service_spec.rb' @@ -2702,7 +2698,6 @@ - './ee/spec/services/projects/group_links/create_service_spec.rb' - './ee/spec/services/projects/group_links/destroy_service_spec.rb' - './ee/spec/services/projects/group_links/update_service_spec.rb' -- './ee/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb' - './ee/spec/services/projects/import_export/export_service_spec.rb' - './ee/spec/services/projects/import_service_spec.rb' - './ee/spec/services/projects/mark_for_deletion_service_spec.rb' -- GitLab