diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2022-03-31 15:23:55 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2022-03-31 17:11:48 +0300 |
commit | f43afbe236a1b17d31a8aaff8df5a593b8d6e523 (patch) | |
tree | 31c7328a146bee9572a785f9808f3f155be35a59 /internal/gitaly | |
parent | a32498721df03d6a16731f7aa4ef5143f3fb60d7 (diff) |
Remove implicit pool creation on link behaviorsmh-remove-implicit-pool-creation
If a repository is being linked to an object pool that does not exist,
the current behavior is to create the pool implicitly. This doesn't
bode well with Praefect as it doesn't know whether a pool was or wasn't
created and can't thus create a metadata record for the pool. The
functionality was removed behind a feature flag. It doesn't seem like
anything relies on the implicit pool creation, so this commit removes
the feature flag and returns an error when linking a repository to a
non-existent pool.
Changelog: changed
Diffstat (limited to 'internal/gitaly')
-rw-r--r-- | internal/gitaly/service/objectpool/link.go | 13 | ||||
-rw-r--r-- | internal/gitaly/service/objectpool/link_test.go | 34 |
2 files changed, 7 insertions, 40 deletions
diff --git a/internal/gitaly/service/objectpool/link.go b/internal/gitaly/service/objectpool/link.go index cb57a1ecf..cbb16f0ac 100644 --- a/internal/gitaly/service/objectpool/link.go +++ b/internal/gitaly/service/objectpool/link.go @@ -4,7 +4,6 @@ import ( "context" "gitlab.com/gitlab-org/gitaly/v14/internal/helper" - "gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -22,18 +21,8 @@ func (s *server) LinkRepositoryToObjectPool(ctx context.Context, req *gitalypb.L repo := s.localrepo(req.GetRepository()) - if featureflag.LinkRepositoryToObjectPoolNotFound.IsDisabled(ctx) { - if err := pool.Init(ctx); err != nil { - return nil, helper.ErrInternal(err) - } - } - if err := pool.Link(ctx, repo); err != nil { - if featureflag.LinkRepositoryToObjectPoolNotFound.IsEnabled(ctx) { - return nil, helper.ErrInternal(err) - } - - return nil, helper.ErrInternal(helper.SanitizeError(err)) + return nil, helper.ErrInternal(err) } return &gitalypb.LinkRepositoryToObjectPoolResponse{}, nil diff --git a/internal/gitaly/service/objectpool/link_test.go b/internal/gitaly/service/objectpool/link_test.go index bd516a521..9b58be35a 100644 --- a/internal/gitaly/service/objectpool/link_test.go +++ b/internal/gitaly/service/objectpool/link_test.go @@ -1,18 +1,14 @@ package objectpool import ( - "context" "os" "path/filepath" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v14/internal/git" "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/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag" "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" @@ -20,10 +16,8 @@ import ( ) func TestLink(t *testing.T) { - testhelper.NewFeatureSets(featureflag.LinkRepositoryToObjectPoolNotFound).Run(t, testLink) -} + ctx := testhelper.Context(t) -func testLink(t *testing.T, ctx context.Context) { cfg, repo, _, _, client := setup(ctx, t, testserver.WithDisablePraefect()) localRepo := localrepo.NewTestRepo(t, cfg, repo) @@ -89,10 +83,8 @@ func testLink(t *testing.T, ctx context.Context) { } func TestLinkIdempotent(t *testing.T) { - testhelper.NewFeatureSets(featureflag.LinkRepositoryToObjectPoolNotFound).Run(t, testLinkIdempotent) -} + ctx := testhelper.Context(t) -func testLinkIdempotent(t *testing.T, ctx context.Context) { cfg, repoProto, _, _, client := setup(ctx, t) pool := initObjectPool(t, cfg, cfg.Storages[0]) @@ -141,11 +133,9 @@ func TestLinkNoClobber(t *testing.T) { } func TestLinkNoPool(t *testing.T) { - testhelper.NewFeatureSets(featureflag.LinkRepositoryToObjectPoolNotFound).Run(t, testLinkNoPool) -} + ctx := testhelper.Context(t) -func testLinkNoPool(t *testing.T, ctx context.Context) { - cfg, repo, _, locator, client := setup(ctx, t) + cfg, repo, _, _, client := setup(ctx, t) pool := initObjectPool(t, cfg, cfg.Storages[0]) _, err := client.CreateObjectPool(ctx, &gitalypb.CreateObjectPoolRequest{ @@ -165,18 +155,6 @@ func testLinkNoPool(t *testing.T, ctx context.Context) { } _, err = client.LinkRepositoryToObjectPool(ctx, request) - if featureflag.LinkRepositoryToObjectPoolNotFound.IsEnabled(ctx) { - testhelper.RequireGrpcCode(t, err, codes.NotFound) - require.Error(t, err, "GetRepoPath: not a git repository:") - return - } - - require.NoError(t, err) - - pool = rewrittenObjectPool(ctx, t, cfg, pool) - - poolRepoPath, err := locator.GetRepoPath(pool) - require.NoError(t, err) - - assert.True(t, storage.IsGitDirectory(poolRepoPath)) + testhelper.RequireGrpcCode(t, err, codes.NotFound) + require.Error(t, err, "GetRepoPath: not a git repository:") } |