Merge branch 'pks-ff-enable-synchronized-hook-executions' into 'master'

hooks: Default-enable synchronized hook executions

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5989

Merged-by: Pavlo Strokov <pstrokov@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>

4 files changed, 20 insertions, 11 deletions

diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index cebbccb09..88b15e6a1 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -26,6 +26,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config/prometheus"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service/hook"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitlab"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/metadata"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/middleware/limithandler"
@@ -356,6 +357,8 @@ func testHooksUpdate(t *testing.T, ctx context.Context, cfg config.Cfg, glValues
 }
 
 func TestHooksPostReceiveFailed(t *testing.T) {
+	t.Parallel()
+
 	secretToken := "secret token"
 	glProtocol := "ssh"
 	changes := "oldhead newhead"
@@ -395,7 +398,9 @@ func TestHooksPostReceiveFailed(t *testing.T) {
 	gitlabClient, err := gitlab.NewHTTPClient(logger, cfg.Gitlab, cfg.TLS, prometheus.Config{})
 	require.NoError(t, err)
 
-	runHookServiceWithGitlabClient(t, cfg, true, gitlabClient)
+	txManager := transaction.NewTrackingManager()
+
+	runHookServiceWithGitlabClient(t, cfg, true, gitlabClient, testserver.WithTransactionManager(txManager))
 
 	customHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "post-receive")
 
@@ -418,6 +423,7 @@ func TestHooksPostReceiveFailed(t *testing.T) {
 				require.Empty(t, stdout.String())
 				require.Empty(t, stderr.String())
 				require.NoFileExists(t, customHookOutputPath)
+				require.Empty(t, txManager.Votes())
 			},
 		},
 		{
@@ -430,12 +436,15 @@ func TestHooksPostReceiveFailed(t *testing.T) {
 				require.Empty(t, stdout.String())
 				require.Empty(t, stderr.String())
 				require.NoFileExists(t, customHookOutputPath)
+				require.Len(t, txManager.Votes(), 1)
 			},
 		},
 	}
 
 	for _, tc := range testcases {
 		t.Run(tc.desc, func(t *testing.T) {
+			txManager.Reset()
+
 			hooksPayload, err := git.NewHooksPayload(
 				cfg,
 				repo,
diff --git a/internal/featureflag/ff_synchronize_hook_executions.go b/internal/featureflag/ff_synchronize_hook_executions.go
index 7dbed213e..305f8359f 100644
--- a/internal/featureflag/ff_synchronize_hook_executions.go
+++ b/internal/featureflag/ff_synchronize_hook_executions.go
@@ -6,5 +6,5 @@ var SynchronizeHookExecutions = NewFeatureFlag(
 	"synchronize_hook_executions",
 	"v16.1.0",
 	"https://gitlab.com/gitlab-org/gitaly/-/issues/5359",
-	false,
+	true,
 )
diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go
index f341884e6..f1303a797 100644
--- a/internal/gitaly/service/smarthttp/receive_pack_test.go
+++ b/internal/gitaly/service/smarthttp/receive_pack_test.go
@@ -794,7 +794,7 @@ func TestPostReceivePack_referenceTransactionHook(t *testing.T) {
 		requireSideband(t, []string{
 			"0049\x01000eunpack ok\n0019ok refs/heads/master\n0019ok refs/heads/branch\n0000",
 		}, response)
-		require.Equal(t, 5, refTransactionServer.called)
+		require.Equal(t, 9, refTransactionServer.called)
 	})
 
 	t.Run("delete", func(t *testing.T) {
@@ -829,7 +829,7 @@ func TestPostReceivePack_referenceTransactionHook(t *testing.T) {
 		requireSideband(t, []string{
 			"0033\x01000eunpack ok\n001cok refs/heads/delete-me\n0000",
 		}, response)
-		require.Equal(t, 3, refTransactionServer.called)
+		require.Equal(t, 6, refTransactionServer.called)
 	})
 }
 
diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go
index 2ab5bbb77..4b73c4ba2 100644
--- a/internal/gitaly/service/ssh/receive_pack_test.go
+++ b/internal/gitaly/service/ssh/receive_pack_test.go
@@ -493,7 +493,7 @@ func TestReceivePack_transactional(t *testing.T) {
 			expectedRefs: map[string]git.ObjectID{
 				"refs/heads/main": commitID,
 			},
-			expectedVotes: 3,
+			expectedVotes: 6,
 		},
 		{
 			desc:          "update",
@@ -508,7 +508,7 @@ func TestReceivePack_transactional(t *testing.T) {
 			expectedRefs: map[string]git.ObjectID{
 				"refs/heads/main": parentCommitID,
 			},
-			expectedVotes: 3,
+			expectedVotes: 6,
 		},
 		{
 			desc:          "creation",
@@ -523,7 +523,7 @@ func TestReceivePack_transactional(t *testing.T) {
 			expectedRefs: map[string]git.ObjectID{
 				"refs/heads/other": commitID,
 			},
-			expectedVotes: 3,
+			expectedVotes: 6,
 		},
 		{
 			desc: "deletion",
@@ -537,7 +537,7 @@ func TestReceivePack_transactional(t *testing.T) {
 			expectedRefs: map[string]git.ObjectID{
 				"refs/heads/other": gittest.DefaultObjectHash.ZeroOID,
 			},
-			expectedVotes: 3,
+			expectedVotes: 6,
 		},
 		{
 			desc:          "multiple commands",
@@ -558,7 +558,7 @@ func TestReceivePack_transactional(t *testing.T) {
 				"refs/heads/a": commitID,
 				"refs/heads/b": commitID,
 			},
-			expectedVotes: 5,
+			expectedVotes: 9,
 		},
 		{
 			desc:          "refused recreation of branch",
@@ -573,7 +573,7 @@ func TestReceivePack_transactional(t *testing.T) {
 			expectedRefs: map[string]git.ObjectID{
 				"refs/heads/a": commitID,
 			},
-			expectedVotes: 1,
+			expectedVotes: 3,
 		},
 		{
 			desc:          "refused recreation and successful delete",
@@ -593,7 +593,7 @@ func TestReceivePack_transactional(t *testing.T) {
 			expectedRefs: map[string]git.ObjectID{
 				"refs/heads/a": commitID,
 			},
-			expectedVotes: 3,
+			expectedVotes: 7,
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {