diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-07-24 14:20:13 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-07-24 14:20:13 +0300 |
commit | 07fb2d1b6f9d5f093b14f100a5473c3839408b8a (patch) | |
tree | aa0f0486ec160ebd3089f81785285366bf24f66e | |
parent | 5ad8502e3782818671ec79a94f6323fa440ba3eb (diff) |
hook: Always synchronize hook executions
In Gitaly Cluster, custom hooks only run on the primary node.
Consequentially, secondary Gitaly nodes may already forge ahead while
the primary is still executing the hooks. Depending on how long they
run, this can cause the secondaries to already lock references for an
extended amount of time while they are waiting for the primary node to
catch up.
To fix this edge case we have introduced "synchronizing" votes in commit
38e01406b (gitaly/hook: Fix packed-refs lock contention by synchronizing
hooks, 2023-06-13). These synchronizing votes cause the secondaries to
wait for the primary to have executed the hooks before they start to
lock any of the references on disk.
We have rolled out this change over a month ago and default-enabled the
new behaviour with 002ec9926 (hooks: Default-enable synchronized hook
executions, 2023-06-29). We couldn't yet remove the flag until now
though because changes to the voting logic need to be handled over at
least two releases due to zero-downtime upgrades.
Now that the flag was released as default-enabled though via v16.2.0, we
can finally remove it. Let's do so.
-rw-r--r-- | internal/featureflag/ff_synchronize_hook_executions.go | 10 | ||||
-rw-r--r-- | internal/gitaly/hook/postreceive_test.go | 36 | ||||
-rw-r--r-- | internal/gitaly/hook/prereceive_test.go | 36 | ||||
-rw-r--r-- | internal/gitaly/hook/transactions.go | 7 | ||||
-rw-r--r-- | internal/gitaly/hook/update_test.go | 35 | ||||
-rw-r--r-- | internal/gitaly/service/hook/post_receive_test.go | 12 | ||||
-rw-r--r-- | internal/gitaly/service/hook/pre_receive_test.go | 26 | ||||
-rw-r--r-- | internal/gitaly/service/operations/apply_patch_test.go | 10 | ||||
-rw-r--r-- | internal/gitaly/service/operations/branches_test.go | 17 | ||||
-rw-r--r-- | internal/gitaly/service/operations/rebase_test.go | 12 | ||||
-rw-r--r-- | internal/gitaly/service/operations/tags_test.go | 10 |
11 files changed, 47 insertions, 164 deletions
diff --git a/internal/featureflag/ff_synchronize_hook_executions.go b/internal/featureflag/ff_synchronize_hook_executions.go deleted file mode 100644 index 305f8359f..000000000 --- a/internal/featureflag/ff_synchronize_hook_executions.go +++ /dev/null @@ -1,10 +0,0 @@ -package featureflag - -// SynchronizeHookExecutions synchronizes execution of Git hooks across all Gitaly nodes taking part in a transaction -// in Gitaly Cluster. This is done to avoid lock contention around the `packed-refs` file. -var SynchronizeHookExecutions = NewFeatureFlag( - "synchronize_hook_executions", - "v16.1.0", - "https://gitlab.com/gitlab-org/gitaly/-/issues/5359", - true, -) diff --git a/internal/gitaly/hook/postreceive_test.go b/internal/gitaly/hook/postreceive_test.go index a63734b42..60c1544c8 100644 --- a/internal/gitaly/hook/postreceive_test.go +++ b/internal/gitaly/hook/postreceive_test.go @@ -68,12 +68,8 @@ func TestPrintAlert(t *testing.T) { func TestPostReceive_customHook(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testPostReceiveCustomHook) -} - -func testPostReceiveCustomHook(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) cfg := testcfg.Build(t) repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ @@ -213,20 +209,14 @@ func testPostReceiveCustomHook(t *testing.T, ctx context.Context) { stdin: "changes\n", hook: "#!/bin/sh\necho foo\n", expectedStdout: "foo\n", - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, - []transaction.PhasedVote{synchronizedVote("post-receive")}, - []transaction.PhasedVote{}, - ), + expectedVotes: []transaction.PhasedVote{synchronizedVote("post-receive")}, }, { - desc: "hook is not executed on secondary", - env: []string{secondaryPayload}, - stdin: "changes\n", - hook: "#!/bin/sh\necho foo\n", - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, - []transaction.PhasedVote{synchronizedVote("post-receive")}, - []transaction.PhasedVote{}, - ), + desc: "hook is not executed on secondary", + env: []string{secondaryPayload}, + stdin: "changes\n", + hook: "#!/bin/sh\necho foo\n", + expectedVotes: []transaction.PhasedVote{synchronizedVote("post-receive")}, }, { desc: "missing changes cause error", @@ -280,12 +270,8 @@ func (m *postreceiveAPIMock) PostReceive(ctx context.Context, glRepository, glID func TestPostReceive_gitlab(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testPostReceiveGitlab) -} - -func testPostReceiveGitlab(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) cfg := testcfg.Build(t) repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ @@ -411,12 +397,8 @@ func testPostReceiveGitlab(t *testing.T, ctx context.Context) { func TestPostReceive_quarantine(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testPostReceiveQuarantine) -} - -func testPostReceiveQuarantine(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) cfg := testcfg.Build(t) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ diff --git a/internal/gitaly/hook/prereceive_test.go b/internal/gitaly/hook/prereceive_test.go index 5cc9cdc3a..930ca8a87 100644 --- a/internal/gitaly/hook/prereceive_test.go +++ b/internal/gitaly/hook/prereceive_test.go @@ -27,12 +27,8 @@ import ( func TestPrereceive_customHooks(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testPrereceiveCustomHooks) -} - -func testPrereceiveCustomHooks(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) cfg := testcfg.Build(t) repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ @@ -169,20 +165,14 @@ func testPrereceiveCustomHooks(t *testing.T, ctx context.Context) { hook: "#!/bin/sh\necho foo\n", stdin: "change\n", expectedStdout: "foo\n", - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, - []transaction.PhasedVote{synchronizedVote("pre-receive")}, - []transaction.PhasedVote{}, - ), + expectedVotes: []transaction.PhasedVote{synchronizedVote("pre-receive")}, }, { - desc: "hook is not executed on secondary", - env: []string{secondaryPayload}, - hook: "#!/bin/sh\necho foo\n", - stdin: "change\n", - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, - []transaction.PhasedVote{synchronizedVote("pre-receive")}, - []transaction.PhasedVote{}, - ), + desc: "hook is not executed on secondary", + env: []string{secondaryPayload}, + hook: "#!/bin/sh\necho foo\n", + stdin: "change\n", + expectedVotes: []transaction.PhasedVote{synchronizedVote("pre-receive")}, }, { desc: "missing changes cause error", @@ -216,12 +206,8 @@ func testPrereceiveCustomHooks(t *testing.T, ctx context.Context) { func TestPrereceive_quarantine(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testPrereceiveQuarantine) -} - -func testPrereceiveQuarantine(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) cfg := testcfg.Build(t) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ @@ -306,12 +292,8 @@ func (m *prereceiveAPIMock) PostReceive(context.Context, string, string, string, func TestPrereceive_gitlab(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testPrereceiveGitlab) -} - -func testPrereceiveGitlab(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) cfg := testcfg.Build(t) repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ diff --git a/internal/gitaly/hook/transactions.go b/internal/gitaly/hook/transactions.go index 9f888fdaa..0966b1328 100644 --- a/internal/gitaly/hook/transactions.go +++ b/internal/gitaly/hook/transactions.go @@ -4,7 +4,6 @@ import ( "context" "fmt" - "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/transaction/txinfo" "gitlab.com/gitlab-org/gitaly/v16/internal/transaction/voting" @@ -64,11 +63,7 @@ func (m *GitLabHookManager) stopTransaction(ctx context.Context, payload git.Hoo // becomes significantly shorter, which alleviates the lock contention. func (m *GitLabHookManager) synchronizeHookExecution(ctx context.Context, payload git.HooksPayload, hook string) error { if err := m.runWithTransaction(ctx, payload, func(ctx context.Context, tx txinfo.Transaction) error { - if featureflag.SynchronizeHookExecutions.IsEnabled(ctx) { - return m.txManager.Vote(ctx, tx, voting.VoteFromData([]byte("synchronize "+hook+" hook")), voting.Synchronized) - } - - return nil + return m.txManager.Vote(ctx, tx, voting.VoteFromData([]byte("synchronize "+hook+" hook")), voting.Synchronized) }); err != nil { return fmt.Errorf("vote failed: %w", err) } diff --git a/internal/gitaly/hook/update_test.go b/internal/gitaly/hook/update_test.go index dbbde536f..ab3b9116c 100644 --- a/internal/gitaly/hook/update_test.go +++ b/internal/gitaly/hook/update_test.go @@ -2,7 +2,6 @@ package hook import ( "bytes" - "context" "fmt" "strings" "testing" @@ -24,12 +23,8 @@ import ( func TestUpdate_customHooks(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testUpdateCustomHooks) -} - -func testUpdateCustomHooks(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) cfg := testcfg.Build(t) repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ @@ -181,22 +176,16 @@ func testUpdateCustomHooks(t *testing.T, ctx context.Context) { newHash: commitB, hook: "#!/bin/sh\necho foo\n", expectedStdout: "foo\n", - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, - []transaction.PhasedVote{synchronizedVote("update")}, - []transaction.PhasedVote{}, - ), + expectedVotes: []transaction.PhasedVote{synchronizedVote("update")}, }, { - desc: "hook is not executed on secondary", - env: []string{secondaryPayload}, - reference: "refs/heads/master", - oldHash: commitA, - newHash: commitB, - hook: "#!/bin/sh\necho foo\n", - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, - []transaction.PhasedVote{synchronizedVote("update")}, - []transaction.PhasedVote{}, - ), + desc: "hook is not executed on secondary", + env: []string{secondaryPayload}, + reference: "refs/heads/master", + oldHash: commitA, + newHash: commitB, + hook: "#!/bin/sh\necho foo\n", + expectedVotes: []transaction.PhasedVote{synchronizedVote("update")}, }, { desc: "hook fails with missing reference", @@ -247,12 +236,8 @@ func testUpdateCustomHooks(t *testing.T, ctx context.Context) { func TestUpdate_quarantine(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testUpdateQuarantine) -} - -func testUpdateQuarantine(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) cfg := testcfg.Build(t) repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ diff --git a/internal/gitaly/service/hook/post_receive_test.go b/internal/gitaly/service/hook/post_receive_test.go index ff7476ff5..12b7f962c 100644 --- a/internal/gitaly/service/hook/post_receive_test.go +++ b/internal/gitaly/service/hook/post_receive_test.go @@ -2,7 +2,6 @@ package hook import ( "bytes" - "context" "io" "path/filepath" "testing" @@ -43,12 +42,8 @@ func TestPostReceiveInvalidArgument(t *testing.T) { func TestHooksMissingStdin(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testHooksMissingStdin) -} - -func testHooksMissingStdin(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) user, password, secretToken := "user", "password", "secret token" tempDir := testhelper.TempDir(t) gitlab.WriteShellSecretFile(t, tempDir, secretToken) @@ -165,10 +160,7 @@ func testHooksMissingStdin(t *testing.T, ctx context.Context) { require.NotEqual(t, int32(0), status, "exit code should be non-zero") } else { require.Equal(t, int32(0), status, "exit code unequal") - require.Equal(t, testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, - []transaction.PhasedVote{synchronizedVote("post-receive")}, - []transaction.PhasedVote{}, - ), txManager.Votes()) + require.Equal(t, []transaction.PhasedVote{synchronizedVote("post-receive")}, txManager.Votes()) } }) } diff --git a/internal/gitaly/service/hook/pre_receive_test.go b/internal/gitaly/service/hook/pre_receive_test.go index a86874b87..b31359b2d 100644 --- a/internal/gitaly/service/hook/pre_receive_test.go +++ b/internal/gitaly/service/hook/pre_receive_test.go @@ -2,7 +2,6 @@ package hook import ( "bytes" - "context" "fmt" "io" "net/http" @@ -377,11 +376,8 @@ exit %d func TestPreReceiveHook_Primary(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testPreReceiveHookPrimary) -} -func testPreReceiveHookPrimary(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) testCases := []struct { desc string @@ -399,10 +395,7 @@ func testPreReceiveHookPrimary(t *testing.T, ctx context.Context) { allowedHandler: allowedHandler(t, true), preReceiveHandler: preReceiveHandler(t, true), expectedExitStatus: 0, - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, - []transaction.PhasedVote{synchronizedVote("pre-receive")}, - []transaction.PhasedVote{}, - ), + expectedVotes: []transaction.PhasedVote{synchronizedVote("pre-receive")}, }, { desc: "primary checks for permissions", @@ -417,10 +410,7 @@ func testPreReceiveHookPrimary(t *testing.T, ctx context.Context) { primary: false, allowedHandler: allowedHandler(t, false), expectedExitStatus: 0, - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, - []transaction.PhasedVote{synchronizedVote("pre-receive")}, - []transaction.PhasedVote{}, - ), + expectedVotes: []transaction.PhasedVote{synchronizedVote("pre-receive")}, }, { desc: "primary tries to increase reference counter", @@ -437,10 +427,7 @@ func testPreReceiveHookPrimary(t *testing.T, ctx context.Context) { allowedHandler: allowedHandler(t, true), preReceiveHandler: preReceiveHandler(t, false), expectedExitStatus: 0, - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, - []transaction.PhasedVote{synchronizedVote("pre-receive")}, - []transaction.PhasedVote{}, - ), + expectedVotes: []transaction.PhasedVote{synchronizedVote("pre-receive")}, }, { desc: "primary executes hook", @@ -458,10 +445,7 @@ func testPreReceiveHookPrimary(t *testing.T, ctx context.Context) { preReceiveHandler: preReceiveHandler(t, true), hookExitCode: 123, expectedExitStatus: 0, - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, - []transaction.PhasedVote{synchronizedVote("pre-receive")}, - []transaction.PhasedVote{}, - ), + expectedVotes: []transaction.PhasedVote{synchronizedVote("pre-receive")}, }, } diff --git a/internal/gitaly/service/operations/apply_patch_test.go b/internal/gitaly/service/operations/apply_patch_test.go index b8dffa014..9c84b9473 100644 --- a/internal/gitaly/service/operations/apply_patch_test.go +++ b/internal/gitaly/service/operations/apply_patch_test.go @@ -3,7 +3,6 @@ package operations import ( - "context" "fmt" "io" "os" @@ -13,7 +12,6 @@ import ( "time" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo" @@ -780,12 +778,8 @@ func TestUserApplyPatch_stableID(t *testing.T) { func TestUserApplyPatch_transactional(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testUserApplyPatchTransactional) -} - -func testUserApplyPatchTransactional(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) txManager := transaction.NewTrackingManager() ctx, _, repoProto, _, client := setupOperationsService(t, ctx, testserver.WithTransactionManager(txManager)) @@ -824,7 +818,7 @@ func testUserApplyPatchTransactional(t *testing.T, ctx context.Context) { require.True(t, response.BranchUpdate.BranchCreated) - require.Equal(t, testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, 15, 12), len(txManager.Votes())) + require.Equal(t, 15, len(txManager.Votes())) } func TestFailedPatchApplyPatch(t *testing.T) { diff --git a/internal/gitaly/service/operations/branches_test.go b/internal/gitaly/service/operations/branches_test.go index 6db52e74b..f33b9708f 100644 --- a/internal/gitaly/service/operations/branches_test.go +++ b/internal/gitaly/service/operations/branches_test.go @@ -11,7 +11,6 @@ import ( "testing" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo" @@ -113,12 +112,8 @@ func TestUserCreateBranch_successful(t *testing.T) { func TestUserCreateBranch_Transactions(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testUserCreateBranchTransactions) -} - -func testUserCreateBranchTransactions(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) cfg := testcfg.Build(t) repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ @@ -196,7 +191,7 @@ func testUserCreateBranchTransactions(t *testing.T, ctx context.Context) { transactionServer.called = 0 _, err = client.UserCreateBranch(ctx, request) require.NoError(t, err) - require.Equal(t, testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, 5, 2), transactionServer.called) + require.Equal(t, 5, transactionServer.called) }) } } @@ -810,12 +805,8 @@ func TestUserDeleteBranch_hooks(t *testing.T) { func TestUserDeleteBranch_transaction(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testUserDeleteBranchTransaction) -} - -func testUserDeleteBranchTransaction(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) cfg := testcfg.Build(t) repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ @@ -870,7 +861,7 @@ func testUserDeleteBranchTransaction(t *testing.T, ctx context.Context) { User: gittest.TestUser, }) require.NoError(t, err) - require.Equal(t, testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, 5, 2), transactionServer.called) + require.Equal(t, 5, transactionServer.called) } func TestUserDeleteBranch_invalidArgument(t *testing.T) { diff --git a/internal/gitaly/service/operations/rebase_test.go b/internal/gitaly/service/operations/rebase_test.go index 632e389c6..16770a933 100644 --- a/internal/gitaly/service/operations/rebase_test.go +++ b/internal/gitaly/service/operations/rebase_test.go @@ -3,14 +3,12 @@ package operations import ( - "context" "errors" "fmt" "io" "testing" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo" @@ -191,12 +189,8 @@ func TestUserRebaseConfirmable_skipEmptyCommits(t *testing.T) { func TestUserRebaseConfirmable_transaction(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testUserRebaseConfirmableTransaction) -} - -func testUserRebaseConfirmableTransaction(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) txManager := transaction.NewTrackingManager() ctx, cfg, repoProto, repoPath, client := setupOperationsService( @@ -225,14 +219,14 @@ func testUserRebaseConfirmableTransaction(t *testing.T, ctx context.Context) { desc: "primary votes and executes hook", withTransaction: true, primary: true, - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, 5, 2), + expectedVotes: 5, expectPreReceiveHook: true, }, { desc: "secondary votes but does not execute hook", withTransaction: true, primary: false, - expectedVotes: testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, 5, 2), + expectedVotes: 5, expectPreReceiveHook: false, }, } { diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go index 157398b45..bbcf7644d 100644 --- a/internal/gitaly/service/operations/tags_test.go +++ b/internal/gitaly/service/operations/tags_test.go @@ -1,7 +1,6 @@ package operations import ( - "context" "fmt" "path/filepath" "strings" @@ -9,7 +8,6 @@ import ( "time" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo" @@ -455,12 +453,8 @@ func TestUserCreateTag_successful(t *testing.T) { func TestUserCreateTag_transactional(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.SynchronizeHookExecutions).Run(t, testUserCreateTagTransactional) -} - -func testUserCreateTagTransactional(t *testing.T, ctx context.Context) { - t.Parallel() + ctx := testhelper.Context(t) cfg := testcfg.Build(t) cfg.SocketPath = runOperationServiceServer(t, cfg, testserver.WithDisablePraefect()) @@ -568,7 +562,7 @@ func testUserCreateTagTransactional(t *testing.T, ctx context.Context) { require.NoFileExists(t, hooksOutputPath) } - require.Equal(t, testhelper.EnabledOrDisabledFlag(ctx, featureflag.SynchronizeHookExecutions, 5, 2), transactionServer.called) + require.Equal(t, 5, transactionServer.called) transactionServer.called = 0 }) } |