diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-12-09 11:46:03 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-12-10 18:04:03 +0300 |
commit | 8e439cebb878a04c7c60c6df5e3c65bac52cea95 (patch) | |
tree | 72043d1299c517f09fb14105b874b0f455126f61 | |
parent | d5c97bec598ba8f5bee409e691279a50224f371c (diff) |
backup: Fix missing feature flag tests
The backup restoration tests aren't testing feature flags for atomic
creation and removal of repositories. This went undetected because the
tests used `context.Background()` to create the context instead of using
`testhelper.Context()`, which would've set up additional sanity checks
for feature flags.
Exercise the feature flags. This uncovers a bug in the test setup if
running with Praefect: because the repository entry does not exist in
the database, Praefect intercepted the repository removal and never
forwarded it to Gitaly. As a result, at the time we try to create the
repository the old repository still exists. This used to work alright,
but now fails with atomic repository creation enabled because the target
repository must not exist yet.
Fix this setup by creating the repository with `CreateRepository()`
first.
-rw-r--r-- | cmd/gitaly-backup/restore_test.go | 35 |
1 files changed, 33 insertions, 2 deletions
diff --git a/cmd/gitaly-backup/restore_test.go b/cmd/gitaly-backup/restore_test.go index a207651ad..0c7828c0e 100644 --- a/cmd/gitaly-backup/restore_test.go +++ b/cmd/gitaly-backup/restore_test.go @@ -7,12 +7,15 @@ import ( "flag" "fmt" "io" + "os" "path/filepath" "testing" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/v14/client" "gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/setup" + "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/testcfg" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testserver" @@ -21,6 +24,14 @@ import ( func TestRestoreSubcommand(t *testing.T) { t.Parallel() + testhelper.NewFeatureSets( + featureflag.AtomicRemoveRepository, + featureflag.TxAtomicRepositoryCreation, + ).Run(t, testRestoreSubcommand) +} + +func testRestoreSubcommand(t *testing.T, ctx context.Context) { + t.Parallel() cfg := testcfg.Build(t) testcfg.BuildGitalyHooks(t, cfg) @@ -29,8 +40,28 @@ func TestRestoreSubcommand(t *testing.T) { path := testhelper.TempDir(t) - existingRepo, existRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0], gittest.CloneRepoOpts{ + conn, err := client.Dial(gitalyAddr, nil) + require.NoError(t, err) + defer testhelper.MustClose(t, conn) + + existingRepo := &gitalypb.Repository{ RelativePath: "existing_repo", + StorageName: cfg.Storages[0].Name, + } + + // We need to create the repository entry in the database such that we can call + // `RemoveRepository()` and have Praefect forward the request to Gitaly as expected. + repoClient := gitalypb.NewRepositoryServiceClient(conn) + _, err = repoClient.CreateRepository(ctx, &gitalypb.CreateRepositoryRequest{Repository: existingRepo}) + require.NoError(t, err) + existingRepo.RelativePath = testhelper.GetReplicaPath(ctx, t, conn, existingRepo) + + // This is a bit awkward: we have to delete the created repository on-disk again and + // recreate it with the seed repository. + existRepoPath := filepath.Join(cfg.Storages[0].Path, existingRepo.RelativePath) + require.NoError(t, os.RemoveAll(existRepoPath)) + gittest.CloneRepo(t, cfg, cfg.Storages[0], gittest.CloneRepoOpts{ + RelativePath: existingRepo.RelativePath, }) existingRepoBundlePath := filepath.Join(path, existingRepo.RelativePath+".bundle") gittest.Exec(t, cfg, "-C", existRepoPath, "bundle", "create", existingRepoBundlePath, "--all") @@ -69,7 +100,7 @@ func TestRestoreSubcommand(t *testing.T) { require.NoError(t, fs.Parse([]string{"-path", path})) require.EqualError(t, - cmd.Run(context.Background(), &stdin, io.Discard), + cmd.Run(ctx, &stdin, io.Discard), "restore: pipeline: 1 failures encountered:\n - invalid: manager: remove repository: could not dial source: invalid connection string: \"invalid\"\n") for _, repo := range repos { |