Merge branch 'smh-test-with-per-repo' into 'master'

Use PerRepositoryElector in test-with-praefect

See merge request gitlab-org/gitaly!3914

4 files changed, 37 insertions, 17 deletions

diff --git a/internal/gitaly/service/namespace/namespace_test.go b/internal/gitaly/service/namespace/namespace_test.go
index 56873a6c1..8adab217a 100644
--- a/internal/gitaly/service/namespace/namespace_test.go
+++ b/internal/gitaly/service/namespace/namespace_test.go
@@ -101,6 +101,8 @@ func getStorageDir(t *testing.T, cfg config.Cfg, storageName string) string {
 }
 
 func TestAddNamespace(t *testing.T) {
+	testhelper.SkipWithPraefect(t, "per_repository election strategy doesn't support storage scoped mutators")
+
 	cfg, client := setupNamespaceService(t)
 	existingStorage := cfg.Storages[0]
 
@@ -163,6 +165,8 @@ func TestAddNamespace(t *testing.T) {
 }
 
 func TestRemoveNamespace(t *testing.T) {
+	testhelper.SkipWithPraefect(t, "per_repository election strategy doesn't support storage scoped mutators")
+
 	cfg, client := setupNamespaceService(t)
 	existingStorage := cfg.Storages[0]
 
@@ -217,6 +221,8 @@ func TestRemoveNamespace(t *testing.T) {
 }
 
 func TestRenameNamespace(t *testing.T) {
+	testhelper.SkipWithPraefect(t, "per_repository election strategy doesn't support storage scoped mutators")
+
 	cfg, client := setupNamespaceService(t)
 	existingStorage := cfg.Storages[0]
 
@@ -285,6 +291,8 @@ func TestRenameNamespace(t *testing.T) {
 }
 
 func TestRenameNamespaceWithNonexistentParentDir(t *testing.T) {
+	testhelper.SkipWithPraefect(t, "per_repository election strategy doesn't support storage scoped mutators")
+
 	cfg, client := setupNamespaceService(t)
 	existingStorage := cfg.Storages[0]
 
diff --git a/internal/praefect/coordinator.go b/internal/praefect/coordinator.go
index 938f47fe7..0cfc43782 100644
--- a/internal/praefect/coordinator.go
+++ b/internal/praefect/coordinator.go
@@ -331,20 +331,27 @@ func (c *Coordinator) directRepositoryScopedMessage(ctx context.Context, call gr
 	var ps *proxy.StreamParameters
 
 	if c.forceCreateRepositories {
-		// This is a hack for the tests: during execution of the gitaly tests under praefect proxy
-		// the repositories are created directly on the filesystem. There is no call for the
-		// CreateRepository that creates records in the database that is why we do it artificially
-		// before redirecting the calls.
-		id, err := c.rs.ReserveRepositoryID(ctx, call.targetRepo.StorageName, call.targetRepo.RelativePath)
+		replicationType, _, err := getReplicationDetails(call.fullMethodName, call.msg)
 		if err != nil {
-			if !errors.Is(err, commonerr.ErrRepositoryAlreadyExists) {
-				return nil, err
-			}
-		} else {
-			if err := c.rs.CreateRepository(ctx, id, call.targetRepo.StorageName, call.targetRepo.RelativePath, call.targetRepo.StorageName, nil, nil, true, true); err != nil {
-				if !errors.As(err, &datastore.RepositoryExistsError{}) {
+			return nil, err
+		}
+
+		if replicationType != datastore.CreateRepo {
+			// This is a hack for the tests: during execution of the gitaly tests under praefect proxy
+			// the repositories are created directly on the filesystem. There is no call for the
+			// CreateRepository that creates records in the database that is why we do it artificially
+			// before redirecting the calls.
+			id, err := c.rs.ReserveRepositoryID(ctx, call.targetRepo.StorageName, call.targetRepo.RelativePath)
+			if err != nil {
+				if !errors.Is(err, commonerr.ErrRepositoryAlreadyExists) {
 					return nil, err
 				}
+			} else {
+				if err := c.rs.CreateRepository(ctx, id, call.targetRepo.StorageName, call.targetRepo.RelativePath, call.targetRepo.StorageName, nil, nil, true, true); err != nil {
+					if !errors.As(err, &datastore.RepositoryExistsError{}) {
+						return nil, err
+					}
+				}
 			}
 		}
 	}
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 1c452e85a..3d87fd898 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -47,6 +47,14 @@ const (
 	DefaultStorageName = "default"
 )
 
+// SkipWithPraefect skips the test if it is being executed with Praefect in front
+// of the Gitaly.
+func SkipWithPraefect(t testing.TB, reason string) {
+	if os.Getenv("GITALY_TEST_WITH_PRAEFECT") == "YesPlease" {
+		t.Skipf(reason)
+	}
+}
+
 // MustReadFile returns the content of a file or fails at once.
 func MustReadFile(t testing.TB, filename string) []byte {
 	content, err := os.ReadFile(filename)
diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go
index 0c7ed2645..7dac962f3 100644
--- a/internal/testhelper/testserver/gitaly.go
+++ b/internal/testhelper/testserver/gitaly.go
@@ -75,17 +75,14 @@ func runPraefectProxy(t testing.TB, cfg config.Cfg, gitalyAddr, praefectBinPath
 	dbName := createDatabase(t)
 
 	conf := praefectconfig.Config{
-		AllowLegacyElectors: true,
-		SocketPath:          praefectServerSocketPath,
+		SocketPath: praefectServerSocketPath,
 		Auth: auth.Config{
 			Token: cfg.Auth.Token,
 		},
 		DB: glsql.GetDBConfig(t, dbName),
 		Failover: praefectconfig.Failover{
-			Enabled:           true,
-			ElectionStrategy:  praefectconfig.ElectionStrategyLocal,
-			BootstrapInterval: config.Duration(time.Microsecond),
-			MonitorInterval:   config.Duration(time.Second),
+			Enabled:          true,
+			ElectionStrategy: praefectconfig.ElectionStrategyLocal,
 		},
 		Replication: praefectconfig.DefaultReplicationConfig(),
 		Logging: gitalylog.Config{