Merge branch '5619-geo-wiki-repository-verification-is-broken' into 'master'

Resolve "Geo: Wiki repository verification is broken"

Closes #5619

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

ee/app/services/geo/repository_verify_secondary_service.rb

@@ -2,8 +2,6 @@ module Geo
   class RepositoryVerifySecondaryService
     include Gitlab::Geo::ProjectLogHelpers
 
-    delegate :project, to: :registry
-
     def initialize(registry, type)
       @registry = registry
       @type     = type.to_sym
@@ -21,6 +19,8 @@ module Geo
 
     attr_reader :registry, :type
 
+    delegate :project, to: :registry
+
     def should_verify_checksum?
       return false if resync?
 
@@ -40,15 +40,15 @@ module Geo
     end
 
     def verify_checksum
-      checksum = project.repository.checksum
+      checksum = repository.checksum
 
       if mismatch?(checksum)
-        update_registry!(failure: "#{type.to_s.capitalize} checksum mismatch: #{repository_path}")
+        update_registry!(failure: "#{type.to_s.capitalize} checksum mismatch: #{repository.disk_path}")
       else
         update_registry!(checksum: checksum)
       end
     rescue ::Gitlab::Git::Repository::NoRepository, ::Gitlab::Git::Repository::ChecksumError, Timeout::Error => e
-      update_registry!(failure: "Error verifying #{type.to_s.capitalize} checksum: #{repository_path}", exception: e)
+      update_registry!(failure: "Error verifying #{type.to_s.capitalize} checksum: #{repository.disk_path}", exception: e)
     end
 
     def mismatch?(checksum)
@@ -62,28 +62,18 @@ module Geo
       }
 
       if failure
-        log_error(failure, exception, type: type, repository_full_path: path_to_repo)
+        log_error(failure, exception, type: type, repository_full_path: repository.path_to_repo)
       end
 
       registry.update!(attrs)
     end
 
-    def repository_path
-      case type
-      when :repository
-        registry.project.disk_path
-      when :wiki
-        registry.project.wiki.disk_path
-      end
-    end
-
-    def path_to_repo
-      case type
-      when :repository
-        project.repository.path_to_repo
-      when :wiki
-        project.wiki.repository.path_to_repo
-      end
+    def repository
+      @repository ||=
+        case type
+        when :repository then project.repository
+        when :wiki then project.wiki.repository
+        end
     end
   end
 end

ee/changelogs/unreleased/5619-geo-wiki-repository-verification-is-broken.yml

+---
+title: Geo - Fix wiki repository verification on a secondary node
+merge_request: 5315
+author:
+type: fixed

ee/spec/services/geo/repository_verify_secondary_service_spec.rb

@@ -10,12 +10,14 @@ describe Geo::RepositoryVerifySecondaryService, :geo do
   end
 
   shared_examples 'verify checksums for repositories/wikis' do |type|
+    let(:repository) { find_repository(type) }
+
     subject(:service)  { described_class.new(registry, type) }
 
     it 'does not calculate the checksum when not running on a secondary' do
       allow(Gitlab::Geo).to receive(:secondary?) { false }
 
-      expect(registry.project.repository).not_to receive(:checksum)
+      expect(repository).not_to receive(:checksum)
 
       service.execute
     end
@@ -23,7 +25,7 @@ describe Geo::RepositoryVerifySecondaryService, :geo do
     it 'does not verify the checksum if resync is needed' do
       registry.assign_attributes("resync_#{type}" => true)
 
-      expect(registry.project.repository).not_to receive(:checksum)
+      expect(repository).not_to receive(:checksum)
 
       service.execute
     end
@@ -31,7 +33,7 @@ describe Geo::RepositoryVerifySecondaryService, :geo do
     it 'does not verify the checksum if primary was never verified' do
       repository_state.assign_attributes("#{type}_verification_checksum" => nil)
 
-      expect(registry.project.repository).not_to receive(:checksum)
+      expect(repository).not_to receive(:checksum)
 
       service.execute
     end
@@ -40,24 +42,31 @@ describe Geo::RepositoryVerifySecondaryService, :geo do
       repository_state.assign_attributes("#{type}_verification_checksum" => 'my_checksum')
       registry.assign_attributes("#{type}_verification_checksum_sha" => 'my_checksum')
 
-      expect(registry.project.repository).not_to receive(:checksum)
+      expect(repository).not_to receive(:checksum)
 
       service.execute
     end
 
     it 'sets checksum when the checksum matches' do
-      expect(registry.project.repository).to receive(:checksum).and_return('my_checksum')
+      expect(repository).to receive(:checksum).and_return('my_checksum')
 
       expect { service.execute }.to change(registry, "#{type}_verification_checksum_sha")
         .from(nil).to('my_checksum')
     end
 
     it 'keeps track of failure when the checksum mismatch' do
-      expect(registry.project.repository).to receive(:checksum).and_return('other_checksum')
+      expect(repository).to receive(:checksum).and_return('other_checksum')
 
       expect { service.execute }.to change(registry, "last_#{type}_verification_failure")
         .from(nil).to(/#{Regexp.quote(type.to_s.capitalize)} checksum mismatch/)
     end
+
+    def find_repository(type)
+      case type
+      when :repository then project.repository
+      when :wiki then project.wiki.repository
+      end
+    end
   end
 
   describe '#execute' do