Add feature flag: geo_use_clone_on_first_sync

config/feature_flags/ops/geo_use_clone_on_first_sync.yml

+---
+name: geo_use_clone_on_first_sync
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77143
+rollout_issue_url:
+milestone: '14.9'
+type: ops
+group: group::geo
+default_enabled: false

ee/app/services/geo/framework_repository_sync_service.rb

@@ -80,13 +80,18 @@ module Geo
       elsif repository.exists?
         fetch_geo_mirror
       else
+        if Feature.enabled?('geo_use_clone_on_first_sync', type: :ops)
           clone_geo_mirror
 
+          @new_repository = true
+        else
           ensure_repository
           # Because we ensure a repository exists by this point, we need to
           # mark it as new, even if fetching the mirror fails, we should run
           # housekeeping to enable object deduplication to run
           @new_repository = true
+          fetch_geo_mirror
+        end
       end
 
       update_root_ref
@@ -103,8 +108,13 @@ module Geo
 
       log_info("Attempting to fetch repository via git")
 
+      if Feature.enabled?('geo_use_clone_on_first_sync', type: :ops)
         clone_geo_mirror(target_repository: temp_repo)
         temp_repo.create_repository unless temp_repo.exists?
+      else
+        temp_repo.create_repository
+        fetch_geo_mirror(target_repository: temp_repo)
+      end
 
       set_temp_repository_as_main
     ensure
@@ -117,9 +127,10 @@ module Geo
 
     # Updates an existing repository using JWT authentication mechanism
     #
-    def fetch_geo_mirror
+    # @param [Repository] target_repository specify a different temporary repository (default: current repository)
+    def fetch_geo_mirror(target_repository: repository)
       # Fetch the repository, using a JWT header for authentication
-      repository.fetch_as_mirror(replicator.remote_url, forced: true, http_authorization_header: replicator.jwt_authentication_header)
+      target_repository.fetch_as_mirror(replicator.remote_url, forced: true, http_authorization_header: replicator.jwt_authentication_header)
     end
 
     # Clone a Geo repository using JWT authentication mechanism

ee/app/services/geo/repository_base_sync_service.rb

@@ -63,13 +63,19 @@ module Geo
       elsif repository.exists?
         fetch_geo_mirror
       else
+        if Feature.enabled?('geo_use_clone_on_first_sync', type: :ops)
           clone_geo_mirror
-        repository.expire_status_cache # after_create
+
+          @new_repository = true
+        else
+          ensure_repository
 
           # Because we ensure a repository exists by this point, we need to
           # mark it as new, even if fetching the mirror fails, we should run
           # housekeeping to enable object deduplication to run
           @new_repository = true
+          fetch_geo_mirror
+        end
       end
 
       update_root_ref
@@ -90,7 +96,13 @@ module Geo
 
       log_info("Attempting to fetch repository via git")
 
+      if Feature.enabled?('geo_use_clone_on_first_sync', type: :ops)
         clone_geo_mirror(target_repository: temp_repo)
+      else
+        # `git fetch` needs an empty bare repository to fetch into
+        temp_repo.create_repository
+        fetch_geo_mirror(target_repository: temp_repo)
+      end
 
       set_temp_repository_as_main
     ensure
@@ -103,9 +115,10 @@ module Geo
 
     # Updates an existing repository using JWT authentication mechanism
     #
-    def fetch_geo_mirror
+    # @param [Repository] target_repository specify a different temporary repository (default: current repository)
+    def fetch_geo_mirror(target_repository: repository)
       # Fetch the repository, using a JWT header for authentication
-      repository.fetch_as_mirror(remote_url,
+      target_repository.fetch_as_mirror(remote_url,
                                        forced: true,
                                        http_authorization_header: jwt_authentication_header)
     end

ee/spec/services/geo/design_repository_sync_service_spec.rb

@@ -13,8 +13,10 @@ RSpec.describe Geo::DesignRepositorySyncService do
   let(:project) { create(:project_empty_repo, :design_repo, namespace: create(:namespace, owner: user)) }
 
   let(:repository) { project.design_repository }
+  let(:temp_repo) { subject.send(:temp_repo) }
   let(:lease_key) { "geo_sync_service:design:#{project.id}" }
   let(:lease_uuid) { 'uuid' }
+  let(:url_to_repo) { "#{primary.url}#{project.full_path}.design.git" }
 
   subject { described_class.new(project) }
 
@@ -26,8 +28,6 @@ RSpec.describe Geo::DesignRepositorySyncService do
   it_behaves_like 'geo base sync fetch'
 
   describe '#execute' do
-    let(:url_to_repo) { "#{primary.url}#{project.full_path}.design.git" }
-
     before do
       # update_highest_role uses exclusive key too:
       allow(Gitlab::ExclusiveLease).to receive(:new).and_call_original
@@ -172,4 +172,53 @@ RSpec.describe Geo::DesignRepositorySyncService do
       subject.send(:mark_sync_as_successful)
     end
   end
+
+  context 'when the repository is redownloaded' do
+    context 'with geo_use_clone_on_first_sync flag disabled' do
+      before do
+        stub_feature_flags(geo_use_clone_on_first_sync: false)
+        allow(subject).to receive(:redownload?).and_return(true)
+      end
+
+      it 'creates a new repository and fetches with JWT credentials' do
+        expect(temp_repo).to receive(:create_repository)
+        expect(temp_repo).to receive(:fetch_as_mirror)
+                               .with(url_to_repo, forced: true, http_authorization_header: anything)
+                               .once
+        expect(subject).to receive(:set_temp_repository_as_main)
+
+        subject.execute
+      end
+
+      it 'cleans temporary repo after redownload' do
+        expect(subject).to receive(:fetch_geo_mirror).with(target_repository: temp_repo)
+        expect(subject).to receive(:clean_up_temporary_repository).twice.and_call_original
+
+        subject.execute
+      end
+    end
+
+    context 'with geo_use_clone_on_first_sync flag enabled' do
+      before do
+        stub_feature_flags(geo_use_clone_on_first_sync: true)
+        allow(subject).to receive(:redownload?).and_return(true)
+      end
+
+      it 'clones a new repository with JWT credentials' do
+        expect(temp_repo).to receive(:clone_as_mirror)
+                               .with(url_to_repo, http_authorization_header: anything)
+                               .once
+        expect(subject).to receive(:set_temp_repository_as_main)
+
+        subject.execute
+      end
+
+      it 'cleans temporary repo after redownload' do
+        expect(subject).to receive(:clone_geo_mirror).with(target_repository: temp_repo)
+        expect(subject).to receive(:clean_up_temporary_repository).twice.and_call_original
+
+        subject.execute
+      end
+    end
+  end
 end

ee/spec/services/geo/framework_repository_sync_service_spec.rb

@@ -150,8 +150,13 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
     end
 
     context 'with a never synced repository' do
-      it 'clones repository with JWT credentials' do
+      context 'with geo_use_clone_on_first_sync flag enabled' do
+        before do
+          stub_feature_flags(geo_use_clone_on_first_sync: true)
           allow(repository).to receive(:exists?) { false }
+        end
+
+        it 'clones repository with JWT credentials' do
           expect(repository).to receive(:clone_as_mirror)
                                   .with(url_to_repo, http_authorization_header: anything)
                                   .once
@@ -160,6 +165,22 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
         end
       end
 
+      context 'with geo_use_clone_on_first_sync flag disabled' do
+        before do
+          stub_feature_flags(geo_use_clone_on_first_sync: false)
+          allow(repository).to receive(:exists?) { false }
+        end
+
+        it 'fetches repository with JWT credentials' do
+          expect(repository).to receive(:fetch_as_mirror)
+                                  .with(url_to_repo, forced: true, http_authorization_header: anything)
+                                  .once
+
+          subject.execute
+        end
+      end
+    end
+
     context 'tracking database' do
       context 'temporary repositories' do
         include_examples 'cleans temporary repositories'
@@ -253,6 +274,45 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
           subject.execute
         end
 
+        it 'tries to redownload when should_be_redownloaded' do
+          allow(subject).to receive(:should_be_redownloaded?) { true }
+
+          expect(subject).to receive(:redownload_repository)
+
+          subject.execute
+        end
+
+        it 'successfully redownloads the repository even if the retry time exceeds max value' do
+          timestamp = Time.current.utc
+          registry.update!(
+            retry_count: described_class::RETRIES_BEFORE_REDOWNLOAD + 2000,
+            retry_at: timestamp,
+            force_to_redownload: true
+          )
+
+          subject.execute
+
+          # The repository should be redownloaded and cleared without errors. If
+          # the timestamp were not capped, we would have seen a "timestamp out
+          # of range" in the first update to the registry.
+          registry.reload
+          expect(registry.retry_at).to be_nil
+        end
+      end
+
+      context 'no repository' do
+        it 'does not raise an error' do
+          registry.update!(force_to_redownload: true)
+
+          expect(repository).to receive(:expire_exists_cache).twice.and_call_original
+          expect(subject).not_to receive(:fail_registry_sync!)
+
+          subject.execute
+        end
+      end
+    end
+
+    context 'when repository is redownloaded' do
       it 'sets the redownload flag to false after success' do
         registry.update!(retry_count: described_class::RETRIES_BEFORE_REDOWNLOAD + 1, force_to_redownload: true)
 
@@ -278,54 +338,50 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
         expect(File.directory?(repo_path)).to be true
       end
 
-        it 'tries to redownload repo when force_redownload flag is set' do
-          registry.update!(
-            retry_count: described_class::RETRIES_BEFORE_REDOWNLOAD - 1,
-            force_to_redownload: true
-          )
+      context 'with geo_use_clone_on_first_sync flag disabled' do
+        before do
+          stub_feature_flags(geo_use_clone_on_first_sync: false)
+          allow(subject).to receive(:should_be_redownloaded?) { true }
+        end
 
-          expect(subject).to receive(:sync_repository)
+        it 'creates a new repository and fetches with JWT credentials' do
+          expect(temp_repo).to receive(:create_repository)
+          expect(temp_repo).to receive(:fetch_as_mirror)
+                                 .with(url_to_repo, forced: true, http_authorization_header: anything)
+                                 .once
+          expect(subject).to receive(:set_temp_repository_as_main)
 
           subject.execute
         end
 
         it 'cleans temporary repo after redownload' do
-          registry.update!(
-            retry_count: described_class::RETRIES_BEFORE_REDOWNLOAD - 1,
-            force_to_redownload: true
-          )
-
-          expect(subject).to receive(:clone_geo_mirror).with(target_repository: temp_repo)
+          expect(subject).to receive(:fetch_geo_mirror).with(target_repository: temp_repo)
           expect(subject).to receive(:clean_up_temporary_repository).twice.and_call_original
           expect(subject.gitlab_shell).to receive(:repository_exists?).twice.with(replicator.model_record.repository_storage, /.git$/)
 
           subject.execute
         end
+      end
 
-        it 'successfully redownloads the repository even if the retry time exceeds max value' do
-          timestamp = Time.current.utc
-          registry.update!(
-            retry_count: described_class::RETRIES_BEFORE_REDOWNLOAD + 2000,
-            retry_at: timestamp,
-            force_to_redownload: true
-          )
+      context 'with geo_use_clone_on_first_sync flag enabled' do
+        before do
+          stub_feature_flags(geo_use_clone_on_first_sync: true)
+          allow(subject).to receive(:should_be_redownloaded?) { true }
+        end
 
-          subject.execute
+        it 'clones a new repository with JWT credentials' do
+          expect(temp_repo).to receive(:clone_as_mirror)
+                                 .with(url_to_repo, http_authorization_header: anything)
+                                 .once
+          expect(subject).to receive(:set_temp_repository_as_main)
 
-          # The repository should be redownloaded and cleared without errors. If
-          # the timestamp were not capped, we would have seen a "timestamp out
-          # of range" in the first update to the registry.
-          registry.reload
-          expect(registry.retry_at).to be_nil
-        end
+          subject.execute
         end
 
-      context 'no repository' do
-        it 'does not raise an error' do
-          registry.update!(force_to_redownload: true)
-
-          expect(repository).to receive(:expire_exists_cache).twice.and_call_original
-          expect(subject).not_to receive(:fail_registry_sync!)
+        it 'cleans temporary repo after redownload' do
+          expect(subject).to receive(:clone_geo_mirror).with(target_repository: temp_repo)
+          expect(subject).to receive(:clean_up_temporary_repository).twice.and_call_original
+          expect(subject.gitlab_shell).to receive(:repository_exists?).twice.with(replicator.model_record.repository_storage, /.git$/)
 
           subject.execute
         end

ee/spec/services/geo/repository_sync_service_spec.rb

@@ -14,6 +14,7 @@ RSpec.describe Geo::RepositorySyncService, :geo do
   let(:temp_repo) { subject.send(:temp_repo) }
   let(:lease_key) { "geo_sync_service:repository:#{project.id}" }
   let(:lease_uuid) { 'uuid'}
+  let(:url_to_repo) { "#{primary.url}#{project.full_path}.git" }
 
   subject { described_class.new(project) }
 
@@ -26,8 +27,6 @@ RSpec.describe Geo::RepositorySyncService, :geo do
   it_behaves_like 'reschedules sync due to race condition instead of waiting for backfill'
 
   describe '#execute' do
-    let(:url_to_repo) { "#{primary.url}#{project.full_path}.git" }
-
     before do
       stub_exclusive_lease(lease_key, lease_uuid)
       stub_exclusive_lease("geo_project_housekeeping:#{project.id}")
@@ -416,13 +415,25 @@ RSpec.describe Geo::RepositorySyncService, :geo do
   end
 
   context 'when the repository is redownloaded' do
+    context 'with geo_use_clone_on_first_sync flag disabled' do
       before do
+        stub_feature_flags(geo_use_clone_on_first_sync: false)
         allow(subject).to receive(:redownload?).and_return(true)
-      allow(subject).to receive(:redownload_repository).and_return(nil)
       end
 
-    it "indicates the repository is new" do
-      expect(Geo::ProjectHousekeepingService).to receive(:new).with(project, new_repository: true).and_call_original
+      it 'creates a new repository and fetches with JWT credentials' do
+        expect(temp_repo).to receive(:create_repository)
+        expect(temp_repo).to receive(:fetch_as_mirror)
+                               .with(url_to_repo, forced: true, http_authorization_header: anything)
+                               .once
+        expect(subject).to receive(:set_temp_repository_as_main)
+
+        subject.execute
+      end
+
+      it 'cleans temporary repo after redownload' do
+        expect(subject).to receive(:fetch_geo_mirror).with(target_repository: temp_repo)
+        expect(subject).to receive(:clean_up_temporary_repository).twice.and_call_original
 
         subject.execute
       end
@@ -435,24 +446,77 @@ RSpec.describe Geo::RepositorySyncService, :geo do
       end
     end
 
+    context 'with geo_use_clone_on_first_sync flag enabled' do
+      before do
+        stub_feature_flags(geo_use_clone_on_first_sync: true)
+        allow(subject).to receive(:redownload?).and_return(true)
+      end
+
+      it 'clones a new repository with JWT credentials' do
+        expect(temp_repo).to receive(:clone_as_mirror)
+                               .with(url_to_repo, http_authorization_header: anything)
+                               .once
+        expect(subject).to receive(:set_temp_repository_as_main)
+        expect(Geo::ProjectHousekeepingService).to receive(:new).with(project, new_repository: true).and_call_original
+
+        subject.execute
+      end
+
+      it 'cleans temporary repo after redownload' do
+        expect(subject).to receive(:clone_geo_mirror).with(target_repository: temp_repo)
+        expect(subject).to receive(:clean_up_temporary_repository).twice.and_call_original
+
+        subject.execute
+      end
+    end
+  end
+
   context 'when repository did not exist' do
     before do
       allow(repository).to receive(:exists?).and_return(false)
+      allow(subject).to receive(:fetch_geo_mirror).and_return(nil)
       allow(subject).to receive(:clone_geo_mirror).and_return(nil)
     end
 
-    it "indicates the repository is new" do
+    context 'with geo_use_clone_on_first_sync flag enabled' do
+      before do
+        stub_feature_flags(geo_use_clone_on_first_sync: true)
+      end
+
+      it "dont indicates the repository is new when there were errors" do
+        allow(subject).to receive(:clone_geo_mirror).and_raise(Gitlab::Shell::Error)
+
+        expect(Geo::ProjectHousekeepingService).to receive(:new).with(project, new_repository: false).and_call_original
+
+        subject.execute
+      end
+
+      it "indicates the repository is new if successful" do
         expect(Geo::ProjectHousekeepingService).to receive(:new).with(project, new_repository: true).and_call_original
 
         subject.execute
       end
+    end
+
+    context 'with geo_use_clone_on_first_sync flag disabled' do
+      before do
+        stub_feature_flags(geo_use_clone_on_first_sync: false)
+      end
 
       it "indicates the repository is new when there were errors" do
         allow(subject).to receive(:fetch_geo_mirror).and_raise(Gitlab::Shell::Error)
+
         expect(Geo::ProjectHousekeepingService).to receive(:new).with(project, new_repository: true).and_call_original
 
         subject.execute
       end
+
+      it "indicates the repository is new if successful" do
+        expect(Geo::ProjectHousekeepingService).to receive(:new).with(project, new_repository: true).and_call_original
+
+        subject.execute
+      end
+    end
   end
 
   context 'when repository already existed' do

ee/spec/services/geo/wiki_sync_service_spec.rb

@@ -14,6 +14,7 @@ RSpec.describe Geo::WikiSyncService, :geo do
   let(:temp_repo) { subject.send(:temp_repo) }
   let(:lease_key) { "geo_sync_service:wiki:#{project.id}" }
   let(:lease_uuid) { 'uuid'}
+  let(:url_to_repo) { "#{primary.url}#{project.full_path}.wiki.git" }
 
   subject { described_class.new(project) }
 
@@ -26,8 +27,6 @@ RSpec.describe Geo::WikiSyncService, :geo do
   it_behaves_like 'reschedules sync due to race condition instead of waiting for backfill'
 
   describe '#execute' do
-    let(:url_to_repo) { "#{primary.url}#{project.full_path}.wiki.git" }
-
     before do
       stub_exclusive_lease(lease_key, lease_uuid)
 
@@ -251,4 +250,53 @@ RSpec.describe Geo::WikiSyncService, :geo do
       end
     end
   end
+
+  context 'when the repository is redownloaded' do
+    context 'with geo_use_clone_on_first_sync flag disabled' do
+      before do
+        stub_feature_flags(geo_use_clone_on_first_sync: false)
+        allow(subject).to receive(:redownload?).and_return(true)
+      end
+
+      it 'creates a new repository and fetches with JWT credentials' do
+        expect(temp_repo).to receive(:create_repository)
+        expect(temp_repo).to receive(:fetch_as_mirror)
+                               .with(url_to_repo, forced: true, http_authorization_header: anything)
+                               .once
+        expect(subject).to receive(:set_temp_repository_as_main)
+
+        subject.execute
+      end
+
+      it 'cleans temporary repo after redownload' do
+        expect(subject).to receive(:fetch_geo_mirror).with(target_repository: temp_repo)
+        expect(subject).to receive(:clean_up_temporary_repository).twice.and_call_original
+
+        subject.execute
+      end
+    end
+
+    context 'with geo_use_clone_on_first_sync flag enabled' do
+      before do
+        stub_feature_flags(geo_use_clone_on_first_sync: true)
+        allow(subject).to receive(:redownload?).and_return(true)
+      end
+
+      it 'clones a new repository with JWT credentials' do
+        expect(temp_repo).to receive(:clone_as_mirror)
+                               .with(url_to_repo, http_authorization_header: anything)
+                               .once
+        expect(subject).to receive(:set_temp_repository_as_main)
+
+        subject.execute
+      end
+
+      it 'cleans temporary repo after redownload' do
+        expect(subject).to receive(:clone_geo_mirror).with(target_repository: temp_repo)
+        expect(subject).to receive(:clean_up_temporary_repository).twice.and_call_original
+
+        subject.execute
+      end
+    end
+  end
 end