backup: Use CreateRespository and FetchBundle to restore bundles

To restore from an incremental backup we will need to apply multiple
bundles to the same repository. This is not possible with
CreateRepositoryFromBundle. So instead we first create an empty
repository and then fetch the contents of each bundle into it.

To maintain compatibility with existing backups we introduce the idea of
"skippable" bundles. It wouldn't make sense to skip a whole restore when
an incremental bundle doesn't exist. So here we're making sure we only
skip non-existing full backups.

4 files changed, 55 insertions, 37 deletions

diff --git a/internal/backup/backup.go b/internal/backup/backup.go
index 70a9b6d91..5ceebcd68 100644
--- a/internal/backup/backup.go
+++ b/internal/backup/backup.go
@@ -42,6 +42,10 @@ type Sink interface {
 type Full struct {
 	// BundlePath is the path of the bundle
 	BundlePath string
+	// SkippableOnNotFound defines if the bundle can be skipped when it does
+	// not exist. This allows us to maintain legacy behaviour where we always
+	// check a specific location for a bundle without knowing if it exists.
+	SkippableOnNotFound bool
 	// RefPath is the path of the ref file
 	RefPath string
 	// CustomHooksPath is the path of the custom hooks archive
@@ -174,36 +178,35 @@ func (mgr *Manager) Restore(ctx context.Context, req *RestoreRequest) error {
 
 	full, err := mgr.locator.FindLatestFull(ctx, req.Repository)
 	if err != nil {
-		return mgr.checkRestoreSkip(ctx, err, req)
+		return fmt.Errorf("manager: %w", err)
 	}
 
-	if err := mgr.restoreBundle(ctx, full.BundlePath, req.Server, req.Repository); err != nil {
-		return mgr.checkRestoreSkip(ctx, err, req)
-	}
-	if err := mgr.restoreCustomHooks(ctx, full.CustomHooksPath, req.Server, req.Repository); err != nil {
+	if err := mgr.createRepository(ctx, req.Server, req.Repository); err != nil {
 		return fmt.Errorf("manager: %w", err)
 	}
-	return nil
-}
+	if err := mgr.restoreBundle(ctx, full.BundlePath, req.Server, req.Repository); err != nil {
+		if full.SkippableOnNotFound && errors.Is(err, ErrDoesntExist) {
+			// For compatibility with existing backups we need to make sure the
+			// repository exists even if there's no bundle for project
+			// repositories (not wiki or snippet repositories).  Gitaly does
+			// not know which repository is which type so here we accept a
+			// parameter to tell us to employ this behaviour. Since the
+			// repository has already been created, we simply skip cleaning up.
+			if req.AlwaysCreate {
+				return nil
+			}
 
-func (mgr *Manager) checkRestoreSkip(ctx context.Context, err error, req *RestoreRequest) error {
-	if errors.Is(err, ErrDoesntExist) {
-		// For compatibility with existing backups we need to always create the
-		// repository even if there's no bundle for project repositories
-		// (not wiki or snippet repositories).  Gitaly does not know which
-		// repository is which type so here we accept a parameter to tell us
-		// to employ this behaviour.
-		if req.AlwaysCreate {
-			if err := mgr.createRepository(ctx, req.Server, req.Repository); err != nil {
-				return fmt.Errorf("manager: %w", err)
+			if err := mgr.removeRepository(ctx, req.Server, req.Repository); err != nil {
+				return fmt.Errorf("manager: remove on skipped: %w", err)
 			}
-			return nil
-		}
 
-		return fmt.Errorf("manager: %w: %s", ErrSkipped, err.Error())
+			return fmt.Errorf("manager: %w: %s", ErrSkipped, err.Error())
+		}
 	}
-
-	return fmt.Errorf("manager: %w", err)
+	if err := mgr.restoreCustomHooks(ctx, full.CustomHooksPath, req.Server, req.Repository); err != nil {
+		return fmt.Errorf("manager: %w", err)
+	}
+	return nil
 }
 
 func (mgr *Manager) isEmpty(ctx context.Context, server storage.ServerInfo, repo *gitalypb.Repository) (bool, error) {
@@ -317,11 +320,11 @@ func (mgr *Manager) restoreBundle(ctx context.Context, path string, server stora
 	if err != nil {
 		return fmt.Errorf("restore bundle: %q: %w", path, err)
 	}
-	stream, err := repoClient.CreateRepositoryFromBundle(ctx)
+	stream, err := repoClient.FetchBundle(ctx)
 	if err != nil {
 		return fmt.Errorf("restore bundle: %q: %w", path, err)
 	}
-	request := &gitalypb.CreateRepositoryFromBundleRequest{Repository: repo}
+	request := &gitalypb.FetchBundleRequest{Repository: repo}
 	bundle := streamio.NewWriter(func(p []byte) error {
 		request.Data = p
 		if err := stream.Send(request); err != nil {
@@ -329,7 +332,7 @@ func (mgr *Manager) restoreBundle(ctx context.Context, path string, server stora
 		}
 
 		// Only set `Repository` on the first `Send` of the stream
-		request = &gitalypb.CreateRepositoryFromBundleRequest{}
+		request = &gitalypb.FetchBundleRequest{}
 
 		return nil
 	})
diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go
index fd1dd04d8..27d015e5e 100644
--- a/internal/backup/backup_test.go
+++ b/internal/backup/backup_test.go
@@ -256,6 +256,7 @@ func testManagerRestore(t *testing.T, cfg config.Cfg, gitalyAddr string) {
 		desc          string
 		repo          *gitalypb.Repository
 		alwaysCreate  bool
+		expectExists  bool
 		expectedPaths []string
 		expectedErrAs error
 		expectVerify  bool
@@ -264,6 +265,7 @@ func testManagerRestore(t *testing.T, cfg config.Cfg, gitalyAddr string) {
 			desc:         "existing repo, without hooks",
 			repo:         existingRepo,
 			expectVerify: true,
+			expectExists: true,
 		},
 		{
 			desc: "existing repo, with hooks",
@@ -273,6 +275,7 @@ func testManagerRestore(t *testing.T, cfg config.Cfg, gitalyAddr string) {
 				"custom_hooks/prepare-commit-msg.sample",
 				"custom_hooks/pre-push.sample",
 			},
+			expectExists: true,
 			expectVerify: true,
 		},
 		{
@@ -284,11 +287,13 @@ func testManagerRestore(t *testing.T, cfg config.Cfg, gitalyAddr string) {
 			desc:         "missing bundle, always create",
 			repo:         missingBundleRepoAlwaysCreate,
 			alwaysCreate: true,
+			expectExists: true,
 		},
 		{
 			desc:         "nonexistent repo",
 			repo:         nonexistentRepo,
 			expectVerify: true,
+			expectExists: true,
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
@@ -307,6 +312,12 @@ func testManagerRestore(t *testing.T, cfg config.Cfg, gitalyAddr string) {
 				require.NoError(t, err)
 			}
 
+			exists, err := repoClient.RepositoryExists(ctx, &gitalypb.RepositoryExistsRequest{
+				Repository: tc.repo,
+			})
+			require.NoError(t, err)
+			require.Equal(t, tc.expectExists, exists.Exists)
+
 			if tc.expectVerify {
 				output := gittest.Exec(t, cfg, "-C", repoPath, "bundle", "verify", bundlePath)
 				require.Contains(t, string(output), "The bundle records a complete history")
diff --git a/internal/backup/locator.go b/internal/backup/locator.go
index 6110240b9..e93126e78 100644
--- a/internal/backup/locator.go
+++ b/internal/backup/locator.go
@@ -43,9 +43,10 @@ func (l LegacyLocator) newFull(repo *gitalypb.Repository) *Full {
 	backupPath := strings.TrimSuffix(repo.RelativePath, ".git")
 
 	return &Full{
-		BundlePath:      backupPath + ".bundle",
-		RefPath:         backupPath + ".refs",
-		CustomHooksPath: filepath.Join(backupPath, "custom_hooks.tar"),
+		SkippableOnNotFound: true,
+		BundlePath:          backupPath + ".bundle",
+		RefPath:             backupPath + ".refs",
+		CustomHooksPath:     filepath.Join(backupPath, "custom_hooks.tar"),
 	}
 }
 
diff --git a/internal/backup/locator_test.go b/internal/backup/locator_test.go
index f8e97a2e5..97b79d0ff 100644
--- a/internal/backup/locator_test.go
+++ b/internal/backup/locator_test.go
@@ -20,9 +20,10 @@ func TestLegacyLocator(t *testing.T) {
 		defer cancel()
 
 		expected := &Full{
-			BundlePath:      repo.RelativePath + ".bundle",
-			RefPath:         repo.RelativePath + ".refs",
-			CustomHooksPath: filepath.Join(repo.RelativePath, "custom_hooks.tar"),
+			SkippableOnNotFound: true,
+			BundlePath:          repo.RelativePath + ".bundle",
+			RefPath:             repo.RelativePath + ".refs",
+			CustomHooksPath:     filepath.Join(repo.RelativePath, "custom_hooks.tar"),
 		}
 
 		full := l.BeginFull(ctx, repo, "abc123")
@@ -36,9 +37,10 @@ func TestLegacyLocator(t *testing.T) {
 		defer cancel()
 
 		expected := &Full{
-			BundlePath:      repo.RelativePath + ".bundle",
-			RefPath:         repo.RelativePath + ".refs",
-			CustomHooksPath: filepath.Join(repo.RelativePath, "custom_hooks.tar"),
+			SkippableOnNotFound: true,
+			BundlePath:          repo.RelativePath + ".bundle",
+			RefPath:             repo.RelativePath + ".refs",
+			CustomHooksPath:     filepath.Join(repo.RelativePath, "custom_hooks.tar"),
 		}
 
 		full, err := l.FindLatestFull(ctx, repo)
@@ -114,9 +116,10 @@ func TestPointerLocator(t *testing.T) {
 			defer cancel()
 
 			expectedFallback := &Full{
-				BundlePath:      repo.RelativePath + ".bundle",
-				RefPath:         repo.RelativePath + ".refs",
-				CustomHooksPath: filepath.Join(repo.RelativePath, "custom_hooks.tar"),
+				SkippableOnNotFound: true,
+				BundlePath:          repo.RelativePath + ".bundle",
+				RefPath:             repo.RelativePath + ".refs",
+				CustomHooksPath:     filepath.Join(repo.RelativePath, "custom_hooks.tar"),
 			}
 
 			fallbackFull, err := l.FindLatestFull(ctx, repo)