Merge branch 'pks-update-remote-mirror-inmemory' into 'master'

Implement in-memory remotes for `update_remote_mirror` [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!63962

Gemfile

@@ -480,7 +480,7 @@ end
 gem 'spamcheck', '~> 0.1.0'
 
 # Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 13.12.0.pre.rc1'
+gem 'gitaly', '~> 14.0.0.pre.rc2'
 
 # KAS GRPC protocol definitions
 gem 'kas-grpc', '~> 0.0.2'

Gemfile.lock

@@ -454,7 +454,7 @@ GEM
       rails (>= 3.2.0)
     git (1.7.0)
       rchardet (~> 1.8)
-    gitaly (13.12.0.pre.rc1)
+    gitaly (14.0.0.pre.rc2)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab (4.16.1)
@@ -1483,7 +1483,7 @@ DEPENDENCIES
   gettext (~> 3.3)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly (~> 13.12.0.pre.rc1)
+  gitaly (~> 14.0.0.pre.rc2)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
   gitlab-dangerfiles (~> 2.1.2)

app/models/remote_mirror.rb

@@ -100,10 +100,11 @@ class RemoteMirror < ApplicationRecord
     update_status == 'started'
   end
 
-  def update_repository
+  def update_repository(inmemory_remote:)
     Gitlab::Git::RemoteMirror.new(
       project.repository.raw,
       remote_name,
+      inmemory_remote ? remote_url : nil,
       **options_for_update
     ).update
   end

app/services/projects/update_remote_mirror_service.rb

@@ -39,12 +39,16 @@ module Projects
 
     def update_mirror(remote_mirror)
       remote_mirror.update_start!
-      remote_mirror.ensure_remote!
 
       # LFS objects must be sent first, or the push has dangling pointers
       send_lfs_objects!(remote_mirror)
 
-      response = remote_mirror.update_repository
+      response = if Feature.enabled?(:update_remote_mirror_inmemory, project, default_enabled: :yaml)
+                   remote_mirror.update_repository(inmemory_remote: true)
+                 else
+                   remote_mirror.ensure_remote!
+                   remote_mirror.update_repository(inmemory_remote: false)
+                 end
 
       if response.divergent_refs.any?
         message = "Some refs have diverged and have not been updated on the remote:"

config/feature_flags/development/update_remote_mirror_inmemory.yml

+---
+name: update_remote_mirror_inmemory
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63962
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/333517
+milestone: '14.0'
+type: development
+group: group::gitaly
+default_enabled: false

lib/gitlab/git/remote_mirror.rb

@@ -5,11 +5,12 @@ module Gitlab
     class RemoteMirror
       include Gitlab::Git::WrapsGitalyErrors
 
-      attr_reader :repository, :ref_name, :only_branches_matching, :ssh_key, :known_hosts, :keep_divergent_refs
+      attr_reader :repository, :ref_name, :remote_url, :only_branches_matching, :ssh_key, :known_hosts, :keep_divergent_refs
 
-      def initialize(repository, ref_name, only_branches_matching: [], ssh_key: nil, known_hosts: nil, keep_divergent_refs: false)
+      def initialize(repository, ref_name, remote_url, only_branches_matching: [], ssh_key: nil, known_hosts: nil, keep_divergent_refs: false)
         @repository = repository
         @ref_name = ref_name
+        @remote_url = remote_url
         @only_branches_matching = only_branches_matching
         @ssh_key = ssh_key
         @known_hosts = known_hosts
@@ -20,6 +21,7 @@ module Gitlab
         wrapped_gitaly_errors do
           repository.gitaly_remote_client.update_remote_mirror(
             ref_name,
+            remote_url,
             only_branches_matching,
             ssh_key: ssh_key,
             known_hosts: known_hosts,

lib/gitlab/gitaly_client/remote_service.rb

@@ -55,13 +55,18 @@ module Gitlab
         encode_utf8(response.ref)
       end
 
-      def update_remote_mirror(ref_name, only_branches_matching, ssh_key: nil, known_hosts: nil, keep_divergent_refs: false)
+      def update_remote_mirror(ref_name, remote_url, only_branches_matching, ssh_key: nil, known_hosts: nil, keep_divergent_refs: false)
         req_enum = Enumerator.new do |y|
           first_request = Gitaly::UpdateRemoteMirrorRequest.new(
-            repository: @gitaly_repo,
-            ref_name: ref_name
+            repository: @gitaly_repo
           )
 
+          if remote_url
+            first_request.remote = Gitaly::UpdateRemoteMirrorRequest::Remote.new(url: remote_url)
+          else
+            first_request.ref_name = ref_name
+          end
+
           first_request.ssh_key = ssh_key if ssh_key.present?
           first_request.known_hosts = known_hosts if known_hosts.present?
           first_request.keep_divergent_refs = keep_divergent_refs

spec/lib/gitlab/git/remote_mirror_spec.rb

@@ -7,16 +7,29 @@ RSpec.describe Gitlab::Git::RemoteMirror do
     let(:project) { create(:project, :repository) }
     let(:repository) { project.repository }
     let(:ref_name) { 'foo' }
+    let(:url) { 'https://example.com' }
     let(:options) { { only_branches_matching: ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true } }
 
-    subject(:remote_mirror) { described_class.new(repository, ref_name, **options) }
+    subject(:remote_mirror) { described_class.new(repository, ref_name, url, **options) }
 
-    it 'delegates to the Gitaly client' do
-      expect(repository.gitaly_remote_client)
-        .to receive(:update_remote_mirror)
-        .with(ref_name, ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true)
+    shared_examples 'an update' do
+      it 'delegates to the Gitaly client' do
+        expect(repository.gitaly_remote_client)
+          .to receive(:update_remote_mirror)
+          .with(ref_name, url, ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true)
+
+        remote_mirror.update # rubocop:disable Rails/SaveBang
+      end
+    end
+
+    context 'with url' do
+      it_behaves_like 'an update'
+    end
+
+    context 'without url' do
+      let(:url) { nil }
 
-      remote_mirror.update # rubocop:disable Rails/SaveBang
+      it_behaves_like 'an update'
     end
 
     it 'wraps gitaly errors' do

spec/lib/gitlab/gitaly_client/remote_service_spec.rb

@@ -67,13 +67,29 @@ RSpec.describe Gitlab::GitalyClient::RemoteService do
     let(:ssh_key) { 'KEY' }
     let(:known_hosts) { 'KNOWN HOSTS' }
 
-    it 'sends an update_remote_mirror message' do
-      expect_any_instance_of(Gitaly::RemoteService::Stub)
-        .to receive(:update_remote_mirror)
-        .with(kind_of(Enumerator), kind_of(Hash))
-        .and_return(double(:update_remote_mirror_response))
+    shared_examples 'an update' do
+      it 'sends an update_remote_mirror message' do
+        expect_any_instance_of(Gitaly::RemoteService::Stub)
+          .to receive(:update_remote_mirror)
+          .with(array_including(gitaly_request_with_params(expected_params)), kind_of(Hash))
+          .and_return(double(:update_remote_mirror_response))
+
+        client.update_remote_mirror(ref_name, url, only_branches_matching, ssh_key: ssh_key, known_hosts: known_hosts, keep_divergent_refs: true)
+      end
+    end
+
+    context 'with remote name' do
+      let(:url) { nil }
+      let(:expected_params) { { ref_name: ref_name } }
+
+      it_behaves_like 'an update'
+    end
+
+    context 'with remote URL' do
+      let(:url) { 'http:://git.example.com/my-repo.git' }
+      let(:expected_params) { { remote: Gitaly::UpdateRemoteMirrorRequest::Remote.new(url: url) } }
 
-      client.update_remote_mirror(ref_name, only_branches_matching, ssh_key: ssh_key, known_hosts: known_hosts, keep_divergent_refs: true)
+      it_behaves_like 'an update'
     end
   end
 

spec/models/remote_mirror_spec.rb

@@ -157,19 +157,34 @@ RSpec.describe RemoteMirror, :mailer do
   end
 
   describe '#update_repository' do
-    it 'performs update including options' do
-      git_remote_mirror = stub_const('Gitlab::Git::RemoteMirror', spy)
-      mirror = build(:remote_mirror)
-
-      expect(mirror).to receive(:options_for_update).and_return(keep_divergent_refs: true)
-      mirror.update_repository
-
-      expect(git_remote_mirror).to have_received(:new).with(
-        mirror.project.repository.raw,
-        mirror.remote_name,
-        keep_divergent_refs: true
-      )
-      expect(git_remote_mirror).to have_received(:update)
+    shared_examples 'an update' do
+      it 'performs update including options' do
+        git_remote_mirror = stub_const('Gitlab::Git::RemoteMirror', spy)
+        mirror = build(:remote_mirror)
+
+        expect(mirror).to receive(:options_for_update).and_return(keep_divergent_refs: true)
+        mirror.update_repository(inmemory_remote: inmemory)
+
+        expect(git_remote_mirror).to have_received(:new).with(
+          mirror.project.repository.raw,
+          mirror.remote_name,
+          inmemory ? mirror.url : nil,
+          keep_divergent_refs: true
+        )
+        expect(git_remote_mirror).to have_received(:update)
+      end
+    end
+
+    context 'with inmemory remote' do
+      let(:inmemory) { true }
+
+      it_behaves_like 'an update'
+    end
+
+    context 'with on-disk remote' do
+      let(:inmemory) { false }
+
+      it_behaves_like 'an update'
     end
   end
 

spec/services/projects/update_remote_mirror_service_spec.rb

@@ -13,21 +13,36 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
 
   describe '#execute' do
     let(:retries) { 0 }
+    let(:inmemory) { true }
 
     subject(:execute!) { service.execute(remote_mirror, retries) }
 
     before do
+      stub_feature_flags(update_remote_mirror_inmemory: inmemory)
       project.repository.add_branch(project.owner, 'existing-branch', 'master')
 
       allow(remote_mirror)
         .to receive(:update_repository)
+        .with(inmemory_remote: inmemory)
         .and_return(double(divergent_refs: []))
     end
 
-    it 'ensures the remote exists' do
-      expect(remote_mirror).to receive(:ensure_remote!)
+    context 'with in-memory remote disabled' do
+      let(:inmemory) { false }
 
-      execute!
+      it 'ensures the remote exists' do
+        expect(remote_mirror).to receive(:ensure_remote!)
+
+        execute!
+      end
+    end
+
+    context 'with in-memory remote enabled' do
+      it 'does not ensure the remote exists' do
+        expect(remote_mirror).not_to receive(:ensure_remote!)
+
+        execute!
+      end
     end
 
     it 'does not fetch the remote repository' do