diff options
author | James Liu <jliu@gitlab.com> | 2024-01-18 09:31:55 +0300 |
---|---|---|
committer | James Liu <jliu@gitlab.com> | 2024-01-18 13:36:15 +0300 |
commit | 396352f28151ef868a8088c9496af7caa971abf5 (patch) | |
tree | cd40978f6523a9a3be64782118f63a2af1a89f35 | |
parent | 8c7ef2ac66421fbdb70025a3ab1ed237e8affa7a (diff) |
backup: Check for backup before deleting repo
Reorders the repository restore logic so that we do not remove the repo
until we've checked that it has an existing backup. If the repo has no
backup, it's skipped from the restore and will be removed later by the
caller.
The previous order of operations caused issues when performing a full
restore that included a dangling repo (a repo created after the backup
was taken). When the `Restore` function was executed against this repo,
it was removed immediately. Then, the remainder of the restore was
skipped since no backup existed for that repo. Since the repo was not
marked as restored, the caller of the `Restore` function tried to remove
it once again, leading to a "repository not found" error since it had
already been erased.
The "missing backup" test case for the restore of a specific backup has
been updated to expect that the repo exists to align with this logic
-rw-r--r-- | internal/backup/backup.go | 8 | ||||
-rw-r--r-- | internal/backup/backup_test.go | 1 |
2 files changed, 5 insertions, 4 deletions
diff --git a/internal/backup/backup.go b/internal/backup/backup.go index a15f47abe..bc54cbaf0 100644 --- a/internal/backup/backup.go +++ b/internal/backup/backup.go @@ -335,10 +335,6 @@ func (mgr *Manager) Restore(ctx context.Context, req *RestoreRequest) error { return fmt.Errorf("manager: %w", err) } - if err := repo.Remove(ctx); err != nil { - return fmt.Errorf("manager: %w", err) - } - var backup *Backup if req.BackupID == "" { backup, err = mgr.locator.FindLatest(ctx, req.VanityRepository) @@ -359,6 +355,10 @@ func (mgr *Manager) Restore(ctx context.Context, req *RestoreRequest) error { defaultBranch, defaultBranchKnown := git.ReferenceName(backup.HeadReference).Branch() + if err := repo.Remove(ctx); err != nil { + return fmt.Errorf("manager: %w", err) + } + if err := repo.Create(ctx, hash, defaultBranch); err != nil { return fmt.Errorf("manager: %w", err) } diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index 81e87a33a..96a06fba3 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -977,6 +977,7 @@ func TestManager_Restore_specific(t *testing.T) { return repo, nil }, expectedErrAs: backup.ErrSkipped, + expectExists: true, }, { desc: "single incremental", |