diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2022-04-07 13:24:21 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2022-04-07 13:24:21 +0300 |
commit | 23efb30243f15dd8c29e222ba0552a21af9b1cbe (patch) | |
tree | b463eaf4cc0445313a58f663edef458a4e0643f8 | |
parent | 3626c6deca3ff88456259ff6132dcd3b6b569671 (diff) | |
parent | f43afbe236a1b17d31a8aaff8df5a593b8d6e523 (diff) |
Merge branch 'smh-remove-implicit-pool-creation' into 'master'
Remove implicit pool creation on link behavior
See merge request gitlab-org/gitaly!4455
4 files changed, 9 insertions, 50 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:") } 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 deleted file mode 100644 index 274ee788c..000000000 --- a/internal/metadata/featureflag/ff_link_repository_to_object_pool_not_found.go +++ /dev/null @@ -1,5 +0,0 @@ -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 6862b2f3f..a0dba9d70 100644 --- a/internal/praefect/replicator_test.go +++ b/internal/praefect/replicator_test.go @@ -52,13 +52,10 @@ func TestMain(m *testing.M) { } func TestReplMgr_ProcessBacklog(t *testing.T) { - testhelper.NewFeatureSets(featureflag.LinkRepositoryToObjectPoolNotFound).Run(t, testReplMgr_ProcessBacklog) -} - -//nolint:revive,stylecheck -func testReplMgr_ProcessBacklog(t *testing.T, ctx context.Context) { t.Parallel() + ctx := testhelper.Context(t) + primaryCfg, testRepoProto, testRepoPath := testcfg.BuildWithRepo(t, testcfg.WithStorages("primary")) testRepo := localrepo.NewTestRepo(t, primaryCfg, testRepoProto) primaryCfg.SocketPath = testserver.RunGitalyServer(t, primaryCfg, nil, setup.RegisterAll, testserver.WithDisablePraefect()) |