diff options
author | Toon Claes <toon@gitlab.com> | 2022-03-10 18:11:45 +0300 |
---|---|---|
committer | Toon Claes <toon@gitlab.com> | 2022-03-10 18:11:45 +0300 |
commit | 0206458c621892e0bc247098eb6f3a21d2424477 (patch) | |
tree | 9d68f7ab7aafad7f26630ab820ca9fee275a48a5 | |
parent | 7e4ced45a42379d240661115c613912d318a1de1 (diff) | |
parent | b8de637479b1f530e8e59705570bcf0e02a6aeec (diff) |
Merge branch 'smh-no-implicit-pool-creation' into 'master'
Disable implicit pool creation on link behind a feature flag
See merge request gitlab-org/gitaly!4397
4 files changed, 40 insertions, 7 deletions
diff --git a/internal/gitaly/service/objectpool/link.go b/internal/gitaly/service/objectpool/link.go index 3e378d40b..cb57a1ecf 100644 --- a/internal/gitaly/service/objectpool/link.go +++ b/internal/gitaly/service/objectpool/link.go @@ -4,6 +4,7 @@ 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" @@ -21,11 +22,17 @@ func (s *server) LinkRepositoryToObjectPool(ctx context.Context, req *gitalypb.L repo := s.localrepo(req.GetRepository()) - if err := pool.Init(ctx); err != nil { - return nil, helper.ErrInternal(err) + 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)) } diff --git a/internal/gitaly/service/objectpool/link_test.go b/internal/gitaly/service/objectpool/link_test.go index 49d9176a0..bd516a521 100644 --- a/internal/gitaly/service/objectpool/link_test.go +++ b/internal/gitaly/service/objectpool/link_test.go @@ -1,6 +1,7 @@ package objectpool import ( + "context" "os" "path/filepath" "testing" @@ -11,6 +12,7 @@ import ( "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" @@ -18,7 +20,10 @@ import ( ) func TestLink(t *testing.T) { - ctx := testhelper.Context(t) + testhelper.NewFeatureSets(featureflag.LinkRepositoryToObjectPoolNotFound).Run(t, testLink) +} + +func testLink(t *testing.T, ctx context.Context) { cfg, repo, _, _, client := setup(ctx, t, testserver.WithDisablePraefect()) localRepo := localrepo.NewTestRepo(t, cfg, repo) @@ -84,7 +89,10 @@ func TestLink(t *testing.T) { } func TestLinkIdempotent(t *testing.T) { - ctx := testhelper.Context(t) + testhelper.NewFeatureSets(featureflag.LinkRepositoryToObjectPoolNotFound).Run(t, testLinkIdempotent) +} + +func testLinkIdempotent(t *testing.T, ctx context.Context) { cfg, repoProto, _, _, client := setup(ctx, t) pool := initObjectPool(t, cfg, cfg.Storages[0]) @@ -133,7 +141,10 @@ func TestLinkNoClobber(t *testing.T) { } func TestLinkNoPool(t *testing.T) { - ctx := testhelper.Context(t) + testhelper.NewFeatureSets(featureflag.LinkRepositoryToObjectPoolNotFound).Run(t, testLinkNoPool) +} + +func testLinkNoPool(t *testing.T, ctx context.Context) { cfg, repo, _, locator, client := setup(ctx, t) pool := initObjectPool(t, cfg, cfg.Storages[0]) @@ -154,6 +165,12 @@ func TestLinkNoPool(t *testing.T) { } _, 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) diff --git a/internal/metadata/featureflag/ff_link_repository_to_object_pool_not_found.go b/internal/metadata/featureflag/ff_link_repository_to_object_pool_not_found.go new file mode 100644 index 000000000..274ee788c --- /dev/null +++ b/internal/metadata/featureflag/ff_link_repository_to_object_pool_not_found.go @@ -0,0 +1,5 @@ +package featureflag + +// LinkRepositoryToObjectPoolNotFound disables the implicit pool creation when linking a repository +// to an object pool that does not exist. +var LinkRepositoryToObjectPoolNotFound = NewFeatureFlag("link_repository_to_object_pool_not_found", false) diff --git a/internal/praefect/replicator_test.go b/internal/praefect/replicator_test.go index 6ddb85128..7a9915a4a 100644 --- a/internal/praefect/replicator_test.go +++ b/internal/praefect/replicator_test.go @@ -26,6 +26,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v14/internal/helper" + "gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v14/internal/middleware/metadatahandler" "gitlab.com/gitlab-org/gitaly/v14/internal/praefect/config" "gitlab.com/gitlab-org/gitaly/v14/internal/praefect/datastore" @@ -50,9 +51,12 @@ func TestMain(m *testing.M) { } func TestReplMgr_ProcessBacklog(t *testing.T) { - t.Parallel() + testhelper.NewFeatureSets(featureflag.LinkRepositoryToObjectPoolNotFound).Run(t, testReplMgr_ProcessBacklog) +} - ctx := testhelper.Context(t) +//nolint:revive,stylecheck +func testReplMgr_ProcessBacklog(t *testing.T, ctx context.Context) { + t.Parallel() primaryCfg, testRepoProto, testRepoPath := testcfg.BuildWithRepo(t, testcfg.WithStorages("primary")) testRepo := localrepo.NewTestRepo(t, primaryCfg, testRepoProto) |