diff options
| author | Sami Hiltunen <shiltunen@gitlab.com> | 2022-03-07 18:49:04 +0300 |
|---|---|---|
| committer | Sami Hiltunen <shiltunen@gitlab.com> | 2022-04-04 13:54:32 +0300 |
| commit | b79eeebc6fb40eae817253bdd03cc1e237e708df (patch) | |
| tree | 9c1fc645ac4f05c261cd710ba3b344c685d301a3 /internal/gitaly/service | |
| parent | 1ad960e9d6c88125c8b64ca8ada057d8cf79589d (diff) | |
Handle DeleteObjectPool calls in Praefect
Praefect currently proxies DeleteObjectPool calls to Gitalys like
any other mutating RPC. This has the same problem as RemoveRepository
previously had; a pool can be deleted from the disk and it's metadata
record still left in the database. This can then cause problems as
Praefect believes a pool replica still exists where one doesn't exist.
Praefect doesn't even treat DeleteObjectPool as a repository removing
RPC. This means the deletions have never been replicated to the
secondaries and the pool metadata records have been left in place. This
then causes the reconciler to repeatedly attempt to reconcile from a
non-existing primary pool replica to the secondaries.
This commit fixes both issues by handling pool deletions in Praefect.
Similarly to RemoveRepository, the metadata record of the pool is first
deleted and only then the pool is attempted to remove from the Gitalys
that have it. This ensures atomicity of the removals when Praefect is
rewriting the replica paths.
With the behavior fixed, the Praefect specific test asserations can be
removed as the behavior now matches what how a plain Gitaly would
behave.
The handler in Praefect is tested via the same tests that test the
handler in Gitaly. Adding separate tests doesn't make sense as external
behavior of the the handlers should match and the handler shouldn't
exists in Praefect if it is removed from Gitaly.
Changelog: fixed
Diffstat (limited to 'internal/gitaly/service')
| -rw-r--r-- | internal/gitaly/service/objectpool/create_test.go | 15 |
1 files changed, 1 insertions, 14 deletions
diff --git a/internal/gitaly/service/objectpool/create_test.go b/internal/gitaly/service/objectpool/create_test.go index 35afaaf0a..c8b77063c 100644 --- a/internal/gitaly/service/objectpool/create_test.go +++ b/internal/gitaly/service/objectpool/create_test.go @@ -1,7 +1,6 @@ package objectpool import ( - "fmt" "path/filepath" "strings" "testing" @@ -10,8 +9,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo" - "gitlab.com/gitlab-org/gitaly/v14/internal/helper" - "gitlab.com/gitlab-org/gitaly/v14/internal/praefect/commonerr" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testserver" "gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb" @@ -221,18 +218,8 @@ func TestDelete(t *testing.T) { request.ObjectPool = nil } - expectedErr := tc.error - if tc.error == errInvalidPoolDir && testhelper.IsPraefectEnabled() { - expectedErr = helper.ErrNotFound(fmt.Errorf( - "mutator call: route repository mutator: get repository id: %w", - commonerr.NewRepositoryNotFoundError(repo.GetStorageName(), tc.relativePath), - )) - } else if tc.error == errMissingPool && testhelper.IsPraefectEnabled() { - expectedErr = helper.ErrInvalidArgumentf("repo scoped: unable to descend OID [1 1] into message gitaly.DeleteObjectPoolRequest: proto field is empty") - } - _, err := client.DeleteObjectPool(ctx, request) - testhelper.RequireGrpcError(t, expectedErr, err) + testhelper.RequireGrpcError(t, tc.error, err) response, err := repositoryClient.RepositoryExists(ctx, &gitalypb.RepositoryExistsRequest{ Repository: pool.ToProto().GetRepository(), |