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

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