diff options
author | James Fargher <proglottis@gmail.com> | 2022-04-07 04:27:33 +0300 |
---|---|---|
committer | James Fargher <proglottis@gmail.com> | 2022-04-07 04:27:33 +0300 |
commit | 3626c6deca3ff88456259ff6132dcd3b6b569671 (patch) | |
tree | d7a958c6c4a28d6789278125dd9d0a7786960e7f | |
parent | 778bdf56489619646d20600a217d464257b4fe1e (diff) | |
parent | eef4eb7da5ce89305c01077e6628836a3caf6ee6 (diff) |
Merge branch 'jc-remove-write-ref-ff' into 'master'
featureflag: Remove TransactionalSymbolicRefUpdates featureflag
See merge request gitlab-org/gitaly!4467
-rw-r--r-- | internal/backup/backup_test.go | 7 | ||||
-rw-r--r-- | internal/git/localrepo/refs.go | 13 | ||||
-rw-r--r-- | internal/git/localrepo/refs_test.go | 33 | ||||
-rw-r--r-- | internal/gitaly/service/repository/create_repository_from_bundle_test.go | 27 | ||||
-rw-r--r-- | internal/gitaly/service/repository/fetch_bundle_test.go | 11 | ||||
-rw-r--r-- | internal/gitaly/service/repository/replicate_test.go | 38 | ||||
-rw-r--r-- | internal/gitaly/service/repository/write_ref_test.go | 46 | ||||
-rw-r--r-- | internal/metadata/featureflag/ff_write_ref_manual_voting.go | 5 |
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) |