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())