Merge branch '53966-make-hashed-storage-migration-safer-and-more-inviting' into 'master'

Hashed Storage rollback mechanism

See merge request gitlab-org/gitlab-ce!23955

5 files changed, 283 insertions, 3 deletions

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 61dbb57ec08..639dd930618 100644
--- a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb
+++ b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb
@@ -70,10 +70,10 @@ describe Projects::HashedStorage::MigrateAttachmentsService do
         FileUtils.mkdir_p(base_path(hashed_storage))
       end
 
-      it 'raises AttachmentMigrationError' do
+      it 'raises AttachmentCannotMoveError' do
         expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage))
 
-        expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentMigrationError)
+        expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentCannotMoveError)
       end
     end
   end
@@ -86,6 +86,8 @@ describe Projects::HashedStorage::MigrateAttachmentsService do
 
   context '#new_disk_path' do
     it 'returns new disk_path for project' do
+      service.execute
+
       expect(service.new_disk_path).to eq(project.disk_path)
     end
   end
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 0772dc4b85b..e77e2198439 100644
--- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb
+++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb
@@ -28,7 +28,17 @@ describe Projects::HashedStorage::MigrateRepositoryService do
       it 'fails when a git operation is in progress' do
         allow(project).to receive(:repo_reference_count) { 1 }
 
-        expect { service.execute }.to raise_error(Projects::HashedStorage::RepositoryMigrationError)
+        expect { service.execute }.to raise_error(Projects::HashedStorage::RepositoryInUseError)
+      end
+    end
+
+    context 'when repository doesnt exist on disk' do
+      let(:project) { create(:project, :legacy_storage) }
+
+      it 'skips the disk change but increase the version' do
+        service.execute
+
+        expect(project.hashed_storage?(:repository)).to be_truthy
       end
     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
new file mode 100644
index 00000000000..6f4154d6011
--- /dev/null
+++ b/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb
@@ -0,0 +1,100 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::HashedStorage::RollbackAttachmentsService do
+  subject(:service) { described_class.new(project, logger: nil) }
+
+  let(:project) { create(:project, :repository, skip_disk_validation: true) }
+  let(:legacy_storage) { Storage::LegacyProject.new(project) }
+  let(:hashed_storage) { Storage::HashedProject.new(project) }
+
+  let!(:upload) { Upload.find_by(path: file_uploader.upload_path) }
+  let(:file_uploader) { build(:file_uploader, project: project) }
+  let(:old_disk_path) { File.join(base_path(hashed_storage), upload.path) }
+  let(:new_disk_path) { File.join(base_path(legacy_storage), upload.path) }
+
+  context '#execute' do
+    context 'when succeeds' do
+      it 'moves attachments to legacy storage layout' do
+        expect(File.file?(old_disk_path)).to be_truthy
+        expect(File.file?(new_disk_path)).to be_falsey
+        expect(File.exist?(base_path(hashed_storage))).to be_truthy
+        expect(File.exist?(base_path(legacy_storage))).to be_falsey
+        expect(FileUtils).to receive(:mv).with(base_path(hashed_storage), base_path(legacy_storage)).and_call_original
+
+        service.execute
+
+        expect(File.exist?(base_path(legacy_storage))).to be_truthy
+        expect(File.exist?(base_path(hashed_storage))).to be_falsey
+        expect(File.file?(old_disk_path)).to be_falsey
+        expect(File.file?(new_disk_path)).to be_truthy
+      end
+
+      it 'returns true' do
+        expect(service.execute).to be_truthy
+      end
+
+      it 'sets skipped to false' do
+        service.execute
+
+        expect(service.skipped?).to be_falsey
+      end
+    end
+
+    context 'when original folder does not exist anymore' do
+      before do
+        FileUtils.rm_rf(base_path(hashed_storage))
+      end
+
+      it 'skips moving folders and go to next' do
+        expect(FileUtils).not_to receive(:mv).with(base_path(hashed_storage), base_path(legacy_storage))
+
+        service.execute
+
+        expect(File.exist?(base_path(legacy_storage))).to be_falsey
+        expect(File.file?(new_disk_path)).to be_falsey
+      end
+
+      it 'returns true' do
+        expect(service.execute).to be_truthy
+      end
+
+      it 'sets skipped to true' do
+        service.execute
+
+        expect(service.skipped?).to be_truthy
+      end
+    end
+
+    context 'when target folder already exists' do
+      before do
+        FileUtils.mkdir_p(base_path(legacy_storage))
+      end
+
+      it 'raises AttachmentCannotMoveError' do
+        expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage))
+
+        expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentCannotMoveError)
+      end
+    end
+  end
+
+  context '#old_disk_path' do
+    it 'returns old disk_path for project' do
+      expect(service.old_disk_path).to eq(project.disk_path)
+    end
+  end
+
+  context '#new_disk_path' do
+    it 'returns new disk_path for project' do
+      service.execute
+
+      expect(service.new_disk_path).to eq(project.full_path)
+    end
+  end
+
+  def base_path(storage)
+    File.join(FileUploader.root, storage.disk_path)
+  end
+end
diff --git a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb
new file mode 100644
index 00000000000..41927934501
--- /dev/null
+++ b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb
@@ -0,0 +1,111 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis_shared_state do
+  include GitHelpers
+
+  let(:gitlab_shell) { Gitlab::Shell.new }
+  let(:project) { create(:project, :repository, :wiki_repo, storage_version: ::Project::HASHED_STORAGE_FEATURES[:repository]) }
+  let(:legacy_storage) { Storage::LegacyProject.new(project) }
+  let(:hashed_storage) { Storage::HashedProject.new(project) }
+
+  subject(:service) { described_class.new(project, project.disk_path) }
+
+  describe '#execute' do
+    let(:old_disk_path) { hashed_storage.disk_path }
+    let(:new_disk_path) { legacy_storage.disk_path }
+
+    before do
+      allow(service).to receive(:gitlab_shell) { gitlab_shell }
+    end
+
+    context 'repository lock' do
+      it 'tries to lock the repository' do
+        expect(service).to receive(:try_to_set_repository_read_only!)
+
+        service.execute
+      end
+
+      it 'fails when a git operation is in progress' do
+        allow(project).to receive(:repo_reference_count) { 1 }
+
+        expect { service.execute }.to raise_error(Projects::HashedStorage::RepositoryInUseError)
+      end
+    end
+
+    context 'when repository doesnt exist on disk' do
+      let(:project) { create(:project) }
+
+      it 'skips the disk change but decrease the version' do
+        service.execute
+
+        expect(project.legacy_storage?).to be_truthy
+      end
+    end
+
+    context 'when succeeds' do
+      it 'renames project and wiki repositories' do
+        service.execute
+
+        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.git")).to be_truthy
+        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_truthy
+      end
+
+      it 'updates project to be legacy and not read-only' do
+        service.execute
+
+        expect(project.legacy_storage?).to be_truthy
+        expect(project.repository_read_only).to be_falsey
+      end
+
+      it 'move operation is called for both repositories' do
+        expect_move_repository(old_disk_path, new_disk_path)
+        expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki")
+
+        service.execute
+      end
+
+      it 'writes project full path to .git/config' do
+        service.execute
+
+        rugged_config = rugged_repo(project.repository).config['gitlab.fullpath']
+
+        expect(rugged_config).to eq project.full_path
+      end
+    end
+
+    context 'when one move fails' do
+      it 'rolls repositories back to original name' do
+        allow(service).to receive(:move_repository).and_call_original
+        allow(service).to receive(:move_repository).with(old_disk_path, new_disk_path).once { false } # will disable first move only
+
+        expect(service).to receive(:rollback_folder_move).and_call_original
+
+        service.execute
+
+        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.git")).to be_falsey
+        expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_falsey
+        expect(project.repository_read_only?).to be_falsey
+      end
+
+      context 'when rollback fails' do
+        before do
+          legacy_storage.ensure_storage_path_exists
+          gitlab_shell.mv_repository(project.repository_storage, old_disk_path, new_disk_path)
+        end
+
+        it 'does not try to move nil repository over existing' do
+          expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage, old_disk_path, new_disk_path)
+          expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki")
+
+          service.execute
+        end
+      end
+    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
+  end
+end
diff --git a/spec/services/projects/hashed_storage/rollback_service_spec.rb b/spec/services/projects/hashed_storage/rollback_service_spec.rb
new file mode 100644
index 00000000000..427d1535559
--- /dev/null
+++ b/spec/services/projects/hashed_storage/rollback_service_spec.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::HashedStorage::RollbackService do
+  let(:project) { create(:project, :empty_repo, :wiki_repo) }
+  let(:logger) { double }
+
+  subject(:service) { described_class.new(project, project.full_path, logger: logger) }
+
+  describe '#execute' do
+    context 'attachments rollback' do
+      let(:attachments_service_class) { Projects::HashedStorage::RollbackAttachmentsService }
+      let(:attachments_service) { attachments_service_class.new(project, logger: logger) }
+
+      it 'delegates rollback to Projects::HashedStorage::RollbackAttachmentsService' do
+        expect(attachments_service_class).to receive(:new)
+            .with(project, logger: logger)
+            .and_return(attachments_service)
+        expect(attachments_service).to receive(:execute)
+
+        service.execute
+      end
+
+      it 'does not delegate rollback if repository is in legacy storage already' do
+        project.storage_version = nil
+        expect(attachments_service_class).not_to receive(:new)
+
+        service.execute
+      end
+    end
+
+    context 'repository rollback' do
+      let(:repository_service_class) { Projects::HashedStorage::RollbackRepositoryService }
+      let(:repository_service) { repository_service_class.new(project, project.full_path, logger: logger) }
+
+      it 'delegates rollback to RollbackRepositoryService' do
+        project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:repository]
+
+        expect(repository_service_class).to receive(:new)
+            .with(project, project.full_path, logger: logger)
+            .and_return(repository_service)
+        expect(repository_service).to receive(:execute)
+
+        service.execute
+      end
+
+      it 'does not delegate rollback if repository is in legacy storage already' do
+        project.storage_version = nil
+
+        expect(repository_service_class).not_to receive(:new)
+
+        service.execute
+      end
+    end
+  end
+end