Merge branch...

Merge branch '7211-geo-synchronization-does-not-seem-to-replicate-default-branch-all-the-time' into 'master'

Geo - Synchronize the default branch in secondary nodes

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

GITALY_SERVER_VERSION

-0.117.2
+0.119.0

Gemfile

@@ -440,7 +440,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC client
-gem 'gitaly-proto', '~> 0.113.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 0.117.0', require: 'gitaly'
 gem 'grpc', '~> 1.11.0'
 
 # Locked until https://github.com/google/protobuf/issues/4210 is closed

Gemfile.lock

@@ -300,7 +300,7 @@ GEM
       gettext_i18n_rails (>= 0.7.1)
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
-    gitaly-proto (0.113.0)
+    gitaly-proto (0.117.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.10)
     github-linguist (5.3.3)
@@ -1072,7 +1072,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly-proto (~> 0.113.0)
+  gitaly-proto (~> 0.117.0)
   github-linguist (~> 5.3.3)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-gollum-lib (~> 4.2)

Gemfile.rails5.lock

@@ -303,7 +303,7 @@ GEM
       gettext_i18n_rails (>= 0.7.1)
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
-    gitaly-proto (0.113.0)
+    gitaly-proto (0.117.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.10)
     github-linguist (5.3.3)
@@ -1081,7 +1081,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly-proto (~> 0.113.0)
+  gitaly-proto (~> 0.117.0)
   github-linguist (~> 5.3.3)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-gollum-lib (~> 4.2)

ee/app/models/ee/repository.rb

@@ -10,7 +10,7 @@ module EE
     MIRROR_REMOTE = "upstream".freeze
 
     included do
-      delegate :checksum, to: :raw_repository
+      delegate :checksum, :find_remote_root_ref, to: :raw_repository
     end
 
     # Transiently sets a configuration variable
@@ -78,6 +78,13 @@ module EE
       log_geo_updated_event
     end
 
+    override :after_change_head
+    def after_change_head
+      super
+    ensure
+      log_geo_updated_event
+    end
+
     def log_geo_updated_event
       return unless ::Gitlab::Geo.primary?
 

ee/app/services/geo/repository_sync_service.rb

@@ -6,9 +6,7 @@ module Geo
 
     def sync_repository
       fetch_repository
-
-      update_gitattributes
-
+      update_root_ref
       mark_sync_as_successful
     rescue Gitlab::Shell::Error => e
       # In some cases repository does not exist, the only way to know about this is to parse the error text.
@@ -47,11 +45,10 @@ module Geo
       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)
+    # Update the default branch querying the remote to determine its HEAD
+    def update_root_ref
+      root_ref = repository.find_remote_root_ref(GEO_REMOTE_NAME)
+      project.change_head(root_ref) if root_ref.present? && root_ref != project.default_branch
     end
 
     def schedule_repack

ee/changelogs/unreleased/7211-geo-synchronization-does-not-seem-to-replicate-default-branch-all-the-time.yml

+---
+title: Geo - Synchronize the default branch in secondary nodes
+merge_request: 7218
+author:
+type: fixed

ee/spec/models/repository_spec.rb

@@ -3,8 +3,12 @@ require 'spec_helper'
 describe Repository do
   include RepoHelpers
   include ::EE::GeoHelpers
+
   TestBlob = Struct.new(:path)
 
+  set(:primary_node)   { create(:geo_node, :primary) }
+  set(:secondary_node) { create(:geo_node) }
+
   let(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
 
@@ -116,8 +120,6 @@ describe Repository do
   end
 
   describe '#keep_around' do
-    set(:primary_node)   { create(:geo_node, :primary) }
-    set(:secondary_node) { create(:geo_node) }
     let(:sha) { sample_commit.id }
 
     context 'on a Geo primary' do
@@ -174,4 +176,20 @@ describe Repository do
       repository.code_owners_blob
     end
   end
+
+  describe '#after_change_head' do
+    it 'creates a RepositoryUpdatedEvent on a Geo primary' do
+      stub_current_geo_node(primary_node)
+
+      expect { repository.after_change_head }
+        .to change { ::Geo::RepositoryUpdatedEvent.count }.by(1)
+    end
+
+    it 'does not create a RepositoryUpdatedEvent on a Geo secondary' do
+      stub_current_geo_node(secondary_node)
+
+      expect { repository.after_change_head }
+        .not_to change { ::Geo::RepositoryUpdatedEvent.count }
+    end
+  end
 end

ee/spec/services/geo/repository_sync_service_spec.rb

@@ -31,6 +31,11 @@ describe Geo::RepositorySyncService do
 
       allow_any_instance_of(Repository).to receive(:fetch_as_mirror)
         .and_return(true)
+
+      allow_any_instance_of(Repository)
+        .to receive(:find_remote_root_ref)
+        .with('geo')
+        .and_return('master')
     end
 
     it 'fetches project repository with JWT credentials' do
@@ -210,10 +215,46 @@ describe Geo::RepositorySyncService do
         context 'with non empty repositories' do
           let(:project) { create(:project, :repository) }
 
-          it 'syncs gitattributes to info/attributes' do
-            expect(repository).to receive(:copy_gitattributes)
+          context 'when when HEAD change' do
+            before do
+              allow(project.repository)
+                .to receive(:find_remote_root_ref)
+                .with('geo')
+                .and_return('feature')
+            end
+
+            it 'syncs gitattributes to info/attributes' do
+              expect(repository).to receive(:copy_gitattributes)
+
+              subject.execute
+            end
+
+            it 'updates the default branch' do
+              expect(project).to receive(:change_head).with('feature').once
+
+              subject.execute
+            end
+          end
+
+          context 'when HEAD does not change' do
+            before do
+              allow(project.repository)
+                .to receive(:find_remote_root_ref)
+                .with('geo')
+                .and_return(project.default_branch)
+            end
+
+            it 'does not sync gitattributes to info/attributes' do
+              expect(repository).not_to receive(:copy_gitattributes)
+
+              subject.execute
+            end
+
+            it 'does not update the default branch' do
+              expect(project).not_to receive(:change_head)
 
-            subject.execute
+              subject.execute
+            end
           end
         end
       end

lib/gitlab/git/repository.rb

@@ -666,6 +666,14 @@ module Gitlab
         end
       end
 
+      def find_remote_root_ref(remote_name)
+        return unless remote_name.present?
+
+        wrapped_gitaly_errors do
+          gitaly_remote_client.find_remote_root_ref(remote_name)
+        end
+      end
+
       AUTOCRLF_VALUES = {
         "true" => true,
         "false" => false,

lib/gitlab/gitaly_client/remote_service.rb

@@ -52,6 +52,18 @@ module Gitlab
         response.result
       end
 
+      def find_remote_root_ref(remote_name)
+        request = Gitaly::FindRemoteRootRefRequest.new(
+          repository: @gitaly_repo,
+          remote: remote_name
+        )
+
+        response = GitalyClient.call(@storage, :remote_service,
+                                     :find_remote_root_ref, request)
+
+        response.ref.presence
+      end
+
       def update_remote_mirror(ref_name, only_branches_matching)
         req_enum = Enumerator.new do |y|
           y.yield Gitaly::UpdateRemoteMirrorRequest.new(

spec/lib/gitlab/git/repository_spec.rb

@@ -583,6 +583,33 @@ describe Gitlab::Git::Repository, :seed_helper do
     end
   end
 
+  describe '#find_remote_root_ref' do
+    it 'gets the remote root ref from GitalyClient' do
+      expect_any_instance_of(Gitlab::GitalyClient::RemoteService)
+        .to receive(:find_remote_root_ref).and_call_original
+
+      expect(repository.find_remote_root_ref('origin')).to eq 'master'
+    end
+
+    it 'returns nil when remote name is nil' do
+      expect_any_instance_of(Gitlab::GitalyClient::RemoteService)
+        .not_to receive(:find_remote_root_ref)
+
+      expect(repository.find_remote_root_ref(nil)).to be_nil
+    end
+
+    it 'returns nil when remote name is empty' do
+      expect_any_instance_of(Gitlab::GitalyClient::RemoteService)
+        .not_to receive(:find_remote_root_ref)
+
+      expect(repository.find_remote_root_ref('')).to be_nil
+    end
+
+    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RemoteService, :find_remote_root_ref do
+      subject { repository.find_remote_root_ref('origin') }
+    end
+  end
+
   describe "#log" do
     shared_examples 'repository log' do
       let(:commit_with_old_name) do

spec/lib/gitlab/gitaly_client/remote_service_spec.rb

@@ -45,6 +45,17 @@ describe Gitlab::GitalyClient::RemoteService do
     end
   end
 
+  describe '#find_remote_root_ref' do
+    it 'sends an find_remote_root_ref message and returns the root ref' do
+      expect_any_instance_of(Gitaly::RemoteService::Stub)
+        .to receive(:find_remote_root_ref)
+        .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
+        .and_return(double(ref: 'master'))
+
+      expect(client.find_remote_root_ref('origin')).to eq 'master'
+    end
+  end
+
   describe '#update_remote_mirror' do
     let(:ref_name) { 'remote_mirror_1' }
     let(:only_branches_matching) { ['my-branch', 'master'] }