Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Steinhardt <psteinhardt@gitlab.com>2023-07-24 14:20:13 +0300
committerPatrick Steinhardt <psteinhardt@gitlab.com>2023-07-24 14:20:13 +0300
commit07fb2d1b6f9d5f093b14f100a5473c3839408b8a (patch)
treeaa0f0486ec160ebd3089f81785285366bf24f66e
parent5ad8502e3782818671ec79a94f6323fa440ba3eb (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.go10
-rw-r--r--internal/gitaly/hook/postreceive_test.go36
-rw-r--r--internal/gitaly/hook/prereceive_test.go36
-rw-r--r--internal/gitaly/hook/transactions.go7
-rw-r--r--internal/gitaly/hook/update_test.go35
-rw-r--r--internal/gitaly/service/hook/post_receive_test.go12
-rw-r--r--internal/gitaly/service/hook/pre_receive_test.go26
-rw-r--r--internal/gitaly/service/operations/apply_patch_test.go10
-rw-r--r--internal/gitaly/service/operations/branches_test.go17
-rw-r--r--internal/gitaly/service/operations/rebase_test.go12
-rw-r--r--internal/gitaly/service/operations/tags_test.go10
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
})
}