Merge branch 'fix-geo-repository-sync-service' into 'master'

Geo - Fix RepositorySyncService when cannot obtain a lease to sync repository

See merge request !2097

app/services/geo/repository_sync_service.rb

@@ -66,7 +66,7 @@ module Geo
       log('Trying to obtain lease to sync repository')
       repository_lease = Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT).try_obtain
 
-      if repository_lease.nil?
+      unless repository_lease
         log('Could not obtain lease to sync repository')
         return
       end

changelogs/unreleased-ee/fix-geo-repository-sync-service.yml

+---
+title: Geo - Fix RepositorySyncService when cannot obtain a lease to sync a repository
+merge_request:
+author:

spec/services/geo/repository_sync_service_spec.rb

@@ -2,9 +2,19 @@ require 'spec_helper'
 
 describe Geo::RepositorySyncService, services: true do
   let!(:primary) { create(:geo_node, :primary, host: 'primary-geo-node') }
+  let(:lease) { double(try_obtain: true) }
 
   subject { described_class.new(project.id) }
 
+  before do
+    allow(Gitlab::ExclusiveLease).to receive(:new)
+      .with(subject.__send__(:lease_key), anything)
+      .and_return(lease)
+
+    allow_any_instance_of(Repository).to receive(:fetch_geo_mirror)
+      .and_return(true)
+  end
+
   describe '#execute' do
     context 'when repository is empty' do
       let(:project) { create(:project_empty_repo) }
@@ -22,8 +32,6 @@ describe Geo::RepositorySyncService, services: true do
       end
 
       it 'expires repository caches' do
-        allow_any_instance_of(Repository).to receive(:fetch_geo_mirror) { true }
-
         expect_any_instance_of(Repository).to receive(:expire_all_method_caches).once
         expect_any_instance_of(Repository).to receive(:expire_branch_cache).once
         expect_any_instance_of(Repository).to receive(:expire_content_cache).once
@@ -38,14 +46,20 @@ describe Geo::RepositorySyncService, services: true do
         subject.execute
       end
 
+      it 'does not fetch project repositories if cannot obtain a lease' do
+        allow(lease).to receive(:try_obtain) { false }
+
+        expect_any_instance_of(Repository).not_to receive(:fetch_geo_mirror)
+
+        subject.execute
+      end
+
       context 'tracking database' do
         it 'tracks repository sync' do
           expect { subject.execute }.to change(Geo::ProjectRegistry, :count).by(1)
         end
 
         it 'stores last_repository_successful_sync_at when succeed' do
-          allow_any_instance_of(Repository).to receive(:fetch_geo_mirror) { true }
-
           subject.execute
 
           registry = Geo::ProjectRegistry.find_by(project_id: project.id)
@@ -86,8 +100,6 @@ describe Geo::RepositorySyncService, services: true do
         end
 
         it 'stores last_repository_successful_sync_at when succeed' do
-          allow_any_instance_of(Repository).to receive(:fetch_geo_mirror) { true }
-
           subject.execute
 
           registry = Geo::ProjectRegistry.find_by(project_id: project.id)
@@ -137,8 +149,6 @@ describe Geo::RepositorySyncService, services: true do
         end
 
         it 'updates registry when succeed' do
-          allow_any_instance_of(Repository).to receive(:fetch_geo_mirror) { true }
-
           subject.execute
 
           registry.reload
@@ -184,10 +194,6 @@ describe Geo::RepositorySyncService, services: true do
       end
 
       context 'tracking database' do
-        before do
-          allow_any_instance_of(Repository).to receive(:fetch_geo_mirror) { true }
-        end
-
         it 'does not create a new registry' do
           expect { subject.execute }.not_to change(Geo::ProjectRegistry, :count)
         end