Add latest changes from gitlab-org/gitlab@master

app/services/projects/hashed_storage/base_attachment_service.rb

@@ -16,6 +16,12 @@ module Projects
       # Returns the logger currently in use
       attr_reader :logger
 
+      def initialize(project:, old_disk_path:, logger: nil)
+        @project = project
+        @old_disk_path = old_disk_path
+        @logger = logger || Rails.logger # rubocop:disable Gitlab/RailsLogger
+      end
+
       # Return whether this operation was skipped or not
       #
       # @return [Boolean] true if skipped of false otherwise
@@ -23,6 +29,14 @@ module Projects
         @skipped
       end
 
+      # Check if target path has discardable content
+      #
+      # @param [String] new_path
+      # @return [Boolean] whether we can discard the target path or not
+      def target_path_discardable?(new_path)
+        false
+      end
+
       protected
 
       def move_folder!(old_path, new_path)
@@ -34,8 +48,13 @@ module Projects
         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"
+          if target_path_discardable?(new_path)
+            discard_path!(new_path)
+          else
+            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
         end
 
         # Create base path folder on the new storage layout
@@ -46,6 +65,16 @@ module Projects
 
         true
       end
+
+      # Rename a path adding a suffix in order to prevent data-loss.
+      #
+      # @param [String] new_path
+      def discard_path!(new_path)
+        discarded_path = "#{new_path}-#{Time.now.utc.to_i}"
+
+        logger.info("Moving existing empty attachments folder from '#{new_path}' to '#{discarded_path}', (PROJECT_ID=#{project.id})")
+        FileUtils.mv(new_path, discarded_path)
+      end
     end
   end
 end

app/services/projects/hashed_storage/base_repository_service.rb

@@ -10,7 +10,7 @@ module Projects
 
       attr_reader :old_disk_path, :new_disk_path, :old_wiki_disk_path, :old_storage_version, :logger, :move_wiki
 
-      def initialize(project, old_disk_path, logger: nil)
+      def initialize(project:, old_disk_path:, logger: nil)
         @project = project
         @logger = logger || Gitlab::AppLogger
         @old_disk_path = old_disk_path

app/services/projects/hashed_storage/migrate_attachments_service.rb

@@ -3,18 +3,19 @@
 module Projects
   module HashedStorage
     class MigrateAttachmentsService < BaseAttachmentService
-      def initialize(project, old_disk_path, logger: nil)
-        @project = project
-        @logger = logger || Rails.logger # rubocop:disable Gitlab/RailsLogger
-        @old_disk_path = old_disk_path
+      extend ::Gitlab::Utils::Override
+
+      # List of paths that can be excluded while evaluation if a target can be discarded
+      DISCARDABLE_PATHS = %w(tmp tmp/cache tmp/work).freeze
+
+      def initialize(project:, old_disk_path:, logger: nil)
+        super
+
         @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
-        origin.sub!(/#{Regexp.escape(project.full_path)}\z/, old_disk_path)
+        origin = find_old_attachments_path(project)
 
         project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:attachments]
         target = FileUploader.absolute_base_dir(project)
@@ -27,13 +28,38 @@ module Projects
           project.save!(validate: false)
 
           yield if block_given?
-        else
-          # Rollback changes
-          project.rollback!
         end
 
         result
       end
+
+      override :target_path_discardable?
+      # Check if target path has discardable content
+      #
+      # @param [String] new_path
+      # @return [Boolean] whether we can discard the target path or not
+      def target_path_discardable?(new_path)
+        return false unless File.directory?(new_path)
+
+        found = Dir.glob(File.join(new_path, '**', '**'))
+
+        (found - discardable_paths(new_path)).empty?
+      end
+
+      private
+
+      def discardable_paths(new_path)
+        DISCARDABLE_PATHS.collect { |path| File.join(new_path, path) }
+      end
+
+      def find_old_attachments_path(project)
+        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
+        #
+        origin.sub(/#{Regexp.escape(project.full_path)}\z/, old_disk_path)
+      end
     end
   end
 end

app/services/projects/hashed_storage/migration_service.rb

@@ -14,12 +14,12 @@ module Projects
       def execute
         # Migrate repository from Legacy to Hashed Storage
         unless project.hashed_storage?(:repository)
-          return false unless migrate_repository
+          return false unless migrate_repository_service.execute
         end
 
         # Migrate attachments from Legacy to Hashed Storage
         unless project.hashed_storage?(:attachments)
-          return false unless migrate_attachments
+          return false unless migrate_attachments_service.execute
         end
 
         true
@@ -27,12 +27,12 @@ module Projects
 
       private
 
-      def migrate_repository
-        HashedStorage::MigrateRepositoryService.new(project, old_disk_path, logger: logger).execute
+      def migrate_repository_service
+        HashedStorage::MigrateRepositoryService.new(project: project, old_disk_path: old_disk_path, logger: logger)
       end
 
-      def migrate_attachments
-        HashedStorage::MigrateAttachmentsService.new(project, old_disk_path, logger: logger).execute
+      def migrate_attachments_service
+        HashedStorage::MigrateAttachmentsService.new(project: project, old_disk_path: old_disk_path, logger: logger)
       end
     end
   end

app/services/projects/hashed_storage/rollback_attachments_service.rb

@@ -3,14 +3,9 @@
 module Projects
   module HashedStorage
     class RollbackAttachmentsService < BaseAttachmentService
-      def initialize(project, logger: nil)
-        @project = project
-        @logger = logger || Rails.logger # rubocop:disable Gitlab/RailsLogger
-        @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)
 

app/services/projects/hashed_storage/rollback_service.rb

@@ -5,32 +5,26 @@ module Projects
     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 # rubocop:disable Gitlab/RailsLogger
-      end
-
       def execute
         # Rollback attachments from Hashed Storage to Legacy
         if project.hashed_storage?(:attachments)
-          return false unless rollback_attachments
+          return false unless rollback_attachments_service.execute
         end
 
         # Rollback repository from Hashed Storage to Legacy
         if project.hashed_storage?(:repository)
-          rollback_repository
+          rollback_repository_service.execute
         end
       end
 
       private
 
-      def rollback_attachments
-        HashedStorage::RollbackAttachmentsService.new(project, logger: logger).execute
+      def rollback_attachments_service
+        HashedStorage::RollbackAttachmentsService.new(project: project, old_disk_path: old_disk_path, logger: logger)
       end
 
-      def rollback_repository
-        HashedStorage::RollbackRepositoryService.new(project, old_disk_path, logger: logger).execute
+      def rollback_repository_service
+        HashedStorage::RollbackRepositoryService.new(project: project, old_disk_path: old_disk_path, logger: logger)
       end
     end
   end

app/workers/hashed_storage/project_migrate_worker.rb

@@ -16,7 +16,7 @@ module HashedStorage
         project = Project.without_deleted.find_by(id: project_id)
         break unless project
 
-        old_disk_path ||= project.disk_path
+        old_disk_path ||= Storage::LegacyProject.new(project).disk_path
 
         ::Projects::HashedStorage::MigrationService.new(project, old_disk_path, logger: logger).execute
       end

changelogs/unreleased/32367-hashed-storage-migration-handle-failed-attachment-migrations-wth-ex.yml

+---
+title: 'Hashed Storage Migration: Handle failed attachment migrations with existing
+  target path'
+merge_request: 19061
+author:
+type: fixed

doc/administration/high_availability/README.md

@@ -229,7 +229,7 @@ users are, how much automation you use, mirroring, and repo/change size.
 | 3 GitLab Rails <br> - Puma workers on each node set to 90% of available CPUs with 16 threads | 32 vCPU, 28.8GB Memory | n1-highcpu-32 |
 | 3 PostgreSQL                  | 4 vCPU, 15GB Memory     | n1-standard-4  |
 | 1 PgBouncer                   | 2 vCPU, 1.8GB Memory    | n1-highcpu-2   |
-| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 90% of available CPUs with 16 threads | 16 vCPU, 60GB Memory   | n1-standard-16 |
+| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 20% of available CPUs | 16 vCPU, 60GB Memory   | n1-standard-16 |
 | 3 Redis Cache + Sentinel <br> - Cache maxmemory set to 90% of available memory | 4 vCPU, 15GB Memory | n1-standard-4 |
 | 3 Redis Persistent + Sentinel | 4 vCPU, 15GB Memory     | n1-standard-4  |
 | 4 Sidekiq                     | 4 vCPU, 15GB Memory     | n1-standard-4  |
@@ -256,7 +256,7 @@ vendors a best effort like for like can be used.
 | 7 GitLab Rails <br> - Puma workers on each node set to 90% of available CPUs with 16 threads | 32 vCPU, 28.8GB Memory | n1-highcpu-32 |
 | 3 PostgreSQL                  | 8 vCPU, 30GB Memory     | n1-standard-8  |
 | 1 PgBouncer                   | 2 vCPU, 1.8GB Memory    | n1-highcpu-2   |
-| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 90% of available CPUs with 16 threads | 32 vCPU, 120GB Memory   | n1-standard-32 |
+| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 20% of available CPUs | 32 vCPU, 120GB Memory   | n1-standard-32 |
 | 3 Redis Cache + Sentinel <br> - Cache maxmemory set to 90% of available memory | 4 vCPU, 15GB Memory | n1-standard-4 |
 | 3 Redis Persistent + Sentinel | 4 vCPU, 15GB Memory     | n1-standard-4  |
 | 4 Sidekiq                     | 4 vCPU, 15GB Memory     | n1-standard-4  |
@@ -285,7 +285,7 @@ may be adjusted prior to certification based on performance testing.
 | 15 GitLab Rails <br> - Puma workers on each node set to 90% of available CPUs with 16 threads | 32 vCPU, 28.8GB Memory | n1-highcpu-32 |
 | 3 PostgreSQL                  | 8 vCPU, 30GB Memory     | n1-standard-8  |
 | 1 PgBouncer                   | 2 vCPU, 1.8GB Memory    | n1-highcpu-2   |
-| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 90% of available CPUs with 16 threads | 64 vCPU, 240GB Memory   | n1-standard-64 |
+| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 20% of available CPUs | 64 vCPU, 240GB Memory   | n1-standard-64 |
 | 3 Redis Cache + Sentinel <br> - Cache maxmemory set to 90% of available memory | 4 vCPU, 15GB Memory | n1-standard-4 |
 | 3 Redis Persistent + Sentinel | 4 vCPU, 15GB Memory     | n1-standard-4  |
 | 4 Sidekiq                     | 4 vCPU, 15GB Memory     | n1-standard-4  |

doc/administration/operations/index.md

@@ -21,3 +21,6 @@ Keep your GitLab instance up and running smoothly.
   performance can have a big impact on GitLab performance, especially for actions
   that read or write Git repositories. This information will help benchmark
   filesystem performance against known good and bad real-world systems.
+- [ChatOps Scripts](https://gitlab.com/gitlab-com/chatops): The GitLab.com Infrastructure team uses this repository to house
+  common ChatOps scripts they use to troubleshoot and maintain the production instance of GitLab.com.
+  These scripts are likely useful to administrators of GitLab instances of all sizes.

doc/ci/chatops/README.md

@@ -58,6 +58,11 @@ ls:
     - echo -e "section_start:$( date +%s ):chat_reply\r\033[0K\n$( ls -la )\nsection_end:$( date +%s ):chat_reply\r\033[0K"
 ```
 
+## GitLab ChatOps Examples
+
+The GitLab.com team created a repository of [common ChatOps scripts they use to interact with our Production instance of GitLab](https://gitlab.com/gitlab-com/chatops). They are likely useful
+to other adminstrators of GitLab instances and can serve as inspiration for ChatOps scripts you can write to interact with your own applications.
+
 ## GitLab ChatOps icon
 
 Say Hi to our ChatOps bot.

doc/user/project/milestones/index.md

@@ -12,23 +12,21 @@ Milestones allow you to organize issues and merge requests into a cohesive group
 
 ## Milestones as Agile sprints
 
-Milestones can be used as Agile sprints.
-Set the milestone start date and due date to represent
-the start and end of your Agile sprint.
-Set the milestone title to the name of your Agile sprint,
-such as `November 2018 sprint`.
-Add an issue to your Agile sprint by associating
-the milestone to the issue.
+Milestones can be used as Agile sprints so that you can track all issues and merge requests related to a particular sprint. To do so:
+
+1. Set the milestone start date and due date to represent the start and end of your Agile sprint.
+1. Set the milestone title to the name of your Agile sprint, such as `November 2018 sprint`.
+1. Add an issue to your Agile sprint by associating the desired milestone from the issue's right-hand sidebar.
 
 ## Milestones as releases
 
-Milestones can be used as releases.
-Set the milestone due date to represent the release date of your release.
-(And leave the milestone start date blank.)
-Set the milestone title to the version of your release,
-such as `Version 9.4`.
-Add an issue to your release by associating
-the milestone to the issue.
+Similarily, milestones can be used as releases. To do so:
+
+1. Set the milestone due date to represent the release date of your release and leave the milestone start date blank.
+1. Set the milestone title to the version of your release, such as `Version 9.4`.
+1. Add an issue to your release by associating the desired milestone from the issue's right-hand sidebar.
+
+Additionally, you can integrate milestones with GitLab's [Releases feature](../releases/index.md#releases-associated-with-milestones).
 
 ## Project milestones and group milestones
 

doc/user/project/releases/index.md

@@ -58,6 +58,31 @@ links from your GitLab instance.
 NOTE: **NOTE**
 You can manipulate links of each release entry with [Release Links API](../../../api/releases/links.md)
 
+#### Releases associated with milestones
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/29020) in GitLab 12.5.
+
+Releases can optionally be associated with one or more
+[project milestones](../milestones/index.md#project-milestones-and-group-milestones)
+by including a `milestones` array in your requests to the
+[Releases API](../../../api/releases/index.md#create-a-release).
+
+Releases display this association with the **Milestone** indicator near
+the top of the Release block on the **Project overview > Releases** page.
+
+![A Release with one associated milestone](img/release_with_milestone_v12_5.png)
+
+Below is an example of milestones with no Releases, one Release, and two
+Releases, respectively.
+
+![Milestones with and without Release associations](img/milestone_list_with_releases_v12_5.png)
+
+This relationship is also visible in the **Releases** section of the sidebar
+when viewing a specific milestone. Below is an example of a milestone
+associated with a large number of Releases.
+
+![Milestone with lots of associated Releases](img/milestone_with_releases_v12_5.png)
+
 ## Releases list
 
 Navigate to **Project > Releases** in order to see the list of releases for a given

lib/gitlab/ci/config/entry/job.rb

@@ -114,7 +114,10 @@ module Gitlab
 
           entry :rules, Entry::Rules,
             description: 'List of evaluable Rules to determine job inclusion.',
-            inherit: false
+            inherit: false,
+            metadata: {
+              allowed_when: %w[on_success on_failure always never manual delayed].freeze
+            }
 
           entry :needs, Entry::Needs,
             description: 'Needs configuration for this job.',

lib/gitlab/ci/config/entry/rules/rule.rb

@@ -8,9 +8,9 @@ module Gitlab
           include ::Gitlab::Config::Entry::Validatable
           include ::Gitlab::Config::Entry::Attributable
 
-          CLAUSES      = %i[if changes exists].freeze
-          ALLOWED_KEYS = %i[if changes exists when start_in].freeze
-          ALLOWED_WHEN = %w[on_success on_failure always never manual delayed].freeze
+          CLAUSES        = %i[if changes exists].freeze
+          ALLOWED_KEYS   = %i[if changes exists when start_in].freeze
+          ALLOWABLE_WHEN = %w[on_success on_failure always never manual delayed].freeze
 
           attributes :if, :changes, :exists, :when, :start_in
 
@@ -25,7 +25,14 @@ module Gitlab
             with_options allow_nil: true do
               validates :if, expression: true
               validates :changes, :exists, array_of_strings: true, length: { maximum: 50 }
-              validates :when, allowed_values: { in: ALLOWED_WHEN }
+              validates :when, allowed_values: { in: ALLOWABLE_WHEN }
+            end
+
+            validate do
+              validates_with Gitlab::Config::Entry::Validators::AllowedValuesValidator,
+                attributes: %i[when],
+                allow_nil: true,
+                in: opt(:allowed_when)
             end
           end
 

package.json

@@ -37,9 +37,9 @@
     "@babel/plugin-syntax-dynamic-import": "^7.2.0",
     "@babel/plugin-syntax-import-meta": "^7.2.0",
     "@babel/preset-env": "^7.6.2",
-    "@gitlab/svgs": "^1.80.0",
-    "@gitlab/ui": "7.5.0",
-    "@gitlab/visual-review-tools": "1.0.3",
+    "@gitlab/svgs": "^1.82.0",
+    "@gitlab/ui": "7.11.0",
+    "@gitlab/visual-review-tools": "1.2.0",
     "@sentry/browser": "^5.7.1",
     "apollo-cache-inmemory": "^1.6.3",
     "apollo-client": "^2.6.4",

spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb

@@ -6,7 +6,17 @@ require 'support/helpers/stub_feature_flags'
 require_dependency 'active_model'
 
 describe Gitlab::Ci::Config::Entry::Rules::Rule do
-  let(:entry) { described_class.new(config) }
+  let(:factory) do
+    Gitlab::Config::Entry::Factory.new(described_class)
+      .metadata(metadata)
+      .value(config)
+  end
+
+  let(:metadata) do
+    { allowed_when: %w[on_success on_failure always never manual delayed] }
+  end
+
+  let(:entry) { factory.create! }
 
   describe '.new' do
     subject { entry }
@@ -212,6 +222,112 @@ describe Gitlab::Ci::Config::Entry::Rules::Rule do
           .to include(/should be a hash/)
       end
     end
+
+    context 'when: validation' do
+      context 'with an invalid boolean when:' do
+        let(:config) do
+          { if: '$THIS == "that"', when: false }
+        end
+
+        it { is_expected.to be_a(described_class) }
+        it { is_expected.not_to be_valid }
+
+        it 'returns an error about invalid when:' do
+          expect(subject.errors).to include(/when unknown value: false/)
+        end
+
+        context 'when composed' do
+          before do
+            subject.compose!
+          end
+
+          it { is_expected.not_to be_valid }
+
+          it 'returns an error about invalid when:' do
+            expect(subject.errors).to include(/when unknown value: false/)
+          end
+        end
+      end
+
+      context 'with an invalid string when:' do
+        let(:config) do
+          { if: '$THIS == "that"', when: 'explode' }
+        end
+
+        it { is_expected.to be_a(described_class) }
+        it { is_expected.not_to be_valid }
+
+        it 'returns an error about invalid when:' do
+          expect(subject.errors).to include(/when unknown value: explode/)
+        end
+
+        context 'when composed' do
+          before do
+            subject.compose!
+          end
+
+          it { is_expected.not_to be_valid }
+
+          it 'returns an error about invalid when:' do
+            expect(subject.errors).to include(/when unknown value: explode/)
+          end
+        end
+      end
+
+      context 'with a string passed in metadata but not allowed in the class' do
+        let(:metadata) { { allowed_when: %w[explode] } }
+
+        let(:config) do
+          { if: '$THIS == "that"', when: 'explode' }
+        end
+
+        it { is_expected.to be_a(described_class) }
+        it { is_expected.not_to be_valid }
+
+        it 'returns an error about invalid when:' do
+          expect(subject.errors).to include(/when unknown value: explode/)
+        end
+
+        context 'when composed' do
+          before do
+            subject.compose!
+          end
+
+          it { is_expected.not_to be_valid }
+
+          it 'returns an error about invalid when:' do
+            expect(subject.errors).to include(/when unknown value: explode/)
+          end
+        end
+      end
+
+      context 'with a string allowed in the class but not passed in metadata' do
+        let(:metadata) { { allowed_when: %w[always never] } }
+
+        let(:config) do
+          { if: '$THIS == "that"', when: 'on_success' }
+        end
+
+        it { is_expected.to be_a(described_class) }
+        it { is_expected.not_to be_valid }
+
+        it 'returns an error about invalid when:' do
+          expect(subject.errors).to include(/when unknown value: on_success/)
+        end
+
+        context 'when composed' do
+          before do
+            subject.compose!
+          end
+
+          it { is_expected.not_to be_valid }
+
+          it 'returns an error about invalid when:' do
+            expect(subject.errors).to include(/when unknown value: on_success/)
+          end
+        end
+      end
+    end
   end
 
   describe '#value' do

spec/lib/gitlab/ci/config/entry/rules_spec.rb

@@ -5,7 +5,14 @@ require 'support/helpers/stub_feature_flags'
 require_dependency 'active_model'
 
 describe Gitlab::Ci::Config::Entry::Rules do
-  let(:entry) { described_class.new(config) }
+  let(:factory) do
+    Gitlab::Config::Entry::Factory.new(described_class)
+      .metadata(metadata)
+      .value(config)
+  end
+
+  let(:metadata) { { allowed_when: %w[always never] } }
+  let(:entry)    { factory.create! }
 
   describe '.new' do
     subject { entry }
@@ -18,7 +25,7 @@ describe Gitlab::Ci::Config::Entry::Rules do
       it { is_expected.to be_a(described_class) }
       it { is_expected.to be_valid }
 
-      context 'after #compose!' do
+      context 'when composed' do
         before do
           subject.compose!
         end
@@ -38,7 +45,7 @@ describe Gitlab::Ci::Config::Entry::Rules do
       it { is_expected.to be_a(described_class) }
       it { is_expected.to be_valid }
 
-      context 'after #compose!' do
+      context 'when composed' do
         before do
           subject.compose!
         end
@@ -54,48 +61,6 @@ describe Gitlab::Ci::Config::Entry::Rules do
 
       it { is_expected.not_to be_valid }
     end
-
-    context 'with an invalid boolean when:' do
-      let(:config) do
-        [{ if: '$THIS == "that"', when: false }]
-      end
-
-      it { is_expected.to be_a(described_class) }
-      it { is_expected.to be_valid }
-
-      context 'after #compose!' do
-        before do
-          subject.compose!
-        end
-
-        it { is_expected.not_to be_valid }
-
-        it 'returns an error about invalid when:' do
-          expect(subject.errors).to include(/when unknown value: false/)
-        end
-      end
-    end
-
-    context 'with an invalid string when:' do
-      let(:config) do
-        [{ if: '$THIS == "that"', when: 'explode' }]
-      end
-
-      it { is_expected.to be_a(described_class) }
-      it { is_expected.to be_valid }
-
-      context 'after #compose!' do
-        before do
-          subject.compose!
-        end
-
-        it { is_expected.not_to be_valid }
-
-        it 'returns an error about invalid when:' do
-          expect(subject.errors).to include(/when unknown value: explode/)
-        end
-      end
-    end
   end
 
   describe '#value' do

spec/services/projects/hashed_storage/base_attachment_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::HashedStorage::BaseAttachmentService do
+  let(:project) { create(:project, :repository, storage_version: 0, skip_disk_validation: true) }
+
+  subject(:service) { described_class.new(project: project, old_disk_path: project.full_path, logger: nil) }
+
+  describe '#old_disk_path' do
+    it { is_expected.to respond_to :old_disk_path }
+  end
+
+  describe '#new_disk_path' do
+    it { is_expected.to respond_to :new_disk_path }
+  end
+
+  describe '#skipped?' do
+    it { is_expected.to respond_to :skipped? }
+  end
+
+  describe '#target_path_discardable?' do
+    it 'returns false' do
+      expect(subject.target_path_discardable?('something/something')).to be_falsey
+    end
+  end
+
+  describe '#discard_path!' do
+    it 'renames target path adding a timestamp at the end' do
+      target_path = Dir.mktmpdir
+      expect(Dir.exist?(target_path)).to be_truthy
+
+      Timecop.freeze do
+        suffix = Time.now.utc.to_i
+        subject.send(:discard_path!, target_path)
+
+        expected_renamed_path = "#{target_path}-#{suffix}"
+
+        expect(Dir.exist?(target_path)).to be_falsey
+        expect(Dir.exist?(expected_renamed_path)).to be_truthy
+      end
+    end
+  end
+
+  describe '#move_folder!' do
+    context 'when old_path is not a directory' do
+      it 'adds information to the logger and returns true' do
+        Tempfile.create do |old_path|
+          new_path = "#{old_path}-new"
+
+          expect(subject.send(:move_folder!, old_path, new_path)).to be_truthy
+        end
+      end
+    end
+  end
+end

spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe Projects::HashedStorage::MigrateAttachmentsService do
-  subject(:service) { described_class.new(project, project.full_path, logger: nil) }
+  subject(:service) { described_class.new(project: project, old_disk_path: project.full_path, logger: nil) }
 
   let(:project) { create(:project, :repository, storage_version: 1, skip_disk_validation: true) }
   let(:legacy_storage) { Storage::LegacyProject.new(project) }
@@ -72,7 +72,23 @@ describe Projects::HashedStorage::MigrateAttachmentsService do
         FileUtils.mkdir_p(base_path(hashed_storage))
       end
 
-      it 'raises AttachmentCannotMoveError' do
+      it 'succeed when target is empty' do
+        expect { service.execute }.not_to raise_error
+      end
+
+      it 'succeed when target include only discardable items' do
+        Projects::HashedStorage::MigrateAttachmentsService::DISCARDABLE_PATHS.each do |path_fragment|
+          discardable_path = File.join(base_path(hashed_storage), path_fragment)
+          FileUtils.mkdir_p(discardable_path)
+        end
+
+        expect { service.execute }.not_to raise_error
+      end
+
+      it 'raises AttachmentCannotMoveError when there are non discardable items on target path' do
+        not_discardable_path = File.join(base_path(hashed_storage), 'something')
+        FileUtils.mkdir_p(not_discardable_path)
+
         expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage))
 
         expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentCannotMoveError)
@@ -100,6 +116,18 @@ describe Projects::HashedStorage::MigrateAttachmentsService do
     end
   end
 
+  context '#target_path_discardable?' do
+    it 'returns true when it include only items on the discardable list' do
+      hashed_attachments_path = File.join(base_path(hashed_storage))
+      Projects::HashedStorage::MigrateAttachmentsService::DISCARDABLE_PATHS.each do |path_fragment|
+        discardable_path = File.join(hashed_attachments_path, path_fragment)
+        FileUtils.mkdir_p(discardable_path)
+      end
+
+      expect(service.target_path_discardable?(hashed_attachments_path)).to be_truthy
+    end
+  end
+
   def base_path(storage)
     File.join(FileUploader.root, storage.disk_path)
   end

spec/services/projects/hashed_storage/migrate_repository_service_spec.rb

@@ -10,7 +10,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do
   let(:legacy_storage) { Storage::LegacyProject.new(project) }
   let(:hashed_storage) { Storage::HashedProject.new(project) }
 
-  subject(:service) { described_class.new(project, project.disk_path) }
+  subject(:service) { described_class.new(project: project, old_disk_path: project.disk_path) }
 
   describe '#execute' do
     let(:old_disk_path) { legacy_storage.disk_path }

spec/services/projects/hashed_storage/migration_service_spec.rb

@@ -10,13 +10,14 @@ describe Projects::HashedStorage::MigrationService do
 
   describe '#execute' do
     context 'repository migration' do
-      let(:repository_service) { Projects::HashedStorage::MigrateRepositoryService.new(project, project.full_path, logger: logger) }
+      let(:repository_service) do
+        Projects::HashedStorage::MigrateRepositoryService.new(project: project,
+                                                              old_disk_path: project.full_path,
+                                                              logger: logger)
+      end
 
       it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do
-        expect(Projects::HashedStorage::MigrateRepositoryService)
-          .to receive(:new)
-                .with(project, project.full_path, logger: logger)
-                .and_return(repository_service)
+        expect(service).to receive(:migrate_repository_service).and_return(repository_service)
         expect(repository_service).to receive(:execute)
 
         service.execute
@@ -31,13 +32,14 @@ describe Projects::HashedStorage::MigrationService do
     end
 
     context 'attachments migration' do
-      let(:attachments_service) { Projects::HashedStorage::MigrateAttachmentsService.new(project, project.full_path, logger: logger) }
+      let(:attachments_service) do
+        Projects::HashedStorage::MigrateAttachmentsService.new(project: project,
+                                                               old_disk_path: project.full_path,
+                                                               logger: logger)
+      end
 
       it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do
-        expect(Projects::HashedStorage::MigrateAttachmentsService)
-          .to receive(:new)
-                .with(project, project.full_path, logger: logger)
-                .and_return(attachments_service)
+        expect(service).to receive(:migrate_attachments_service).and_return(attachments_service)
         expect(attachments_service).to receive(:execute)
 
         service.execute

spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe Projects::HashedStorage::RollbackAttachmentsService do
-  subject(:service) { described_class.new(project, logger: nil) }
+  subject(:service) { described_class.new(project: project, old_disk_path: project.disk_path, logger: nil) }
 
   let(:project) { create(:project, :repository, skip_disk_validation: true) }
   let(:legacy_storage) { Storage::LegacyProject.new(project) }

spec/services/projects/hashed_storage/rollback_repository_service_spec.rb

@@ -10,7 +10,7 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis
   let(:legacy_storage) { Storage::LegacyProject.new(project) }
   let(:hashed_storage) { Storage::HashedProject.new(project) }
 
-  subject(:service) { described_class.new(project, project.disk_path) }
+  subject(:service) { described_class.new(project: project, old_disk_path: project.disk_path) }
 
   describe '#execute' do
     let(:old_disk_path) { hashed_storage.disk_path }

spec/services/projects/hashed_storage/rollback_service_spec.rb

@@ -6,17 +6,15 @@ 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) }
+  subject(:service) { described_class.new(project, project.disk_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) }
+      let(:attachments_service) { attachments_service_class.new(project: project, old_disk_path: project.disk_path, 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(service).to receive(:rollback_attachments_service).and_return(attachments_service)
         expect(attachments_service).to receive(:execute)
 
         service.execute
@@ -31,15 +29,12 @@ describe Projects::HashedStorage::RollbackService do
     end
 
     context 'repository rollback' do
+      let(:project) { create(:project, :empty_repo, :wiki_repo, storage_version: ::Project::HASHED_STORAGE_FEATURES[:repository]) }
       let(:repository_service_class) { Projects::HashedStorage::RollbackRepositoryService }
-      let(:repository_service) { repository_service_class.new(project, project.full_path, logger: logger) }
+      let(:repository_service) { repository_service_class.new(project: project, old_disk_path: project.disk_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(service).to receive(:rollback_repository_service).and_return(repository_service)
         expect(repository_service).to receive(:execute)
 
         service.execute

yarn.lock

@@ -717,15 +717,15 @@
   dependencies:
     vue-eslint-parser "^6.0.4"
 
-"@gitlab/svgs@^1.80.0":
-  version "1.80.0"
-  resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.80.0.tgz#52b2d25f002cdfe9bd7c366a043c1849687ad64b"
-  integrity sha512-hsyX3EZV/hk9bMTvvoxVcNC0EO6sy771BC2vXjqGtzjye4hTs0BTAzu3V0UPWuDompHtKXi/plVcJU+NxNLQ6Q==
+"@gitlab/svgs@^1.82.0":
+  version "1.82.0"
+  resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.82.0.tgz#c059c460afc13ebfe9df370521ca8963fa5afb80"
+  integrity sha512-9L4Brys2LCk44lHvFsCFDKN768lYjoMVYDb4PD7FSjqUEruQQ1SRj0rvb1RWKLhiTCDKrtDOXkH6I1TTEms24w==
 
-"@gitlab/ui@7.5.0":
-  version "7.5.0"
-  resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-7.5.0.tgz#d25567157d20bb64741ab51b6b9f770ea49e634d"
-  integrity sha512-h7RxNMtQ1+KHK2uV+nb5d7UlqBVOtj9VGXqRXqVinc1b1x0onnvFFnYjgxf7XbXdsZq85ZyTlZa1SkduRig+Eg==
+"@gitlab/ui@7.11.0":
+  version "7.11.0"
+  resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-7.11.0.tgz#b5c981f3b1edbf0ad75bcca8fa1cd81017676b3b"
+  integrity sha512-PxZkgdY2j/XdriTdp3jsnsif9cgcxd1wUF8PVOho2HIyJqU244E8ELewIXkDozQq3p3ZXzWnjR/GvYcNMZtGmA==
   dependencies:
     "@babel/standalone" "^7.0.0"
     "@gitlab/vue-toasted" "^1.2.1"
@@ -740,10 +740,10 @@
     vue "^2.6.10"
     vue-loader "^15.4.2"
 
-"@gitlab/visual-review-tools@1.0.3":
-  version "1.0.3"
-  resolved "https://registry.yarnpkg.com/@gitlab/visual-review-tools/-/visual-review-tools-1.0.3.tgz#b49c4a6fd8af3a1517d7e7d04096562f8bcb5d14"
-  integrity sha512-96j+0+Ivon5nYvT2doDCLQoBzU/GZYfQGLBmZZE3FZVMsIPAEsqDcSV/6+XCikUzU3B8VnH6er6l9OxE5x1RVw==
+"@gitlab/visual-review-tools@1.2.0":
+  version "1.2.0"
+  resolved "https://registry.yarnpkg.com/@gitlab/visual-review-tools/-/visual-review-tools-1.2.0.tgz#8d6757917193c1023012bb4a316dc1a97309a27a"
+  integrity sha512-GaV/lYLmOF0hWtv8K8MLWGaCZ7PL1LF4D0/gargXYf9HO0Cw4wtz4oWyaLS15wFposJIYdPIHSNfrLVk4Dk9sQ==
 
 "@gitlab/vue-toasted@^1.2.1":
   version "1.2.1"