Merge branch '299853-fj-move-project-repository-storage-classes-to-namespace' into 'master'

Move project repository storage classes to namespace

See merge request gitlab-org/gitlab!54793

app/models/project.rb

@@ -345,7 +345,7 @@ class Project < ApplicationRecord
 
   has_many :daily_build_group_report_results, class_name: 'Ci::DailyBuildGroupReportResult'
 
-  has_many :repository_storage_moves, class_name: 'ProjectRepositoryStorageMove', inverse_of: :container
+  has_many :repository_storage_moves, class_name: 'Projects::RepositoryStorageMove', inverse_of: :container
 
   has_many :webide_pipelines, -> { webide_source }, class_name: 'Ci::Pipeline', inverse_of: :project
   has_many :reviews, inverse_of: :project

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
-  extend ::Gitlab::Utils::Override
-  include RepositoryStorageMovable
-
-  belongs_to :container, class_name: 'Project', inverse_of: :repository_storage_moves, foreign_key: :project_id
-  alias_attribute :project, :container
-  scope :with_projects, -> { includes(container: :route) }
-
-  override :update_repository_storage
-  def update_repository_storage(new_storage)
-    container.update_column(:repository_storage, new_storage)
-  end
-
-  override :schedule_repository_storage_update_worker
-  def schedule_repository_storage_update_worker
-    ProjectUpdateRepositoryStorageWorker.perform_async(
-      project_id,
-      destination_storage_name,
-      id
-    )
-  end
-
-  private
-
-  override :error_key
-  def error_key
-    :project
-  end
+# This is a compatibility class to avoid calling a non-existent
+# class from sidekiq during deployment.
+#
+# This class was moved to a namespace in https://gitlab.com/gitlab-org/gitlab/-/issues/299853.
+# we cannot remove this class entirely because there can be jobs
+# referencing it.
+#
+# We can get rid of this class in 14.0
+# https://gitlab.com/gitlab-org/gitlab/-/issues/322393
+class ProjectRepositoryStorageMove < Projects::RepositoryStorageMove
 end

app/models/projects/repository_storage_move.rb

+# frozen_string_literal: true
+
+# Projects::RepositoryStorageMove are details of repository storage moves for a
+# project. For example, moving a project to another gitaly node to help
+# balance storage capacity.
+module Projects
+  class RepositoryStorageMove < ApplicationRecord
+    extend ::Gitlab::Utils::Override
+    include RepositoryStorageMovable
+
+    self.table_name = 'project_repository_storage_moves'
+
+    belongs_to :container, class_name: 'Project', inverse_of: :repository_storage_moves, foreign_key: :project_id
+    alias_attribute :project, :container
+    scope :with_projects, -> { includes(container: :route) }
+
+    override :update_repository_storage
+    def update_repository_storage(new_storage)
+      container.update_column(:repository_storage, new_storage)
+    end
+
+    override :schedule_repository_storage_update_worker
+    def schedule_repository_storage_update_worker
+      Projects::UpdateRepositoryStorageWorker.perform_async(
+        project_id,
+        destination_storage_name,
+        id
+      )
+    end
+
+    private
+
+    override :error_key
+    def error_key
+      :project
+    end
+  end
+end

app/services/projects/schedule_bulk_repository_shard_moves_service.rb

@@ -25,7 +25,7 @@ module Projects
 
     override :schedule_bulk_worker_klass
     def self.schedule_bulk_worker_klass
-      ::ProjectScheduleBulkRepositoryShardMovesWorker
+      ::Projects::ScheduleBulkRepositoryShardMovesWorker
     end
   end
 end

app/workers/all_queues.yml

@@ -2020,6 +2020,22 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: projects_schedule_bulk_repository_shard_moves
+  :feature_category: :gitaly
+  :has_external_dependencies:
+  :urgency: :throttled
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
+- :name: projects_update_repository_storage
+  :feature_category: :gitaly
+  :has_external_dependencies:
+  :urgency: :throttled
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: prometheus_create_default_alerts
   :feature_category: :incident_management
   :has_external_dependencies:

app/workers/project_schedule_bulk_repository_shard_moves_worker.rb

 # frozen_string_literal: true
 
-class ProjectScheduleBulkRepositoryShardMovesWorker
-  include ApplicationWorker
-
+# This is a compatibility class to avoid calling a non-existent
+# class from sidekiq during deployment.
+#
+# This class was moved to a namespace in https://gitlab.com/gitlab-org/gitlab/-/issues/299853.
+# we cannot remove this class entirely because there can be jobs
+# referencing it.
+#
+# We can get rid of this class in 14.0
+# https://gitlab.com/gitlab-org/gitlab/-/issues/322393
+class ProjectScheduleBulkRepositoryShardMovesWorker < Projects::ScheduleBulkRepositoryShardMovesWorker
   idempotent!
-  feature_category :gitaly
   urgency :throttled
-
-  def perform(source_storage_name, destination_storage_name = nil)
-    Projects::ScheduleBulkRepositoryShardMovesService.new.execute(source_storage_name, destination_storage_name)
-  end
 end

app/workers/project_update_repository_storage_worker.rb

 # frozen_string_literal: true
 
-class ProjectUpdateRepositoryStorageWorker # rubocop:disable Scalability/IdempotentWorker
-  extend ::Gitlab::Utils::Override
-  include UpdateRepositoryStorageWorker
-
-  private
-
-  override :find_repository_storage_move
-  def find_repository_storage_move(repository_storage_move_id)
-    ProjectRepositoryStorageMove.find(repository_storage_move_id)
-  end
-
-  override :find_container
-  def find_container(container_id)
-    Project.find(container_id)
-  end
-
-  override :update_repository_storage
-  def update_repository_storage(repository_storage_move)
-    ::Projects::UpdateRepositoryStorageService.new(repository_storage_move).execute
-  end
+# This is a compatibility class to avoid calling a non-existent
+# class from sidekiq during deployment.
+#
+# This class was moved to a namespace in https://gitlab.com/gitlab-org/gitlab/-/issues/299853.
+# we cannot remove this class entirely because there can be jobs
+# referencing it.
+#
+# We can get rid of this class in 14.0
+# https://gitlab.com/gitlab-org/gitlab/-/issues/322393
+class ProjectUpdateRepositoryStorageWorker < Projects::UpdateRepositoryStorageWorker
+  idempotent!
+  urgency :throttled
 end

app/workers/projects/schedule_bulk_repository_shard_moves_worker.rb

+# frozen_string_literal: true
+
+module Projects
+  class ScheduleBulkRepositoryShardMovesWorker
+    include ApplicationWorker
+
+    idempotent!
+    feature_category :gitaly
+    urgency :throttled
+
+    def perform(source_storage_name, destination_storage_name = nil)
+      Projects::ScheduleBulkRepositoryShardMovesService.new.execute(source_storage_name, destination_storage_name)
+    end
+  end
+end

app/workers/projects/update_repository_storage_worker.rb

+# frozen_string_literal: true
+
+module Projects
+  class UpdateRepositoryStorageWorker # rubocop:disable Scalability/IdempotentWorker
+    extend ::Gitlab::Utils::Override
+    include ::UpdateRepositoryStorageWorker
+
+    private
+
+    override :find_repository_storage_move
+    def find_repository_storage_move(repository_storage_move_id)
+      ::Projects::RepositoryStorageMove.find(repository_storage_move_id)
+    end
+
+    override :find_container
+    def find_container(container_id)
+      Project.find(container_id)
+    end
+
+    override :update_repository_storage
+    def update_repository_storage(repository_storage_move)
+      ::Projects::UpdateRepositoryStorageService.new(repository_storage_move).execute
+    end
+  end
+end

config/sidekiq_queues.yml

@@ -282,6 +282,10 @@
   - 1
 - - projects_git_garbage_collect
   - 1
+- - projects_schedule_bulk_repository_shard_moves
+  - 1
+- - projects_update_repository_storage
+  - 1
 - - prometheus_create_default_alerts
   - 1
 - - propagate_integration

db/post_migrate/20210210093901_backfill_updated_at_after_repository_storage_move.rb

@@ -10,16 +10,16 @@ class BackfillUpdatedAtAfterRepositoryStorageMove < ActiveRecord::Migration[6.0]
 
   disable_ddl_transaction!
 
-  class ProjectRepositoryStorageMove < ActiveRecord::Base
+  class RepositoryStorageMove < ActiveRecord::Base
     include EachBatch
 
     self.table_name = 'project_repository_storage_moves'
   end
 
   def up
-    ProjectRepositoryStorageMove.reset_column_information
+    RepositoryStorageMove.reset_column_information
 
-    ProjectRepositoryStorageMove.select(:project_id).distinct.each_batch(of: BATCH_SIZE, column: :project_id) do |batch, index|
+    RepositoryStorageMove.select(:project_id).distinct.each_batch(of: BATCH_SIZE, column: :project_id) do |batch, index|
       migrate_in(
         INTERVAL * index,
         MIGRATION_CLASS,

lib/api/entities/project_repository_storage_move.rb → lib/api/entities/projects/repository_storage_move.rb

@@ -2,8 +2,10 @@
 
 module API
   module Entities
-    class ProjectRepositoryStorageMove < BasicRepositoryStorageMove
+    module Projects
+      class RepositoryStorageMove < BasicRepositoryStorageMove
         expose :project, using: Entities::ProjectIdentity
       end
     end
+  end
 end

lib/api/project_repository_storage_moves.rb

@@ -11,28 +11,28 @@ module API
     resource :project_repository_storage_moves do
       desc 'Get a list of all project repository storage moves' do
         detail 'This feature was introduced in GitLab 13.0.'
-        success Entities::ProjectRepositoryStorageMove
+        success Entities::Projects::RepositoryStorageMove
       end
       params do
         use :pagination
       end
       get do
-        storage_moves = ProjectRepositoryStorageMove.with_projects.order_created_at_desc
+        storage_moves = ::Projects::RepositoryStorageMove.with_projects.order_created_at_desc
 
-        present paginate(storage_moves), with: Entities::ProjectRepositoryStorageMove, current_user: current_user
+        present paginate(storage_moves), with: Entities::Projects::RepositoryStorageMove, current_user: current_user
       end
 
       desc 'Get a project repository storage move' do
         detail 'This feature was introduced in GitLab 13.0.'
-        success Entities::ProjectRepositoryStorageMove
+        success Entities::Projects::RepositoryStorageMove
       end
       params do
         requires :repository_storage_move_id, type: Integer, desc: 'The ID of a project repository storage move'
       end
       get ':repository_storage_move_id' do
-        storage_move = ProjectRepositoryStorageMove.find(params[:repository_storage_move_id])
+        storage_move = ::Projects::RepositoryStorageMove.find(params[:repository_storage_move_id])
 
-        present storage_move, with: Entities::ProjectRepositoryStorageMove, current_user: current_user
+        present storage_move, with: Entities::Projects::RepositoryStorageMove, current_user: current_user
       end
 
       desc 'Schedule bulk project repository storage moves' do
@@ -58,7 +58,7 @@ module API
     resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       desc 'Get a list of all project repository storage moves' do
         detail 'This feature was introduced in GitLab 13.1.'
-        success Entities::ProjectRepositoryStorageMove
+        success Entities::Projects::RepositoryStorageMove
       end
       params do
         use :pagination
@@ -66,12 +66,12 @@ module API
       get ':id/repository_storage_moves' do
         storage_moves = user_project.repository_storage_moves.with_projects.order_created_at_desc
 
-        present paginate(storage_moves), with: Entities::ProjectRepositoryStorageMove, current_user: current_user
+        present paginate(storage_moves), with: Entities::Projects::RepositoryStorageMove, current_user: current_user
       end
 
       desc 'Get a project repository storage move' do
         detail 'This feature was introduced in GitLab 13.1.'
-        success Entities::ProjectRepositoryStorageMove
+        success Entities::Projects::RepositoryStorageMove
       end
       params do
         requires :repository_storage_move_id, type: Integer, desc: 'The ID of a project repository storage move'
@@ -79,12 +79,12 @@ module API
       get ':id/repository_storage_moves/:repository_storage_move_id' do
         storage_move = user_project.repository_storage_moves.find(params[:repository_storage_move_id])
 
-        present storage_move, with: Entities::ProjectRepositoryStorageMove, current_user: current_user
+        present storage_move, with: Entities::Projects::RepositoryStorageMove, current_user: current_user
       end
 
       desc 'Schedule a project repository storage move' do
         detail 'This feature was introduced in GitLab 13.1.'
-        success Entities::ProjectRepositoryStorageMove
+        success Entities::Projects::RepositoryStorageMove
       end
       params do
         optional :destination_storage_name, type: String, desc: 'The destination storage shard'
@@ -95,7 +95,7 @@ module API
         )
 
         if storage_move.schedule
-          present storage_move, with: Entities::ProjectRepositoryStorageMove, current_user: current_user
+          present storage_move, with: Entities::Projects::RepositoryStorageMove, current_user: current_user
         else
           render_validation_error!(storage_move)
         end

lib/gitlab/background_migration/backfill_project_updated_at_after_repository_storage_move.rb

@@ -5,7 +5,7 @@ module Gitlab
     # Update existent project update_at column after their repository storage was moved
     class BackfillProjectUpdatedAtAfterRepositoryStorageMove
       def perform(*project_ids)
-        updated_repository_storages = ProjectRepositoryStorageMove.select("project_id, MAX(updated_at) as updated_at").where(project_id: project_ids).group(:project_id)
+        updated_repository_storages = Projects::RepositoryStorageMove.select("project_id, MAX(updated_at) as updated_at").where(project_id: project_ids).group(:project_id)
 
         Project.connection.execute <<-SQL
           WITH repository_storage_cte as (

spec/factories/project_repository_storage_moves.rb

 # frozen_string_literal: true
 
 FactoryBot.define do
-  factory :project_repository_storage_move, class: 'ProjectRepositoryStorageMove' do
+  factory :project_repository_storage_move, class: 'Projects::RepositoryStorageMove' do
     container { association(:project) }
 
     source_storage_name { 'default' }
 
     trait :scheduled do
-      state { ProjectRepositoryStorageMove.state_machines[:state].states[:scheduled].value }
+      state { Projects::RepositoryStorageMove.state_machines[:state].states[:scheduled].value }
     end
 
     trait :started do
-      state { ProjectRepositoryStorageMove.state_machines[:state].states[:started].value }
+      state { Projects::RepositoryStorageMove.state_machines[:state].states[:started].value }
     end
 
     trait :replicated do
-      state { ProjectRepositoryStorageMove.state_machines[:state].states[:replicated].value }
+      state { Projects::RepositoryStorageMove.state_machines[:state].states[:replicated].value }
     end
 
     trait :finished do
-      state { ProjectRepositoryStorageMove.state_machines[:state].states[:finished].value }
+      state { Projects::RepositoryStorageMove.state_machines[:state].states[:finished].value }
     end
 
     trait :failed do
-      state { ProjectRepositoryStorageMove.state_machines[:state].states[:failed].value }
+      state { Projects::RepositoryStorageMove.state_machines[:state].states[:failed].value }
     end
   end
 end

spec/lib/api/entities/project_repository_storage_move_spec.rb → spec/lib/api/entities/projects/repository_storage_move_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe API::Entities::ProjectRepositoryStorageMove do
+RSpec.describe API::Entities::Projects::RepositoryStorageMove do
   describe '#as_json' do
     subject { entity.as_json }
 

spec/models/project_repository_storage_move_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe ProjectRepositoryStorageMove, type: :model do
     let(:container) { project }
     let(:repository_storage_factory_key) { :project_repository_storage_move }
     let(:error_key) { :project }
-    let(:repository_storage_worker) { ProjectUpdateRepositoryStorageWorker }
+    let(:repository_storage_worker) { Projects::UpdateRepositoryStorageWorker }
   end
 
   describe 'state transitions' do

spec/models/projects/repository_storage_move_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::RepositoryStorageMove, type: :model do
+  let_it_be_with_refind(:project) { create(:project) }
+
+  it_behaves_like 'handles repository moves' do
+    let(:container) { project }
+    let(:repository_storage_factory_key) { :project_repository_storage_move }
+    let(:error_key) { :project }
+    let(:repository_storage_worker) { Projects::UpdateRepositoryStorageWorker }
+  end
+
+  describe 'state transitions' do
+    let(:storage) { 'test_second_storage' }
+
+    before do
+      stub_storage_settings(storage => { 'path' => 'tmp/tests/extra_storage' })
+    end
+
+    context 'when started' do
+      subject(:storage_move) { create(:project_repository_storage_move, :started, container: project, destination_storage_name: storage) }
+
+      context 'and transits to replicated' do
+        it 'sets the repository storage and marks the container as writable' do
+          storage_move.finish_replication!
+
+          expect(project.repository_storage).to eq(storage)
+          expect(project).not_to be_repository_read_only
+        end
+      end
+    end
+  end
+end

spec/requests/api/project_repository_storage_moves_spec.rb

@@ -7,6 +7,6 @@ RSpec.describe API::ProjectRepositoryStorageMoves do
     let_it_be(:container) { create(:project, :repository).tap { |project| project.track_project_repository } }
     let_it_be(:storage_move) { create(:project_repository_storage_move, :scheduled, container: container) }
     let(:repository_storage_move_factory) { :project_repository_storage_move }
-    let(:bulk_worker_klass) { ProjectScheduleBulkRepositoryShardMovesWorker }
+    let(:bulk_worker_klass) { Projects::ScheduleBulkRepositoryShardMovesWorker }
   end
 end

spec/requests/api/projects_spec.rb

@@ -2818,7 +2818,7 @@ RSpec.describe API::Projects do
             Sidekiq::Testing.fake! do
               put(api("/projects/#{new_project.id}", user), params: { repository_storage: unknown_storage, issues_enabled: false })
             end
-          end.not_to change(ProjectUpdateRepositoryStorageWorker.jobs, :size)
+          end.not_to change(Projects::UpdateRepositoryStorageWorker.jobs, :size)
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response['issues_enabled']).to eq(false)
@@ -2845,7 +2845,7 @@ RSpec.describe API::Projects do
             Sidekiq::Testing.fake! do
               put(api("/projects/#{new_project.id}", admin), params: { repository_storage: 'test_second_storage' })
             end
-          end.to change(ProjectUpdateRepositoryStorageWorker.jobs, :size).by(1)
+          end.to change(Projects::UpdateRepositoryStorageWorker.jobs, :size).by(1)
 
           expect(response).to have_gitlab_http_status(:ok)
         end

spec/services/projects/schedule_bulk_repository_shard_moves_service_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe Projects::ScheduleBulkRepositoryShardMovesService do
   it_behaves_like 'moves repository shard in bulk' do
     let_it_be_with_reload(:container) { create(:project, :repository).tap { |project| project.track_project_repository } }
 
-    let(:move_service_klass) { ProjectRepositoryStorageMove }
-    let(:bulk_worker_klass) { ::ProjectScheduleBulkRepositoryShardMovesWorker }
+    let(:move_service_klass) { Projects::RepositoryStorageMove }
+    let(:bulk_worker_klass) { ::Projects::ScheduleBulkRepositoryShardMovesWorker }
   end
 end

spec/services/projects/update_service_spec.rb

@@ -551,7 +551,7 @@ RSpec.describe Projects::UpdateService do
 
             expect(project).to be_repository_read_only
             expect(project.repository_storage_moves.last).to have_attributes(
-              state: ::ProjectRepositoryStorageMove.state_machines[:state].states[:scheduled].value,
+              state: ::Projects::RepositoryStorageMove.state_machines[:state].states[:scheduled].value,
               source_storage_name: 'default',
               destination_storage_name: 'test_second_storage'
             )

spec/workers/project_schedule_bulk_repository_shard_moves_worker_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe ProjectScheduleBulkRepositoryShardMovesWorker do
   it_behaves_like 'schedules bulk repository shard moves' do
     let_it_be_with_reload(:container) { create(:project, :repository).tap { |project| project.track_project_repository } }
 
-    let(:move_service_klass) { ProjectRepositoryStorageMove }
-    let(:worker_klass) { ProjectUpdateRepositoryStorageWorker }
+    let(:move_service_klass) { Projects::RepositoryStorageMove }
+    let(:worker_klass) { Projects::UpdateRepositoryStorageWorker }
   end
 end

spec/workers/project_update_repository_storage_worker_spec.rb

@@ -10,6 +10,6 @@ RSpec.describe ProjectUpdateRepositoryStorageWorker do
     let_it_be(:repository_storage_move) { create(:project_repository_storage_move) }
 
     let(:service_klass) { Projects::UpdateRepositoryStorageService }
-    let(:repository_storage_move_klass) { ProjectRepositoryStorageMove }
+    let(:repository_storage_move_klass) { Projects::RepositoryStorageMove }
   end
 end

spec/workers/projects/schedule_bulk_repository_shard_moves_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::ScheduleBulkRepositoryShardMovesWorker do
+  it_behaves_like 'schedules bulk repository shard moves' do
+    let_it_be_with_reload(:container) { create(:project, :repository).tap { |project| project.track_project_repository } }
+
+    let(:move_service_klass) { Projects::RepositoryStorageMove }
+    let(:worker_klass) { Projects::UpdateRepositoryStorageWorker }
+  end
+end

spec/workers/projects/update_repository_storage_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::UpdateRepositoryStorageWorker do
+  subject { described_class.new }
+
+  it_behaves_like 'an update storage move worker' do
+    let_it_be_with_refind(:container) { create(:project, :repository) }
+    let_it_be(:repository_storage_move) { create(:project_repository_storage_move) }
+
+    let(:service_klass) { Projects::UpdateRepositoryStorageService }
+    let(:repository_storage_move_klass) { Projects::RepositoryStorageMove }
+  end
+end