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

7 files changed, 191 insertions, 56 deletions

diff --git a/app/services/projects/hashed_storage/base_attachment_service.rb b/app/services/projects/hashed_storage/base_attachment_service.rb
new file mode 100644
index 00000000000..828ab616bab
--- /dev/null
+++ b/app/services/projects/hashed_storage/base_attachment_service.rb
@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+module Projects
+  module HashedStorage
+    AttachmentMigrationError = Class.new(StandardError)
+
+    AttachmentCannotMoveError = Class.new(StandardError)
+
+    class BaseAttachmentService < BaseService
+      # Returns the disk_path value before the execution
+      attr_reader :old_disk_path
+
+      # Returns the disk_path value after the execution
+      attr_reader :new_disk_path
+
+      # Returns the logger currently in use
+      attr_reader :logger
+
+      # Return whether this operation was skipped or not
+      #
+      # @return [Boolean] true if skipped of false otherwise
+      def skipped?
+        @skipped
+      end
+
+      protected
+
+      def move_folder!(old_path, new_path)
+        unless File.directory?(old_path)
+          logger.info("Skipped attachments move from '#{old_path}' to '#{new_path}', source path doesn't exist or is not a directory (PROJECT_ID=#{project.id})")
+          @skipped = true
+
+          return true
+        end
+
+        if File.exist?(new_path)
+          logger.error("Cannot move attachments from '#{old_path}' to '#{new_path}', target path already exist (PROJECT_ID=#{project.id})")
+          raise AttachmentCannotMoveError, "Target path '#{new_path}' already exists"
+        end
+
+        # Create base path folder on the new storage layout
+        FileUtils.mkdir_p(File.dirname(new_path))
+
+        FileUtils.mv(old_path, new_path)
+        logger.info("Project attachments moved from '#{old_path}' to '#{new_path}' (PROJECT_ID=#{project.id})")
+
+        true
+      end
+    end
+  end
+end
diff --git a/app/services/projects/hashed_storage/base_repository_service.rb b/app/services/projects/hashed_storage/base_repository_service.rb
index 761c81d776f..f97a28b8c3b 100644
--- a/app/services/projects/hashed_storage/base_repository_service.rb
+++ b/app/services/projects/hashed_storage/base_repository_service.rb
@@ -2,11 +2,8 @@
 
 module Projects
   module HashedStorage
-    # Returned when there is an error with the Hashed Storage migration
-    RepositoryMigrationError = Class.new(StandardError)
-
-    # Returned when there is an error with the Hashed Storage rollback
-    RepositoryRollbackError = Class.new(StandardError)
+    # Returned when repository can't be made read-only because there is already a git transfer in progress
+    RepositoryInUseError = Class.new(StandardError)
 
     class BaseRepositoryService < BaseService
       include Gitlab::ShellAdapter
@@ -38,7 +35,10 @@ module Projects
         # project was not originally empty.
         if !from_exists && !to_exists
           logger.warn "Can't find a repository on either source or target paths for #{project.full_path} (ID=#{project.id}) ..."
-          return false
+
+          # We return true so we still reflect the change in the database.
+          # Next time the repository is (re)created it will be under the new storage layout
+          return true
         elsif !from_exists
           # Repository have been moved already.
           return true
@@ -52,6 +52,16 @@ module Projects
         move_repository(new_disk_path, old_disk_path)
         move_repository("#{new_disk_path}.wiki", old_wiki_disk_path)
       end
+
+      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 RepositoryInUseError, migration_error
+        end
+      end
     end
   end
 end
diff --git a/app/services/projects/hashed_storage/migrate_attachments_service.rb b/app/services/projects/hashed_storage/migrate_attachments_service.rb
index 03e0685d2cd..9eaeb6eb4e7 100644
--- a/app/services/projects/hashed_storage/migrate_attachments_service.rb
+++ b/app/services/projects/hashed_storage/migrate_attachments_service.rb
@@ -2,62 +2,37 @@
 
 module Projects
   module HashedStorage
-    AttachmentMigrationError = Class.new(StandardError)
-
-    class MigrateAttachmentsService < BaseService
-      attr_reader :logger, :old_disk_path, :new_disk_path
-
+    class MigrateAttachmentsService < BaseAttachmentService
       def initialize(project, old_disk_path, logger: nil)
         @project = project
         @logger = logger || Rails.logger
         @old_disk_path = old_disk_path
-        @new_disk_path = project.disk_path
         @skipped = false
       end
 
       def execute
         origin = FileUploader.absolute_base_dir(project)
-        # It's possible that old_disk_path does not match project.disk_path. For example, that happens when we rename a project
+        # It's possible that old_disk_path does not match project.disk_path.
+        # For example, that happens when we rename a project
         origin.sub!(/#{Regexp.escape(project.full_path)}\z/, old_disk_path)
 
         project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:attachments]
         target = FileUploader.absolute_base_dir(project)
 
-        result = move_folder!(origin, target)
-        project.save!
-
-        if result && block_given?
-          yield
-        end
-
-        result
-      end
-
-      def skipped?
-        @skipped
-      end
+        @new_disk_path = project.disk_path
 
-      private
+        result = move_folder!(origin, target)
 
-      def move_folder!(old_path, new_path)
-        unless File.directory?(old_path)
-          logger.info("Skipped attachments migration from '#{old_path}' to '#{new_path}', source path doesn't exist or is not a directory (PROJECT_ID=#{project.id})")
-          @skipped = true
-          return true
-        end
+        if result
+          project.save!
 
-        if File.exist?(new_path)
-          logger.error("Cannot migrate attachments from '#{old_path}' to '#{new_path}', target path already exist (PROJECT_ID=#{project.id})")
-          raise AttachmentMigrationError, "Target path '#{new_path}' already exist"
+          yield if block_given?
+        else
+          # Rollback changes
+          project.rollback!
         end
 
-        # Create hashed storage base path folder
-        FileUtils.mkdir_p(File.dirname(new_path))
-
-        FileUtils.mv(old_path, new_path)
-        logger.info("Migrated project attachments from '#{old_path}' to '#{new_path}' (PROJECT_ID=#{project.id})")
-
-        true
+        result
       end
     end
   end
diff --git a/app/services/projects/hashed_storage/migrate_repository_service.rb b/app/services/projects/hashed_storage/migrate_repository_service.rb
index 9c672283c7e..5afa8732c0a 100644
--- a/app/services/projects/hashed_storage/migrate_repository_service.rb
+++ b/app/services/projects/hashed_storage/migrate_repository_service.rb
@@ -15,7 +15,7 @@ module Projects
         result = move_repository(old_disk_path, new_disk_path)
 
         if move_wiki
-          result &&= move_repository("#{old_wiki_disk_path}", "#{new_disk_path}.wiki")
+          result &&= move_repository(old_wiki_disk_path, "#{new_disk_path}.wiki")
         end
 
         if result
@@ -35,18 +35,6 @@ module Projects
 
         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 RepositoryMigrationError, migration_error
-        end
-      end
     end
   end
 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..6c370ac47e9
--- /dev/null
+++ b/app/services/projects/hashed_storage/rollback_attachments_service.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module Projects
+  module HashedStorage
+    class RollbackAttachmentsService < BaseAttachmentService
+      def initialize(project, logger: nil)
+        @project = project
+        @logger = logger || Rails.logger
+        @old_disk_path = project.disk_path
+      end
+
+      def execute
+        origin = FileUploader.absolute_base_dir(project)
+        project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:repository]
+        target = FileUploader.absolute_base_dir(project)
+
+        @new_disk_path = FileUploader.base_dir(project)
+
+        result = move_folder!(origin, target)
+
+        if result
+          project.save!
+
+          yield if block_given?
+        else
+          # Rollback changes
+          project.rollback!
+        end
+
+        result
+      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..b5c971c70a5
--- /dev/null
+++ b/app/services/projects/hashed_storage/rollback_repository_service.rb
@@ -0,0 +1,40 @@
+# 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
+    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