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.

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

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

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

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

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

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

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

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]

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

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

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

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

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

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