Merge branch 'hashed-storage-attachments-migration-path' into 'master'

Hashed storage attachments migration path

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

app/models/project.rb

@@ -273,8 +273,9 @@ class Project < ActiveRecord::Base
   scope :pending_delete, -> { where(pending_delete: true) }
   scope :without_deleted, -> { where(pending_delete: false) }
 
-  scope :with_hashed_storage, -> { where('storage_version >= 1') }
+  scope :with_storage_feature, ->(feature) { where('storage_version >= :version', version: HASHED_STORAGE_FEATURES[feature]) }
-  scope :with_legacy_storage, -> { where(storage_version: [nil, 0]) }
+  scope :without_storage_feature, ->(feature) { where('storage_version < :version OR storage_version IS NULL', version: HASHED_STORAGE_FEATURES[feature]) }
+  scope :with_unmigrated_storage, -> { where('storage_version < :version OR storage_version IS NULL', version: LATEST_STORAGE_VERSION) }
 
   scope :sorted_by_activity, -> { reorder(last_activity_at: :desc) }
   scope :sorted_by_stars, -> { reorder('projects.star_count DESC') }

app/models/storage/hashed_project.rb

@@ -4,7 +4,6 @@ module Storage
     delegate :gitlab_shell, :repository_storage_path, to: :project
 
     ROOT_PATH_PREFIX = '@hashed'.freeze
-    STORAGE_VERSION = 1
 
     def initialize(project)
       @project = project

app/services/projects/hashed_storage/migrate_attachments_service.rb

@@ -0,0 +1,54 @@
+module Projects
+  module HashedStorage
+    AttachmentMigrationError = Class.new(StandardError)
+
+    class MigrateAttachmentsService < BaseService
+      attr_reader :logger, :old_path, :new_path
+
+      def initialize(project, logger = nil)
+        @project = project
+        @logger = logger || Rails.logger
+      end
+
+      def execute
+        @old_path = project.full_path
+        @new_path = project.disk_path
+
+        origin = FileUploader.dynamic_path_segment(project)
+        project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:attachments]
+        target = FileUploader.dynamic_path_segment(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 migration from '#{old_path}' to '#{new_path}', source path doesn't exist or is not a directory (PROJECT_ID=#{project.id})")
+          return
+        end
+
+        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"
+        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
+      end
+    end
+  end
+end

app/services/projects/hashed_storage/migrate_repository_service.rb

@@ -0,0 +1,70 @@
+module Projects
+  module HashedStorage
+    class MigrateRepositoryService < BaseService
+      include Gitlab::ShellAdapter
+
+      attr_reader :old_disk_path, :new_disk_path, :old_wiki_disk_path, :old_storage_version, :logger
+
+      def initialize(project, logger = nil)
+        @project = project
+        @logger = logger || Rails.logger
+      end
+
+      def execute
+        @old_disk_path = project.disk_path
+        has_wiki = project.wiki.repository_exists?
+
+        @old_storage_version = project.storage_version
+        project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:repository]
+        project.ensure_storage_path_exists
+
+        @new_disk_path = project.disk_path
+
+        result = move_repository(@old_disk_path, @new_disk_path)
+
+        if has_wiki
+          @old_wiki_disk_path = "#{@old_disk_path}.wiki"
+          result &&= move_repository("#{@old_wiki_disk_path}", "#{@new_disk_path}.wiki")
+        end
+
+        unless result
+          rollback_folder_move
+          project.storage_version = nil
+        end
+
+        project.repository_read_only = false
+        project.save!
+
+        if result && block_given?
+          yield
+        end
+
+        result
+      end
+
+      private
+
+      def move_repository(from_name, to_name)
+        from_exists = gitlab_shell.exists?(project.repository_storage_path, "#{from_name}.git")
+        to_exists = gitlab_shell.exists?(project.repository_storage_path, "#{to_name}.git")
+
+        # If we don't find the repository on either original or target we should log that as it could be an issue if the
+        # 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
+        elsif !from_exists
+          # Repository have been moved already.
+          return true
+        end
+
+        gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name)
+      end
+
+      def rollback_folder_move
+        move_repository(@new_disk_path, @old_disk_path)
+        move_repository("#{@new_disk_path}.wiki", "#{@old_disk_path}.wiki")
+      end
+    end
+  end
+end

app/services/projects/hashed_storage_migration_service.rb

@@ -1,68 +1,22 @@
 module Projects
   class HashedStorageMigrationService < BaseService
-    include Gitlab::ShellAdapter
+    attr_reader :logger
-
-    attr_reader :old_disk_path, :new_disk_path
 
     def initialize(project, logger = nil)
       @project = project
-      @logger ||= Rails.logger
+      @logger = logger || Rails.logger
     end
 
     def execute
-      return if project.hashed_storage?(:repository)
+      # Migrate repository from Legacy to Hashed Storage
-
+      unless project.hashed_storage?(:repository)
-      @old_disk_path = project.disk_path
+        return unless HashedStorage::MigrateRepositoryService.new(project, logger).execute
-      has_wiki = project.wiki.repository_exists?
-
-      project.storage_version = Storage::HashedProject::STORAGE_VERSION
-      project.ensure_storage_path_exists
-
-      @new_disk_path = project.disk_path
-
-      result = move_repository(@old_disk_path, @new_disk_path)
-
-      if has_wiki
-        result &&= move_repository("#{@old_disk_path}.wiki", "#{@new_disk_path}.wiki")
       end
 
-      unless result
+      # Migrate attachments from Legacy to Hashed Storage
-        rollback_folder_move
+      unless project.hashed_storage?(:attachments)
-        return
+        HashedStorage::MigrateAttachmentsService.new(project, logger).execute
       end
-
-      project.repository_read_only = false
-      project.save!
-
-      block_given? ? yield : result
-    end
-
-    private
-
-    def move_repository(from_name, to_name)
-      from_exists = gitlab_shell.exists?(project.repository_storage_path, "#{from_name}.git")
-      to_exists = gitlab_shell.exists?(project.repository_storage_path, "#{to_name}.git")
-
-      # If we don't find the repository on either original or target we should log that as it could be an issue if the
-      # 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
-      elsif !from_exists
-        # Repository have been moved already.
-        return true
-      end
-
-      gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name)
-    end
-
-    def rollback_folder_move
-      move_repository(@new_disk_path, @old_disk_path)
-      move_repository("#{@new_disk_path}.wiki", "#{@old_disk_path}.wiki")
-    end
-
-    def logger
-      @logger
     end
   end
 end

app/uploaders/file_uploader.rb

@@ -31,12 +31,19 @@ class FileUploader < GitlabUploader
   # Returns a String without a trailing slash
   def self.dynamic_path_segment(project)
     if project.hashed_storage?(:attachments)
-      File.join(CarrierWave.root, base_dir, project.disk_path)
+      dynamic_path_builder(project.disk_path)
     else
-      File.join(CarrierWave.root, base_dir, project.full_path)
+      dynamic_path_builder(project.full_path)
     end
   end
 
+  # Auxiliary method to build dynamic path segment when not using a project model
+  #
+  # Prefer to use the `.dynamic_path_segment` as it includes Hashed Storage specific logic
+  def self.dynamic_path_builder(path)
+    File.join(CarrierWave.root, base_dir, path)
+  end
+
   attr_accessor :model
   attr_reader :secret
 

app/workers/project_migrate_hashed_storage_worker.rb

@@ -2,10 +2,34 @@ class ProjectMigrateHashedStorageWorker
   include Sidekiq::Worker
   include DedicatedSidekiqQueue
 
+  LEASE_TIMEOUT = 30.seconds.to_i
+
   def perform(project_id)
     project = Project.find_by(id: project_id)
     return if project.nil? || project.pending_delete?
 
-    ::Projects::HashedStorageMigrationService.new(project, logger).execute
+    uuid = lease_for(project_id).try_obtain
+    if uuid
+      ::Projects::HashedStorageMigrationService.new(project, logger).execute
+    else
+      false
+    end
+  rescue => ex
+    cancel_lease_for(project_id, uuid) if uuid
+    raise ex
+  end
+
+  def lease_for(project_id)
+    Gitlab::ExclusiveLease.new(lease_key(project_id), timeout: LEASE_TIMEOUT)
+  end
+
+  private
+
+  def lease_key(project_id)
+    "project_migrate_hashed_storage_worker:#{project_id}"
+  end
+
+  def cancel_lease_for(project_id, uuid)
+    Gitlab::ExclusiveLease.cancel(lease_key(project_id), uuid)
   end
 end

changelogs/unreleased/hashed-storage-attachments-migration-path.yml

@@ -0,0 +1,5 @@
+---
+title: Hashed Storage migration script now supports migrating project attachments
+merge_request: 15352
+author:
+type: added

doc/administration/raketasks/storage.md

@@ -1,10 +1,43 @@
 # Repository Storage Rake Tasks
 
 This is a collection of rake tasks you can use to help you list and migrate 
-existing projects from Legacy storage to the new Hashed storage type.
+existing projects and attachments associated with it from Legacy storage to 
+the new Hashed storage type.
 
 You can read more about the storage types [here][storage-types].
 
+## Migrate existing projects to Hashed storage
+
+Before migrating your existing projects, you should 
+[enable hashed storage][storage-migration] for the new projects as well.
+
+This task will schedule all your existing projects and attachments associated with it to be migrated to the 
+**Hashed** storage type:
+
+**Omnibus Installation**
+
+```bash
+gitlab-rake gitlab:storage:migrate_to_hashed
+```
+
+**Source Installation**
+
+```bash
+rake gitlab:storage:migrate_to_hashed
+
+```
+
+You can monitor the progress in the _Admin > Monitoring > Background jobs_ screen.
+There is a specific Queue you can watch to see how long it will take to finish: **project_migrate_hashed_storage**
+
+After it reaches zero, you can confirm every project has been migrated by running the commands bellow. 
+If you find it necessary, you can run this migration script again to schedule missing projects.
+
+Any error or warning will be logged in the sidekiq's log file.
+
+You only need the `gitlab:storage:migrate_to_hashed` rake task to migrate your repositories, but we have additional
+commands below that helps you inspect projects and attachments in both legacy and hashed storage.
+
 ## List projects on Legacy storage
 
 To have a simple summary of projects using **Legacy** storage:
@@ -73,35 +106,73 @@ rake gitlab:storage:list_hashed_projects
 
 ```
 
-## Migrate existing projects to Hashed storage
+## List attachments on Legacy storage
 
-Before migrating your existing projects, you should 
+To have a simple summary of project attachments using **Legacy** storage:
-[enable hashed storage][storage-migration] for the new projects as well.
-
-This task will schedule all your existing projects to be migrated to the 
-**Hashed** storage type:
 
 **Omnibus Installation**
 
 ```bash
-gitlab-rake gitlab:storage:migrate_to_hashed
+gitlab-rake gitlab:storage:legacy_attachments
 ```
 
 **Source Installation**
 
 ```bash
-rake gitlab:storage:migrate_to_hashed
+rake gitlab:storage:legacy_attachments
 
 ```
 
-You can monitor the progress in the _Admin > Monitoring > Background jobs_ screen.
+------
-There is a specific Queue you can watch to see how long it will take to finish: **project_migrate_hashed_storage**
 
-After it reaches zero, you can confirm every project has been migrated by running the commands above.
+To list project attachments using **Legacy** storage:
-If you find it necessary, you can run this migration script again to schedule missing projects.
 
-Any error or warning will be logged in the sidekiq log file.
+**Omnibus Installation**
 
+```bash
+gitlab-rake gitlab:storage:list_legacy_attachments
+```
+
+**Source Installation**
+
+```bash
+rake gitlab:storage:list_legacy_attachments
+
+```
+
+## List attachments on Hashed storage
+
+To have a simple summary of project attachments using **Hashed** storage:
+
+**Omnibus Installation**
+
+```bash
+gitlab-rake gitlab:storage:hashed_attachments
+```
+
+**Source Installation**
+
+```bash
+rake gitlab:storage:hashed_attachments
+
+```
+
+------
+
+To list project attachments using **Hashed** storage:
+
+**Omnibus Installation**
+
+```bash
+gitlab-rake gitlab:storage:list_hashed_attachments
+```
+
+**Source Installation**
+
+```bash
+rake gitlab:storage:list_hashed_attachments
+
+```
 
 [storage-types]: ../repository_storage_types.md
 [storage-migration]: ../repository_storage_types.md#how-to-migrate-to-hashed-storage

lib/tasks/gitlab/storage.rake

@@ -2,10 +2,10 @@ namespace :gitlab do
   namespace :storage do
     desc 'GitLab | Storage | Migrate existing projects to Hashed Storage'
     task migrate_to_hashed: :environment do
-      legacy_projects_count = Project.with_legacy_storage.count
+      legacy_projects_count = Project.with_unmigrated_storage.count
 
       if legacy_projects_count == 0
-        puts 'There are no projects using legacy storage. Nothing to do!'
+        puts 'There are no projects requiring storage migration. Nothing to do!'
 
         next
       end
@@ -23,22 +23,42 @@ namespace :gitlab do
 
     desc 'Gitlab | Storage | Summary of existing projects using Legacy Storage'
     task legacy_projects: :environment do
-      projects_summary(Project.with_legacy_storage)
+      relation_summary('projects', Project.without_storage_feature(:repository))
     end
 
     desc 'Gitlab | Storage | List existing projects using Legacy Storage'
     task list_legacy_projects: :environment do
-      projects_list(Project.with_legacy_storage)
+      projects_list('projects using Legacy Storage', Project.without_storage_feature(:repository))
     end
 
     desc 'Gitlab | Storage | Summary of existing projects using Hashed Storage'
     task hashed_projects: :environment do
-      projects_summary(Project.with_hashed_storage)
+      relation_summary('projects using Hashed Storage', Project.with_storage_feature(:repository))
     end
 
     desc 'Gitlab | Storage | List existing projects using Hashed Storage'
     task list_hashed_projects: :environment do
-      projects_list(Project.with_hashed_storage)
+      projects_list('projects using Hashed Storage', Project.with_storage_feature(:repository))
+    end
+
+    desc 'Gitlab | Storage | Summary of project attachments using Legacy Storage'
+    task legacy_attachments: :environment do
+      relation_summary('attachments using Legacy Storage', legacy_attachments_relation)
+    end
+
+    desc 'Gitlab | Storage | List existing project attachments using Legacy Storage'
+    task list_legacy_attachments: :environment do
+      attachments_list('attachments using Legacy Storage', legacy_attachments_relation)
+    end
+
+    desc 'Gitlab | Storage | Summary of project attachments using Hashed Storage'
+    task hashed_attachments: :environment do
+      relation_summary('attachments using Hashed Storage', hashed_attachments_relation)
+    end
+
+    desc 'Gitlab | Storage | List existing project attachments using Hashed Storage'
+    task list_hashed_attachments: :environment do
+      attachments_list('attachments using Hashed Storage', hashed_attachments_relation)
     end
 
     def batch_size
@@ -46,29 +66,43 @@ namespace :gitlab do
     end
 
     def project_id_batches(&block)
-      Project.with_legacy_storage.in_batches(of: batch_size, start: ENV['ID_FROM'], finish: ENV['ID_TO']) do |relation| # rubocop: disable Cop/InBatches
+      Project.with_unmigrated_storage.in_batches(of: batch_size, start: ENV['ID_FROM'], finish: ENV['ID_TO']) do |relation| # rubocop: disable Cop/InBatches
         ids = relation.pluck(:id)
 
         yield ids.min, ids.max
       end
     end
 
-    def projects_summary(relation)
+    def legacy_attachments_relation
-      projects_count = relation.count
+      Upload.joins(<<~SQL).where('projects.storage_version < :version OR projects.storage_version IS NULL', version: Project::HASHED_STORAGE_FEATURES[:attachments])
-      puts "* Found #{projects_count} projects".color(:green)
+        JOIN projects
-
+          ON (uploads.model_type='Project' AND uploads.model_id=projects.id)
-      projects_count
+      SQL
     end
 
-    def projects_list(relation)
+    def hashed_attachments_relation
-      projects_count = projects_summary(relation)
+      Upload.joins(<<~SQL).where('projects.storage_version >= :version', version: Project::HASHED_STORAGE_FEATURES[:attachments])
+        JOIN projects
+        ON (uploads.model_type='Project' AND uploads.model_id=projects.id)
+      SQL
+    end
+
+    def relation_summary(relation_name, relation)
+      relation_count = relation.count
+      puts "* Found #{relation_count} #{relation_name}".color(:green)
+
+      relation_count
+    end
+
+    def projects_list(relation_name, relation)
+      relation_count = relation_summary(relation_name, relation)
 
       projects = relation.with_route
       limit = ENV.fetch('LIMIT', 500).to_i
 
-      return unless projects_count > 0
+      return unless relation_count > 0
 
-      puts "  ! Displaying first #{limit} projects..." if projects_count > limit
+      puts "  ! Displaying first #{limit} #{relation_name}..." if relation_count > limit
 
       counter = 0
       projects.find_in_batches(batch_size: batch_size) do |batch|
@@ -81,5 +115,26 @@ namespace :gitlab do
         end
       end
     end
+
+    def attachments_list(relation_name, relation)
+      relation_count = relation_summary(relation_name, relation)
+
+      limit = ENV.fetch('LIMIT', 500).to_i
+
+      return unless relation_count > 0
+
+      puts "  ! Displaying first #{limit} #{relation_name}..." if relation_count > limit
+
+      counter = 0
+      relation.find_in_batches(batch_size: batch_size) do |batch|
+        batch.each do |upload|
+          counter += 1
+
+          puts "  - #{upload.path} (id: #{upload.id})".color(:red)
+
+          return if counter >= limit # rubocop:disable Lint/NonLocalExitFromIterator
+        end
+      end
+    end
   end
 end

spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb

@@ -0,0 +1,63 @@
+require 'spec_helper'
+
+describe Projects::HashedStorage::MigrateAttachmentsService do
+  subject(:service) { described_class.new(project) }
+  let(:project) { create(:project) }
+  let(:legacy_storage) { Storage::LegacyProject.new(project) }
+  let(:hashed_storage) { Storage::HashedProject.new(project) }
+
+  let!(:upload) { Upload.find_by(path: file_uploader.relative_path) }
+  let(:file_uploader) { build(:file_uploader, project: project) }
+  let(:old_path) { File.join(base_path(legacy_storage), upload.path) }
+  let(:new_path) { File.join(base_path(hashed_storage), upload.path) }
+
+  context '#execute' do
+    context 'when succeeds' do
+      it 'moves attachments to hashed storage layout' do
+        expect(File.file?(old_path)).to be_truthy
+        expect(File.file?(new_path)).to be_falsey
+        expect(File.exist?(base_path(legacy_storage))).to be_truthy
+        expect(File.exist?(base_path(hashed_storage))).to be_falsey
+        expect(FileUtils).to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage)).and_call_original
+
+        service.execute
+
+        expect(File.exist?(base_path(hashed_storage))).to be_truthy
+        expect(File.exist?(base_path(legacy_storage))).to be_falsey
+        expect(File.file?(old_path)).to be_falsey
+        expect(File.file?(new_path)).to be_truthy
+      end
+    end
+
+    context 'when original folder does not exist anymore' do
+      before do
+        FileUtils.rm_rf(base_path(legacy_storage))
+      end
+
+      it 'skips moving folders and go to next' do
+        expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage))
+
+        service.execute
+
+        expect(File.exist?(base_path(hashed_storage))).to be_falsey
+        expect(File.file?(new_path)).to be_falsey
+      end
+    end
+
+    context 'when target folder already exists' do
+      before do
+        FileUtils.mkdir_p(base_path(hashed_storage))
+      end
+
+      it 'raises AttachmentMigrationError' 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)
+      end
+    end
+  end
+
+  def base_path(storage)
+    FileUploader.dynamic_path_builder(storage.disk_path)
+  end
+end

spec/services/projects/hashed_storage/migrate_repository_service_spec.rb

@@ -0,0 +1,76 @@
+require 'spec_helper'
+
+describe Projects::HashedStorage::MigrateRepositoryService do
+  let(:gitlab_shell) { Gitlab::Shell.new }
+  let(:project) { create(:project, :empty_repo, :wiki_repo) }
+  let(:service) { described_class.new(project) }
+  let(:legacy_storage) { Storage::LegacyProject.new(project) }
+  let(:hashed_storage) { Storage::HashedProject.new(project) }
+
+  describe '#execute' do
+    before do
+      allow(service).to receive(:gitlab_shell) { gitlab_shell }
+    end
+
+    context 'when succeeds' do
+      it 'renames project and wiki repositories' do
+        service.execute
+
+        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_truthy
+        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_truthy
+      end
+
+      it 'updates project to be hashed and not read-only' do
+        service.execute
+
+        expect(project.hashed_storage?(:repository)).to be_truthy
+        expect(project.repository_read_only).to be_falsey
+      end
+
+      it 'move operation is called for both repositories' do
+        expect_move_repository(project.disk_path, hashed_storage.disk_path)
+        expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.disk_path}.wiki")
+
+        service.execute
+      end
+    end
+
+    context 'when one move fails' do
+      it 'rollsback repositories to original name' do
+        from_name = project.disk_path
+        to_name = hashed_storage.disk_path
+        allow(service).to receive(:move_repository).and_call_original
+        allow(service).to receive(:move_repository).with(from_name, to_name).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_path, "#{hashed_storage.disk_path}.git")).to be_falsey
+        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_falsey
+        expect(project.repository_read_only?).to be_falsey
+      end
+
+      context 'when rollback fails' do
+        let(:from_name) { legacy_storage.disk_path }
+        let(:to_name) { hashed_storage.disk_path }
+
+        before do
+          hashed_storage.ensure_storage_path_exists
+          gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name)
+        end
+
+        it 'does not try to move nil repository over hashed' do
+          expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage_path, from_name, to_name)
+          expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.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_path, from_name, to_name).and_call_original
+    end
+  end
+end

spec/services/projects/hashed_storage_migration_service_spec.rb

@@ -1,74 +1,44 @@
 require 'spec_helper'
 
 describe Projects::HashedStorageMigrationService do
-  let(:gitlab_shell) { Gitlab::Shell.new }
   let(:project) { create(:project, :empty_repo, :wiki_repo) }
-  let(:service) { described_class.new(project) }
+  subject(:service) { described_class.new(project) }
-  let(:legacy_storage) { Storage::LegacyProject.new(project) }
-  let(:hashed_storage) { Storage::HashedProject.new(project) }
 
   describe '#execute' do
-    before do
+    context 'repository migration' do
-      allow(service).to receive(:gitlab_shell) { gitlab_shell }
+      let(:repository_service) { Projects::HashedStorage::MigrateRepositoryService.new(project, subject.logger) }
-    end
+
+      it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do
+        expect(Projects::HashedStorage::MigrateRepositoryService).to receive(:new).with(project, subject.logger).and_return(repository_service)
+        expect(repository_service).to receive(:execute)
 
-    context 'when succeeds' do
-      it 'renames project and wiki repositories' do
         service.execute
-
-        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_truthy
-        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_truthy
       end
 
-      it 'updates project to be hashed and not read-only' do
+      it 'does not delegate migration if repository is already migrated' do
-        service.execute
+        project.storage_version = ::Project::LATEST_STORAGE_VERSION
-
+        expect(Projects::HashedStorage::MigrateRepositoryService).not_to receive(:new)
-        expect(project.hashed_storage?(:repository)).to be_truthy
-        expect(project.repository_read_only).to be_falsey
-      end
-
-      it 'move operation is called for both repositories' do
-        expect_move_repository(project.disk_path, hashed_storage.disk_path)
-        expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.disk_path}.wiki")
 
         service.execute
       end
     end
 
-    context 'when one move fails' do
+    context 'attachments migration' do
-      it 'rollsback repositories to original name' do
+      let(:attachments_service) { Projects::HashedStorage::MigrateAttachmentsService.new(project, subject.logger) }
-        from_name = project.disk_path
-        to_name = hashed_storage.disk_path
-        allow(service).to receive(:move_repository).and_call_original
-        allow(service).to receive(:move_repository).with(from_name, to_name).once { false } # will disable first move only
 
-        expect(service).to receive(:rollback_folder_move).and_call_original
+      it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do
+        expect(Projects::HashedStorage::MigrateAttachmentsService).to receive(:new).with(project, subject.logger).and_return(attachments_service)
+        expect(attachments_service).to receive(:execute)
 
         service.execute
-
-        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_falsey
-        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_falsey
       end
 
-      context 'when rollback fails' do
+      it 'does not delegate migration if attachments are already migrated' do
-        before do
+        project.storage_version = ::Project::LATEST_STORAGE_VERSION
-          from_name = legacy_storage.disk_path
+        expect(Projects::HashedStorage::MigrateAttachmentsService).not_to receive(:new)
-          to_name = hashed_storage.disk_path
 
-          hashed_storage.ensure_storage_path_exists
+        service.execute
-          gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name)
-        end
-
-        it 'does not try to move nil repository over hashed' do
-          expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.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_path, from_name, to_name).and_call_original
-    end
   end
 end

spec/workers/project_migrate_hashed_storage_worker_spec.rb

@@ -1,29 +1,53 @@
 require 'spec_helper'
 
-describe ProjectMigrateHashedStorageWorker do
+describe ProjectMigrateHashedStorageWorker, :clean_gitlab_redis_shared_state do
   describe '#perform' do
     let(:project) { create(:project, :empty_repo) }
     let(:pending_delete_project) { create(:project, :empty_repo, pending_delete: true) }
 
-    it 'skips when project no longer exists' do
+    context 'when have exclusive lease' do
-      nonexistent_id = 999999999999
+      before do
+        lease = subject.lease_for(project.id)
 
-      expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
+        allow(Gitlab::ExclusiveLease).to receive(:new).and_return(lease)
-      subject.perform(nonexistent_id)
+        allow(lease).to receive(:try_obtain).and_return(true)
+      end
+
+      it 'skips when project no longer exists' do
+        nonexistent_id = 999999999999
+
+        expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
+        subject.perform(nonexistent_id)
+      end
+
+      it 'skips when project is pending delete' do
+        expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
+
+        subject.perform(pending_delete_project.id)
+      end
+
+      it 'delegates removal to service class' do
+        service = double('service')
+        expect(::Projects::HashedStorageMigrationService).to receive(:new).with(project, subject.logger).and_return(service)
+        expect(service).to receive(:execute)
+
+        subject.perform(project.id)
+      end
     end
 
-    it 'skips when project is pending delete' do
+    context 'when dont have exclusive lease' do
-      expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
+      before do
+        lease = subject.lease_for(project.id)
 
-      subject.perform(pending_delete_project.id)
+        allow(Gitlab::ExclusiveLease).to receive(:new).and_return(lease)
-    end
+        allow(lease).to receive(:try_obtain).and_return(false)
+      end
 
-    it 'delegates removal to service class' do
+      it 'skips when dont have lease' do
-      service = double('service')
+        expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
-      expect(::Projects::HashedStorageMigrationService).to receive(:new).with(project, subject.logger).and_return(service)
-      expect(service).to receive(:execute)
 
-      subject.perform(project.id)
+        subject.perform(project.id)
+      end
     end
   end
 end