Merge branch 'mk/fix-geo-error-syncing-repository-does-not-exist' into 'master'

Mark empty repos as synced in Geo

See merge request gitlab-org/gitlab-ee!4757

ee/app/services/geo/base_sync_service.rb

@@ -52,8 +52,22 @@ module Geo
 
     private
 
+    def fetch_repository(redownload)
+      log_info("Trying to fetch #{type}")
+      update_registry!(started_at: DateTime.now)
+
+      if redownload
+        log_info("Redownloading #{type}")
+        fetch_geo_mirror(build_temporary_repository)
+        set_temp_repository_as_main
+      else
+        ensure_repository
+        fetch_geo_mirror(repository)
+      end
+    end
+
     def retry_count
-      registry.public_send("#{type}_retry_count") || 0 # rubocop:disable GitlabSecurity/PublicSend
+      registry.public_send("#{type}_retry_count") || -1 # rubocop:disable GitlabSecurity/PublicSend
     end
 
     def should_be_retried?
@@ -68,10 +82,6 @@ module Geo
       (RETRY_BEFORE_REDOWNLOAD..RETRY_LIMIT) === retry_count
     end
 
-    def sync_repository
-      raise NotImplementedError, 'This class should implement sync_repository method'
-    end
-
     def current_node
       ::Gitlab::Geo.current_node
     end

ee/app/services/geo/repository_sync_service.rb

@@ -5,32 +5,21 @@ module Geo
     private
 
     def sync_repository(redownload = false)
-      fetch_project_repository(redownload)
-      expire_repository_caches
-    end
-
-    def fetch_project_repository(redownload)
-      log_info('Trying to fetch project repository')
-      update_registry!(started_at: DateTime.now)
-
-      if redownload
-        log_info('Redownloading repository')
-        fetch_geo_mirror(build_temporary_repository)
-        set_temp_repository_as_main
-      else
-        project.ensure_repository
-        fetch_geo_mirror(project.repository)
-      end
+      fetch_repository(redownload)
 
       update_gitattributes
 
-      update_registry!(finished_at: DateTime.now, attrs: { last_repository_sync_failure: nil })
-      log_info('Finished repository sync',
-               update_delay_s: update_delay_in_seconds,
-               download_time_s: download_time_in_seconds)
+      mark_sync_as_successful
     rescue Gitlab::Shell::Error,
            Gitlab::Git::RepositoryMirroring::RemoteError => e
-      fail_registry!('Error syncing repository', e)
+      # In some cases repository does not exist, the only way to know about this is to parse the error text.
+      # If it does not exist we should consider it as successfully downloaded.
+      if e.message.include? Gitlab::GitAccess::ERROR_MESSAGES[:no_repo]
+        log_info('Repository is not found, marking it as successfully synced')
+        mark_sync_as_successful
+      else
+        fail_registry!('Error syncing repository', e)
+      end
     rescue Gitlab::Git::Repository::NoRepository => e
       log_info('Setting force_to_redownload flag')
       fail_registry!('Invalid repository', e, force_to_redownload_repository: true)
@@ -39,6 +28,15 @@ module Geo
       project.repository.after_create
     ensure
       clean_up_temporary_repository if redownload
+      expire_repository_caches
+    end
+
+    def mark_sync_as_successful
+      update_registry!(finished_at: DateTime.now, attrs: { last_repository_sync_failure: nil })
+
+      log_info('Finished repository sync',
+               update_delay_s: update_delay_in_seconds,
+               download_time_s: download_time_in_seconds)
     end
 
     def expire_repository_caches
@@ -54,15 +52,15 @@ module Geo
       project.repository
     end
 
+    def ensure_repository
+      project.ensure_repository
+    end
+
     # Update info/attributes file using the contents of .gitattributes file from the default branch
     def update_gitattributes
       return if project.default_branch.nil?
 
       repository.copy_gitattributes(project.default_branch)
     end
-
-    def retry_count
-      registry.public_send("#{type}_retry_count") || -1 # rubocop:disable GitlabSecurity/PublicSend
-    end
   end
 end

ee/app/services/geo/wiki_sync_service.rb

@@ -5,30 +5,16 @@ module Geo
     private
 
     def sync_repository(redownload = false)
-      fetch_wiki_repository(redownload)
-    end
-
-    def fetch_wiki_repository(redownload)
-      log_info('Fetching wiki repository')
-      update_registry!(started_at: DateTime.now)
-
-      if redownload
-        log_info('Redownloading wiki')
-        fetch_geo_mirror(build_temporary_repository)
-        set_temp_repository_as_main
-      else
-        project.wiki.ensure_repository
-        fetch_geo_mirror(project.wiki.repository)
-      end
+      fetch_repository(redownload)
 
       mark_sync_as_successful
     rescue Gitlab::Git::RepositoryMirroring::RemoteError,
            Gitlab::Shell::Error,
            ProjectWiki::CouldNotCreateWikiError => e
-      # In some cases repository does not exists, the only way to know about this is to parse the error text.
-      # If it does not exist we should consider it as successfuly downloaded.
+      # In some cases repository does not exist, the only way to know about this is to parse the error text.
+      # If it does not exist we should consider it as successfully downloaded.
       if e.message.include? Gitlab::GitAccess::ERROR_MESSAGES[:no_repo]
-        log_info('Repository is not found, marking it as successfully synced')
+        log_info('Wiki repository is not found, marking it as successfully synced')
         mark_sync_as_successful
       else
         fail_registry!('Error syncing wiki repository', e)
@@ -48,6 +34,10 @@ module Geo
       project.wiki.repository
     end
 
+    def ensure_repository
+      project.wiki.ensure_repository
+    end
+
     def mark_sync_as_successful
       update_registry!(finished_at: DateTime.now, attrs: { last_wiki_sync_failure: nil })
 
@@ -55,9 +45,5 @@ module Geo
                update_delay_s: update_delay_in_seconds,
                download_time_s: download_time_in_seconds)
     end
-
-    def retry_count
-      registry.public_send("#{type}_retry_count") || -1 # rubocop:disable GitlabSecurity/PublicSend
-    end
   end
 end

ee/changelogs/unreleased/mk-fix-geo-error-syncing-repository-does-not-exist.yml

+---
+title: Mark empty repos as synced in Geo
+merge_request: 4757
+author:
+type: fixed

ee/spec/services/geo/base_sync_service_spec.rb

@@ -5,8 +5,6 @@ describe Geo::BaseSyncService do
 
   subject { described_class.new(project) }
 
-  it_behaves_like 'geo base sync execution'
-
   describe '#lease_key' do
     it 'returns a key in the correct pattern' do
       allow(described_class).to receive(:type) { :wiki }

ee/spec/services/geo/repository_sync_service_spec.rb

@@ -115,6 +115,19 @@ describe Geo::RepositorySyncService do
       expect(Geo::ProjectRegistry.last.repository_retry_count).to eq(1)
     end
 
+    it 'marks sync as successful if no repository found' do
+      registry = create(:geo_project_registry, project: project)
+
+      allow(repository).to receive(:fetch_as_mirror)
+        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .and_raise(Gitlab::Shell::Error.new(Gitlab::GitAccess::ERROR_MESSAGES[:no_repo]))
+
+      subject.execute
+
+      expect(registry.reload.resync_repository).to be false
+      expect(registry.reload.last_repository_successful_sync_at).not_to be nil
+    end
+
     context 'tracking database' do
       it 'creates a new registry if does not exists' do
         expect { subject.execute }.to change(Geo::ProjectRegistry, :count).by(1)
@@ -205,7 +218,7 @@ describe Geo::RepositorySyncService do
       it 'tries to fetch repo' do
         create(:geo_project_registry, project: project, repository_retry_count: Geo::BaseSyncService::RETRY_BEFORE_REDOWNLOAD - 1)
 
-        expect_any_instance_of(described_class).to receive(:fetch_project_repository).with(false)
+        expect(subject).to receive(:sync_repository).with(no_args)
 
         subject.execute
       end
@@ -221,7 +234,7 @@ describe Geo::RepositorySyncService do
       it 'tries to redownload repo' do
         create(:geo_project_registry, project: project, repository_retry_count: Geo::BaseSyncService::RETRY_BEFORE_REDOWNLOAD + 1)
 
-        expect(subject).to receive(:fetch_project_repository).with(true).and_call_original
+        expect(subject).to receive(:sync_repository).with(true).and_call_original
         expect(subject.gitlab_shell).to receive(:mv_repository).exactly(2).times.and_call_original
         expect(subject.gitlab_shell).to receive(:remove_repository).exactly(3).times.and_call_original
 
@@ -242,7 +255,7 @@ describe Geo::RepositorySyncService do
           force_to_redownload_repository: true
         )
 
-        expect_any_instance_of(described_class).to receive(:fetch_project_repository).with(true)
+        expect(subject).to receive(:sync_repository).with(true)
 
         subject.execute
       end

ee/spec/support/services/base_sync_service_shared_examples.rb

@@ -4,11 +4,11 @@ shared_examples 'geo base sync execution' do
 
     context 'when can acquire exclusive lease' do
       before do
-        allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) { 12345 }
+        exclusive_lease = double(:exclusive_lease, try_obtain: 12345)
+        expect(subject).to receive(:exclusive_lease).and_return(exclusive_lease)
       end
 
       it 'executes the synchronization' do
-        subject.class.type ||= :wiki
         expect(subject).to receive(:sync_repository)
 
         subject.execute
@@ -17,7 +17,8 @@ shared_examples 'geo base sync execution' do
 
     context 'when exclusive lease is not acquired' do
       before do
-        allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) { nil }
+        exclusive_lease = double(:exclusive_lease, try_obtain: nil)
+        expect(subject).to receive(:exclusive_lease).and_return(exclusive_lease)
       end
 
       it 'is does not execute synchronization' do