return not found error when getting a primary of a non-existent repo

This commit changes the PerRepositoryElector to return a not found
error when attempting to get the primary of a repository that does
not exist. This way we can return an appropriate error to Rails which
performs some calls to repositories that do not exist always.

2 files changed, 31 insertions, 9 deletions

diff --git a/internal/praefect/nodes/per_repository.go b/internal/praefect/nodes/per_repository.go
index 8e209f0d0..637a18f1a 100644
--- a/internal/praefect/nodes/per_repository.go
+++ b/internal/praefect/nodes/per_repository.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/lib/pq"
 	"github.com/sirupsen/logrus"
+	"gitlab.com/gitlab-org/gitaly/internal/praefect/commonerr"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/datastore/glsql"
 )
 
@@ -148,23 +149,26 @@ WHERE NOT EXISTS (
 
 // GetPrimary returns the primary storage of a repository.
 func (pr *PerRepositoryElector) GetPrimary(ctx context.Context, virtualStorage, relativePath string) (string, error) {
-	var primary string
+	var primary sql.NullString
 	if err := pr.db.QueryRowContext(ctx, `
 SELECT "primary"
 FROM repositories
 WHERE virtual_storage = $1
 AND relative_path = $2
-AND "primary" IS NOT NULL
 		`,
 		virtualStorage,
 		relativePath,
 	).Scan(&primary); err != nil {
 		if errors.Is(err, sql.ErrNoRows) {
-			return "", ErrNoPrimary
+			return "", commonerr.NewRepositoryNotFoundError(virtualStorage, relativePath)
 		}
 
 		return "", fmt.Errorf("scan: %w", err)
 	}
 
-	return primary, nil
+	if !primary.Valid {
+		return "", ErrNoPrimary
+	}
+
+	return primary.String, nil
 }
diff --git a/internal/praefect/nodes/per_repository_test.go b/internal/praefect/nodes/per_repository_test.go
index 2c5c5bca2..d416f8998 100644
--- a/internal/praefect/nodes/per_repository_test.go
+++ b/internal/praefect/nodes/per_repository_test.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/praefect/commonerr"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/datastore"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/datastore/glsql"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
@@ -99,6 +100,13 @@ func TestPerRepositoryElector(t *testing.T) {
 		},
 		{
 			desc: "no valid primary",
+			state: state{
+				"virtual-storage-1": {
+					"relative-path-1": {
+						"gitaly-1": {generation: 0},
+					},
+				},
+			},
 			steps: steps{
 				{
 					healthyNodes: map[string][]string{
@@ -395,18 +403,28 @@ func TestPerRepositoryElector(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc: "repository does not exist",
+			steps: steps{
+				{
+					error:   commonerr.NewRepositoryNotFoundError("virtual-storage-1", "relative-path-1"),
+					primary: noPrimary(),
+				},
+			},
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			db := getDB(t)
 
-			_, err := db.ExecContext(ctx,
-				`INSERT INTO repositories (virtual_storage, relative_path) VALUES ('virtual-storage-1', 'relative-path-1')`,
-			)
-			require.NoError(t, err)
-
 			rs := datastore.NewPostgresRepositoryStore(db, nil)
 			for virtualStorage, relativePaths := range tc.state {
 				for relativePath, storages := range relativePaths {
+					_, err := db.ExecContext(ctx,
+						`INSERT INTO repositories (virtual_storage, relative_path) VALUES ($1, $2)`,
+						virtualStorage, relativePath,
+					)
+					require.NoError(t, err)
+
 					for storage, record := range storages {
 						require.NoError(t, rs.SetGeneration(ctx, virtualStorage, relativePath, storage, record.generation))