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-06-06 16:22:33 +0300
committerPatrick Steinhardt <psteinhardt@gitlab.com>2023-06-07 13:40:06 +0300
commit6b52f52bfe4bfa3449d6892493cf19dd01531028 (patch)
treed0d74ed4966d89a4df2dab0fa0c177194788cf85
parent709ffb628fce42a5f426ff7743555ab9a47ecd52 (diff)
commonerr: Move repository-related errors into storage package
The commonerr package bundles a set of common errors that we use across the project. Right now the current set only includes storage-related errors though that relate to repositories being missing or invalid. And while we use these common errors only in the Praefect codebase right now, it would make sense to have Praefect and Gitaly share these errors so that we ideally have a single source of truth for these. Move the errors into the storage package so that it becomes less awkward to share them between Gitaly and Praefect. This feels like a natural place for these errors given that the errors can be directly attributed to the storage level.
-rw-r--r--internal/cli/praefect/subcmd_track_repository.go4
-rw-r--r--internal/gitaly/storage/locator.go37
-rw-r--r--internal/praefect/commonerr/error.go35
-rw-r--r--internal/praefect/coordinator.go10
-rw-r--r--internal/praefect/coordinator_test.go12
-rw-r--r--internal/praefect/datastore/assignment_test.go6
-rw-r--r--internal/praefect/datastore/repository_store.go38
-rw-r--r--internal/praefect/datastore/repository_store_test.go28
-rw-r--r--internal/praefect/datastore/storage_provider_test.go4
-rw-r--r--internal/praefect/nodes/manager.go4
-rw-r--r--internal/praefect/nodes/per_repository.go4
-rw-r--r--internal/praefect/nodes/per_repository_test.go6
-rw-r--r--internal/praefect/reconciler/reconciler_test.go6
-rw-r--r--internal/praefect/remove_repository.go6
-rw-r--r--internal/praefect/rename_repository.go5
-rw-r--r--internal/praefect/router_node_manager.go4
-rw-r--r--internal/praefect/router_per_repository_test.go3
-rw-r--r--internal/praefect/service/info/metadata.go4
-rw-r--r--internal/praefect/service/info/server.go4
19 files changed, 107 insertions, 113 deletions
diff --git a/internal/cli/praefect/subcmd_track_repository.go b/internal/cli/praefect/subcmd_track_repository.go
index 1c28cbe28..d8381194b 100644
--- a/internal/cli/praefect/subcmd_track_repository.go
+++ b/internal/cli/praefect/subcmd_track_repository.go
@@ -12,10 +12,10 @@ import (
"github.com/sirupsen/logrus"
"github.com/urfave/cli/v2"
glcli "gitlab.com/gitlab-org/gitaly/v16/internal/cli"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/middleware/metadatahandler"
"gitlab.com/gitlab-org/gitaly/v16/internal/log"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/config"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore/glsql"
@@ -275,7 +275,7 @@ func (req *trackRepositoryRequest) trackRepository(
) (int64, error) {
repositoryID, err := ds.ReserveRepositoryID(ctx, req.VirtualStorage, req.RelativePath)
if err != nil {
- if errors.Is(err, commonerr.ErrRepositoryAlreadyExists) {
+ if errors.Is(err, storage.ErrRepositoryAlreadyExists) {
fmt.Fprintf(w, "repository is already tracked in praefect database")
return 0, nil
}
diff --git a/internal/gitaly/storage/locator.go b/internal/gitaly/storage/locator.go
index 7aaa2239f..9815b9342 100644
--- a/internal/gitaly/storage/locator.go
+++ b/internal/gitaly/storage/locator.go
@@ -9,6 +9,40 @@ import (
"strings"
)
+// RepositoryNotFoundError is returned when attempting to operate on a repository that does not
+// exist in the virtual storage.
+type RepositoryNotFoundError struct {
+ virtualStorage string
+ relativePath string
+}
+
+// NewRepositoryNotFoundError returns a new repository not found error for the given repository.
+func NewRepositoryNotFoundError(virtualStorage string, relativePath string) error {
+ return RepositoryNotFoundError{virtualStorage: virtualStorage, relativePath: relativePath}
+}
+
+// Error returns the error message.
+func (err RepositoryNotFoundError) Error() string {
+ return fmt.Sprintf("repository %q/%q not found", err.virtualStorage, err.relativePath)
+}
+
+var (
+ // ErrRepositoryNotFound is returned when operating on a repository that doesn't exist.
+ //
+ // This somewhat duplicates the above RepositoryNotFoundError but doesn't specify which
+ // repository was not found. With repository IDs in use, the virtual storage and relative
+ // path won't be available everywhere anymore.
+ ErrRepositoryNotFound = errors.New("repository not found")
+
+ // ErrRepositoryAlreadyExists is returned when attempting to create a repository that
+ // already exists.
+ ErrRepositoryAlreadyExists = errors.New("repository already exists")
+
+ // ErrRelativePathEscapesRoot is returned when a relative path is passed that escapes the
+ // storage's root directory.
+ ErrRelativePathEscapesRoot = errors.New("relative path escapes root directory")
+)
+
// Repository represents a storage-scoped repository.
type Repository interface {
GetStorageName() string
@@ -61,9 +95,6 @@ func WithRepositoryVerificationSkipped() GetRepoPathOption {
}
}
-//nolint:revive // This is unintentionally missing documentation.
-var ErrRelativePathEscapesRoot = errors.New("relative path escapes root directory")
-
// ValidateRelativePath validates a relative path by joining it with rootDir and verifying the result
// is either rootDir or a path within rootDir. Returns clean relative path from rootDir to relativePath
// or an ErrRelativePathEscapesRoot if the resulting path is not contained within rootDir.
diff --git a/internal/praefect/commonerr/error.go b/internal/praefect/commonerr/error.go
deleted file mode 100644
index d113f4485..000000000
--- a/internal/praefect/commonerr/error.go
+++ /dev/null
@@ -1,35 +0,0 @@
-// Package commonerr contains common errors between different Praefect components. Ideally
-// this package's contents would be in praefect's root package but this is not possible at the moment
-// due to cyclic imports.
-package commonerr
-
-import (
- "errors"
- "fmt"
-)
-
-// RepositoryNotFoundError is returned when attempting to operate on a repository
-// that does not exist in the virtual storage.
-type RepositoryNotFoundError struct {
- virtualStorage string
- relativePath string
-}
-
-// NewRepositoryNotFoundError returns a new repository not found error for the given repository.
-func NewRepositoryNotFoundError(virtualStorage string, relativePath string) error {
- return RepositoryNotFoundError{virtualStorage: virtualStorage, relativePath: relativePath}
-}
-
-// Error returns the error message.
-func (err RepositoryNotFoundError) Error() string {
- return fmt.Sprintf("repository %q/%q not found", err.virtualStorage, err.relativePath)
-}
-
-// ErrRepositoryNotFound is returned when operating on a repository that doesn't exist.
-//
-// This somewhat duplicates the above RepositoryNotFoundError but doesn't specify which repository was not found.
-// With repository IDs in use, the virtual storage and relative path won't be available everywhere anymore.
-var ErrRepositoryNotFound = errors.New("repository not found")
-
-// ErrRepositoryAlreadyExists is returned when attempting to create a repository that already exists.
-var ErrRepositoryAlreadyExists = errors.New("repository already exists")
diff --git a/internal/praefect/coordinator.go b/internal/praefect/coordinator.go
index 24eb35825..bab8957c0 100644
--- a/internal/praefect/coordinator.go
+++ b/internal/praefect/coordinator.go
@@ -12,11 +12,11 @@ import (
"github.com/sirupsen/logrus"
gitalyerrors "gitlab.com/gitlab-org/gitaly/v16/internal/errors"
"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/metadata"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/middleware/metadatahandler"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/proxy"
"gitlab.com/gitlab-org/gitaly/v16/internal/helper"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/config"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/metrics"
@@ -263,11 +263,11 @@ func (c *Coordinator) directRepositoryScopedMessage(ctx context.Context, call gr
}
if err != nil {
- if errors.As(err, new(commonerr.RepositoryNotFoundError)) {
+ if errors.As(err, new(storage.RepositoryNotFoundError)) {
return nil, structerr.NewNotFound("%w", err)
}
- if errors.Is(err, commonerr.ErrRepositoryNotFound) {
+ if errors.Is(err, storage.ErrRepositoryNotFound) {
return nil, structerr.NewNotFound("%w", err)
}
@@ -407,7 +407,7 @@ func (c *Coordinator) mutatorStreamParameters(ctx context.Context, call grpcCall
// ReplicateRepository RPC should also be able to replicate if repository ID already exists in Praefect.
if call.fullMethodName == "/gitaly.RepositoryService/ReplicateRepository" &&
- errors.Is(err, commonerr.ErrRepositoryAlreadyExists) {
+ errors.Is(err, storage.ErrRepositoryAlreadyExists) {
change = datastore.UpdateRepo
route, err = c.router.RouteRepositoryMutator(ctx, virtualStorage, targetRepo.RelativePath, additionalRepoRelativePath)
}
@@ -702,7 +702,7 @@ func (c *Coordinator) StreamDirector(ctx context.Context, fullMethodName string,
return nil, structerr.NewInvalidArgument("%w", err)
}
- if errors.Is(err, commonerr.ErrRepositoryAlreadyExists) {
+ if errors.Is(err, storage.ErrRepositoryAlreadyExists) {
return nil, structerr.NewAlreadyExists("%w", err)
}
diff --git a/internal/praefect/coordinator_test.go b/internal/praefect/coordinator_test.go
index d843f382a..150b87ece 100644
--- a/internal/praefect/coordinator_test.go
+++ b/internal/praefect/coordinator_test.go
@@ -25,10 +25,10 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
gconfig "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
gitaly_metadata "gitlab.com/gitlab-org/gitaly/v16/internal/grpc/metadata"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/middleware/metadatahandler"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/proxy"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/config"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/nodes"
@@ -303,7 +303,7 @@ func TestStreamDirectorMutator(t *testing.T) {
request: &gitalypb.UserCreateTagRequest{
Repository: targetRepo,
},
- expectedErr: structerr.NewNotFound("%w", fmt.Errorf("mutator call: route repository mutator: %w", fmt.Errorf("get repository id: %w", commonerr.NewRepositoryNotFoundError(targetRepo.StorageName, targetRepo.RelativePath)))),
+ expectedErr: structerr.NewNotFound("%w", fmt.Errorf("mutator call: route repository mutator: %w", fmt.Errorf("get repository id: %w", storage.NewRepositoryNotFoundError(targetRepo.StorageName, targetRepo.RelativePath)))),
}
},
},
@@ -334,7 +334,7 @@ func TestStreamDirectorMutator(t *testing.T) {
expectedErr: structerr.NewNotFound("%w", fmt.Errorf("mutator call: route repository mutator: %w",
fmt.Errorf("resolve additional replica path: %w",
fmt.Errorf("get additional repository id: %w",
- commonerr.NewRepositoryNotFoundError(additionalRepo.StorageName, additionalRepo.RelativePath),
+ storage.NewRepositoryNotFoundError(additionalRepo.StorageName, additionalRepo.RelativePath),
),
),
)),
@@ -696,7 +696,7 @@ func TestStreamDirectorMutator_ReplicateRepository(t *testing.T) {
router := mockRouter{
// Simulate scenario where target repository already exists and error is returned.
routeRepositoryCreation: func(ctx context.Context, virtualStorage, relativePath, additionalRepoRelativePath string) (RepositoryMutatorRoute, error) {
- return RepositoryMutatorRoute{}, fmt.Errorf("reserve repository id: %w", commonerr.ErrRepositoryAlreadyExists)
+ return RepositoryMutatorRoute{}, fmt.Errorf("reserve repository id: %w", storage.ErrRepositoryAlreadyExists)
},
// Pass through normally to handle route creation.
routeRepositoryMutator: func(ctx context.Context, virtualStorage, relativePath, additionalRepoRelativePath string) (RepositoryMutatorRoute, error) {
@@ -1086,10 +1086,10 @@ func TestStreamDirectorAccessor(t *testing.T) {
desc: "repository not found",
router: mockRouter{
routeRepositoryAccessorFunc: func(_ context.Context, virtualStorage, relativePath string, _ bool) (RepositoryAccessorRoute, error) {
- return RepositoryAccessorRoute{}, commonerr.NewRepositoryNotFoundError(virtualStorage, relativePath)
+ return RepositoryAccessorRoute{}, storage.NewRepositoryNotFoundError(virtualStorage, relativePath)
},
},
- error: structerr.NewNotFound("%w", fmt.Errorf("accessor call: route repository accessor: %w", commonerr.NewRepositoryNotFoundError(targetRepo.StorageName, targetRepo.RelativePath))),
+ error: structerr.NewNotFound("%w", fmt.Errorf("accessor call: route repository accessor: %w", storage.NewRepositoryNotFoundError(targetRepo.StorageName, targetRepo.RelativePath))),
},
} {
t.Run(tc.desc, func(t *testing.T) {
diff --git a/internal/praefect/datastore/assignment_test.go b/internal/praefect/datastore/assignment_test.go
index 1f97fe40e..73ec3c8d8 100644
--- a/internal/praefect/datastore/assignment_test.go
+++ b/internal/praefect/datastore/assignment_test.go
@@ -6,7 +6,7 @@ import (
"testing"
"github.com/stretchr/testify/require"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore/glsql"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testdb"
@@ -83,7 +83,7 @@ func TestAssignmentStore_GetHostAssignments(t *testing.T) {
rs := NewPostgresRepositoryStore(db, nil)
for _, assignment := range tc.existingAssignments {
repositoryID, err := rs.GetRepositoryID(ctx, assignment.virtualStorage, assignment.relativePath)
- if errors.Is(err, commonerr.NewRepositoryNotFoundError(assignment.virtualStorage, assignment.relativePath)) {
+ if errors.Is(err, storage.NewRepositoryNotFoundError(assignment.virtualStorage, assignment.relativePath)) {
repositoryID, err = rs.ReserveRepositoryID(ctx, assignment.virtualStorage, assignment.relativePath)
require.NoError(t, err)
@@ -99,7 +99,7 @@ func TestAssignmentStore_GetHostAssignments(t *testing.T) {
repositoryID, err := rs.GetRepositoryID(ctx, tc.virtualStorage, "relative-path")
if err != nil {
- require.Equal(t, commonerr.NewRepositoryNotFoundError(tc.virtualStorage, "relative-path"), err)
+ require.Equal(t, storage.NewRepositoryNotFoundError(tc.virtualStorage, "relative-path"), err)
}
actualAssignments, err := NewAssignmentStore(
diff --git a/internal/praefect/datastore/repository_store.go b/internal/praefect/datastore/repository_store.go
index 968daf15a..c6a41dff3 100644
--- a/internal/praefect/datastore/repository_store.go
+++ b/internal/praefect/datastore/repository_store.go
@@ -10,7 +10,7 @@ import (
"time"
"gitlab.com/gitlab-org/gitaly/v16/internal/datastructure"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore/glsql"
)
@@ -90,7 +90,7 @@ type RepositoryStore interface {
// SetGeneration sets the repository's generation on the given storage. If the generation is higher
// than the virtual storage's generation, it is set to match as well to guarantee monotonic increments.
SetGeneration(ctx context.Context, repositoryID int64, storage, relativePath string, generation int) error
- // GetReplicaPath gets the replica path of a repository. Returns a commonerr.ErrRepositoryNotFound if a record
+ // GetReplicaPath gets the replica path of a repository. Returns a storage.ErrRepositoryNotFound if a record
// for the repository ID is not found.
GetReplicaPath(ctx context.Context, repositoryID int64) (string, error)
// GetReplicatedGeneration returns the generation propagated by applying the replication. If the generation would
@@ -110,7 +110,7 @@ type RepositoryStore interface {
// SetAuthoritativeReplica sets the given replica of a repsitory as the authoritative one by setting its generation as the latest one.
SetAuthoritativeReplica(ctx context.Context, virtualStorage, relativePath, storage string) error
// DeleteRepository deletes the database records associated with the repository. It returns the replica path and the storages
- // which are known to have a replica at the time of deletion. commonerr.RepositoryNotFoundError is returned when
+ // which are known to have a replica at the time of deletion. storage.RepositoryNotFoundError is returned when
// the repository is not tracked by the Praefect datastore.
DeleteRepository(ctx context.Context, virtualStorage, relativePath string) (string, []string, error)
// DeleteAllRepositories deletes the database records associated with
@@ -282,7 +282,7 @@ SELECT
}
if !repositoryExists {
- return commonerr.ErrRepositoryNotFound
+ return storage.ErrRepositoryNotFound
}
if !repositoryUpdated {
@@ -326,7 +326,7 @@ ON CONFLICT (repository_id, storage) DO UPDATE SET
}
// SetAuthoritativeReplica sets the given replica of a repsitory as the authoritative one by setting its generation as the latest one.
-func (rs *PostgresRepositoryStore) SetAuthoritativeReplica(ctx context.Context, virtualStorage, relativePath, storage string) error {
+func (rs *PostgresRepositoryStore) SetAuthoritativeReplica(ctx context.Context, virtualStorage, relativePath, storageName string) error {
result, err := rs.db.ExecContext(ctx, `
WITH updated_repository AS (
UPDATE repositories
@@ -342,7 +342,7 @@ FROM updated_repository
ON CONFLICT (virtual_storage, relative_path, storage) DO UPDATE
SET repository_id = EXCLUDED.repository_id,
generation = EXCLUDED.generation
- `, virtualStorage, relativePath, storage)
+ `, virtualStorage, relativePath, storageName)
if err != nil {
return fmt.Errorf("exec: %w", err)
}
@@ -350,7 +350,7 @@ ON CONFLICT (virtual_storage, relative_path, storage) DO UPDATE
if rowsAffected, err := result.RowsAffected(); err != nil {
return fmt.Errorf("rows affected: %w", err)
} else if rowsAffected == 0 {
- return commonerr.NewRepositoryNotFoundError(virtualStorage, relativePath)
+ return storage.NewRepositoryNotFoundError(virtualStorage, relativePath)
}
return nil
@@ -513,7 +513,7 @@ GROUP BY replica_path
`, virtualStorage, relativePath,
).Scan(&replicaPath, &storages); err != nil {
if errors.Is(err, sql.ErrNoRows) {
- return "", nil, commonerr.NewRepositoryNotFoundError(virtualStorage, relativePath)
+ return "", nil, storage.NewRepositoryNotFoundError(virtualStorage, relativePath)
}
return "", nil, fmt.Errorf("scan: %w", err)
@@ -590,7 +590,7 @@ WHERE repository_id = (SELECT repository_id FROM repository)
`, virtualStorage, relativePath, newRelativePath)
if err != nil {
if glsql.IsUniqueViolation(err, "repository_lookup_index") {
- return commonerr.ErrRepositoryAlreadyExists
+ return storage.ErrRepositoryAlreadyExists
}
return fmt.Errorf("query: %w", err)
@@ -599,7 +599,7 @@ WHERE repository_id = (SELECT repository_id FROM repository)
if rowsAffected, err := result.RowsAffected(); err != nil {
return fmt.Errorf("rows affected: %w", err)
} else if rowsAffected == 0 {
- return commonerr.ErrRepositoryNotFound
+ return storage.ErrRepositoryNotFound
}
return nil
@@ -662,8 +662,8 @@ WHERE repositories.virtual_storage = $1
AND repositories.relative_path = $2
GROUP BY replica_path
`, virtualStorage, relativePath)
- if errors.Is(err, commonerr.ErrRepositoryNotFound) {
- return "", nil, commonerr.NewRepositoryNotFoundError(virtualStorage, relativePath)
+ if errors.Is(err, storage.ErrRepositoryNotFound) {
+ return "", nil, storage.NewRepositoryNotFoundError(virtualStorage, relativePath)
}
return replicaPath, storages, err
@@ -676,7 +676,7 @@ func (rs *PostgresRepositoryStore) getConsistentStorages(ctx context.Context, qu
if err := rs.db.QueryRowContext(ctx, query, params...).Scan(&replicaPath, &storages); err != nil {
if errors.Is(err, sql.ErrNoRows) {
- return "", nil, commonerr.ErrRepositoryNotFound
+ return "", nil, storage.ErrRepositoryNotFound
}
return "", nil, fmt.Errorf("query: %w", err)
@@ -787,7 +787,7 @@ func (rs *PostgresRepositoryStore) GetRepositoryMetadata(ctx context.Context, re
}
if len(metadata) == 0 {
- return RepositoryMetadata{}, commonerr.ErrRepositoryNotFound
+ return RepositoryMetadata{}, storage.ErrRepositoryNotFound
}
return metadata[0], nil
@@ -808,7 +808,7 @@ func (rs *PostgresRepositoryStore) GetRepositoryMetadataByPath(ctx context.Conte
}
if len(metadata) == 0 {
- return RepositoryMetadata{}, commonerr.ErrRepositoryNotFound
+ return RepositoryMetadata{}, storage.ErrRepositoryNotFound
}
return metadata[0], nil
@@ -990,7 +990,7 @@ WHERE NOT EXISTS (
)
`, virtualStorage, relativePath).Scan(&id); err != nil {
if errors.Is(err, sql.ErrNoRows) {
- return 0, commonerr.ErrRepositoryAlreadyExists
+ return 0, storage.ErrRepositoryAlreadyExists
}
return 0, fmt.Errorf("scan: %w", err)
@@ -1010,7 +1010,7 @@ WHERE virtual_storage = $1
AND relative_path = $2
`, virtualStorage, relativePath).Scan(&id); err != nil {
if errors.Is(err, sql.ErrNoRows) {
- return 0, commonerr.NewRepositoryNotFoundError(virtualStorage, relativePath)
+ return 0, storage.NewRepositoryNotFoundError(virtualStorage, relativePath)
}
return 0, fmt.Errorf("scan: %w", err)
@@ -1019,7 +1019,7 @@ AND relative_path = $2
return id, nil
}
-// GetReplicaPath gets the replica path of a repository. Returns a commonerr.ErrRepositoryNotFound if a record
+// GetReplicaPath gets the replica path of a repository. Returns a storage.ErrRepositoryNotFound if a record
// for the repository ID is not found.
func (rs *PostgresRepositoryStore) GetReplicaPath(ctx context.Context, repositoryID int64) (string, error) {
var replicaPath string
@@ -1027,7 +1027,7 @@ func (rs *PostgresRepositoryStore) GetReplicaPath(ctx context.Context, repositor
ctx, "SELECT replica_path FROM repositories WHERE repository_id = $1", repositoryID,
).Scan(&replicaPath); err != nil {
if errors.Is(err, sql.ErrNoRows) {
- return "", commonerr.ErrRepositoryNotFound
+ return "", storage.ErrRepositoryNotFound
}
return "", fmt.Errorf("scan: %w", err)
diff --git a/internal/praefect/datastore/repository_store_test.go b/internal/praefect/datastore/repository_store_test.go
index c1a59cb98..5ca18d27b 100644
--- a/internal/praefect/datastore/repository_store_test.go
+++ b/internal/praefect/datastore/repository_store_test.go
@@ -8,7 +8,7 @@ import (
"time"
"github.com/stretchr/testify/require"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore/glsql"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testdb"
@@ -133,7 +133,7 @@ func TestRepositoryStore_Postgres(t *testing.T) {
require.Equal(t,
rs.IncrementGeneration(ctx, 1, "primary", []string{"secondary-1"}),
- commonerr.ErrRepositoryNotFound,
+ storage.ErrRepositoryNotFound,
)
requireState(t, ctx, db, virtualStorageState{}, storageState{})
})
@@ -358,7 +358,7 @@ func TestRepositoryStore_Postgres(t *testing.T) {
rs := newRepositoryStore(t, nil)
require.Equal(t,
- commonerr.NewRepositoryNotFoundError(vs, repo),
+ storage.NewRepositoryNotFoundError(vs, repo),
rs.SetAuthoritativeReplica(ctx, vs, repo, stor),
)
})
@@ -752,7 +752,7 @@ func TestRepositoryStore_Postgres(t *testing.T) {
rs := newRepositoryStore(t, nil)
replicaPath, storages, err := rs.DeleteRepository(ctx, vs, repo)
- require.Equal(t, commonerr.NewRepositoryNotFoundError(vs, repo), err)
+ require.Equal(t, storage.NewRepositoryNotFoundError(vs, repo), err)
require.Empty(t, replicaPath)
require.Empty(t, storages)
})
@@ -898,7 +898,7 @@ func TestRepositoryStore_Postgres(t *testing.T) {
rs := newRepositoryStore(t, nil)
require.Equal(t,
- commonerr.ErrRepositoryNotFound,
+ storage.ErrRepositoryNotFound,
rs.RenameRepositoryInPlace(ctx, vs, repo, "new-relative-path"),
)
})
@@ -910,7 +910,7 @@ func TestRepositoryStore_Postgres(t *testing.T) {
require.NoError(t, rs.CreateRepository(ctx, 2, vs, "relative-path-2", "replica-path-2", "primary", nil, nil, true, false))
require.Equal(t,
- commonerr.ErrRepositoryAlreadyExists,
+ storage.ErrRepositoryAlreadyExists,
rs.RenameRepositoryInPlace(ctx, vs, "relative-path-1", "relative-path-2"),
)
})
@@ -1007,12 +1007,12 @@ func TestRepositoryStore_Postgres(t *testing.T) {
t.Run("no records", func(t *testing.T) {
replicaPath, secondaries, err := rs.GetConsistentStorages(ctx, vs, repo)
- require.Equal(t, commonerr.NewRepositoryNotFoundError(vs, repo), err)
+ require.Equal(t, storage.NewRepositoryNotFoundError(vs, repo), err)
require.Empty(t, replicaPath)
require.Empty(t, secondaries)
replicaPath, secondaries, err = rs.GetConsistentStoragesByRepositoryID(ctx, 1)
- require.Equal(t, commonerr.ErrRepositoryNotFound, err)
+ require.Equal(t, storage.ErrRepositoryNotFound, err)
require.Empty(t, replicaPath)
require.Empty(t, secondaries)
})
@@ -1101,12 +1101,12 @@ func TestRepositoryStore_Postgres(t *testing.T) {
requireState(t, ctx, db, virtualStorageState{}, storageState{})
replicaPath, secondaries, err := rs.GetConsistentStorages(ctx, vs, repo)
- require.Equal(t, commonerr.NewRepositoryNotFoundError(vs, repo), err)
+ require.Equal(t, storage.NewRepositoryNotFoundError(vs, repo), err)
require.Empty(t, secondaries)
require.Empty(t, replicaPath)
replicaPath, secondaries, err = rs.GetConsistentStoragesByRepositoryID(ctx, 1)
- require.Equal(t, commonerr.ErrRepositoryNotFound, err)
+ require.Equal(t, storage.ErrRepositoryNotFound, err)
require.Empty(t, secondaries)
require.Empty(t, replicaPath)
})
@@ -1208,7 +1208,7 @@ func TestRepositoryStore_Postgres(t *testing.T) {
require.NoError(t, rs.CreateRepository(ctx, id, vs, repo, "replica-path", stor, nil, nil, false, false))
id, err = rs.ReserveRepositoryID(ctx, vs, repo)
- require.Equal(t, commonerr.ErrRepositoryAlreadyExists, err)
+ require.Equal(t, storage.ErrRepositoryAlreadyExists, err)
require.Equal(t, int64(0), id)
id, err = rs.ReserveRepositoryID(ctx, vs, repo+"-2")
@@ -1220,7 +1220,7 @@ func TestRepositoryStore_Postgres(t *testing.T) {
rs := newRepositoryStore(t, nil)
id, err := rs.GetRepositoryID(ctx, vs, repo)
- require.Equal(t, commonerr.NewRepositoryNotFoundError(vs, repo), err)
+ require.Equal(t, storage.NewRepositoryNotFoundError(vs, repo), err)
require.Equal(t, int64(0), id)
require.NoError(t, rs.CreateRepository(ctx, 1, vs, repo, "replica-path", stor, nil, nil, false, false))
@@ -1234,7 +1234,7 @@ func TestRepositoryStore_Postgres(t *testing.T) {
rs := newRepositoryStore(t, nil)
replicaPath, err := rs.GetReplicaPath(ctx, 1)
- require.Equal(t, err, commonerr.ErrRepositoryNotFound)
+ require.Equal(t, err, storage.ErrRepositoryNotFound)
require.Empty(t, replicaPath)
require.NoError(t, rs.CreateRepository(ctx, 1, vs, repo, "replica-path", stor, nil, nil, false, false))
@@ -1669,7 +1669,7 @@ func TestPostgresRepositoryStore_GetRepositoryMetadata(t *testing.T) {
var expectedErr error
if tc.nonExistentRepository {
- expectedErr = commonerr.ErrRepositoryNotFound
+ expectedErr = storage.ErrRepositoryNotFound
expectedMetadata = RepositoryMetadata{}
}
diff --git a/internal/praefect/datastore/storage_provider_test.go b/internal/praefect/datastore/storage_provider_test.go
index 410195c8a..abe683fbe 100644
--- a/internal/praefect/datastore/storage_provider_test.go
+++ b/internal/praefect/datastore/storage_provider_test.go
@@ -16,7 +16,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitaly/v16/internal/datastructure"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore/glsql"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testdb"
@@ -101,7 +101,7 @@ func TestCachingStorageProvider_GetSyncedNodes(t *testing.T) {
cache.Connected()
_, _, err = cache.GetConsistentStorages(ctx, "vs", "/repo/path")
- require.Equal(t, commonerr.NewRepositoryNotFoundError("vs", "/repo/path"), err)
+ require.Equal(t, storage.NewRepositoryNotFoundError("vs", "/repo/path"), err)
// "populate" metric is not set as there was an error and we don't want this result to be cached
err = testutil.CollectAndCompare(cache, strings.NewReader(`
diff --git a/internal/praefect/nodes/manager.go b/internal/praefect/nodes/manager.go
index e6260f7e1..6894d61ee 100644
--- a/internal/praefect/nodes/manager.go
+++ b/internal/praefect/nodes/manager.go
@@ -13,10 +13,10 @@ import (
"github.com/sirupsen/logrus"
gitalyauth "gitlab.com/gitlab-org/gitaly/v16/auth"
"gitlab.com/gitlab-org/gitaly/v16/internal/datastructure"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/client"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/proxy"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/sidechannel"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/config"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/metrics"
@@ -274,7 +274,7 @@ func (n *Mgr) GetPrimary(ctx context.Context, virtualStorage string, _ int64) (s
//nolint:revive // This is unintentionally missing documentation.
func (n *Mgr) GetSyncedNode(ctx context.Context, virtualStorageName, repoPath string) (Node, error) {
_, upToDateStorages, err := n.csg.GetConsistentStorages(ctx, virtualStorageName, repoPath)
- if err != nil && !errors.As(err, new(commonerr.RepositoryNotFoundError)) {
+ if err != nil && !errors.As(err, new(storage.RepositoryNotFoundError)) {
return nil, err
}
diff --git a/internal/praefect/nodes/per_repository.go b/internal/praefect/nodes/per_repository.go
index 254724198..a7e2b97d3 100644
--- a/internal/praefect/nodes/per_repository.go
+++ b/internal/praefect/nodes/per_repository.go
@@ -8,7 +8,7 @@ import (
"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
"github.com/sirupsen/logrus"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore/glsql"
)
@@ -103,7 +103,7 @@ WHERE snapshot.repository_id = $1
repositoryID,
).Scan(&current, &previous); err != nil {
if errors.Is(err, sql.ErrNoRows) {
- return "", commonerr.ErrRepositoryNotFound
+ return "", storage.ErrRepositoryNotFound
}
return "", fmt.Errorf("scan: %w", err)
diff --git a/internal/praefect/nodes/per_repository_test.go b/internal/praefect/nodes/per_repository_test.go
index 93d5ef806..1e96737b3 100644
--- a/internal/praefect/nodes/per_repository_test.go
+++ b/internal/praefect/nodes/per_repository_test.go
@@ -8,7 +8,7 @@ import (
"github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testdb"
@@ -476,7 +476,7 @@ func TestPerRepositoryElector(t *testing.T) {
desc: "repository does not exist",
steps: steps{
{
- error: commonerr.ErrRepositoryNotFound,
+ error: storage.ErrRepositoryNotFound,
primary: noPrimary(),
noBlockedQuery: true,
},
@@ -514,7 +514,7 @@ func TestPerRepositoryElector(t *testing.T) {
for _, event := range tc.existingJobs {
repositoryID, err := rs.GetRepositoryID(ctx, event.Job.VirtualStorage, event.Job.RelativePath)
if err != nil {
- require.Equal(t, commonerr.NewRepositoryNotFoundError(event.Job.VirtualStorage, event.Job.RelativePath), err)
+ require.Equal(t, storage.NewRepositoryNotFoundError(event.Job.VirtualStorage, event.Job.RelativePath), err)
}
event.Job.RepositoryID = repositoryID
diff --git a/internal/praefect/reconciler/reconciler_test.go b/internal/praefect/reconciler/reconciler_test.go
index bf68e4898..f421df15e 100644
--- a/internal/praefect/reconciler/reconciler_test.go
+++ b/internal/praefect/reconciler/reconciler_test.go
@@ -9,10 +9,10 @@ import (
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/require"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/middleware/metadatahandler"
"gitlab.com/gitlab-org/gitaly/v16/internal/helper"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testdb"
@@ -1041,7 +1041,7 @@ func TestReconciler(t *testing.T) {
var err error
existing.Job.RepositoryID, err = rs.GetRepositoryID(ctx, existing.Job.VirtualStorage, existing.Job.RelativePath)
if err != nil {
- require.Equal(t, commonerr.NewRepositoryNotFoundError(existing.Job.VirtualStorage, existing.Job.RelativePath), err)
+ require.Equal(t, storage.NewRepositoryNotFoundError(existing.Job.VirtualStorage, existing.Job.RelativePath), err)
}
var id int64
@@ -1099,7 +1099,7 @@ func TestReconciler(t *testing.T) {
for i, job := range jobs {
id, err := rs.GetRepositoryID(ctx, job.VirtualStorage, job.RelativePath)
if err != nil {
- require.Equal(t, commonerr.NewRepositoryNotFoundError(job.VirtualStorage, job.RelativePath), err)
+ require.Equal(t, storage.NewRepositoryNotFoundError(job.VirtualStorage, job.RelativePath), err)
}
jobs[i].RepositoryID = id
diff --git a/internal/praefect/remove_repository.go b/internal/praefect/remove_repository.go
index 2d8c9a17c..12c1805bd 100644
--- a/internal/praefect/remove_repository.go
+++ b/internal/praefect/remove_repository.go
@@ -9,7 +9,7 @@ import (
"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
"github.com/sirupsen/logrus"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
@@ -63,9 +63,9 @@ func removeRepositoryHandler(rs datastore.RepositoryStore, conns Connections, pa
virtualStorage := repo.StorageName
replicaPath, storages, err := rs.DeleteRepository(ctx, virtualStorage, repo.RelativePath)
if err != nil {
- if errors.As(err, new(commonerr.RepositoryNotFoundError)) {
+ if errors.As(err, new(storage.RepositoryNotFoundError)) {
if errorOnNotFound {
- if errors.As(err, new(commonerr.RepositoryNotFoundError)) {
+ if errors.As(err, new(storage.RepositoryNotFoundError)) {
return structerr.NewNotFound("repository does not exist")
}
}
diff --git a/internal/praefect/rename_repository.go b/internal/praefect/rename_repository.go
index b7c849460..2c2adc5b7 100644
--- a/internal/praefect/rename_repository.go
+++ b/internal/praefect/rename_repository.go
@@ -6,7 +6,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
@@ -59,13 +58,13 @@ func RenameRepositoryHandler(virtualStoragesNames []string, rs datastore.Reposit
req.GetRepository().GetRelativePath(),
req.GetRelativePath(),
); err != nil {
- if errors.Is(err, commonerr.ErrRepositoryNotFound) {
+ if errors.Is(err, storage.ErrRepositoryNotFound) {
return structerr.NewNotFound(
`GetRepoPath: not a git repository: "%s/%s"`,
req.GetRepository().GetStorageName(),
req.GetRepository().GetRelativePath(),
)
- } else if errors.Is(err, commonerr.ErrRepositoryAlreadyExists) {
+ } else if errors.Is(err, storage.ErrRepositoryAlreadyExists) {
return structerr.NewAlreadyExists("target repo exists already")
}
diff --git a/internal/praefect/router_node_manager.go b/internal/praefect/router_node_manager.go
index 0159a9cd1..2fc2bae61 100644
--- a/internal/praefect/router_node_manager.go
+++ b/internal/praefect/router_node_manager.go
@@ -6,7 +6,7 @@ import (
"fmt"
"gitlab.com/gitlab-org/gitaly/v16/internal/datastructure"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/nodes"
)
@@ -86,7 +86,7 @@ func (r *nodeManagerRouter) RouteRepositoryMutator(ctx context.Context, virtualS
// paths of repositories and thus returns an empty string. This breaks the tests. Instead, we'll just keep
// using the relative path in NodeManagerRouter.
_, consistentStorages, err := r.rs.GetConsistentStorages(ctx, virtualStorage, relativePath)
- if err != nil && !errors.As(err, new(commonerr.RepositoryNotFoundError)) {
+ if err != nil && !errors.As(err, new(storage.RepositoryNotFoundError)) {
return RepositoryMutatorRoute{}, fmt.Errorf("consistent storages: %w", err)
}
diff --git a/internal/praefect/router_per_repository_test.go b/internal/praefect/router_per_repository_test.go
index 75c67a941..dc68d0dfe 100644
--- a/internal/praefect/router_per_repository_test.go
+++ b/internal/praefect/router_per_repository_test.go
@@ -9,7 +9,6 @@ import (
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/nodes"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
@@ -841,7 +840,7 @@ func TestPerRepositoryRouterRouteRepositoryCreation(t *testing.T) {
primaryPick: 0,
repositoryExists: true,
expectedPrimaryCandidates: []int{3},
- expectedErr: fmt.Errorf("reserve repository id: %w", commonerr.ErrRepositoryAlreadyExists),
+ expectedErr: fmt.Errorf("reserve repository id: %w", storage.ErrRepositoryAlreadyExists),
},
{
desc: "additional repository doesn't exist",
diff --git a/internal/praefect/service/info/metadata.go b/internal/praefect/service/info/metadata.go
index b3820f67d..2e204512a 100644
--- a/internal/praefect/service/info/metadata.go
+++ b/internal/praefect/service/info/metadata.go
@@ -4,7 +4,7 @@ import (
"context"
"errors"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
@@ -29,7 +29,7 @@ func (s *Server) GetRepositoryMetadata(ctx context.Context, req *gitalypb.GetRep
metadata, err := getMetadata()
if err != nil {
- if errors.Is(err, commonerr.ErrRepositoryNotFound) {
+ if errors.Is(err, storage.ErrRepositoryNotFound) {
return nil, structerr.NewNotFound("%w", err)
}
diff --git a/internal/praefect/service/info/server.go b/internal/praefect/service/info/server.go
index e7e76915e..c099d2528 100644
--- a/internal/praefect/service/info/server.go
+++ b/internal/praefect/service/info/server.go
@@ -4,7 +4,7 @@ import (
"context"
"errors"
- "gitlab.com/gitlab-org/gitaly/v16/internal/praefect/commonerr"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/config"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/service"
@@ -80,7 +80,7 @@ func (s *Server) SetAuthoritativeStorage(ctx context.Context, req *gitalypb.SetA
}
if err := s.rs.SetAuthoritativeReplica(ctx, req.VirtualStorage, req.RelativePath, req.AuthoritativeStorage); err != nil {
- if errors.As(err, &commonerr.RepositoryNotFoundError{}) {
+ if errors.As(err, &storage.RepositoryNotFoundError{}) {
return nil, structerr.NewInvalidArgument("repository %q does not exist on virtual storage %q", req.RelativePath, req.VirtualStorage)
}