Merge branch 'rs-ls-remote-heads' into 'master'

Experimental remote branches via ls-remote

See merge request gitlab-org/gitaly!2086

3 files changed, 59 insertions, 6 deletions

diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index c08b395de..cabd296ec 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -12,6 +12,8 @@ const (
 	GoFetchInternalRemote = "go_fetch_internal_remote"
 	// GoUpdateHook will bypass the ruby update hook and use the go implementation of custom hooks
 	GoUpdateHook = "go_update_hook"
+	// RemoteBranchesLsRemote will use `ls-remote` for remote branches
+	RemoteBranchesLsRemote = "ruby_remote_branches_ls_remote"
 )
 
 const (
diff --git a/internal/service/remote/update_remote_mirror_test.go b/internal/service/remote/update_remote_mirror_test.go
index 885abe9e0..618f5f13c 100644
--- a/internal/service/remote/update_remote_mirror_test.go
+++ b/internal/service/remote/update_remote_mirror_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -64,6 +65,9 @@ func TestSuccessfulUpdateRemoteMirrorRequest(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
+	// Ensure this flag doesn't alter existing behavior
+	ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.RemoteBranchesLsRemote)
+
 	firstRequest := &gitalypb.UpdateRemoteMirrorRequest{
 		Repository:           testRepo,
 		RefName:              remoteName,
@@ -150,6 +154,9 @@ func TestSuccessfulUpdateRemoteMirrorRequestWithWildcards(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
+	// Ensure this flag doesn't alter existing behavior
+	ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.RemoteBranchesLsRemote)
+
 	firstRequest := &gitalypb.UpdateRemoteMirrorRequest{
 		Repository:           testRepo,
 		RefName:              remoteName,
@@ -219,6 +226,9 @@ func TestSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefs(t *testing.T)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
+	// Ensure this flag doesn't alter existing behavior
+	ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.RemoteBranchesLsRemote)
+
 	firstRequest := &gitalypb.UpdateRemoteMirrorRequest{
 		Repository:        testRepo,
 		RefName:           remoteName,
@@ -296,6 +306,9 @@ func TestFailedUpdateRemoteMirrorRequestDueToValidation(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
+			// Ensure this flag doesn't alter existing behavior
+			ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.RemoteBranchesLsRemote)
+
 			stream, err := client.UpdateRemoteMirror(ctx)
 			require.NoError(t, err)
 			require.NoError(t, stream.Send(tc.request))
diff --git a/ruby/lib/gitlab/git/repository_mirroring.rb b/ruby/lib/gitlab/git/repository_mirroring.rb
index e564a4cbd..47ecfe8d4 100644
--- a/ruby/lib/gitlab/git/repository_mirroring.rb
+++ b/ruby/lib/gitlab/git/repository_mirroring.rb
@@ -26,6 +26,23 @@ module Gitlab
           end
         end
 
+        # Run an experiment to see if ls-remote gives us the same data
+        #
+        # See https://gitlab.com/gitlab-org/gitaly/-/issues/2670
+        if feature_enabled?(:remote_branches_ls_remote)
+          ls_remote_branches = experimental_remote_branches(remote_name)
+
+          control_refs = branches.collect(&:name)
+          experiment_refs = ls_remote_branches.collect(&:name)
+
+          if control_refs != experiment_refs
+            diff = experiment_refs.difference(control_refs).map { |r| "+#{r}" }
+            diff.concat(control_refs.difference(experiment_refs).map { |r| "-#{r}" })
+
+            Rails.logger.warn("experimental_remote_branches returned differing values from control: #{diff.join(', ')}")
+          end
+        end
+
         branches
       end
 
@@ -48,10 +65,27 @@ module Gitlab
         rugged.config["remote.#{remote_name}.prune"] = true
       end
 
+      # Experimental: Get a list of remote branches via `ls-remote`
+      def experimental_remote_branches(remote, env: {})
+        list_remote_refs(remote, env: env).map do |line|
+          target, refname = line.strip.split("\t")
+
+          if target.nil? || refname.nil?
+            Rails.logger.info("Empty or invalid list of heads for remote: #{remote}")
+            break []
+          end
+
+          next unless refname.start_with?('refs/heads/')
+
+          target_commit = Gitlab::Git::Commit.find(self, target)
+          Gitlab::Git::Branch.new(self, refname, target, target_commit)
+        end.compact
+      end
+
       def remote_tags(remote, env: {})
         # Each line has this format: "dc872e9fa6963f8f03da6c8f6f264d0845d6b092\trefs/tags/v1.10.0\n"
         # We want to convert it to: [{ 'v1.10.0' => 'dc872e9fa6963f8f03da6c8f6f264d0845d6b092' }, ...]
-        list_remote_tags(remote, env: env).map do |line|
+        list_remote_refs(remote, env: env).map do |line|
           target, refname = line.strip.split("\t")
 
           # When the remote repo does not have tags.
@@ -60,6 +94,8 @@ module Gitlab
             break []
           end
 
+          next unless refname.start_with?('refs/tags/')
+
           # We're only interested in tag references
           # See: http://stackoverflow.com/questions/15472107/when-listing-git-ls-remote-why-theres-after-the-tag-name
           next if refname.end_with?('^{}')
@@ -88,19 +124,21 @@ module Gitlab
         end
       end
 
-      def list_remote_tags(remote, env:)
-        tag_list, exit_code, error = nil
-        cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} ls-remote --tags #{remote}]
+      def list_remote_refs(remote, env:)
+        ref_list, exit_code, error = nil
+
+        # List heads and tags, ignoring stuff like `refs/merge-requests` and `refs/pull`
+        cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} ls-remote --heads --tags #{remote}]
 
         Open3.popen3(env, *cmd) do |_stdin, stdout, stderr, wait_thr|
-          tag_list  = stdout.read
+          ref_list  = stdout.read
           error     = stderr.read
           exit_code = wait_thr.value.exitstatus
         end
 
         raise RemoteError, error unless exit_code.zero?
 
-        tag_list.split("\n")
+        ref_list.each_line(chomp: true)
       end
     end
   end