Store status of project repository storage moves

Creates a new table project_repository_storage_moves

app/models/project.rb

@@ -324,6 +324,8 @@ class Project < ApplicationRecord
 
   has_many :daily_report_results, class_name: 'Ci::DailyReportResult'
 
+  has_many :repository_storage_moves, class_name: 'ProjectRepositoryStorageMove'
+
   accepts_nested_attributes_for :variables, allow_destroy: true
   accepts_nested_attributes_for :project_feature, update_only: true
   accepts_nested_attributes_for :import_data
@@ -2073,7 +2075,18 @@ class Project < ApplicationRecord
 
     raise ArgumentError unless ::Gitlab.config.repositories.storages.key?(new_repository_storage_key)
 
-    run_after_commit { ProjectUpdateRepositoryStorageWorker.perform_async(id, new_repository_storage_key) }
+    storage_move = repository_storage_moves.create!(
+      source_storage_name: repository_storage,
+      destination_storage_name: new_repository_storage_key
+    )
+
+    run_after_commit do
+      ProjectUpdateRepositoryStorageWorker.perform_async(
+        id, new_repository_storage_key,
+        repository_storage_move_id: storage_move.id
+      )
+    end
+
     self.repository_read_only = true
   end
 

app/models/project_repository_storage_move.rb

+# frozen_string_literal: true
+
+# ProjectRepositoryStorageMove are details of repository storage moves for a
+# project. For example, moving a project to another gitaly node to help
+# balance storage capacity.
+class ProjectRepositoryStorageMove < ApplicationRecord
+  belongs_to :project, inverse_of: :repository_storage_moves
+
+  enum state: { scheduled: 1, started: 2, finished: 3, failed: 4 }
+
+  validates :project, presence: true
+  validates :state, presence: true
+  validates :source_storage_name,
+    presence: true,
+    inclusion: { in: ->(_) { Gitlab.config.repositories.storages.keys } }
+  validates :destination_storage_name,
+    presence: true,
+    inclusion: { in: ->(_) { Gitlab.config.repositories.storages.keys } }
+end

app/services/projects/update_repository_storage_service.rb

@@ -2,32 +2,42 @@
 
 module Projects
   class UpdateRepositoryStorageService < BaseService
-    include Gitlab::ShellAdapter
-
     Error = Class.new(StandardError)
     SameFilesystemError = Class.new(Error)
 
-    def initialize(project)
-      @project = project
+    attr_reader :repository_storage_move
+    delegate :destination_storage_name, to: :repository_storage_move
+
+    def initialize(repository_storage_move)
+      super(repository_storage_move.project)
+      @repository_storage_move = repository_storage_move
     end
 
-    def execute(new_repository_storage_key)
-      raise SameFilesystemError if same_filesystem?(project.repository.storage, new_repository_storage_key)
+    def execute
+      repository_storage_move.update!(state: :started)
 
-      mirror_repositories(new_repository_storage_key)
+      raise SameFilesystemError if same_filesystem?(repository.storage, destination_storage_name)
 
-      mark_old_paths_for_archive
+      mirror_repositories
 
-      project.update!(repository_storage: new_repository_storage_key, repository_read_only: false)
-      project.leave_pool_repository
-      project.track_project_repository
+      project.transaction do
+        mark_old_paths_for_archive
+
+        repository_storage_move.update!(state: :finished)
+        project.update!(repository_storage: destination_storage_name, repository_read_only: false)
+        project.leave_pool_repository
+        project.track_project_repository
+      end
 
       enqueue_housekeeping
 
       success
 
     rescue StandardError => e
-      project.update!(repository_read_only: false)
+      project.transaction do
+        repository_storage_move.update!(state: :failed)
+        project.update!(repository_read_only: false)
+      end
 
       Gitlab::ErrorTracking.track_exception(e, project_path: project.full_path)
 
@@ -40,15 +50,15 @@ module Projects
       Gitlab::GitalyClient.filesystem_id(old_storage) == Gitlab::GitalyClient.filesystem_id(new_storage)
     end
 
-    def mirror_repositories(new_repository_storage_key)
-      mirror_repository(new_repository_storage_key)
+    def mirror_repositories
+      mirror_repository
 
       if project.wiki.repository_exists?
-        mirror_repository(new_repository_storage_key, type: Gitlab::GlRepository::WIKI)
+        mirror_repository(type: Gitlab::GlRepository::WIKI)
       end
     end
 
-    def mirror_repository(new_storage_key, type: Gitlab::GlRepository::PROJECT)
+    def mirror_repository(type: Gitlab::GlRepository::PROJECT)
       unless wait_for_pushes(type)
         raise Error, s_('UpdateRepositoryStorage|Timeout waiting for %{type} repository pushes') % { type: type.name }
       end
@@ -60,7 +70,7 @@ module Projects
 
       # Initialize a git repository on the target path
       new_repository = Gitlab::Git::Repository.new(
-        new_storage_key,
+        destination_storage_name,
         raw_repository.relative_path,
         raw_repository.gl_repository,
         full_path

app/workers/project_update_repository_storage_worker.rb

@@ -5,9 +5,19 @@ class ProjectUpdateRepositoryStorageWorker # rubocop:disable Scalability/Idempot
 
   feature_category :gitaly
 
-  def perform(project_id, new_repository_storage_key)
-    project = Project.find(project_id)
+  def perform(project_id, new_repository_storage_key, repository_storage_move_id: nil)
+    repository_storage_move =
+      if repository_storage_move_id
+        ProjectRepositoryStorageMove.find(repository_storage_move_id)
+      else
+        # maintain compatibility with workers queued before release
+        project = Project.find(project_id)
+        project.repository_storage_moves.create!(
+          source_storage_name: project.repository_storage,
+          destination_storage_name: new_repository_storage_key
+        )
+      end
 
-    ::Projects::UpdateRepositoryStorageService.new(project).execute(new_repository_storage_key)
+    ::Projects::UpdateRepositoryStorageService.new(repository_storage_move).execute
   end
 end

db/migrate/20200407222647_create_project_repository_storage_moves.rb

+# frozen_string_literal: true
+
+class CreateProjectRepositoryStorageMoves < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    create_table :project_repository_storage_moves do |t|
+      t.timestamps_with_timezone
+      t.integer :project_id, limit: 8, null: false
+      t.integer :state, limit: 2, default: 1, null: false
+      t.text :source_storage_name, null: false
+      t.text :destination_storage_name, null: false
+    end
+
+    add_index :project_repository_storage_moves, :project_id
+
+    add_text_limit(:project_repository_storage_moves, :source_storage_name, 255, constraint_name: 'project_repository_storage_moves_source_storage_name')
+    add_text_limit(:project_repository_storage_moves, :destination_storage_name, 255, constraint_name: 'project_repository_storage_moves_destination_storage_name')
+  end
+
+  def down
+    remove_check_constraint(:project_repository_storage_moves, 'project_repository_storage_moves_source_storage_name')
+    remove_check_constraint(:project_repository_storage_moves, 'project_repository_storage_moves_destination_storage_name')
+
+    drop_table :project_repository_storage_moves
+  end
+end

db/migrate/20200429015603_add_fk_to_project_repository_storage_moves.rb

+# frozen_string_literal: true
+
+class AddFkToProjectRepositoryStorageMoves < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_foreign_key :project_repository_storage_moves, :projects, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :project_repository_storage_moves, :projects
+    end
+  end
+end

db/structure.sql

@@ -5171,6 +5171,27 @@ CREATE SEQUENCE public.project_repository_states_id_seq
 
 ALTER SEQUENCE public.project_repository_states_id_seq OWNED BY public.project_repository_states.id;
 
+CREATE TABLE public.project_repository_storage_moves (
+    id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    project_id bigint NOT NULL,
+    state smallint DEFAULT 1 NOT NULL,
+    source_storage_name text NOT NULL,
+    destination_storage_name text NOT NULL,
+    CONSTRAINT project_repository_storage_moves_destination_storage_name CHECK ((char_length(destination_storage_name) <= 255)),
+    CONSTRAINT project_repository_storage_moves_source_storage_name CHECK ((char_length(source_storage_name) <= 255))
+);
+
+CREATE SEQUENCE public.project_repository_storage_moves_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE public.project_repository_storage_moves_id_seq OWNED BY public.project_repository_storage_moves.id;
+
 CREATE TABLE public.project_settings (
     project_id integer NOT NULL,
     created_at timestamp with time zone NOT NULL,
@@ -7640,6 +7661,8 @@ ALTER TABLE ONLY public.project_repositories ALTER COLUMN id SET DEFAULT nextval
 
 ALTER TABLE ONLY public.project_repository_states ALTER COLUMN id SET DEFAULT nextval('public.project_repository_states_id_seq'::regclass);
 
+ALTER TABLE ONLY public.project_repository_storage_moves ALTER COLUMN id SET DEFAULT nextval('public.project_repository_storage_moves_id_seq'::regclass);
+
 ALTER TABLE ONLY public.project_statistics ALTER COLUMN id SET DEFAULT nextval('public.project_statistics_id_seq'::regclass);
 
 ALTER TABLE ONLY public.project_tracing_settings ALTER COLUMN id SET DEFAULT nextval('public.project_tracing_settings_id_seq'::regclass);
@@ -8525,6 +8548,9 @@ ALTER TABLE ONLY public.project_repositories
 ALTER TABLE ONLY public.project_repository_states
     ADD CONSTRAINT project_repository_states_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.project_repository_storage_moves
+    ADD CONSTRAINT project_repository_storage_moves_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY public.project_settings
     ADD CONSTRAINT project_settings_pkey PRIMARY KEY (project_id);
 
@@ -10188,6 +10214,8 @@ CREATE INDEX index_project_repositories_on_shard_id ON public.project_repositori
 
 CREATE UNIQUE INDEX index_project_repository_states_on_project_id ON public.project_repository_states USING btree (project_id);
 
+CREATE INDEX index_project_repository_storage_moves_on_project_id ON public.project_repository_storage_moves USING btree (project_id);
+
 CREATE UNIQUE INDEX index_project_settings_on_push_rule_id ON public.project_settings USING btree (push_rule_id);
 
 CREATE INDEX index_project_statistics_on_namespace_id ON public.project_statistics USING btree (namespace_id);
@@ -11774,6 +11802,9 @@ ALTER TABLE ONLY public.merge_request_diff_files
 ALTER TABLE ONLY public.status_page_settings
     ADD CONSTRAINT fk_rails_506e5ba391 FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.project_repository_storage_moves
+    ADD CONSTRAINT fk_rails_5106dbd44a FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.x509_commit_signatures
     ADD CONSTRAINT fk_rails_53fe41188f FOREIGN KEY (x509_certificate_id) REFERENCES public.x509_certificates(id) ON DELETE CASCADE;
 
@@ -13575,6 +13606,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200407171133
 20200407171417
 20200407182205
+20200407222647
 20200408110856
 20200408133211
 20200408153842
@@ -13638,5 +13670,6 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200424050250
 20200424101920
 20200427064130
+20200429015603
 \.
 

ee/app/services/ee/projects/update_repository_storage_service.rb

@@ -6,11 +6,11 @@ module EE
       extend ::Gitlab::Utils::Override
 
       override :mirror_repositories
-      def mirror_repositories(new_repository_storage_key)
+      def mirror_repositories
         super
 
         if project.design_repository.exists?
-          mirror_repository(new_repository_storage_key, type: ::Gitlab::GlRepository::DESIGN)
+          mirror_repository(type: ::Gitlab::GlRepository::DESIGN)
         end
       end
 

ee/spec/services/projects/update_repository_storage_service_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 describe Projects::UpdateRepositoryStorageService do
   include Gitlab::ShellAdapter
 
-  subject { described_class.new(project) }
+  subject { described_class.new(repository_storage_move) }
 
   before do
     allow(Gitlab.config.repositories.storages).to receive(:keys).and_return(%w[default test_second_storage])
@@ -16,6 +16,8 @@ describe Projects::UpdateRepositoryStorageService do
       include_examples 'moves repository to another storage', 'design' do
         let(:project) { create(:project, :repository, repository_read_only: true) }
         let(:repository) { project.design_repository }
+        let(:destination) { 'test_second_storage' }
+        let(:repository_storage_move) { create(:project_repository_storage_move, project: project, destination_storage_name: destination) }
 
         before do
           project.design_repository.create_if_not_exists

spec/factories/project_repository_storage_moves.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :project_repository_storage_move, class: 'ProjectRepositoryStorageMove' do
+    project
+
+    source_storage_name { 'default' }
+    destination_storage_name { 'default' }
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -489,6 +489,7 @@ project:
 - compliance_framework_setting
 - metrics_users_starred_dashboards
 - alert_management_alerts
+- repository_storage_moves
 award_emoji:
 - awardable
 - user

spec/models/project_repository_storage_move_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ProjectRepositoryStorageMove, type: :model do
+  describe 'associations' do
+    it { is_expected.to belong_to(:project) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:project) }
+    it { is_expected.to validate_presence_of(:state) }
+    it { is_expected.to validate_presence_of(:source_storage_name) }
+    it { is_expected.to validate_presence_of(:destination_storage_name) }
+
+    context 'source_storage_name inclusion' do
+      subject { build(:project_repository_storage_move, source_storage_name: 'missing') }
+
+      it "does not allow repository storages that don't match a label in the configuration" do
+        expect(subject).not_to be_valid
+        expect(subject.errors[:source_storage_name].first).to match(/is not included in the list/)
+      end
+    end
+
+    context 'destination_storage_name inclusion' do
+      subject { build(:project_repository_storage_move, destination_storage_name: 'missing') }
+
+      it "does not allow repository storages that don't match a label in the configuration" do
+        expect(subject).not_to be_valid
+        expect(subject.errors[:destination_storage_name].first).to match(/is not included in the list/)
+      end
+    end
+  end
+end

spec/models/project_spec.rb

@@ -115,6 +115,7 @@ describe Project do
     it { is_expected.to have_many(:alert_management_alerts) }
     it { is_expected.to have_many(:jira_imports) }
     it { is_expected.to have_many(:metrics_users_starred_dashboards).inverse_of(:project) }
+    it { is_expected.to have_many(:repository_storage_moves) }
 
     it_behaves_like 'model with repository' do
       let_it_be(:container) { create(:project, :repository, path: 'somewhere') }
@@ -2837,12 +2838,16 @@ describe Project do
     end
 
     it 'schedules the transfer of the repository to the new storage and locks the project' do
-      expect(ProjectUpdateRepositoryStorageWorker).to receive(:perform_async).with(project.id, 'test_second_storage')
+      expect(ProjectUpdateRepositoryStorageWorker).to receive(:perform_async).with(project.id, 'test_second_storage', repository_storage_move_id: anything)
 
       project.change_repository_storage('test_second_storage')
       project.save!
 
       expect(project).to be_repository_read_only
+      expect(project.repository_storage_moves.last).to have_attributes(
+        source_storage_name: "default",
+        destination_storage_name: "test_second_storage"
+      )
     end
 
     it "doesn't schedule the transfer if the repository is already read-only" do

spec/services/projects/fork_service_spec.rb

@@ -320,7 +320,11 @@ describe Projects::ForkService do
       allow_any_instance_of(Gitlab::Git::Repository).to receive(:checksum)
         .and_return(::Gitlab::Git::BLANK_SHA)
 
-      Projects::UpdateRepositoryStorageService.new(project).execute('test_second_storage')
+      storage_move = project.repository_storage_moves.create!(
+        source_storage_name: project.repository_storage,
+        destination_storage_name: 'test_second_storage'
+      )
+      Projects::UpdateRepositoryStorageService.new(storage_move).execute
       fork_after_move = fork_project(project)
       pool_repository_before_move = PoolRepository.joins(:shard)
                                       .find_by(source_project: project, shards: { name: 'default' })

spec/services/projects/update_repository_storage_service_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 describe Projects::UpdateRepositoryStorageService do
   include Gitlab::ShellAdapter
 
-  subject { described_class.new(project) }
+  subject { described_class.new(repository_storage_move) }
 
   describe "#execute" do
     let(:time) { Time.now }
@@ -17,6 +17,8 @@ describe Projects::UpdateRepositoryStorageService do
 
     context 'without wiki and design repository' do
       let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: false) }
+      let(:destination) { 'test_second_storage' }
+      let(:repository_storage_move) { create(:project_repository_storage_move, project: project, destination_storage_name: destination) }
       let!(:checksum) { project.repository.checksum }
       let(:project_repository_double) { double(:repository) }
 
@@ -42,7 +44,7 @@ describe Projects::UpdateRepositoryStorageService do
           expect(project_repository_double).to receive(:checksum)
             .and_return(checksum)
 
-          result = subject.execute('test_second_storage')
+          result = subject.execute
 
           expect(result[:status]).to eq(:success)
           expect(project).not_to be_repository_read_only
@@ -53,8 +55,10 @@ describe Projects::UpdateRepositoryStorageService do
       end
 
       context 'when the filesystems are the same' do
+        let(:destination) { project.repository_storage }
+
         it 'bails out and does nothing' do
-          result = subject.execute(project.repository_storage)
+          result = subject.execute
 
           expect(result[:status]).to eq(:error)
           expect(result[:message]).to match(/SameFilesystemError/)
@@ -73,7 +77,7 @@ describe Projects::UpdateRepositoryStorageService do
             .and_raise(Gitlab::Git::CommandError)
           expect(GitlabShellWorker).not_to receive(:perform_async)
 
-          result = subject.execute('test_second_storage')
+          result = subject.execute
 
           expect(result[:status]).to eq(:error)
           expect(project).not_to be_repository_read_only
@@ -94,7 +98,7 @@ describe Projects::UpdateRepositoryStorageService do
             .and_return('not matching checksum')
           expect(GitlabShellWorker).not_to receive(:perform_async)
 
-          result = subject.execute('test_second_storage')
+          result = subject.execute
 
           expect(result[:status]).to eq(:error)
           expect(project).not_to be_repository_read_only
@@ -116,7 +120,7 @@ describe Projects::UpdateRepositoryStorageService do
           expect(project_repository_double).to receive(:checksum)
             .and_return(checksum)
 
-          result = subject.execute('test_second_storage')
+          result = subject.execute
 
           expect(result[:status]).to eq(:success)
           expect(project.repository_storage).to eq('test_second_storage')
@@ -129,6 +133,8 @@ describe Projects::UpdateRepositoryStorageService do
       include_examples 'moves repository to another storage', 'wiki' do
         let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: true) }
         let(:repository) { project.wiki.repository }
+        let(:destination) { 'test_second_storage' }
+        let(:repository_storage_move) { create(:project_repository_storage_move, project: project, destination_storage_name: destination) }
 
         before do
           project.create_wiki

spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb

@@ -47,7 +47,7 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
 
       old_repository_path = repository.full_path
 
-      result = subject.execute('test_second_storage')
+      result = subject.execute
 
       expect(result[:status]).to eq(:success)
       expect(project).not_to be_repository_read_only
@@ -62,7 +62,7 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
       end
 
       it 'does not enqueue a GC run' do
-        expect { subject.execute('test_second_storage') }
+        expect { subject.execute }
           .not_to change(GitGarbageCollectWorker.jobs, :count)
       end
     end
@@ -75,20 +75,22 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
       it 'does not enqueue a GC run if housekeeping is disabled' do
         stub_application_setting(housekeeping_enabled: false)
 
-        expect { subject.execute('test_second_storage') }
+        expect { subject.execute }
           .not_to change(GitGarbageCollectWorker.jobs, :count)
       end
 
       it 'enqueues a GC run' do
-        expect { subject.execute('test_second_storage') }
+        expect { subject.execute }
           .to change(GitGarbageCollectWorker.jobs, :count).by(1)
       end
     end
   end
 
   context 'when the filesystems are the same' do
+    let(:destination) { project.repository_storage }
+
     it 'bails out and does nothing' do
-      result = subject.execute(project.repository_storage)
+      result = subject.execute
 
       expect(result[:status]).to eq(:error)
       expect(result[:message]).to match(/SameFilesystemError/)
@@ -114,7 +116,7 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
 
       expect(GitlabShellWorker).not_to receive(:perform_async)
 
-      result = subject.execute('test_second_storage')
+      result = subject.execute
 
       expect(result[:status]).to eq(:error)
       expect(project).not_to be_repository_read_only
@@ -142,7 +144,7 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
 
       expect(GitlabShellWorker).not_to receive(:perform_async)
 
-      result = subject.execute('test_second_storage')
+      result = subject.execute
 
       expect(result[:status]).to eq(:error)
       expect(project).not_to be_repository_read_only

spec/workers/project_update_repository_storage_worker_spec.rb

@@ -9,12 +9,40 @@ describe ProjectUpdateRepositoryStorageWorker do
   subject { described_class.new }
 
   describe "#perform" do
-    it "calls the update repository storage service" do
-      expect_next_instance_of(Projects::UpdateRepositoryStorageService) do |instance|
-        expect(instance).to receive(:execute).with('new_storage')
+    let(:service) { double(:update_repository_storage_service) }
+
+    before do
+      allow(Gitlab.config.repositories.storages).to receive(:keys).and_return(%w[default test_second_storage])
+    end
+
+    context 'without repository storage move' do
+      it "calls the update repository storage service" do
+        expect(Projects::UpdateRepositoryStorageService).to receive(:new).and_return(service)
+        expect(service).to receive(:execute)
+
+        expect do
+          subject.perform(project.id, 'test_second_storage')
+        end.to change(ProjectRepositoryStorageMove, :count).by(1)
+
+        storage_move = project.repository_storage_moves.last
+        expect(storage_move).to have_attributes(
+          source_storage_name: "default",
+          destination_storage_name: "test_second_storage"
+        )
       end
+    end
+
+    context 'with repository storage move' do
+      let!(:repository_storage_move) { create(:project_repository_storage_move) }
 
-      subject.perform(project.id, 'new_storage')
+      it "calls the update repository storage service" do
+        expect(Projects::UpdateRepositoryStorageService).to receive(:new).and_return(service)
+        expect(service).to receive(:execute)
+
+        expect do
+          subject.perform(nil, nil, repository_storage_move_id: repository_storage_move.id)
+        end.not_to change(ProjectRepositoryStorageMove, :count)
+      end
     end
   end
 end