From eef4eb7da5ce89305c01077e6628836a3caf6ee6 Mon Sep 17 00:00:00 2001 From: John Cai Date: Wed, 6 Apr 2022 11:32:12 -0400 Subject: 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 --- .../create_repository_from_bundle_test.go | 27 +++---------- .../gitaly/service/repository/fetch_bundle_test.go | 11 +----- .../gitaly/service/repository/replicate_test.go | 38 ++++-------------- .../gitaly/service/repository/write_ref_test.go | 46 ++++++++++------------ 4 files changed, 34 insertions(+), 88 deletions(-) (limited to 'internal/gitaly/service') 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) { -- cgit v1.2.3