featureflag: Remove TransactionalSymbolicRefUpdates featureflag

This feature flag has been set to default on, and deleted from
production. There have been no observable issues, so it's now safe to
fully remove the feature flag.

Changelog: changed

8 files changed, 48 insertions, 132 deletions

diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go
index a7b22fd14..cd577b6a3 100644
--- a/internal/backup/backup_test.go
+++ b/internal/backup/backup_test.go
@@ -1,7 +1,6 @@
 package backup
 
 import (
-	"context"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -13,7 +12,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/setup"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/storage"
-	"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/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testserver"
@@ -264,12 +262,9 @@ func TestManager_Create_incremental(t *testing.T) {
 }
 
 func TestManager_Restore(t *testing.T) {
-	testhelper.NewFeatureSets(featureflag.TransactionalSymbolicRefUpdates).Run(t, testManagerRestore)
-}
-
-func testManagerRestore(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 	testcfg.BuildGitalyHooks(t, cfg)
 
diff --git a/internal/git/localrepo/refs.go b/internal/git/localrepo/refs.go
index 30cc93000..5b0f9e4df 100644
--- a/internal/git/localrepo/refs.go
+++ b/internal/git/localrepo/refs.go
@@ -14,7 +14,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/command"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/safe"
 )
 
@@ -168,17 +167,7 @@ func (repo *Repo) UpdateRef(ctx context.Context, reference git.ReferenceName, ne
 // SetDefaultBranch sets the repository's HEAD to point to the given reference.
 // It will not verify the reference actually exists.
 func (repo *Repo) SetDefaultBranch(ctx context.Context, txManager transaction.Manager, reference git.ReferenceName) error {
-	if featureflag.TransactionalSymbolicRefUpdates.IsEnabled(ctx) {
-		return repo.setDefaultBranchWithTransaction(ctx, txManager, reference)
-	}
-
-	if err := repo.ExecAndWait(ctx, git.SubCmd{
-		Name: "symbolic-ref",
-		Args: []string{"HEAD", reference.String()},
-	}, git.WithRefTxHook(repo)); err != nil {
-		return err
-	}
-	return nil
+	return repo.setDefaultBranchWithTransaction(ctx, txManager, reference)
 }
 
 // setDefaultBranchWithTransaction sets the repostory's HEAD to point to the given reference
diff --git a/internal/git/localrepo/refs_test.go b/internal/git/localrepo/refs_test.go
index df5c1c7ec..bd50f1347 100644
--- a/internal/git/localrepo/refs_test.go
+++ b/internal/git/localrepo/refs_test.go
@@ -18,7 +18,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
 	"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/safe"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
@@ -456,10 +455,7 @@ func TestRepo_UpdateRef(t *testing.T) {
 }
 
 func TestRepo_SetDefaultBranch(t *testing.T) {
-	testhelper.NewFeatureSets(featureflag.TransactionalSymbolicRefUpdates).Run(t, testRepoSetBranchFeatures)
-}
-
-func testRepoSetBranchFeatures(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	_, repo, _ := setupRepo(t)
 
 	txManager := transaction.NewTrackingManager()
@@ -498,19 +494,17 @@ func testRepoSetBranchFeatures(t *testing.T, ctx context.Context) {
 
 			require.Equal(t, tc.expectedRef, newRef)
 
-			if featureflag.TransactionalSymbolicRefUpdates.IsEnabled(ctx) {
-				require.Len(t, txManager.Votes(), 2)
-				h := voting.NewVoteHash()
-				_, err = h.Write([]byte("ref: " + tc.ref.String() + "\n"))
-				require.NoError(t, err)
-				vote, err := h.Vote()
-				require.NoError(t, err)
+			require.Len(t, txManager.Votes(), 2)
+			h := voting.NewVoteHash()
+			_, err = h.Write([]byte("ref: " + tc.ref.String() + "\n"))
+			require.NoError(t, err)
+			vote, err := h.Vote()
+			require.NoError(t, err)
 
-				require.Equal(t, voting.Prepared, txManager.Votes()[0].Phase)
-				require.Equal(t, vote.String(), txManager.Votes()[0].Vote.String())
-				require.Equal(t, voting.Committed, txManager.Votes()[1].Phase)
-				require.Equal(t, vote.String(), txManager.Votes()[1].Vote.String())
-			}
+			require.Equal(t, voting.Prepared, txManager.Votes()[0].Phase)
+			require.Equal(t, vote.String(), txManager.Votes()[0].Vote.String())
+			require.Equal(t, voting.Committed, txManager.Votes()[1].Phase)
+			require.Equal(t, vote.String(), txManager.Votes()[1].Vote.String())
 		})
 	}
 }
@@ -535,10 +529,7 @@ func (b *blockingManager) Stop(_ context.Context, _ txinfo.Transaction) error {
 }
 
 func TestRepo_SetDefaultBranch_errors(t *testing.T) {
-	ctx := featureflag.ContextWithFeatureFlag(
-		testhelper.Context(t),
-		featureflag.TransactionalSymbolicRefUpdates,
-		true)
+	ctx := testhelper.Context(t)
 
 	t.Run("malformed refname", func(t *testing.T) {
 		_, repo, _ := setupRepo(t)
diff --git a/internal/gitaly/service/repository/create_repository_from_bundle_test.go b/internal/gitaly/service/repository/create_repository_from_bundle_test.go
index 033c161e3..f0bfba7e3 100644
--- a/internal/gitaly/service/repository/create_repository_from_bundle_test.go
+++ b/internal/gitaly/service/repository/create_repository_from_bundle_test.go
@@ -2,7 +2,6 @@ package repository
 
 import (
 	"bytes"
-	"context"
 	"io"
 	"os"
 	"path/filepath"
@@ -16,7 +15,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/praefect/praefectutil"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/tempdir"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
@@ -31,11 +29,8 @@ import (
 
 func TestCreateRepositoryFromBundle_successful(t *testing.T) {
 	t.Parallel()
+	ctx := testhelper.Context(t)
 
-	testhelper.NewFeatureSets(featureflag.TransactionalSymbolicRefUpdates).Run(t, testCreateRepositoryFromBundleSuccessful)
-}
-
-func testCreateRepositoryFromBundleSuccessful(t *testing.T, ctx context.Context) {
 	cfg, repo, repoPath, client := setupRepositoryService(ctx, t)
 
 	locator := config.NewLocator(cfg)
@@ -105,10 +100,7 @@ func testCreateRepositoryFromBundleSuccessful(t *testing.T, ctx context.Context)
 func TestCreateRepositoryFromBundle_transactional(t *testing.T) {
 	t.Parallel()
 
-	testhelper.NewFeatureSets(featureflag.TransactionalSymbolicRefUpdates).Run(t, testCreateRepositoryFromBundleTransactional)
-}
-
-func testCreateRepositoryFromBundleTransactional(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	txManager := transaction.NewTrackingManager()
 
 	cfg, repoProto, repoPath, client := setupRepositoryService(ctx, t, testserver.WithTransactionManager(txManager))
@@ -175,10 +167,7 @@ func testCreateRepositoryFromBundleTransactional(t *testing.T, ctx context.Conte
 func TestCreateRepositoryFromBundle_invalidBundle(t *testing.T) {
 	t.Parallel()
 
-	testhelper.NewFeatureSets(featureflag.TransactionalSymbolicRefUpdates).Run(t, testCreateRepositoryFromBundleInvalidBundle)
-}
-
-func testCreateRepositoryFromBundleInvalidBundle(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	cfg, client := setupRepositoryServiceWithoutRepo(t)
 
 	stream, err := client.CreateRepositoryFromBundle(ctx)
@@ -215,10 +204,7 @@ func testCreateRepositoryFromBundleInvalidBundle(t *testing.T, ctx context.Conte
 func TestCreateRepositoryFromBundle_invalidArgument(t *testing.T) {
 	t.Parallel()
 
-	testhelper.NewFeatureSets(featureflag.TransactionalSymbolicRefUpdates).Run(t, testCreateRepositoryFromBundleInvalidArgument)
-}
-
-func testCreateRepositoryFromBundleInvalidArgument(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	_, client := setupRepositoryServiceWithoutRepo(t)
 
 	stream, err := client.CreateRepositoryFromBundle(ctx)
@@ -233,10 +219,7 @@ func testCreateRepositoryFromBundleInvalidArgument(t *testing.T, ctx context.Con
 func TestCreateRepositoryFromBundle_existingRepository(t *testing.T) {
 	t.Parallel()
 
-	testhelper.NewFeatureSets(featureflag.TransactionalSymbolicRefUpdates).Run(t, testCreateRepositoryFromBundleExistingRepository)
-}
-
-func testCreateRepositoryFromBundleExistingRepository(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	cfg, client := setupRepositoryServiceWithoutRepo(t)
 
 	// The above test creates the second repository on the server. As this test can run with Praefect in front of it,
diff --git a/internal/gitaly/service/repository/fetch_bundle_test.go b/internal/gitaly/service/repository/fetch_bundle_test.go
index ed84aee27..f11327fce 100644
--- a/internal/gitaly/service/repository/fetch_bundle_test.go
+++ b/internal/gitaly/service/repository/fetch_bundle_test.go
@@ -14,7 +14,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/hook"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata"
-	"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/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testserver"
@@ -28,10 +27,7 @@ import (
 func TestServer_FetchBundle_success(t *testing.T) {
 	t.Parallel()
 
-	testhelper.NewFeatureSets(featureflag.TransactionalSymbolicRefUpdates).Run(t, testServerFetchBundlesuccess)
-}
-
-func testServerFetchBundlesuccess(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	cfg, _, repoPath, client := setupRepositoryService(ctx, t)
 
 	tmp := testhelper.TempDir(t)
@@ -76,10 +72,7 @@ func testServerFetchBundlesuccess(t *testing.T, ctx context.Context) {
 func TestServer_FetchBundle_transaction(t *testing.T) {
 	t.Parallel()
 
-	testhelper.NewFeatureSets(featureflag.TransactionalSymbolicRefUpdates).Run(t, testServerFetchBundleTransaction)
-}
-
-func testServerFetchBundleTransaction(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
 	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
 	testcfg.BuildGitalyHooks(t, cfg)
diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go
index 07d011d18..6ac836783 100644
--- a/internal/gitaly/service/repository/replicate_test.go
+++ b/internal/gitaly/service/repository/replicate_test.go
@@ -28,7 +28,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata"
-	"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/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testserver"
@@ -41,12 +40,8 @@ import (
 
 func TestReplicateRepository(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(
-		featureflag.TransactionalSymbolicRefUpdates,
-	).Run(t, testReplicateRepository)
-}
 
-func testReplicateRepository(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	cfgBuilder := testcfg.NewGitalyCfgBuilder(testcfg.WithStorages("default", "replica"))
 	cfg := cfgBuilder.Build(t)
 
@@ -118,12 +113,8 @@ func testReplicateRepository(t *testing.T, ctx context.Context) {
 
 func TestReplicateRepositoryTransactional(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(
-		featureflag.TransactionalSymbolicRefUpdates,
-	).Run(t, testReplicateRepositoryTransactional)
-}
 
-func testReplicateRepositoryTransactional(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	cfgBuilder := testcfg.NewGitalyCfgBuilder(testcfg.WithStorages("default", "replica"))
 	cfg := cfgBuilder.Build(t)
 
@@ -303,12 +294,9 @@ func TestReplicateRepositoryInvalidArguments(t *testing.T) {
 
 func TestReplicateRepository_BadRepository(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(
-		featureflag.TransactionalSymbolicRefUpdates,
-	).Run(t, testReplicateRepositoryBadRepository)
-}
 
-func testReplicateRepositoryBadRepository(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
+
 	for _, tc := range []struct {
 		desc          string
 		invalidSource bool
@@ -390,12 +378,8 @@ func testReplicateRepositoryBadRepository(t *testing.T, ctx context.Context) {
 
 func TestReplicateRepository_FailedFetchInternalRemote(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(
-		featureflag.TransactionalSymbolicRefUpdates,
-	).Run(t, testReplicateRepositoryFailedFetchInternalRemote)
-}
 
-func testReplicateRepositoryFailedFetchInternalRemote(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t, testcfg.WithStorages("default", "replica"))
 	testcfg.BuildGitalyHooks(t, cfg)
 	testcfg.BuildGitalySSH(t, cfg)
@@ -475,12 +459,8 @@ func listenGitalySSHCalls(t *testing.T, conf config.Cfg) func() gitalySSHParams
 
 func TestFetchInternalRemote_successful(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(
-		featureflag.TransactionalSymbolicRefUpdates,
-	).Run(t, testFetchInternalRemoteSuccessful)
-}
 
-func testFetchInternalRemoteSuccessful(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	remoteCfg, remoteRepo, remoteRepoPath := testcfg.BuildWithRepo(t)
 	testcfg.BuildGitalyHooks(t, remoteCfg)
 	gittest.WriteCommit(t, remoteCfg, remoteRepoPath, gittest.WithBranch("master"))
@@ -566,12 +546,8 @@ func testFetchInternalRemoteSuccessful(t *testing.T, ctx context.Context) {
 
 func TestFetchInternalRemote_failure(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(
-		featureflag.TransactionalSymbolicRefUpdates,
-	).Run(t, testFetchInternalRemoteFailure)
-}
 
-func testFetchInternalRemoteFailure(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	cfg, repoProto, _ := testcfg.BuildWithRepo(t)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 	ctx = testhelper.MergeIncomingMetadata(ctx, testcfg.GitalyServersMetadataFromCfg(t, cfg))
diff --git a/internal/gitaly/service/repository/write_ref_test.go b/internal/gitaly/service/repository/write_ref_test.go
index fdd6cf5e6..5e3b7f92a 100644
--- a/internal/gitaly/service/repository/write_ref_test.go
+++ b/internal/gitaly/service/repository/write_ref_test.go
@@ -2,7 +2,6 @@ package repository
 
 import (
 	"bytes"
-	"context"
 	"path/filepath"
 	"testing"
 
@@ -10,7 +9,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata"
-	"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/testserver"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/transaction/txinfo"
@@ -57,37 +55,33 @@ func TestWriteRefSuccessful(t *testing.T) {
 		},
 	}
 
-	testhelper.NewFeatureSets(featureflag.TransactionalSymbolicRefUpdates).Run(t, func(t *testing.T, ctx context.Context) {
-		ctx, err := txinfo.InjectTransaction(ctx, 1, "node", true)
-		require.NoError(t, err)
-		ctx = metadata.IncomingToOutgoing(ctx)
+	ctx, err := txinfo.InjectTransaction(testhelper.Context(t), 1, "node", true)
+	require.NoError(t, err)
+	ctx = metadata.IncomingToOutgoing(ctx)
 
-		for _, tc := range testCases {
-			t.Run(tc.desc, func(t *testing.T) {
-				txManager.Reset()
-				_, err = client.WriteRef(ctx, tc.req)
-				require.NoError(t, err)
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			txManager.Reset()
+			_, err = client.WriteRef(ctx, tc.req)
+			require.NoError(t, err)
 
-				if featureflag.TransactionalSymbolicRefUpdates.IsEnabled(ctx) {
-					require.Len(t, txManager.Votes(), tc.expectedVotes)
-				}
+			require.Len(t, txManager.Votes(), tc.expectedVotes)
 
-				if bytes.Equal(tc.req.Ref, []byte("HEAD")) {
-					content := testhelper.MustReadFile(t, filepath.Join(repoPath, "HEAD"))
+			if bytes.Equal(tc.req.Ref, []byte("HEAD")) {
+				content := testhelper.MustReadFile(t, filepath.Join(repoPath, "HEAD"))
 
-					refRevision := bytes.Join([][]byte{[]byte("ref: "), tc.req.Revision, []byte("\n")}, nil)
+				refRevision := bytes.Join([][]byte{[]byte("ref: "), tc.req.Revision, []byte("\n")}, nil)
 
-					require.EqualValues(t, refRevision, content)
-					return
-				}
-				rev := gittest.Exec(t, cfg, "--git-dir", repoPath, "log", "--pretty=%H", "-1", string(tc.req.Ref))
+				require.EqualValues(t, refRevision, content)
+				return
+			}
+			rev := gittest.Exec(t, cfg, "--git-dir", repoPath, "log", "--pretty=%H", "-1", string(tc.req.Ref))
 
-				rev = bytes.Replace(rev, []byte("\n"), nil, 1)
+			rev = bytes.Replace(rev, []byte("\n"), nil, 1)
 
-				require.Equal(t, string(tc.req.Revision), string(rev))
-			})
-		}
-	})
+			require.Equal(t, string(tc.req.Revision), string(rev))
+		})
+	}
 }
 
 func TestWriteRefValidationError(t *testing.T) {
diff --git a/internal/metadata/featureflag/ff_write_ref_manual_voting.go b/internal/metadata/featureflag/ff_write_ref_manual_voting.go
deleted file mode 100644
index d0ec0caed..000000000
--- a/internal/metadata/featureflag/ff_write_ref_manual_voting.go
+++ /dev/null
@@ -1,5 +0,0 @@
-package featureflag
-
-// TransactionalSymbolicRefUpdates allows the WriteRef RPC to use an implementation to update HEAD that does
-// its own transaction voting.
-var TransactionalSymbolicRefUpdates = NewFeatureFlag("transactional_symbolic_ref_updates", true)