workers: Remove support for asynchronous remote removal

There are no users left of the worker which asynchronously removes
a repository's on-disk remotes given that we only use in-memory remotes
nowadays. While we can now remove the infrastructure to asynchronously
remove remotes, we still need to retain the worker to make sure all jobs
are drained on an upgrade. The worker's logic is stubbed out though to
just do nothing: the worst that can happen with this is that we retain
on-disk remotes in Gitaly storages, but Gitaly will eventually clean
them up via repository maintenance anyway.

app/models/repository.rb

@@ -942,21 +942,6 @@ class Repository
     fetch_remote(remote_name, url: url, refmap: refmap, forced: forced, prune: prune)
   end
 
-  def async_remove_remote(remote_name)
-    return unless remote_name
-    return unless project
-
-    job_id = RepositoryRemoveRemoteWorker.perform_async(project.id, remote_name)
-
-    if job_id
-      Gitlab::AppLogger.info("Remove remote job scheduled for #{project.id} with remote name: #{remote_name} job ID #{job_id}.")
-    else
-      Gitlab::AppLogger.info("Remove remote job failed to create for #{project.id} with remote name #{remote_name}.")
-    end
-
-    job_id
-  end
-
   def fetch_source_branch!(source_repository, source_branch, local_ref)
     raw_repository.fetch_source_branch!(source_repository.raw_repository, source_branch, local_ref)
   end

app/workers/repository_remove_remote_worker.rb

@@ -16,22 +16,13 @@ class RepositoryRemoveRemoteWorker # rubocop:disable Scalability/IdempotentWorke
   attr_reader :project, :remote_name
 
   def perform(project_id, remote_name)
-    @remote_name = remote_name
-    @project = Project.find_by_id(project_id)
-
-    return unless @project
-
-    logger.info("Removing remote #{remote_name} from project #{project.id}")
-
-    try_obtain_lease do
-      remove_remote = @project.repository.remove_remote(remote_name)
-
-      if remove_remote
-        logger.info("Remote #{remote_name} was successfully removed from project #{project.id}")
-      else
-        logger.error("Could not remove remote #{remote_name} from project #{project.id}")
-      end
-    end
+    # On-disk remotes are slated for removal, and GitLab doesn't create any of
+    # them anymore. For backwards compatibility, we need to keep the worker
+    # though such that we can be sure to drain all jobs on an update. Making
+    # this a no-op is fine though: the worst that can happen is that we still
+    # have old remotes lingering in the repository's config, but Gitaly will
+    # start to clean these up in repository maintenance.
+    # https://gitlab.com/gitlab-org/gitlab/-/issues/336745
   end
 
   def lease_timeout

ee/app/models/ee/project.rb

@@ -29,9 +29,6 @@ module EE
       before_save :set_override_pull_mirror_available, unless: -> { ::Gitlab::CurrentSettings.mirror_available }
       before_save :set_next_execution_timestamp_to_now, if: ->(project) { project.mirror? && project.mirror_changed? && project.import_state }
 
-      after_update :remove_mirror_repository_reference,
-        if: ->(project) { project.mirror? && project.import_url_updated? }
-
       after_create :create_security_setting, unless: :security_setting
 
       belongs_to :mirror_user, class_name: 'User'
@@ -569,12 +566,6 @@ module EE
       saved_change_to_import_url? && saved_changes['import_url'].first
     end
 
-    def remove_mirror_repository_reference
-      run_after_commit do
-        repository.async_remove_remote(::Repository::MIRROR_REMOTE)
-      end
-    end
-
     def username_only_import_url
       bare_url = read_attribute(:import_url)
       return bare_url unless ::Gitlab::UrlSanitizer.valid?(bare_url)

ee/spec/models/project_spec.rb

@@ -1064,7 +1064,7 @@ RSpec.describe Project do
     it 'removes previous remote' do
       project = create(:project, :repository, :mirror)
 
-      expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(project.id, ::Repository::MIRROR_REMOTE).and_call_original
+      expect(RepositoryRemoveRemoteWorker).not_to receive(:perform_async)
 
       project.update(import_url: "http://test.com")
     end

spec/models/repository_spec.rb

@@ -1094,38 +1094,6 @@ RSpec.describe Repository do
     end
   end
 
-  describe '#async_remove_remote' do
-    before do
-      masterrev = repository.find_branch('master').dereferenced_target
-      create_remote_branch('joe', 'remote_branch', masterrev)
-    end
-
-    context 'when worker is scheduled successfully' do
-      before do
-        masterrev = repository.find_branch('master').dereferenced_target
-        create_remote_branch('remote_name', 'remote_branch', masterrev)
-
-        allow(RepositoryRemoveRemoteWorker).to receive(:perform_async).and_return('1234')
-      end
-
-      it 'returns job_id' do
-        expect(repository.async_remove_remote('joe')).to eq('1234')
-      end
-    end
-
-    context 'when worker does not schedule successfully' do
-      before do
-        allow(RepositoryRemoveRemoteWorker).to receive(:perform_async).and_return(nil)
-      end
-
-      it 'returns nil' do
-        expect(Gitlab::AppLogger).to receive(:info).with("Remove remote job failed to create for #{project.id} with remote name joe.")
-
-        expect(repository.async_remove_remote('joe')).to be_nil
-      end
-    end
-  end
-
   describe '#fetch_as_mirror' do
     let(:url) { "http://example.com" }
     let(:remote_name) { "remote-name" }

spec/services/notification_service_spec.rb

@@ -3368,10 +3368,6 @@ RSpec.describe NotificationService, :mailer do
         project.add_maintainer(u_maintainer2)
         project.add_developer(u_developer)
 
-        # Mock remote update
-        allow(project.repository).to receive(:async_remove_remote)
-        allow(project.repository).to receive(:add_remote)
-
         reset_delivered_emails!
       end
 

spec/workers/repository_remove_remote_worker_spec.rb

@@ -24,37 +24,25 @@ RSpec.describe RepositoryRemoveRemoteWorker do
           .and_return(project)
       end
 
-      it 'does not remove remote when cannot obtain lease' do
+      it 'does nothing when cannot obtain lease' do
         stub_exclusive_lease_taken(lease_key, timeout: lease_timeout)
 
         expect(project.repository)
           .not_to receive(:remove_remote)
-
         expect(subject)
-          .to receive(:log_error)
-          .with("Cannot obtain an exclusive lease for #{lease_key}. There must be another instance already in execution.")
+          .not_to receive(:log_error)
 
         subject.perform(project.id, remote_name)
       end
 
-      it 'removes remote from repository when obtain a lease' do
+      it 'does nothing when obtain a lease' do
         stub_exclusive_lease(lease_key, timeout: lease_timeout)
-        masterrev = project.repository.find_branch('master').dereferenced_target
-        create_remote_branch(remote_name, 'remote_branch', masterrev)
 
         expect(project.repository)
-          .to receive(:remove_remote)
-          .with(remote_name)
-          .and_call_original
+          .not_to receive(:remove_remote)
 
         subject.perform(project.id, remote_name)
       end
     end
   end
-
-  def create_remote_branch(remote_name, branch_name, target)
-    rugged = rugged_repo(project.repository)
-
-    rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id)
-  end
 end