From b4f205020797319f06c52f769f385876e6427309 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 5 Mar 2019 03:59:17 +0100 Subject: [PATCH] Skip project validation when switching storage layouts This is a fix for the Hashed Storage migration and Rollback procedure to ignore any project-level validation error that can happen in a long-running instance. There are many situations where defaults and acceptable values changed but, because we didn't provide a migration to "valid" attributes, it can happen that project will not be `valid? => true`. Because the changes we are making are limited to setting a project as read_only or changing the storage_level, it's safe to bypass validation. --- .../projects/hashed_storage/migrate_attachments_service.rb | 2 +- .../projects/hashed_storage/migrate_repository_service.rb | 2 +- .../projects/hashed_storage/rollback_attachments_service.rb | 2 +- .../projects/hashed_storage/rollback_repository_service.rb | 2 +- .../unreleased/56618-hashed-storage-skip-validation.yml | 5 +++++ .../hashed_storage/migrate_attachments_service_spec.rb | 6 ++++++ .../hashed_storage/migrate_repository_service_spec.rb | 6 ++++++ .../hashed_storage/rollback_attachments_service_spec.rb | 6 ++++++ .../hashed_storage/rollback_repository_service_spec.rb | 6 ++++++ 9 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/56618-hashed-storage-skip-validation.yml diff --git a/app/services/projects/hashed_storage/migrate_attachments_service.rb b/app/services/projects/hashed_storage/migrate_attachments_service.rb index 9eaeb6eb4e7..3d0b8f58612 100644 --- a/app/services/projects/hashed_storage/migrate_attachments_service.rb +++ b/app/services/projects/hashed_storage/migrate_attachments_service.rb @@ -24,7 +24,7 @@ module Projects result = move_folder!(origin, target) if result - project.save! + project.save!(validate: false) yield if block_given? else diff --git a/app/services/projects/hashed_storage/migrate_repository_service.rb b/app/services/projects/hashed_storage/migrate_repository_service.rb index 5afa8732c0a..e8393128d58 100644 --- a/app/services/projects/hashed_storage/migrate_repository_service.rb +++ b/app/services/projects/hashed_storage/migrate_repository_service.rb @@ -27,7 +27,7 @@ module Projects end project.repository_read_only = false - project.save! + project.save!(validate: false) if result && block_given? yield diff --git a/app/services/projects/hashed_storage/rollback_attachments_service.rb b/app/services/projects/hashed_storage/rollback_attachments_service.rb index 6c370ac47e9..5c6b92f965c 100644 --- a/app/services/projects/hashed_storage/rollback_attachments_service.rb +++ b/app/services/projects/hashed_storage/rollback_attachments_service.rb @@ -19,7 +19,7 @@ module Projects result = move_folder!(origin, target) if result - project.save! + project.save!(validate: false) yield if block_given? else diff --git a/app/services/projects/hashed_storage/rollback_repository_service.rb b/app/services/projects/hashed_storage/rollback_repository_service.rb index b5c971c70a5..67733f4770b 100644 --- a/app/services/projects/hashed_storage/rollback_repository_service.rb +++ b/app/services/projects/hashed_storage/rollback_repository_service.rb @@ -27,7 +27,7 @@ module Projects end project.repository_read_only = false - project.save! + project.save!(validate: false) if result && block_given? yield diff --git a/changelogs/unreleased/56618-hashed-storage-skip-validation.yml b/changelogs/unreleased/56618-hashed-storage-skip-validation.yml new file mode 100644 index 00000000000..c6b32d0bfec --- /dev/null +++ b/changelogs/unreleased/56618-hashed-storage-skip-validation.yml @@ -0,0 +1,5 @@ +--- +title: Skip Project validation during Hashed Storage migration or rollback +merge_request: 25753 +author: +type: fixed diff --git a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb index 639dd930618..efe15139717 100644 --- a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb @@ -76,6 +76,12 @@ describe Projects::HashedStorage::MigrateAttachmentsService do expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentCannotMoveError) end end + + it 'works even when project validation fails' do + allow(project).to receive(:valid?) { false } + + expect { service.execute }.to change { project.hashed_storage?(:attachments) }.to(true) + end end context '#old_disk_path' do diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index e77e2198439..42b0d256cbf 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -102,6 +102,12 @@ describe Projects::HashedStorage::MigrateRepositoryService do end end + it 'works even when project validation fails' do + allow(project).to receive(:valid?) { false } + + expect { service.execute }.to change { project.hashed_storage?(:repository) }.to(true) + end + def expect_move_repository(from_name, to_name) expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage, from_name, to_name).and_call_original end diff --git a/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb b/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb index 6f4154d6011..815c85e0866 100644 --- a/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb +++ b/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb @@ -78,6 +78,12 @@ describe Projects::HashedStorage::RollbackAttachmentsService do expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentCannotMoveError) end end + + it 'works even when project validation fails' do + allow(project).to receive(:valid?) { false } + + expect { service.execute }.to change { project.hashed_storage?(:attachments) }.to(false) + end end context '#old_disk_path' do diff --git a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb index 41927934501..bd4354a7df3 100644 --- a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb @@ -104,6 +104,12 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis end end + it 'works even when project validation fails' do + allow(project).to receive(:valid?) { false } + + expect { service.execute }.to change { project.legacy_storage? }.to(true) + end + def expect_move_repository(from_name, to_name) expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage, from_name, to_name).and_call_original end