Merge branch 'remove_flag_go_fetch_internal_remote' into 'master'

Remove feature flag go_fetch_internal_remote

See merge request gitlab-org/gitaly!2203

4 files changed, 44 insertions, 98 deletions

diff --git a/changelogs/unreleased/remove_flag_go_fetch_internal_remote.yml b/changelogs/unreleased/remove_flag_go_fetch_internal_remote.yml
new file mode 100644
index 000000000..c0a59a379
--- /dev/null
+++ b/changelogs/unreleased/remove_flag_go_fetch_internal_remote.yml
@@ -0,0 +1,5 @@
+---
+title: Remove feature flag go_fetch_internal_remote
+merge_request: 2203
+author:
+type: changed
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index 3a3bc04b5..6d3eee60f 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -12,8 +12,6 @@ const (
 	// note: there doesn't need to be a separate gitaly ruby feature flag name. The same featureflag string can be used for both the go code
 	// and being passed into gitaly-ruby
 	GitalyRubyCallHookRPC = "call_hook_rpc"
-	// GoFetchInternalRemote enables a go implementation of FetchInternalRemote
-	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
diff --git a/internal/service/remote/fetch_internal_remote.go b/internal/service/remote/fetch_internal_remote.go
index 7f3512c3a..6365e3cec 100644
--- a/internal/service/remote/fetch_internal_remote.go
+++ b/internal/service/remote/fetch_internal_remote.go
@@ -8,8 +8,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/gitalyssh"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
-	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -26,10 +24,6 @@ func (s *server) FetchInternalRemote(ctx context.Context, req *gitalypb.FetchInt
 		return nil, status.Errorf(codes.InvalidArgument, "FetchInternalRemote: %v", err)
 	}
 
-	if featureflag.IsDisabled(ctx, featureflag.GoFetchInternalRemote) {
-		return s.rubyFetchInternalRemote(ctx, req)
-	}
-
 	env, err := gitalyssh.UploadPackEnv(ctx, &gitalypb.SSHUploadPackRequest{Repository: req.RemoteRepository})
 	if err != nil {
 		return nil, err
@@ -60,20 +54,6 @@ func (s *server) FetchInternalRemote(ctx context.Context, req *gitalypb.FetchInt
 	return &gitalypb.FetchInternalRemoteResponse{Result: true}, nil
 }
 
-func (s *server) rubyFetchInternalRemote(ctx context.Context, req *gitalypb.FetchInternalRemoteRequest) (*gitalypb.FetchInternalRemoteResponse, error) {
-	client, err := s.ruby.RemoteServiceClient(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	clientCtx, err := rubyserver.SetHeaders(ctx, req.GetRepository())
-	if err != nil {
-		return nil, err
-	}
-
-	return client.FetchInternalRemote(clientCtx, req)
-}
-
 func validateFetchInternalRemoteRequest(req *gitalypb.FetchInternalRemoteRequest) error {
 	if req.GetRepository() == nil {
 		return fmt.Errorf("empty Repository")
diff --git a/internal/service/remote/fetch_internal_remote_test.go b/internal/service/remote/fetch_internal_remote_test.go
index 73dff1c50..3dc4079ee 100644
--- a/internal/service/remote/fetch_internal_remote_test.go
+++ b/internal/service/remote/fetch_internal_remote_test.go
@@ -7,7 +7,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	serverPkg "gitlab.com/gitlab-org/gitaly/internal/server"
 	"gitlab.com/gitlab-org/gitaly/internal/service/remote"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
@@ -27,45 +26,27 @@ func TestSuccessfulFetchInternalRemote(t *testing.T) {
 	remoteRepo, remoteRepoPath, remoteCleanupFn := testhelper.NewTestRepo(t)
 	defer remoteCleanupFn()
 
-	for _, tc := range []struct {
-		CaseName     string
-		FeatureFlags []string
-	}{
-		{
-			CaseName: "ruby",
-		},
-		{
-			CaseName:     "go",
-			FeatureFlags: []string{featureflag.GoFetchInternalRemote},
-		},
-	} {
-		t.Run(tc.CaseName, func(t *testing.T) {
-			repo, repoPath, cleanupFn := testhelper.InitBareRepo(t)
-			defer cleanupFn()
-
-			ctx, cancel := testhelper.Context()
-			defer cancel()
-
-			md := testhelper.GitalyServersMetadata(t, serverSocketPath)
-			ctx = metadata.NewOutgoingContext(ctx, md)
-			for _, feature := range tc.FeatureFlags {
-				ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, feature)
-			}
-
-			request := &gitalypb.FetchInternalRemoteRequest{
-				Repository:       repo,
-				RemoteRepository: remoteRepo,
-			}
-
-			c, err := client.FetchInternalRemote(ctx, request)
-			require.NoError(t, err)
-			require.True(t, c.GetResult())
-
-			remoteRefs := testhelper.GetRepositoryRefs(t, remoteRepoPath)
-			refs := testhelper.GetRepositoryRefs(t, repoPath)
-			require.Equal(t, remoteRefs, refs)
-		})
+	repo, repoPath, cleanupFn := testhelper.InitBareRepo(t)
+	defer cleanupFn()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	md := testhelper.GitalyServersMetadata(t, serverSocketPath)
+	ctx = metadata.NewOutgoingContext(ctx, md)
+
+	request := &gitalypb.FetchInternalRemoteRequest{
+		Repository:       repo,
+		RemoteRepository: remoteRepo,
 	}
+
+	c, err := client.FetchInternalRemote(ctx, request)
+	require.NoError(t, err)
+	require.True(t, c.GetResult())
+
+	remoteRefs := testhelper.GetRepositoryRefs(t, remoteRepoPath)
+	refs := testhelper.GetRepositoryRefs(t, repoPath)
+	require.Equal(t, remoteRefs, refs)
 }
 
 func TestFailedFetchInternalRemote(t *testing.T) {
@@ -75,44 +56,26 @@ func TestFailedFetchInternalRemote(t *testing.T) {
 	client, conn := remote.NewRemoteClient(t, serverSocketPath)
 	defer conn.Close()
 
-	for _, tc := range []struct {
-		CaseName     string
-		FeatureFlags []string
-	}{
-		{
-			CaseName: "ruby",
-		},
-		{
-			CaseName:     "go",
-			FeatureFlags: []string{featureflag.GoFetchInternalRemote},
-		},
-	} {
-		t.Run(tc.CaseName, func(t *testing.T) {
-			repo, _, cleanupFn := testhelper.InitBareRepo(t)
-			defer cleanupFn()
-
-			ctx, cancel := testhelper.Context()
-			defer cancel()
-
-			md := testhelper.GitalyServersMetadata(t, serverSocketPath)
-			ctx = metadata.NewOutgoingContext(ctx, md)
-			for _, feature := range tc.FeatureFlags {
-				ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, feature)
-			}
-
-			// Non-existing remote repo
-			remoteRepo := &gitalypb.Repository{StorageName: "default", RelativePath: "fake.git"}
-
-			request := &gitalypb.FetchInternalRemoteRequest{
-				Repository:       repo,
-				RemoteRepository: remoteRepo,
-			}
-
-			c, err := client.FetchInternalRemote(ctx, request)
-			require.NoError(t, err, "FetchInternalRemote is not supposed to return an error when 'git fetch' fails")
-			require.False(t, c.GetResult())
-		})
+	repo, _, cleanupFn := testhelper.InitBareRepo(t)
+	defer cleanupFn()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	md := testhelper.GitalyServersMetadata(t, serverSocketPath)
+	ctx = metadata.NewOutgoingContext(ctx, md)
+
+	// Non-existing remote repo
+	remoteRepo := &gitalypb.Repository{StorageName: "default", RelativePath: "fake.git"}
+
+	request := &gitalypb.FetchInternalRemoteRequest{
+		Repository:       repo,
+		RemoteRepository: remoteRepo,
 	}
+
+	c, err := client.FetchInternalRemote(ctx, request)
+	require.NoError(t, err, "FetchInternalRemote is not supposed to return an error when 'git fetch' fails")
+	require.False(t, c.GetResult())
 }
 
 func TestFailedFetchInternalRemoteDueToValidations(t *testing.T) {