Remove UpdateRemoteMirror Go port's feature flag

UpdateRemoteMirror's Go port has been running successfully in production
and was default enabled in 14.1. This commit removes the feature flag for
so we can proceed removing the Ruby implementation in a later release.

Changelog: removed

4 files changed, 61 insertions, 156 deletions

diff --git a/internal/gitaly/service/remote/testhelper_test.go b/internal/gitaly/service/remote/testhelper_test.go
index e7d3d0262..dac64c19a 100644
--- a/internal/gitaly/service/remote/testhelper_test.go
+++ b/internal/gitaly/service/remote/testhelper_test.go
@@ -40,14 +40,8 @@ func TestWithRubySidecar(t *testing.T) {
 	t.Cleanup(rubySrv.Stop)
 
 	fs := []func(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server){
-		testSuccessfulUpdateRemoteMirrorRequest,
-		testSuccessfulUpdateRemoteMirrorRequestWithWildcards,
-		testUpdateRemoteMirrorInmemory,
-		testSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefs,
-		testFailedUpdateRemoteMirrorRequestDueToValidation,
 		testSuccessfulAddRemote,
 		testAddRemoteTransactional,
-		testUpdateRemoteMirror,
 	}
 
 	for _, f := range fs {
diff --git a/internal/gitaly/service/remote/update_remote_mirror.go b/internal/gitaly/service/remote/update_remote_mirror.go
index 734f18ffc..baab1000e 100644
--- a/internal/gitaly/service/remote/update_remote_mirror.go
+++ b/internal/gitaly/service/remote/update_remote_mirror.go
@@ -10,11 +10,9 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/ref"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 )
 
@@ -36,14 +34,6 @@ func (s *server) UpdateRemoteMirror(stream gitalypb.RemoteService_UpdateRemoteMi
 		return helper.ErrInvalidArgument(err)
 	}
 
-	if featureflag.GoUpdateRemoteMirror.IsEnabled(stream.Context()) {
-		if err := s.goUpdateRemoteMirror(stream, firstRequest); err != nil {
-			return helper.ErrInternal(err)
-		}
-
-		return nil
-	}
-
 	if err := s.updateRemoteMirror(stream, firstRequest); err != nil {
 		return helper.ErrInternal(err)
 	}
@@ -51,56 +41,8 @@ func (s *server) UpdateRemoteMirror(stream gitalypb.RemoteService_UpdateRemoteMi
 	return nil
 }
 
-// updateRemoteMirror has lots of decorated errors to help us debug
-// https://gitlab.com/gitlab-org/gitaly/issues/2156.
 func (s *server) updateRemoteMirror(stream gitalypb.RemoteService_UpdateRemoteMirrorServer, firstRequest *gitalypb.UpdateRemoteMirrorRequest) error {
 	ctx := stream.Context()
-	client, err := s.ruby.RemoteServiceClient(ctx)
-	if err != nil {
-		return fmt.Errorf("get stub: %v", err)
-	}
-
-	clientCtx, err := rubyserver.SetHeaders(ctx, s.locator, firstRequest.GetRepository())
-	if err != nil {
-		return fmt.Errorf("set headers: %v", err)
-	}
-
-	rubyStream, err := client.UpdateRemoteMirror(clientCtx)
-	if err != nil {
-		return fmt.Errorf("create client: %v", err)
-	}
-
-	if err := rubyStream.Send(firstRequest); err != nil {
-		return fmt.Errorf("first request to gitaly-ruby: %v", err)
-	}
-
-	err = rubyserver.Proxy(func() error {
-		// Do not wrap errors in this callback: we must faithfully relay io.EOF
-		request, err := stream.Recv()
-		if err != nil {
-			return err
-		}
-
-		return rubyStream.Send(request)
-	})
-	if err != nil {
-		return fmt.Errorf("proxy request to gitaly-ruby: %v", err)
-	}
-
-	response, err := rubyStream.CloseAndRecv()
-	if err != nil {
-		return fmt.Errorf("close stream to gitaly-ruby: %v", err)
-	}
-
-	if err := stream.SendAndClose(response); err != nil {
-		return fmt.Errorf("close stream to client: %v", err)
-	}
-
-	return nil
-}
-
-func (s *server) goUpdateRemoteMirror(stream gitalypb.RemoteService_UpdateRemoteMirrorServer, firstRequest *gitalypb.UpdateRemoteMirrorRequest) error {
-	ctx := stream.Context()
 
 	branchMatchers := firstRequest.GetOnlyBranchesMatching()
 	for {
@@ -336,10 +278,6 @@ func validateUpdateRemoteMirrorRequest(ctx context.Context, req *gitalypb.Update
 		if remote.GetUrl() == "" {
 			return fmt.Errorf("remote is missing URL")
 		}
-
-		if featureflag.GoUpdateRemoteMirror.IsDisabled(ctx) {
-			return fmt.Errorf("in-memory remotes require `gitaly_go_update_remote_mirror` feature flag")
-		}
 	}
 
 	return nil
diff --git a/internal/gitaly/service/remote/update_remote_mirror_test.go b/internal/gitaly/service/remote/update_remote_mirror_test.go
index 7e535c48f..85b9a46ce 100644
--- a/internal/gitaly/service/remote/update_remote_mirror_test.go
+++ b/internal/gitaly/service/remote/update_remote_mirror_test.go
@@ -15,27 +15,17 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/repository"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testassert"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testserver"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
-func testUpdateRemoteMirror(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets([]featureflag.FeatureFlag{
-		featureflag.GoUpdateRemoteMirror,
-	}).Run(t, func(t *testing.T, ctx context.Context) {
-		testUpdateRemoteMirrorFeatured(t, ctx, cfg, rubySrv)
-	})
-}
-
 type commandFactoryWrapper struct {
 	git.CommandFactory
 	newFunc func(context.Context, repository.GitRepo, git.Cmd, ...git.CmdOpt) (*command.Command, error)
@@ -45,7 +35,11 @@ func (w commandFactoryWrapper) New(ctx context.Context, repo repository.GitRepo,
 	return w.newFunc(ctx, repo, sc, opts...)
 }
 
-func testUpdateRemoteMirrorFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
+func TestUpdateRemoteMirror(t *testing.T) {
+	t.Parallel()
+
+	cfg := testcfg.Build(t)
+
 	testhelper.ConfigureGitalyGit2GoBin(t, cfg)
 
 	type refs map[string][]string
@@ -449,6 +443,9 @@ func testUpdateRemoteMirrorFeatured(t *testing.T, ctx context.Context, cfg confi
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
+			ctx, cancel := testhelper.Context()
+			defer cancel()
+
 			mirrorRepoPb, mirrorRepoPath, cleanMirrorRepo := gittest.InitBareRepoAt(t, cfg, cfg.Storages[0])
 			defer cleanMirrorRepo()
 
@@ -506,7 +503,7 @@ func testUpdateRemoteMirrorFeatured(t *testing.T, ctx context.Context, cfg confi
 				}
 			}
 
-			addr := testserver.RunGitalyServer(t, cfg, rubySrv, func(srv *grpc.Server, deps *service.Dependencies) {
+			addr := testserver.RunGitalyServer(t, cfg, nil, func(srv *grpc.Server, deps *service.Dependencies) {
 				cmdFactory := deps.GetGitCmdFactory()
 				if tc.wrapCommandFactory != nil {
 					cmdFactory = tc.wrapCommandFactory(t, deps.GetGitCmdFactory())
@@ -570,16 +567,15 @@ func testUpdateRemoteMirrorFeatured(t *testing.T, ctx context.Context, cfg confi
 	}
 }
 
-func testSuccessfulUpdateRemoteMirrorRequest(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets([]featureflag.FeatureFlag{
-		featureflag.GoUpdateRemoteMirror,
-	}).Run(t, func(t *testing.T, ctx context.Context) {
-		testSuccessfulUpdateRemoteMirrorRequestFeatured(t, ctx, cfg, rubySrv)
-	})
-}
+func TestSuccessfulUpdateRemoteMirrorRequest(t *testing.T) {
+	t.Parallel()
+
+	cfg := testcfg.Build(t)
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 
-func testSuccessfulUpdateRemoteMirrorRequestFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	serverSocketPath := testserver.RunGitalyServer(t, cfg, rubySrv, func(srv *grpc.Server, deps *service.Dependencies) {
+	serverSocketPath := testserver.RunGitalyServer(t, cfg, nil, func(srv *grpc.Server, deps *service.Dependencies) {
 		gitalypb.RegisterRemoteServiceServer(srv, NewServer(
 			deps.GetCfg(),
 			deps.GetRubyServer(),
@@ -677,16 +673,15 @@ func testSuccessfulUpdateRemoteMirrorRequestFeatured(t *testing.T, ctx context.C
 	require.NotContains(t, mirrorRefs, "refs/tags/v1.1.0")
 }
 
-func testSuccessfulUpdateRemoteMirrorRequestWithWildcards(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets([]featureflag.FeatureFlag{
-		featureflag.GoUpdateRemoteMirror,
-	}).Run(t, func(t *testing.T, ctx context.Context) {
-		testSuccessfulUpdateRemoteMirrorRequestWithWildcardsFeatured(t, ctx, cfg, rubySrv)
-	})
-}
+func TestSuccessfulUpdateRemoteMirrorRequestWithWildcards(t *testing.T) {
+	t.Parallel()
 
-func testSuccessfulUpdateRemoteMirrorRequestWithWildcardsFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	serverSocketPath := testserver.RunGitalyServer(t, cfg, rubySrv, func(srv *grpc.Server, deps *service.Dependencies) {
+	cfg := testcfg.Build(t)
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	serverSocketPath := testserver.RunGitalyServer(t, cfg, nil, func(srv *grpc.Server, deps *service.Dependencies) {
 		gitalypb.RegisterRemoteServiceServer(srv, NewServer(
 			deps.GetCfg(),
 			deps.GetRubyServer(),
@@ -768,8 +763,12 @@ func testSuccessfulUpdateRemoteMirrorRequestWithWildcardsFeatured(t *testing.T,
 	require.NotContains(t, mirrorRefs, "refs/tags/v1.1.0")
 }
 
-func testUpdateRemoteMirrorInmemory(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	serverSocketPath := testserver.RunGitalyServer(t, cfg, rubySrv, func(srv *grpc.Server, deps *service.Dependencies) {
+func TestUpdateRemoteMirrorInmemory(t *testing.T) {
+	t.Parallel()
+
+	cfg := testcfg.Build(t)
+
+	serverSocketPath := testserver.RunGitalyServer(t, cfg, nil, func(srv *grpc.Server, deps *service.Dependencies) {
 		gitalypb.RegisterRemoteServiceServer(srv, NewServer(
 			deps.GetCfg(),
 			deps.GetRubyServer(),
@@ -793,56 +792,34 @@ func testUpdateRemoteMirrorInmemory(t *testing.T, cfg config.Cfg, rubySrv *rubys
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	t.Run("Ruby implementation fails gracefully", func(t *testing.T) {
-		ctx := featureflag.OutgoingCtxWithFeatureFlagValue(ctx, featureflag.GoUpdateRemoteMirror, "false")
-
-		stream, err := client.UpdateRemoteMirror(ctx)
-		require.NoError(t, err)
-
-		require.NoError(t, stream.Send(&gitalypb.UpdateRemoteMirrorRequest{
-			Repository: localRepo,
-			Remote: &gitalypb.UpdateRemoteMirrorRequest_Remote{
-				Url: remotePath,
-			},
-		}))
-
-		_, err = stream.CloseAndRecv()
-		testassert.GrpcEqualErr(t, status.Error(codes.InvalidArgument, "in-memory remotes require `gitaly_go_update_remote_mirror` feature flag"), err)
-	})
+	stream, err := client.UpdateRemoteMirror(ctx)
+	require.NoError(t, err)
 
-	t.Run("Go implementation succeeds", func(t *testing.T) {
-		ctx := featureflag.OutgoingCtxWithFeatureFlags(ctx, featureflag.GoUpdateRemoteMirror)
+	require.NoError(t, stream.Send(&gitalypb.UpdateRemoteMirrorRequest{
+		Repository: localRepo,
+		Remote: &gitalypb.UpdateRemoteMirrorRequest_Remote{
+			Url: remotePath,
+		},
+	}))
 
-		stream, err := client.UpdateRemoteMirror(ctx)
-		require.NoError(t, err)
+	response, err := stream.CloseAndRecv()
+	require.NoError(t, err)
+	testassert.ProtoEqual(t, &gitalypb.UpdateRemoteMirrorResponse{}, response)
 
-		require.NoError(t, stream.Send(&gitalypb.UpdateRemoteMirrorRequest{
-			Repository: localRepo,
-			Remote: &gitalypb.UpdateRemoteMirrorRequest_Remote{
-				Url: remotePath,
-			},
-		}))
+	localRefs := string(gittest.Exec(t, cfg, "-C", localPath, "for-each-ref"))
+	remoteRefs := string(gittest.Exec(t, cfg, "-C", remotePath, "for-each-ref"))
+	require.Equal(t, localRefs, remoteRefs)
+}
 
-		response, err := stream.CloseAndRecv()
-		require.NoError(t, err)
-		testassert.ProtoEqual(t, &gitalypb.UpdateRemoteMirrorResponse{}, response)
+func TestSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefs(t *testing.T) {
+	t.Parallel()
 
-		localRefs := string(gittest.Exec(t, cfg, "-C", localPath, "for-each-ref"))
-		remoteRefs := string(gittest.Exec(t, cfg, "-C", remotePath, "for-each-ref"))
-		require.Equal(t, localRefs, remoteRefs)
-	})
-}
+	cfg := testcfg.Build(t)
 
-func testSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefs(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets([]featureflag.FeatureFlag{
-		featureflag.GoUpdateRemoteMirror,
-	}).Run(t, func(t *testing.T, ctx context.Context) {
-		testSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefsFeatured(t, ctx, cfg, rubySrv)
-	})
-}
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 
-func testSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefsFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	serverSocketPath := testserver.RunGitalyServer(t, cfg, rubySrv, func(srv *grpc.Server, deps *service.Dependencies) {
+	serverSocketPath := testserver.RunGitalyServer(t, cfg, nil, func(srv *grpc.Server, deps *service.Dependencies) {
 		gitalypb.RegisterRemoteServiceServer(srv, NewServer(
 			deps.GetCfg(),
 			deps.GetRubyServer(),
@@ -926,16 +903,15 @@ func testSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefsFeatured(t *tes
 	require.NotContains(t, mirrorRefs, "refs/heads/not-merged-branch")
 }
 
-func testFailedUpdateRemoteMirrorRequestDueToValidation(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets([]featureflag.FeatureFlag{
-		featureflag.GoUpdateRemoteMirror,
-	}).Run(t, func(t *testing.T, ctx context.Context) {
-		testFailedUpdateRemoteMirrorRequestDueToValidationFeatured(t, ctx, cfg, rubySrv)
-	})
-}
+func TestFailedUpdateRemoteMirrorRequestDueToValidation(t *testing.T) {
+	t.Parallel()
+
+	cfg := testcfg.Build(t)
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 
-func testFailedUpdateRemoteMirrorRequestDueToValidationFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	serverSocketPath := testserver.RunGitalyServer(t, cfg, rubySrv, func(srv *grpc.Server, deps *service.Dependencies) {
+	serverSocketPath := testserver.RunGitalyServer(t, cfg, nil, func(srv *grpc.Server, deps *service.Dependencies) {
 		gitalypb.RegisterRemoteServiceServer(srv, NewServer(
 			deps.GetCfg(),
 			deps.GetRubyServer(),
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index 1466b8cc8..7597475ca 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -4,8 +4,6 @@ package featureflag
 // In order to support coverage of combined features usage all feature flags should be marked as enabled for the test.
 // NOTE: if you add a new feature flag please add it to the `All` list defined below.
 var (
-	// GoUpdateRemoteMirror enables the Go implementation of UpdateRemoteMirror
-	GoUpdateRemoteMirror = FeatureFlag{Name: "go_update_remote_mirror", OnByDefault: true}
 	// GoSetConfig enables git2go implementation of SetConfig.
 	GoSetConfig = FeatureFlag{Name: "go_set_config", OnByDefault: false}
 	// CreateRepositoryFromBundleAtomicFetch will add the `--atomic` flag to git-fetch(1) in
@@ -34,7 +32,6 @@ var (
 
 // All includes all feature flags.
 var All = []FeatureFlag{
-	GoUpdateRemoteMirror,
 	GoSetConfig,
 	CreateRepositoryFromBundleAtomicFetch,
 	ResolveConflictsWithHooks,