Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Steinhardt <psteinhardt@gitlab.com>2021-08-24 09:14:28 +0300
committerPatrick Steinhardt <psteinhardt@gitlab.com>2021-08-24 09:20:29 +0300
commit7db733a65560bd1aae8c21e6746c3b4ffde02918 (patch)
tree330bc2c4bf9076e088cc4e0240d9f3c32b3c991f
parentbb2e3f4a916f031f38c9fb1c4fc955f50f0e4275 (diff)
quarantine: Always enable use of quarantine directories
Quarantine directories have been introduced for two reasons: first they are a correctness fix, where new objects don't end up in the target repo in case the request fails. And second, they allow us to speed up access checks enumerating new objects direcytl via the quarantine directory. Quarantine directories have been tested since August 9th in production now without any issues and can thus be deemed stable. Remove the feature flags to make them generally available. Changelog: performance
-rw-r--r--internal/gitaly/service/conflicts/resolve_conflicts.go17
-rw-r--r--internal/gitaly/service/operations/branches.go5
-rw-r--r--internal/gitaly/service/operations/branches_test.go43
-rw-r--r--internal/gitaly/service/operations/cherry_pick.go3
-rw-r--r--internal/gitaly/service/operations/cherry_pick_test.go76
-rw-r--r--internal/gitaly/service/operations/commit_files.go3
-rw-r--r--internal/gitaly/service/operations/commit_files_test.go94
-rw-r--r--internal/gitaly/service/operations/merge.go5
-rw-r--r--internal/gitaly/service/operations/merge_test.go107
-rw-r--r--internal/gitaly/service/operations/rebase.go3
-rw-r--r--internal/gitaly/service/operations/rebase_test.go120
-rw-r--r--internal/gitaly/service/operations/revert.go3
-rw-r--r--internal/gitaly/service/operations/revert_test.go76
-rw-r--r--internal/gitaly/service/operations/server.go9
-rw-r--r--internal/gitaly/service/operations/submodules.go3
-rw-r--r--internal/gitaly/service/operations/submodules_test.go62
-rw-r--r--internal/gitaly/service/operations/update_branches_test.go37
-rw-r--r--internal/metadata/featureflag/feature_flags.go6
18 files changed, 184 insertions, 488 deletions
diff --git a/internal/gitaly/service/conflicts/resolve_conflicts.go b/internal/gitaly/service/conflicts/resolve_conflicts.go
index 267a3116c..ecc6ae983 100644
--- a/internal/gitaly/service/conflicts/resolve_conflicts.go
+++ b/internal/gitaly/service/conflicts/resolve_conflicts.go
@@ -20,7 +20,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/git/repository"
"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
"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"
@@ -145,19 +144,11 @@ func (s *server) resolveConflicts(header *gitalypb.ResolveConflictsRequestHeader
return err
}
- var quarantineRepo *localrepo.Repo
- var quarantineDir *quarantine.Dir
- if featureflag.QuarantinedResolveConflicts.IsEnabled(ctx) {
- var err error
- quarantineDir, err = quarantine.New(ctx, header.GetRepository(), s.locator)
- if err != nil {
- return helper.ErrInternalf("creating object quarantine: %w", err)
- }
-
- quarantineRepo = s.localrepo(quarantineDir.QuarantinedRepo())
- } else {
- quarantineRepo = s.localrepo(header.GetRepository())
+ quarantineDir, err := quarantine.New(ctx, header.GetRepository(), s.locator)
+ if err != nil {
+ return helper.ErrInternalf("creating object quarantine: %w", err)
}
+ quarantineRepo := s.localrepo(quarantineDir.QuarantinedRepo())
if err := s.repoWithBranchCommit(ctx,
quarantineRepo,
diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go
index 2fb9a2e03..b0dacfb46 100644
--- a/internal/gitaly/service/operations/branches.go
+++ b/internal/gitaly/service/operations/branches.go
@@ -6,7 +6,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/git"
"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
- "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"
@@ -25,7 +24,7 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB
return nil, status.Errorf(codes.InvalidArgument, "empty start point")
}
- quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository(), featureflag.Quarantine)
+ quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository())
if err != nil {
return nil, err
}
@@ -117,7 +116,7 @@ func (s *Server) UserUpdateBranch(ctx context.Context, req *gitalypb.UserUpdateB
referenceName := git.NewReferenceNameFromBranchName(string(req.BranchName))
- quarantineDir, _, err := s.quarantinedRepo(ctx, req.GetRepository(), featureflag.Quarantine)
+ quarantineDir, _, err := s.quarantinedRepo(ctx, req.GetRepository())
if err != nil {
return nil, err
}
diff --git a/internal/gitaly/service/operations/branches_test.go b/internal/gitaly/service/operations/branches_test.go
index b99235da7..19854e913 100644
--- a/internal/gitaly/service/operations/branches_test.go
+++ b/internal/gitaly/service/operations/branches_test.go
@@ -13,7 +13,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service"
"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/hook"
"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
- "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/testassert"
"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
@@ -40,12 +39,9 @@ func (s *testTransactionServer) VoteTransaction(ctx context.Context, in *gitalyp
func TestSuccessfulCreateBranchRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulCreateBranchRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulCreateBranchRequest(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -122,12 +118,9 @@ func testSuccessfulCreateBranchRequest(t *testing.T, ctx context.Context) {
func TestUserCreateBranchWithTransaction(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserCreateBranchWithTransaction)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserCreateBranchWithTransaction(t *testing.T, ctx context.Context) {
cfg, repo, repoPath := testcfg.BuildWithRepo(t)
transactionServer := &testTransactionServer{}
@@ -200,12 +193,9 @@ func testUserCreateBranchWithTransaction(t *testing.T, ctx context.Context) {
func TestSuccessfulGitHooksForUserCreateBranchRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulGitHooksForUserCreateBranchRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulGitHooksForUserCreateBranchRequest(t *testing.T, ctx context.Context) {
ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
branchName := "new-branch"
@@ -235,12 +225,9 @@ func testSuccessfulGitHooksForUserCreateBranchRequest(t *testing.T, ctx context.
func TestSuccessfulCreateBranchRequestWithStartPointRefPrefix(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulCreateBranchRequestWithStartPointRefPrefix)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulCreateBranchRequestWithStartPointRefPrefix(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -313,12 +300,9 @@ func testSuccessfulCreateBranchRequestWithStartPointRefPrefix(t *testing.T, ctx
func TestFailedUserCreateBranchDueToHooks(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserCreateBranchDueToHooks)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserCreateBranchDueToHooks(t *testing.T, ctx context.Context) {
ctx, _, repo, repoPath, client := setupOperationsService(t, ctx)
request := &gitalypb.UserCreateBranchRequest{
@@ -343,12 +327,9 @@ func testFailedUserCreateBranchDueToHooks(t *testing.T, ctx context.Context) {
func TestFailedUserCreateBranchRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserCreateBranchRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserCreateBranchRequest(t *testing.T, ctx context.Context) {
ctx, _, repo, _, client := setupOperationsService(t, ctx)
testCases := []struct {
diff --git a/internal/gitaly/service/operations/cherry_pick.go b/internal/gitaly/service/operations/cherry_pick.go
index d043f19dc..bdeaa6fd4 100644
--- a/internal/gitaly/service/operations/cherry_pick.go
+++ b/internal/gitaly/service/operations/cherry_pick.go
@@ -10,7 +10,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
"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,7 +20,7 @@ func (s *Server) UserCherryPick(ctx context.Context, req *gitalypb.UserCherryPic
return nil, status.Errorf(codes.InvalidArgument, "UserCherryPick: %v", err)
}
- quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository(), featureflag.Quarantine)
+ quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository())
if err != nil {
return nil, err
}
diff --git a/internal/gitaly/service/operations/cherry_pick_test.go b/internal/gitaly/service/operations/cherry_pick_test.go
index e86165cd0..e7e08b5d9 100644
--- a/internal/gitaly/service/operations/cherry_pick_test.go
+++ b/internal/gitaly/service/operations/cherry_pick_test.go
@@ -1,7 +1,6 @@
package operations
import (
- "context"
"fmt"
"strings"
"testing"
@@ -10,7 +9,6 @@ import (
"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/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testassert"
"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
@@ -21,12 +19,9 @@ import (
func TestServer_UserCherryPick_successful(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserCherryPickSuccessful)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserCherryPickSuccessful(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -180,12 +175,9 @@ func testServerUserCherryPickSuccessful(t *testing.T, ctx context.Context) {
func TestServer_UserCherryPick_successfulGitHooks(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserCherryPickSuccessfulGitHooks)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserCherryPickSuccessfulGitHooks(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -223,12 +215,9 @@ func testServerUserCherryPickSuccessfulGitHooks(t *testing.T, ctx context.Contex
func TestServer_UserCherryPick_stableID(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserCherryPickStableID)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserCherryPickStableID(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -284,12 +273,9 @@ func testServerUserCherryPickStableID(t *testing.T, ctx context.Context) {
func TestServer_UserCherryPick_failedValidations(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserCherryPickFailedValidations)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserCherryPickFailedValidations(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, _, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -361,12 +347,9 @@ func testServerUserCherryPickFailedValidations(t *testing.T, ctx context.Context
func TestServer_UserCherryPick_failedWithPreReceiveError(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserCherryPickFailedWithPreReceiveError)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserCherryPickFailedWithPreReceiveError(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -401,12 +384,9 @@ func testServerUserCherryPickFailedWithPreReceiveError(t *testing.T, ctx context
func TestServer_UserCherryPick_failedWithCreateTreeError(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserCherryPickFailedWithCreateTreeError)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserCherryPickFailedWithCreateTreeError(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -435,12 +415,9 @@ func testServerUserCherryPickFailedWithCreateTreeError(t *testing.T, ctx context
func TestServer_UserCherryPick_failedWithCommitError(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserCherryPickFailedWithCommitError)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserCherryPickFailedWithCommitError(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -470,12 +447,9 @@ func testServerUserCherryPickFailedWithCommitError(t *testing.T, ctx context.Con
func TestServer_UserCherryPick_failedWithConflict(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserCherryPickFailedWithConflict)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserCherryPickFailedWithConflict(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -504,12 +478,9 @@ func testServerUserCherryPickFailedWithConflict(t *testing.T, ctx context.Contex
func TestServer_UserCherryPick_successfulWithGivenCommits(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserCherryPickSuccessfulWithGivenCommits)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserCherryPickSuccessfulWithGivenCommits(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -564,12 +535,9 @@ func testServerUserCherryPickSuccessfulWithGivenCommits(t *testing.T, ctx contex
func TestServer_UserCherryPick_quarantine(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserCherryPickQuarantine)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserCherryPickQuarantine(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -598,7 +566,5 @@ func testServerUserCherryPickQuarantine(t *testing.T, ctx context.Context) {
exists, err := repo.HasRevision(ctx, oid.Revision()+"^{commit}")
require.NoError(t, err)
- // The new commit will be in the target repository in case quarantines are disabled.
- // Otherwise, it should've been discarded.
- require.Equal(t, !featureflag.Quarantine.IsEnabled(ctx), exists)
+ require.False(t, exists, "quarantined commit should have been discarded")
}
diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go
index f38955e0c..7a4525ce4 100644
--- a/internal/gitaly/service/operations/commit_files.go
+++ b/internal/gitaly/service/operations/commit_files.go
@@ -18,7 +18,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/storage"
"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"
@@ -119,7 +118,7 @@ func validatePath(rootPath, relPath string) (string, error) {
}
func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommitFilesRequestHeader, stream gitalypb.OperationService_UserCommitFilesServer) error {
- quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, header.GetRepository(), featureflag.Quarantine)
+ quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, header.GetRepository())
if err != nil {
return err
}
diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index 14a8985ed..0613e829a 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -16,7 +16,6 @@ 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/helper/text"
- "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/testassert"
"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
@@ -33,12 +32,9 @@ var (
func TestUserCommitFiles(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserCommitFiles)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserCommitFiles(t *testing.T, ctx context.Context) {
ctx, cfg, _, _, client := setupOperationsService(t, ctx)
const (
@@ -948,12 +944,9 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
func TestUserCommitFilesStableCommitID(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserCommitFilesStableCommitID)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserCommitFilesStableCommitID(t *testing.T, ctx context.Context) {
ctx, cfg, _, _, client := setupOperationsService(t, ctx)
repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
@@ -1011,12 +1004,9 @@ func testUserCommitFilesStableCommitID(t *testing.T, ctx context.Context) {
func TestUserCommitFilesQuarantine(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserCommitFilesQuarantine)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserCommitFilesQuarantine(t *testing.T, ctx context.Context) {
ctx, cfg, _, _, client := setupOperationsService(t, ctx)
repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
@@ -1047,20 +1037,15 @@ func testUserCommitFilesQuarantine(t *testing.T, ctx context.Context) {
exists, err := repo.HasRevision(ctx, oid.Revision()+"^{commit}")
require.NoError(t, err)
- // The new commit will be in the target repository in case quarantines are disabled.
- // Otherwise, it should've been discarded.
- require.Equal(t, !featureflag.Quarantine.IsEnabled(ctx), exists)
+ require.False(t, exists, "quarantined commit should have been discarded")
}
func TestSuccessfulUserCommitFilesRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserCommitFilesRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulUserCommitFilesRequest(t *testing.T, ctx context.Context) {
ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
newRepo, newRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
@@ -1169,12 +1154,9 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctx context.Context) {
func TestSuccessfulUserCommitFilesRequestMove(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserCommitFilesRequestMove)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulUserCommitFilesRequestMove(t *testing.T, ctx context.Context) {
ctx, cfg, _, _, client := setupOperationsService(t, ctx)
branchName := "master"
@@ -1230,12 +1212,9 @@ func testSuccessfulUserCommitFilesRequestMove(t *testing.T, ctx context.Context)
func TestSuccessfulUserCommitFilesRequestForceCommit(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserCommitFilesRequestForceCommit)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulUserCommitFilesRequestForceCommit(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -1280,12 +1259,9 @@ func testSuccessfulUserCommitFilesRequestForceCommit(t *testing.T, ctx context.C
func TestSuccessfulUserCommitFilesRequestStartSha(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserCommitFilesRequestStartSha)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulUserCommitFilesRequestStartSha(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, _, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -1318,21 +1294,17 @@ func testSuccessfulUserCommitFilesRequestStartSha(t *testing.T, ctx context.Cont
func TestSuccessfulUserCommitFilesRequestStartShaRemoteRepository(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserCommitFilesRemoteRepositoryRequest(func(header *gitalypb.UserCommitFilesRequest) {
+ testSuccessfulUserCommitFilesRemoteRepositoryRequest(func(header *gitalypb.UserCommitFilesRequest) {
setStartSha(header, "1e292f8fedd741b75372e19097c76d327140c312")
- }))
+ })
}
func TestSuccessfulUserCommitFilesRequestStartBranchRemoteRepository(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserCommitFilesRemoteRepositoryRequest(func(header *gitalypb.UserCommitFilesRequest) {
+ testSuccessfulUserCommitFilesRemoteRepositoryRequest(func(header *gitalypb.UserCommitFilesRequest) {
setStartBranchName(header, []byte("master"))
- }))
+ })
}
func testSuccessfulUserCommitFilesRemoteRepositoryRequest(setHeader func(header *gitalypb.UserCommitFilesRequest)) func(*testing.T, context.Context) {
@@ -1376,12 +1348,9 @@ func testSuccessfulUserCommitFilesRemoteRepositoryRequest(setHeader func(header
func TestSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *testing.T, ctx context.Context) {
ctx, cfg, _, _, client := setupOperationsService(t, ctx)
repoProto, _ := gittest.InitRepo(t, cfg, cfg.Storages[0])
@@ -1432,12 +1401,9 @@ func testSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *tes
func TestFailedUserCommitFilesRequestDueToHooks(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserCommitFilesRequestDueToHooks)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserCommitFilesRequestDueToHooks(t *testing.T, ctx context.Context) {
ctx, _, repoProto, repoPath, client := setupOperationsService(t, ctx)
branchName := "feature"
@@ -1469,12 +1435,9 @@ func testFailedUserCommitFilesRequestDueToHooks(t *testing.T, ctx context.Contex
func TestFailedUserCommitFilesRequestDueToIndexError(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserCommitFilesRequestDueToIndexError)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserCommitFilesRequestDueToIndexError(t *testing.T, ctx context.Context) {
ctx, _, repo, _, client := setupOperationsService(t, ctx)
testCases := []struct {
@@ -1537,12 +1500,9 @@ func testFailedUserCommitFilesRequestDueToIndexError(t *testing.T, ctx context.C
func TestFailedUserCommitFilesRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserCommitFilesRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserCommitFilesRequest(t *testing.T, ctx context.Context) {
ctx, _, repo, _, client := setupOperationsService(t, ctx)
branchName := "feature"
diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index 7d894fb2b..49ab228e8 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -12,7 +12,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
"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"
)
@@ -48,7 +47,7 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
return helper.ErrInvalidArgument(err)
}
- quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, firstRequest.GetRepository(), featureflag.Quarantine)
+ quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, firstRequest.GetRepository())
if err != nil {
return err
}
@@ -171,7 +170,7 @@ func (s *Server) UserFFBranch(ctx context.Context, in *gitalypb.UserFFBranchRequ
// While we're creating a quarantine directory, we know that it won't ever have any new
// objects given that we're doing a fast-forward merge. We still want to create one such
// that Rails can efficiently compute new objects.
- quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, in.GetRepository(), featureflag.Quarantine)
+ quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, in.GetRepository())
if err != nil {
return nil, err
}
diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go
index 6392f8f26..7647069d6 100644
--- a/internal/gitaly/service/operations/merge_test.go
+++ b/internal/gitaly/service/operations/merge_test.go
@@ -1,7 +1,6 @@
package operations
import (
- "context"
"fmt"
"io/ioutil"
"os"
@@ -18,7 +17,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
- "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/testassert"
"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
@@ -36,12 +34,9 @@ var (
func TestSuccessfulMerge(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulMerge)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulMerge(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -79,14 +74,8 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) {
firstResponse, err := mergeBidi.Recv()
require.NoError(t, err, "receive first response")
- // If we've got a quarantine directory, then we shouldn't be able to read the commit before
- // we have applied the merge.
_, err = repo.ReadCommit(ctx, git.Revision(firstResponse.CommitId))
- if featureflag.Quarantine.IsEnabled(ctx) {
- require.EqualError(t, err, localrepo.ErrObjectNotFound.Error())
- } else {
- require.NoError(t, err, "look up git commit before merge is applied")
- }
+ require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should not exist in the normal repo given that it is quarantined")
require.NoError(t, mergeBidi.Send(&gitalypb.UserMergeBranchRequest{Apply: true}), "apply merge")
@@ -129,12 +118,9 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) {
func TestUserMergeBranch_quarantine(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserMergeBranchQuarantine)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserMergeBranchQuarantine(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -172,20 +158,15 @@ func testUserMergeBranchQuarantine(t *testing.T, ctx context.Context) {
exists, err := repo.HasRevision(ctx, oid.Revision()+"^{commit}")
require.NoError(t, err)
- // The new commit will be in the target repository in case quarantines are disabled.
- // Otherwise, it should've been discarded.
- require.Equal(t, !featureflag.Quarantine.IsEnabled(ctx), exists)
+ require.False(t, exists, "quarantined commit should have been discarded")
}
func TestSuccessfulMerge_stableMergeIDs(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulMergeStableMergeIDs)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulMergeStableMergeIDs(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -254,12 +235,9 @@ func testSuccessfulMergeStableMergeIDs(t *testing.T, ctx context.Context) {
func TestAbortedMerge(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testAbortedMerge)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testAbortedMerge(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -322,12 +300,9 @@ func testAbortedMerge(t *testing.T, ctx context.Context) {
func TestFailedMergeConcurrentUpdate(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedMergeConcurrentUpdate)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedMergeConcurrentUpdate(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -370,12 +345,9 @@ func testFailedMergeConcurrentUpdate(t *testing.T, ctx context.Context) {
func TestUserMergeBranch_ambiguousReference(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserMergeBranchAmbiguousReference)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserMergeBranchAmbiguousReference(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -438,12 +410,9 @@ func testUserMergeBranchAmbiguousReference(t *testing.T, ctx context.Context) {
func TestFailedMergeDueToHooks(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedMergeDueToHooks)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedMergeDueToHooks(t *testing.T, ctx context.Context) {
ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
gittest.Exec(t, cfg, "-C", repoPath, "branch", mergeBranchName, mergeBranchHeadBefore)
@@ -492,12 +461,9 @@ func testFailedMergeDueToHooks(t *testing.T, ctx context.Context) {
func TestUserMergeBranch_conflict(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserMergeBranchConflict)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserMergeBranchConflict(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
const mergeIntoBranch = "mergeIntoBranch"
@@ -537,12 +503,9 @@ func testUserMergeBranchConflict(t *testing.T, ctx context.Context) {
func TestSuccessfulUserFFBranchRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserFFBranchRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulUserFFBranchRequest(t *testing.T, ctx context.Context) {
ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
commitID := "cfe32cf61b73a0d5e9f13e774abde7ff789b1660"
@@ -573,12 +536,9 @@ func testSuccessfulUserFFBranchRequest(t *testing.T, ctx context.Context) {
func TestFailedUserFFBranchRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserFFBranchRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserFFBranchRequest(t *testing.T, ctx context.Context) {
ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
commitID := "cfe32cf61b73a0d5e9f13e774abde7ff789b1660"
@@ -665,12 +625,9 @@ func testFailedUserFFBranchRequest(t *testing.T, ctx context.Context) {
func TestFailedUserFFBranchDueToHooks(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserFFBranchDueToHooks)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserFFBranchDueToHooks(t *testing.T, ctx context.Context) {
ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
commitID := "cfe32cf61b73a0d5e9f13e774abde7ff789b1660"
@@ -700,12 +657,9 @@ func testFailedUserFFBranchDueToHooks(t *testing.T, ctx context.Context) {
func TestUserFFBranch_ambiguousReference(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserFFBranchAmbiguousReference)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserFFBranchAmbiguousReference(t *testing.T, ctx context.Context) {
ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
branchName := "test-ff-target-branch"
@@ -748,8 +702,9 @@ func testUserFFBranchAmbiguousReference(t *testing.T, ctx context.Context) {
func TestSuccessfulUserMergeToRefRequest(t *testing.T) {
t.Parallel()
- ctx, cleanup := testhelper.Context()
- defer cleanup()
+
+ ctx, cancel := testhelper.Context()
+ defer cancel()
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
@@ -854,6 +809,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) {
func TestConflictsOnUserMergeToRefRequest(t *testing.T) {
t.Parallel()
+
ctx, cleanup := testhelper.Context()
defer cleanup()
@@ -991,6 +947,7 @@ func buildUserMergeToRefRequest(t testing.TB, cfg config.Cfg, repo *gitalypb.Rep
func TestUserMergeToRef_stableMergeID(t *testing.T) {
t.Parallel()
+
ctx, cleanup := testhelper.Context()
defer cleanup()
@@ -1043,6 +1000,7 @@ func TestUserMergeToRef_stableMergeID(t *testing.T) {
func TestFailedUserMergeToRefRequest(t *testing.T) {
t.Parallel()
+
ctx, cleanup := testhelper.Context()
defer cleanup()
@@ -1139,6 +1097,7 @@ func TestFailedUserMergeToRefRequest(t *testing.T) {
func TestUserMergeToRefIgnoreHooksRequest(t *testing.T) {
t.Parallel()
+
ctx, cleanup := testhelper.Context()
defer cleanup()
diff --git a/internal/gitaly/service/operations/rebase.go b/internal/gitaly/service/operations/rebase.go
index 632a5c34d..3e0baad93 100644
--- a/internal/gitaly/service/operations/rebase.go
+++ b/internal/gitaly/service/operations/rebase.go
@@ -11,7 +11,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
"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"
@@ -34,7 +33,7 @@ func (s *Server) UserRebaseConfirmable(stream gitalypb.OperationService_UserReba
ctx := stream.Context()
- quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, header.GetRepository(), featureflag.Quarantine)
+ quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, header.GetRepository())
if err != nil {
return helper.ErrInternalf("creating repo quarantine: %w", err)
}
diff --git a/internal/gitaly/service/operations/rebase_test.go b/internal/gitaly/service/operations/rebase_test.go
index 15d3182ad..31dc79d7c 100644
--- a/internal/gitaly/service/operations/rebase_test.go
+++ b/internal/gitaly/service/operations/rebase_test.go
@@ -17,7 +17,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
"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/testhelper"
"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testassert"
"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
@@ -37,12 +36,9 @@ var (
func TestSuccessfulUserRebaseConfirmableRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserRebaseConfirmableRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulUserRebaseConfirmableRequest(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
pushOptions := []string{"ci.skip", "test=value"}
@@ -68,11 +64,7 @@ func testSuccessfulUserRebaseConfirmableRequest(t *testing.T, ctx context.Contex
require.NoError(t, err, "receive first response")
_, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha()))
- if featureflag.Quarantine.IsEnabled(ctx) {
- require.Equal(t, localrepo.ErrObjectNotFound, err)
- } else {
- require.NoError(t, err, "look up git commit before rebase is applied")
- }
+ require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should not exist in the normal repo given that it is quarantined")
applyRequest := buildApplyRequest(true)
require.NoError(t, rebaseStream.Send(applyRequest), "apply rebase")
@@ -104,12 +96,9 @@ func testSuccessfulUserRebaseConfirmableRequest(t *testing.T, ctx context.Contex
func TestUserRebaseConfirmableTransaction(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserRebaseConfirmableTransaction)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserRebaseConfirmableTransaction(t *testing.T, ctx context.Context) {
var voteCount int
txManager := &transaction.MockManager{
VoteFn: func(context.Context, txinfo.Transaction, voting.Vote) error {
@@ -203,12 +192,9 @@ func testUserRebaseConfirmableTransaction(t *testing.T, ctx context.Context) {
func TestUserRebaseConfirmableStableCommitIDs(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserRebaseConfirmableStableCommitIDs)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserRebaseConfirmableStableCommitIDs(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
cfg.Gitlab.URL = setupAndStartGitlabServer(t, gittest.GlID, "project-1", cfg)
@@ -278,12 +264,9 @@ func testUserRebaseConfirmableStableCommitIDs(t *testing.T, ctx context.Context)
func TestFailedRebaseUserRebaseConfirmableRequestDueToInvalidHeader(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedRebaseUserRebaseConfirmableRequestDueToInvalidHeader)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedRebaseUserRebaseConfirmableRequestDueToInvalidHeader(t *testing.T, ctx context.Context) {
ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
repoCopy, _ := gittest.CloneRepo(t, cfg, cfg.Storages[0])
@@ -342,12 +325,9 @@ func testFailedRebaseUserRebaseConfirmableRequestDueToInvalidHeader(t *testing.T
func TestAbortedUserRebaseConfirmable(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testAbortedUserRebaseConfirmable)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testAbortedUserRebaseConfirmable(t *testing.T, ctx context.Context) {
ctx, cfg, _, _, client := setupOperationsService(t, ctx)
testCases := []struct {
@@ -405,12 +385,9 @@ func testAbortedUserRebaseConfirmable(t *testing.T, ctx context.Context) {
func TestFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserRebaseConfirmableDueToApplyBeingFalse)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -429,11 +406,7 @@ func testFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T, ctx conte
require.NoError(t, err, "receive first response")
_, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha()))
- if featureflag.Quarantine.IsEnabled(ctx) {
- require.Equal(t, localrepo.ErrObjectNotFound, err)
- } else {
- require.NoError(t, err, "look up git commit before rebase is applied")
- }
+ require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should not exist in the normal repo given that it is quarantined")
applyRequest := buildApplyRequest(false)
require.NoError(t, rebaseStream.Send(applyRequest), "apply rebase")
@@ -444,11 +417,7 @@ func testFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T, ctx conte
require.False(t, secondResponse.GetRebaseApplied(), "the second rebase is not applied")
_, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha()))
- if featureflag.Quarantine.IsEnabled(ctx) {
- require.Equal(t, localrepo.ErrObjectNotFound, err)
- } else {
- require.NoError(t, err, "look up git commit before rebase is applied")
- }
+ require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should have been discarded")
newBranchSha := getBranchSha(t, cfg, repoPath, rebaseBranchName)
require.Equal(t, branchSha, newBranchSha, "branch should not change when the rebase is not applied")
@@ -458,12 +427,9 @@ func testFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T, ctx conte
func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserRebaseConfirmableRequestDueToPreReceiveError)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -487,11 +453,7 @@ func testFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T, ct
require.NoError(t, err, "receive first response")
_, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha()))
- if featureflag.Quarantine.IsEnabled(ctx) {
- require.Equal(t, localrepo.ErrObjectNotFound, err)
- } else {
- require.NoError(t, err, "look up git commit before rebase is applied")
- }
+ require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should not exist in the normal repo given that it is quarantined")
applyRequest := buildApplyRequest(true)
require.NoError(t, rebaseStream.Send(applyRequest), "apply rebase")
@@ -505,11 +467,7 @@ func testFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T, ct
require.Equal(t, io.EOF, err)
_, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha()))
- if featureflag.Quarantine.IsEnabled(ctx) {
- require.Equal(t, localrepo.ErrObjectNotFound, err)
- } else {
- require.NoError(t, err, "look up git commit after rebase is applied")
- }
+ require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should have been discarded")
newBranchSha := getBranchSha(t, cfg, repoPath, rebaseBranchName)
require.Equal(t, branchSha, newBranchSha, "branch should not change when the rebase fails due to PreReceiveError")
@@ -521,12 +479,9 @@ func testFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T, ct
func TestFailedUserRebaseConfirmableDueToGitError(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserRebaseConfirmableDueToGitError)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserRebaseConfirmableDueToGitError(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repoCopyProto, _ := gittest.CloneRepo(t, cfg, cfg.Storages[0])
@@ -559,12 +514,9 @@ func getBranchSha(t *testing.T, cfg config.Cfg, repoPath string, branchName stri
func TestRebaseRequestWithDeletedFile(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testRebaseRequestWithDeletedFile)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testRebaseRequestWithDeletedFile(t *testing.T, ctx context.Context) {
ctx, cfg, _, _, client := setupOperationsService(t, ctx)
repoProto, repoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0], gittest.CloneRepoOpts{
WithWorktree: true,
@@ -594,11 +546,7 @@ func testRebaseRequestWithDeletedFile(t *testing.T, ctx context.Context) {
require.NoError(t, err, "receive first response")
_, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha()))
- if featureflag.Quarantine.IsEnabled(ctx) {
- require.Equal(t, err, localrepo.ErrObjectNotFound)
- } else {
- require.NoError(t, err, "look up git commit before rebase is applied")
- }
+ require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should not exist in the normal repo given that it is quarantined")
applyRequest := buildApplyRequest(true)
require.NoError(t, rebaseStream.Send(applyRequest), "apply rebase")
@@ -623,12 +571,9 @@ func testRebaseRequestWithDeletedFile(t *testing.T, ctx context.Context) {
func TestRebaseOntoRemoteBranch(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testRebaseOntoRemoteBranch)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testRebaseOntoRemoteBranch(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -661,11 +606,7 @@ func testRebaseOntoRemoteBranch(t *testing.T, ctx context.Context) {
require.NoError(t, err, "receive first response")
_, err = repo.ReadCommit(ctx, git.Revision(remoteBranchHash))
- if featureflag.Quarantine.IsEnabled(ctx) {
- require.Equal(t, localrepo.ErrObjectNotFound, err)
- } else {
- require.NoError(t, err, "remote commit does now exist in local repository")
- }
+ require.Equal(t, localrepo.ErrObjectNotFound, err, "commit should not exist in the normal repo given that it is quarantined")
applyRequest := buildApplyRequest(true)
require.NoError(t, rebaseStream.Send(applyRequest), "apply rebase")
@@ -690,12 +631,9 @@ func testRebaseOntoRemoteBranch(t *testing.T, ctx context.Context) {
func TestRebaseFailedWithCode(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testRebaseFailedWithCode)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testRebaseFailedWithCode(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
branchSha := getBranchSha(t, cfg, repoPath, rebaseBranchName)
diff --git a/internal/gitaly/service/operations/revert.go b/internal/gitaly/service/operations/revert.go
index 04597ef43..d5c1e17f8 100644
--- a/internal/gitaly/service/operations/revert.go
+++ b/internal/gitaly/service/operations/revert.go
@@ -11,7 +11,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
"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"
)
@@ -20,7 +19,7 @@ func (s *Server) UserRevert(ctx context.Context, req *gitalypb.UserRevertRequest
return nil, helper.ErrInvalidArgument(err)
}
- quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository(), featureflag.Quarantine)
+ quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository())
if err != nil {
return nil, err
}
diff --git a/internal/gitaly/service/operations/revert_test.go b/internal/gitaly/service/operations/revert_test.go
index de930566d..ab432d818 100644
--- a/internal/gitaly/service/operations/revert_test.go
+++ b/internal/gitaly/service/operations/revert_test.go
@@ -1,7 +1,6 @@
package operations
import (
- "context"
"fmt"
"strings"
"testing"
@@ -10,7 +9,6 @@ import (
"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/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
"google.golang.org/grpc/codes"
@@ -20,12 +18,9 @@ import (
func TestServer_UserRevert_successful(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserRevertSuccessful)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserRevertSuccessful(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -182,12 +177,9 @@ func testServerUserRevertSuccessful(t *testing.T, ctx context.Context) {
func TestServer_UserRevert_quarantine(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserRevertQuarantine)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserRevertQuarantine(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -216,20 +208,15 @@ func testServerUserRevertQuarantine(t *testing.T, ctx context.Context) {
exists, err := repo.HasRevision(ctx, oid.Revision()+"^{commit}")
require.NoError(t, err)
- // The new commit will be in the target repository in case quarantines are disabled.
- // Otherwise, it should've been discarded.
- require.Equal(t, !featureflag.Quarantine.IsEnabled(ctx), exists)
+ require.False(t, exists, "quarantined commit should have been discarded")
}
func TestServer_UserRevert_stableID(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserRevertStableID)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserRevertStableID(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, _, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -283,12 +270,9 @@ func testServerUserRevertStableID(t *testing.T, ctx context.Context) {
func TestServer_UserRevert_successfulIntoEmptyRepo(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserRevertSuccessfulIntoEmptyRepo)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserRevertSuccessfulIntoEmptyRepo(t *testing.T, ctx context.Context) {
ctx, cfg, startRepoProto, _, client := setupOperationsService(t, ctx)
startRepo := localrepo.NewTestRepo(t, cfg, startRepoProto)
@@ -334,12 +318,9 @@ func testServerUserRevertSuccessfulIntoEmptyRepo(t *testing.T, ctx context.Conte
func TestServer_UserRevert_successfulGitHooks(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserRevertSuccessfulGitHooks)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserRevertSuccessfulGitHooks(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -377,12 +358,9 @@ func testServerUserRevertSuccessfulGitHooks(t *testing.T, ctx context.Context) {
func TestServer_UserRevert_failuedDueToValidations(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserRevertFailuedDueToValidations)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserRevertFailuedDueToValidations(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, _, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -454,12 +432,9 @@ func testServerUserRevertFailuedDueToValidations(t *testing.T, ctx context.Conte
func TestServer_UserRevert_failedDueToPreReceiveError(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserRevertFailedDueToPreReceiveError)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserRevertFailedDueToPreReceiveError(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -494,12 +469,9 @@ func testServerUserRevertFailedDueToPreReceiveError(t *testing.T, ctx context.Co
func TestServer_UserRevert_failedDueToCreateTreeErrorConflict(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserRevertFailedDueToCreateTreeErrorConflict)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserRevertFailedDueToCreateTreeErrorConflict(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -528,12 +500,9 @@ func testServerUserRevertFailedDueToCreateTreeErrorConflict(t *testing.T, ctx co
func TestServer_UserRevert_failedDueToCreateTreeErrorEmpty(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserRevertFailedDueToCreateTreeErrorEmpty)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserRevertFailedDueToCreateTreeErrorEmpty(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -566,12 +535,9 @@ func testServerUserRevertFailedDueToCreateTreeErrorEmpty(t *testing.T, ctx conte
func TestServer_UserRevert_failedDueToCommitError(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testServerUserRevertFailedDueToCommitError)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testServerUserRevertFailedDueToCommitError(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
diff --git a/internal/gitaly/service/operations/server.go b/internal/gitaly/service/operations/server.go
index 24f959198..7ffe24f2d 100644
--- a/internal/gitaly/service/operations/server.go
+++ b/internal/gitaly/service/operations/server.go
@@ -16,7 +16,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/rubyserver"
"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/storage"
"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"
)
@@ -61,14 +60,8 @@ func (s *Server) localrepo(repo repository.GitRepo) *localrepo.Repo {
}
func (s *Server) quarantinedRepo(
- ctx context.Context, repo *gitalypb.Repository, flags ...featureflag.FeatureFlag,
+ ctx context.Context, repo *gitalypb.Repository,
) (*quarantine.Dir, *localrepo.Repo, error) {
- for _, flag := range flags {
- if flag.IsDisabled(ctx) {
- return nil, s.localrepo(repo), nil
- }
- }
-
quarantineDir, err := quarantine.New(ctx, repo, s.locator)
if err != nil {
return nil, nil, helper.ErrInternalf("creating object quarantine: %w", err)
diff --git a/internal/gitaly/service/operations/submodules.go b/internal/gitaly/service/operations/submodules.go
index d7ed6f3c1..11aae0f32 100644
--- a/internal/gitaly/service/operations/submodules.go
+++ b/internal/gitaly/service/operations/submodules.go
@@ -12,7 +12,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
"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"
@@ -61,7 +60,7 @@ func validateUserUpdateSubmoduleRequest(req *gitalypb.UserUpdateSubmoduleRequest
}
func (s *Server) userUpdateSubmodule(ctx context.Context, req *gitalypb.UserUpdateSubmoduleRequest) (*gitalypb.UserUpdateSubmoduleResponse, error) {
- quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository(), featureflag.Quarantine)
+ quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository())
if err != nil {
return nil, err
}
diff --git a/internal/gitaly/service/operations/submodules_test.go b/internal/gitaly/service/operations/submodules_test.go
index 51009204d..9c8c7c7fa 100644
--- a/internal/gitaly/service/operations/submodules_test.go
+++ b/internal/gitaly/service/operations/submodules_test.go
@@ -2,7 +2,6 @@ package operations
import (
"bytes"
- "context"
"fmt"
"strings"
"testing"
@@ -12,7 +11,6 @@ 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/git/lstree"
- "gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
"google.golang.org/grpc/codes"
@@ -22,12 +20,9 @@ import (
func TestSuccessfulUserUpdateSubmoduleRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserUpdateSubmoduleRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulUserUpdateSubmoduleRequest(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -109,12 +104,9 @@ func testSuccessfulUserUpdateSubmoduleRequest(t *testing.T, ctx context.Context)
func TestUserUpdateSubmoduleStableID(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserUpdateSubmoduleStableID)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserUpdateSubmoduleStableID(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, _, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -160,12 +152,9 @@ func testUserUpdateSubmoduleStableID(t *testing.T, ctx context.Context) {
func TestUserUpdateSubmoduleQuarantine(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testUserUpdateSubmoduleQuarantine)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testUserUpdateSubmoduleQuarantine(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -192,20 +181,15 @@ func testUserUpdateSubmoduleQuarantine(t *testing.T, ctx context.Context) {
exists, err := repo.HasRevision(ctx, oid.Revision()+"^{commit}")
require.NoError(t, err)
- // The new commit will be in the target repository in case quarantines are disabled.
- // Otherwise, it should've been discarded.
- require.Equal(t, !featureflag.Quarantine.IsEnabled(ctx), exists)
+ require.False(t, exists, "quarantined commit should have been discarded")
}
func TestFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserUpdateSubmoduleRequestDueToValidations)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T, ctx context.Context) {
ctx, _, repo, _, client := setupOperationsService(t, ctx)
testCases := []struct {
@@ -323,12 +307,9 @@ func testFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T, ctx cont
func TestFailedUserUpdateSubmoduleRequestDueToInvalidBranch(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserUpdateSubmoduleRequestDueToInvalidBranch)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserUpdateSubmoduleRequestDueToInvalidBranch(t *testing.T, ctx context.Context) {
ctx, _, repo, _, client := setupOperationsService(t, ctx)
request := &gitalypb.UserUpdateSubmoduleRequest{
@@ -348,12 +329,9 @@ func testFailedUserUpdateSubmoduleRequestDueToInvalidBranch(t *testing.T, ctx co
func TestFailedUserUpdateSubmoduleRequestDueToInvalidSubmodule(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserUpdateSubmoduleRequestDueToInvalidSubmodule)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserUpdateSubmoduleRequestDueToInvalidSubmodule(t *testing.T, ctx context.Context) {
ctx, _, repo, _, client := setupOperationsService(t, ctx)
request := &gitalypb.UserUpdateSubmoduleRequest{
@@ -373,12 +351,9 @@ func testFailedUserUpdateSubmoduleRequestDueToInvalidSubmodule(t *testing.T, ctx
func TestFailedUserUpdateSubmoduleRequestDueToSameReference(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserUpdateSubmoduleRequestDueToSameReference)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserUpdateSubmoduleRequestDueToSameReference(t *testing.T, ctx context.Context) {
ctx, _, repo, _, client := setupOperationsService(t, ctx)
request := &gitalypb.UserUpdateSubmoduleRequest{
@@ -401,12 +376,9 @@ func testFailedUserUpdateSubmoduleRequestDueToSameReference(t *testing.T, ctx co
func TestFailedUserUpdateSubmoduleRequestDueToRepositoryEmpty(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserUpdateSubmoduleRequestDueToRepositoryEmpty)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserUpdateSubmoduleRequestDueToRepositoryEmpty(t *testing.T, ctx context.Context) {
ctx, cfg, _, _, client := setupOperationsService(t, ctx)
repo, _ := gittest.InitRepo(t, cfg, cfg.Storages[0])
diff --git a/internal/gitaly/service/operations/update_branches_test.go b/internal/gitaly/service/operations/update_branches_test.go
index 48e5be18d..72bc5bd4c 100644
--- a/internal/gitaly/service/operations/update_branches_test.go
+++ b/internal/gitaly/service/operations/update_branches_test.go
@@ -1,7 +1,6 @@
package operations
import (
- "context"
"crypto/sha1"
"fmt"
"testing"
@@ -10,7 +9,6 @@ import (
"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/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testassert"
"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
@@ -27,12 +25,9 @@ var (
func TestSuccessfulUserUpdateBranchRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserUpdateBranchRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulUserUpdateBranchRequest(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -107,12 +102,9 @@ func testSuccessfulUserUpdateBranchRequest(t *testing.T, ctx context.Context) {
func TestSuccessfulUserUpdateBranchRequestToDelete(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulUserUpdateBranchRequestToDelete)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulUserUpdateBranchRequestToDelete(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -181,12 +173,9 @@ func testSuccessfulUserUpdateBranchRequestToDelete(t *testing.T, ctx context.Con
func TestSuccessfulGitHooksForUserUpdateBranchRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testSuccessfulGitHooksForUserUpdateBranchRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulGitHooksForUserUpdateBranchRequest(t *testing.T, ctx context.Context) {
ctx, cfg, _, _, client := setupOperationsService(t, ctx)
for _, hookName := range GitlabHooks {
@@ -218,12 +207,9 @@ func testSuccessfulGitHooksForUserUpdateBranchRequest(t *testing.T, ctx context.
func TestFailedUserUpdateBranchDueToHooks(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserUpdateBranchDueToHooks)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserUpdateBranchDueToHooks(t *testing.T, ctx context.Context) {
ctx, _, repoProto, repoPath, client := setupOperationsService(t, ctx)
request := &gitalypb.UserUpdateBranchRequest{
@@ -255,12 +241,9 @@ func testFailedUserUpdateBranchDueToHooks(t *testing.T, ctx context.Context) {
func TestFailedUserUpdateBranchRequest(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.Quarantine,
- }).Run(t, testFailedUserUpdateBranchRequest)
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testFailedUserUpdateBranchRequest(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, _, client := setupOperationsService(t, ctx)
revDoesntExist := fmt.Sprintf("%x", sha1.Sum([]byte("we need a non existent sha")))
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index 21308f113..3804ddf43 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -6,18 +6,12 @@ package featureflag
var (
// GoSetConfig enables git2go implementation of SetConfig.
GoSetConfig = FeatureFlag{Name: "go_set_config", OnByDefault: true}
- // QuarantinedResolveCOnflicts enables use of a quarantine object directory for ResolveConflicts.
- QuarantinedResolveConflicts = FeatureFlag{Name: "quarantined_resolve_conflicts", OnByDefault: false}
// GoUserApplyPatch enables the Go implementation of UserApplyPatch
GoUserApplyPatch = FeatureFlag{Name: "go_user_apply_patch", OnByDefault: true}
- // Quarantine enables the use of quarantine directories.
- Quarantine = FeatureFlag{Name: "quarantine", OnByDefault: false}
)
// All includes all feature flags.
var All = []FeatureFlag{
GoSetConfig,
- QuarantinedResolveConflicts,
GoUserApplyPatch,
- Quarantine,
}