Adds Rollback functionality to HashedStorage migration

We are adding sidekiq workers and service classes to allow to rollback
a hashed storage migration. There are some refactoring involved as well
as part of the code can be reused by both the migration and the rollback
logic.

14 files changed, 501 insertions, 1 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 47baf899222..400e86123f7 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1976,6 +1976,16 @@ class Project < ActiveRecord::Base
     end
   end
 
+  def rollback_to_legacy_storage!
+    return if legacy_storage?
+
+    if git_transfer_in_progress?
+      ProjectRollbackHashedStorageWorker.perform_in(Gitlab::ReferenceCounter::REFERENCE_EXPIRE_TIME, id)
+    else
+      ProjectRollbackHashedStorageWorker.perform_async(id)
+    end
+  end
+
   def git_transfer_in_progress?
     repo_reference_count > 0 || wiki_reference_count > 0
   end
diff --git a/app/services/projects/hashed_storage/rollback_attachments_service.rb b/app/services/projects/hashed_storage/rollback_attachments_service.rb
new file mode 100644
index 00000000000..5183609ab85
--- /dev/null
+++ b/app/services/projects/hashed_storage/rollback_attachments_service.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+module Projects
+  module HashedStorage
+    AttachmentRollbackError = Class.new(StandardError)
+
+    class RollbackAttachmentsService < BaseService
+      attr_reader :logger, :old_disk_path
+
+      def initialize(project, logger: nil)
+        @project = project
+        @logger = logger || Rails.logger
+      end
+
+      def execute
+        origin = FileUploader.absolute_base_dir(project)
+        project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:repository]
+        target = FileUploader.absolute_base_dir(project)
+
+        result = move_folder!(origin, target)
+        project.save!
+
+        if result && block_given?
+          yield
+        end
+
+        result
+      end
+
+      private
+
+      def move_folder!(old_path, new_path)
+        unless File.directory?(old_path)
+          logger.info("Skipped attachments rollback from '#{old_path}' to '#{new_path}', source path doesn't exist or is not a directory (PROJECT_ID=#{project.id})")
+          return true
+        end
+
+        if File.exist?(new_path)
+          logger.error("Cannot rollback attachments from '#{old_path}' to '#{new_path}', target path already exist (PROJECT_ID=#{project.id})")
+          raise AttachmentRollbackError, "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)
+        logger.info("Rolledback project attachments from '#{old_path}' to '#{new_path}' (PROJECT_ID=#{project.id})")
+
+        true
+      end
+    end
+  end
+end
diff --git a/app/services/projects/hashed_storage/rollback_repository_service.rb b/app/services/projects/hashed_storage/rollback_repository_service.rb
new file mode 100644
index 00000000000..b57d05c50ca
--- /dev/null
+++ b/app/services/projects/hashed_storage/rollback_repository_service.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+module Projects
+  module HashedStorage
+    class RollbackRepositoryService < BaseRepositoryService
+      def execute
+        try_to_set_repository_read_only!
+
+        @old_storage_version = project.storage_version
+        project.storage_version = nil
+        project.ensure_storage_path_exists
+
+        @new_disk_path = project.disk_path
+
+        result = move_repository(old_disk_path, new_disk_path)
+
+        if move_wiki
+          result &&= move_repository("#{old_wiki_disk_path}", "#{new_disk_path}.wiki")
+        end
+
+        if result
+          project.write_repository_config
+          project.track_project_repository
+        else
+          rollback_folder_move
+          project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:repository]
+        end
+
+        project.repository_read_only = false
+        project.save!
+
+        if result && block_given?
+          yield
+        end
+
+        result
+      end
+
+      private
+
+      def try_to_set_repository_read_only!
+        # Mitigate any push operation to start during migration
+        unless project.set_repository_read_only!
+          migration_error = "Target repository '#{old_disk_path}' cannot be made read-only as there is a git transfer in progress"
+          logger.error migration_error
+
+          raise RepositoryRollbackError, migration_error
+        end
+      end
+    end
+  end
+end
diff --git a/app/services/projects/hashed_storage/rollback_service.rb b/app/services/projects/hashed_storage/rollback_service.rb
new file mode 100644
index 00000000000..25767f5de5e
--- /dev/null
+++ b/app/services/projects/hashed_storage/rollback_service.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+module Projects
+  module HashedStorage
+    class RollbackService < BaseService
+      attr_reader :logger, :old_disk_path
+
+      def initialize(project, old_disk_path, logger: nil)
+        @project = project
+        @old_disk_path = old_disk_path
+        @logger = logger || Rails.logger
+      end
+
+      def execute
+        # Rollback attachments from Hashed Storage to Legacy
+        if project.hashed_storage?(:attachments)
+          return false unless rollback_attachments
+        end
+
+        # Rollback repository from Hashed Storage to Legacy
+        if project.hashed_storage?(:repository)
+          rollback_repository
+        end
+      end
+
+      private
+
+      def rollback_attachments
+        HashedStorage::RollbackAttachmentsService.new(project, logger: logger).execute
+      end
+
+      def rollback_repository
+        HashedStorage::RollbackRepositoryService.new(project, old_disk_path, logger: logger).execute
+      end
+    end
+  end
+end
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 337f39b2091..82351c1334f 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -127,6 +127,7 @@
 - project_destroy
 - project_export
 - project_migrate_hashed_storage
+- project_rollback_hashed_storage
 - project_service
 - propagate_service_template
 - reactive_caching
diff --git a/app/workers/project_rollback_hashed_storage_worker.rb b/app/workers/project_rollback_hashed_storage_worker.rb
new file mode 100644
index 00000000000..38faffd2bfa
--- /dev/null
+++ b/app/workers/project_rollback_hashed_storage_worker.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+class ProjectRollbackHashedStorageWorker
+  include ApplicationWorker
+
+  LEASE_TIMEOUT = 30.seconds.to_i
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def perform(project_id, old_disk_path = nil)
+    uuid = lease_for(project_id).try_obtain
+
+    if uuid
+      project = Project.find_by(id: project_id)
+      return if project.nil? || project.pending_delete?
+
+      old_disk_path ||= project.disk_path
+
+      ::Projects::HashedStorage::RollbackService.new(project, old_disk_path, logger: logger).execute
+    else
+      return false
+    end
+
+  ensure
+    cancel_lease_for(project_id, uuid) if uuid
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+
+  def lease_for(project_id)
+    Gitlab::ExclusiveLease.new(lease_key(project_id), timeout: LEASE_TIMEOUT)
+  end
+
+  private
+
+  def lease_key(project_id)
+    # we share the same lease key for both migration and rollback so they don't run simultaneously
+    "#{ProjectMigrateHashedStorageWorker::LEASE_KEY_SEGMENT}:#{project_id}"
+  end
+
+  def cancel_lease_for(project_id, uuid)
+    Gitlab::ExclusiveLease.cancel(lease_key(project_id), uuid)
+  end
+end
diff --git a/changelogs/unreleased/53966-make-hashed-storage-migration-safer-and-more-inviting.yml b/changelogs/unreleased/53966-make-hashed-storage-migration-safer-and-more-inviting.yml
new file mode 100644
index 00000000000..556a238ff7d
--- /dev/null
+++ b/changelogs/unreleased/53966-make-hashed-storage-migration-safer-and-more-inviting.yml
@@ -0,0 +1,5 @@
+---
+title: Hashed Storage rollback mechanism
+merge_request: 23955
+author:
+type: added
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index 1f40363e126..cef123b86ae 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -68,6 +68,7 @@
   - [background_migration, 1]
   - [gcp_cluster, 1]
   - [project_migrate_hashed_storage, 1]
+  - [project_rollback_hashed_storage, 1]
   - [hashed_storage, 1]
   - [pages_domain_verification, 1]
   - [object_storage_upload, 1]
diff --git a/lib/gitlab/hashed_storage/migrator.rb b/lib/gitlab/hashed_storage/migrator.rb
index bf463077dcc..73ed57ea584 100644
--- a/lib/gitlab/hashed_storage/migrator.rb
+++ b/lib/gitlab/hashed_storage/migrator.rb
@@ -45,8 +45,15 @@ module Gitlab
         Rails.logger.error("#{err.message} migrating storage of #{project.full_path} (ID=#{project.id}), trace - #{err.backtrace}")
       end
 
+      # Flag a project to be rolled-back to Legacy Storage
+      #
+      # @param [Project] project that will be rolled-back
       def rollback(project)
-        # TODO: implement rollback strategy
+        Rails.logger.info "Starting storage rollback of #{project.full_path} (ID=#{project.id})..."
+
+        project.rollback_to_legacy_storage!
+      rescue => err
+        Rails.logger.error("#{err.message} rolling-back storage of #{project.full_path} (ID=#{project.id}), trace - #{err.backtrace}")
       end
 
       private
diff --git a/spec/lib/gitlab/hashed_storage/migrator_spec.rb b/spec/lib/gitlab/hashed_storage/migrator_spec.rb
index 3942f168ceb..7c2582bb27a 100644
--- a/spec/lib/gitlab/hashed_storage/migrator_spec.rb
+++ b/spec/lib/gitlab/hashed_storage/migrator_spec.rb
@@ -88,4 +88,50 @@ describe Gitlab::HashedStorage::Migrator do
       end
     end
   end
+
+  describe '#rollback' do
+    let(:project) { create(:project, :empty_repo) }
+
+    it 'enqueues project rollback job' do
+      Sidekiq::Testing.fake! do
+        expect { subject.rollback(project) }.to change(ProjectRollbackHashedStorageWorker.jobs, :size).by(1)
+      end
+    end
+
+    it 'rescues and log exceptions' do
+      allow(project).to receive(:rollback_to_hashed_storage!).and_raise(StandardError)
+
+      expect { subject.rollback(project) }.not_to raise_error
+    end
+
+    it 'rolls-back project storage' do
+      perform_enqueued_jobs do
+        subject.rollback(project)
+      end
+
+      expect(project.reload.legacy_storage?).to be_truthy
+    end
+
+    it 'has rolled-back project set as writable' do
+      perform_enqueued_jobs do
+        subject.rollback(project)
+      end
+
+      expect(project.reload.repository_read_only?).to be_falsey
+    end
+
+    context 'when project is already on legacy storage' do
+      let(:project) { create(:project, :legacy_storage, :empty_repo) }
+
+      it 'doesnt enqueue any rollback job' do
+        Sidekiq::Testing.fake! do
+          expect { subject.rollback(project) }.not_to change(ProjectRollbackHashedStorageWorker.jobs, :size)
+        end
+      end
+
+      it 'returns false' do
+        expect(subject.rollback(project)).to be_falsey
+      end
+    end
+  end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 9fb0d04ca9e..6dc89b352f3 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -3452,6 +3452,20 @@ describe Project do
         project.migrate_to_hashed_storage!
       end
     end
+
+    describe '#rollback_to_legacy_storage!' do
+      let(:project) { create(:project, :empty_repo, :legacy_storage) }
+
+      it 'returns nil' do
+        expect(project.rollback_to_legacy_storage!).to be_nil
+      end
+
+      it 'does not run validations' do
+        expect(project).not_to receive(:valid?)
+
+        project.rollback_to_legacy_storage!
+      end
+    end
   end
 
   context 'hashed storage' do
@@ -3532,6 +3546,30 @@ describe Project do
         end
       end
     end
+
+    describe '#rollback_to_legacy_storage!' do
+      let(:project) { create(:project, :repository, skip_disk_validation: true) }
+
+      it 'returns true' do
+        expect(project.rollback_to_legacy_storage!).to be_truthy
+      end
+
+      it 'does not run validations' do
+        expect(project).not_to receive(:valid?)
+
+        project.rollback_to_legacy_storage!
+      end
+
+      it 'does not flag as read-only' do
+        expect { project.rollback_to_legacy_storage! }.not_to change { project.repository_read_only }
+      end
+
+      it 'enqueues a job' do
+        Sidekiq::Testing.fake! do
+          expect { project.rollback_to_legacy_storage! }.to change(ProjectRollbackHashedStorageWorker.jobs, :size).by(1)
+        end
+      end
+    end
   end
 
   describe '#gl_repository' 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
new file mode 100644
index 00000000000..c9d0a085d21
--- /dev/null
+++ b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb
@@ -0,0 +1,101 @@
+# 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::RepositoryRollbackError)
+      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 'rollsback repositories 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
diff --git a/spec/workers/project_rollback_hashed_storage_worker_spec.rb b/spec/workers/project_rollback_hashed_storage_worker_spec.rb
new file mode 100644
index 00000000000..aed7493763d
--- /dev/null
+++ b/spec/workers/project_rollback_hashed_storage_worker_spec.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProjectRollbackHashedStorageWorker, :clean_gitlab_redis_shared_state do
+  include ExclusiveLeaseHelpers
+
+  describe '#perform' do
+    let(:project) { create(:project, :empty_repo) }
+    let(:lease_key) { "project_migrate_hashed_storage_worker:#{project.id}" }
+    let(:lease_timeout) { described_class::LEASE_TIMEOUT }
+    let(:rollback_service) { ::Projects::HashedStorage::RollbackService }
+
+    it 'skips when project no longer exists' do
+      expect(rollback_service).not_to receive(:new)
+
+      subject.perform(-1)
+    end
+
+    it 'skips when project is pending delete' do
+      pending_delete_project = create(:project, :empty_repo, pending_delete: true)
+
+      expect(rollback_service).not_to receive(:new)
+
+      subject.perform(pending_delete_project.id)
+    end
+
+    it 'delegates rollback to service class when have exclusive lease' do
+      stub_exclusive_lease(lease_key, 'uuid', timeout: lease_timeout)
+
+      service_spy = spy
+
+      allow(rollback_service)
+        .to receive(:new).with(project, project.disk_path, logger: subject.logger)
+        .and_return(service_spy)
+
+      subject.perform(project.id)
+
+      expect(service_spy).to have_received(:execute)
+    end
+
+    it 'skips when it cant acquire the exclusive lease' do
+      stub_exclusive_lease_taken(lease_key, timeout: lease_timeout)
+
+      expect(rollback_service).not_to receive(:new)
+
+      subject.perform(project.id)
+    end
+  end
+end