From c8126c6c41c013fd857b1c9ba36fb837e4b843f3 Mon Sep 17 00:00:00 2001 From: James Fargher Date: Mon, 15 Jan 2024 11:34:09 +1300 Subject: backup: Write latest manifest file Manifests currently cannot restore the "latest" backup because it would require an expensive object-storage directory traversal. So instead it defers to the pointer layout. The problem with this is that you then loose manifest only features like setting the default branch properly. Here we write two manifests: the normal backup manifest as before and an additional "latest" manifest file. This latest manifest is overwritten on each backup taken. For WORM we would ideally not overwrite any files, but until this is implemented we need something to fill the latest restore gap. Changelog: changed --- internal/backup/locator.go | 29 ++++++++++++++++++++--------- internal/backup/locator_test.go | 9 +++++++-- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/internal/backup/locator.go b/internal/backup/locator.go index 1e1c42550..216037aea 100644 --- a/internal/backup/locator.go +++ b/internal/backup/locator.go @@ -294,17 +294,10 @@ func (l ManifestLocator) Commit(ctx context.Context, backup *Backup) (returnErr return err } - f, err := l.Sink.GetWriter(ctx, manifestPath(backup.Repository, backup.ID)) - if err != nil { + if err := l.writeManifest(ctx, backup, backup.ID); err != nil { return fmt.Errorf("manifest: commit: %w", err) } - defer func() { - if err := f.Close(); err != nil && returnErr == nil { - returnErr = fmt.Errorf("manifest: commit: %w", err) - } - }() - - if err := toml.NewEncoder(f).Encode(backup); err != nil { + if err := l.writeManifest(ctx, backup, "+latest"); err != nil { return fmt.Errorf("manifest: commit: %w", err) } @@ -340,6 +333,24 @@ func (l ManifestLocator) Find(ctx context.Context, repo storage.Repository, back return &backup, nil } +func (l ManifestLocator) writeManifest(ctx context.Context, backup *Backup, backupID string) (returnErr error) { + f, err := l.Sink.GetWriter(ctx, manifestPath(backup.Repository, backupID)) + if err != nil { + return fmt.Errorf("write manifest: %w", err) + } + defer func() { + if err := f.Close(); err != nil && returnErr == nil { + returnErr = fmt.Errorf("write manifest: %w", err) + } + }() + + if err := toml.NewEncoder(f).Encode(backup); err != nil { + return fmt.Errorf("write manifest: %w", err) + } + + return nil +} + func manifestPath(repo storage.Repository, backupID string) string { storageName := repo.GetStorageName() // Other locators strip the .git suffix off of relative paths. This suffix diff --git a/internal/backup/locator_test.go b/internal/backup/locator_test.go index 90cac3664..cca9ee8e8 100644 --- a/internal/backup/locator_test.go +++ b/internal/backup/locator_test.go @@ -470,7 +470,9 @@ custom_hooks_path = '%[1]s/%[2]s/001.custom_hooks.tar' require.NoError(t, l.Commit(ctx, incremental)) manifest := testhelper.MustReadFile(t, filepath.Join(backupPath, "manifests", repo.StorageName, repo.RelativePath, backupID+".toml")) - require.Equal(t, fmt.Sprintf(`object_format = 'sha1' + latestManifest := testhelper.MustReadFile(t, filepath.Join(backupPath, "manifests", repo.StorageName, repo.RelativePath, "+latest.toml")) + + expectedManifest := fmt.Sprintf(`object_format = 'sha1' [[steps]] bundle_path = '%[1]s/%[2]s/001.bundle' @@ -482,7 +484,10 @@ bundle_path = '%[1]s/%[2]s/002.bundle' ref_path = '%[1]s/%[2]s/002.refs' previous_ref_path = '%[1]s/%[2]s/001.refs' custom_hooks_path = '%[1]s/%[2]s/002.custom_hooks.tar' -`, repo.RelativePath, backupID), string(manifest)) +`, repo.RelativePath, backupID) + + require.Equal(t, expectedManifest, string(manifest)) + require.Equal(t, expectedManifest, string(latestManifest)) }) } -- cgit v1.2.3 From 0a3c32cb49864bf5a045aaf3e32d034117f6ab0e Mon Sep 17 00:00:00 2001 From: James Fargher Date: Mon, 15 Jan 2024 14:10:20 +1300 Subject: backup: Try read latest manifest when available Now that a latest manifest is written, we can use this manifest when loading the "latest" backup. --- internal/backup/locator.go | 32 ++++++++--- internal/backup/locator_test.go | 117 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 6 deletions(-) diff --git a/internal/backup/locator.go b/internal/backup/locator.go index 216037aea..c0ccca3b3 100644 --- a/internal/backup/locator.go +++ b/internal/backup/locator.go @@ -267,6 +267,8 @@ func (l PointerLocator) writeLatest(ctx context.Context, path, target string) (r return nil } +const latestManifestName = "+latest" + // ManifestLocator locates backup paths based on manifest files that are // written to a predetermined path: // @@ -297,34 +299,52 @@ func (l ManifestLocator) Commit(ctx context.Context, backup *Backup) (returnErr if err := l.writeManifest(ctx, backup, backup.ID); err != nil { return fmt.Errorf("manifest: commit: %w", err) } - if err := l.writeManifest(ctx, backup, "+latest"); err != nil { - return fmt.Errorf("manifest: commit: %w", err) + if err := l.writeManifest(ctx, backup, latestManifestName); err != nil { + return fmt.Errorf("manifest: commit latest: %w", err) } return nil } -// FindLatest passes through to Fallback +// FindLatest loads the manifest called +latest. If this manifest does not +// exist, the Fallback is used. func (l ManifestLocator) FindLatest(ctx context.Context, repo storage.Repository) (*Backup, error) { - return l.Fallback.FindLatest(ctx, repo) + backup, err := l.readManifest(ctx, repo, latestManifestName) + switch { + case errors.Is(err, ErrDoesntExist): + return l.Fallback.FindLatest(ctx, repo) + case err != nil: + return nil, fmt.Errorf("manifest: find latest: %w", err) + } + + return backup, nil } // Find loads the manifest for the provided repo and backupID. If this manifest // does not exist, the fallback is used. func (l ManifestLocator) Find(ctx context.Context, repo storage.Repository, backupID string) (*Backup, error) { - f, err := l.Sink.GetReader(ctx, manifestPath(repo, backupID)) + backup, err := l.readManifest(ctx, repo, backupID) switch { case errors.Is(err, ErrDoesntExist): return l.Fallback.Find(ctx, repo, backupID) case err != nil: return nil, fmt.Errorf("manifest: find: %w", err) } + + return backup, nil +} + +func (l ManifestLocator) readManifest(ctx context.Context, repo storage.Repository, backupID string) (*Backup, error) { + f, err := l.Sink.GetReader(ctx, manifestPath(repo, backupID)) + if err != nil { + return nil, fmt.Errorf("read manifest: %w", err) + } defer f.Close() var backup Backup if err := toml.NewDecoder(f).Decode(&backup); err != nil { - return nil, fmt.Errorf("manifest: find: %w", err) + return nil, fmt.Errorf("read manifest: %w", err) } backup.ID = backupID diff --git a/internal/backup/locator_test.go b/internal/backup/locator_test.go index cca9ee8e8..1d6010554 100644 --- a/internal/backup/locator_test.go +++ b/internal/backup/locator_test.go @@ -610,3 +610,120 @@ custom_hooks_path = 'path/to/002.custom_hooks.tar' }) } } + +func TestManifestLocator_FindLatest(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + desc string + repo storage.Repository + setup func(t *testing.T, ctx context.Context, backupPath string) + expectedBackup *Backup + }{ + { + desc: "finds manifest", + repo: &gitalypb.Repository{ + StorageName: "default", + RelativePath: "vanity/repo.git", + }, + setup: func(t *testing.T, ctx context.Context, backupPath string) { + testhelper.WriteFiles(t, backupPath, map[string]any{ + "vanity/repo/LATEST": "abc123", + "vanity/repo/abc123/LATEST": "002", + "manifests/default/vanity/repo.git/+latest.toml": `object_format = 'sha1' + +[[steps]] +bundle_path = 'manifest-path/to/001.bundle' +ref_path = 'manifest-path/to/001.refs' +custom_hooks_path = 'manifest-path/to/001.custom_hooks.tar' + +[[steps]] +bundle_path = 'manifest-path/to/002.bundle' +ref_path = 'manifest-path/to/002.refs' +previous_ref_path = 'manifest-path/to/001.refs' +custom_hooks_path = 'manifest-path/to/002.custom_hooks.tar' +`, + }) + }, + expectedBackup: &Backup{ + ID: "+latest", + Repository: &gitalypb.Repository{ + StorageName: "default", + RelativePath: "vanity/repo.git", + }, + ObjectFormat: "sha1", + Steps: []Step{ + { + BundlePath: "manifest-path/to/001.bundle", + RefPath: "manifest-path/to/001.refs", + CustomHooksPath: "manifest-path/to/001.custom_hooks.tar", + }, + { + BundlePath: "manifest-path/to/002.bundle", + RefPath: "manifest-path/to/002.refs", + PreviousRefPath: "manifest-path/to/001.refs", + CustomHooksPath: "manifest-path/to/002.custom_hooks.tar", + }, + }, + }, + }, + { + desc: "fallback", + repo: &gitalypb.Repository{ + StorageName: "default", + RelativePath: "vanity/repo.git", + }, + setup: func(t *testing.T, ctx context.Context, backupPath string) { + testhelper.WriteFiles(t, backupPath, map[string]any{ + "vanity/repo/LATEST": "abc123", + "vanity/repo/abc123/LATEST": "002", + }) + }, + expectedBackup: &Backup{ + ID: "abc123", + Repository: &gitalypb.Repository{ + StorageName: "default", + RelativePath: "vanity/repo.git", + }, + ObjectFormat: "sha1", + Steps: []Step{ + { + BundlePath: "vanity/repo/abc123/001.bundle", + RefPath: "vanity/repo/abc123/001.refs", + CustomHooksPath: "vanity/repo/abc123/001.custom_hooks.tar", + }, + { + BundlePath: "vanity/repo/abc123/002.bundle", + RefPath: "vanity/repo/abc123/002.refs", + PreviousRefPath: "vanity/repo/abc123/001.refs", + CustomHooksPath: "vanity/repo/abc123/002.custom_hooks.tar", + }, + }, + }, + }, + } { + tc := tc + t.Run(tc.desc, func(t *testing.T) { + t.Parallel() + + ctx := testhelper.Context(t) + backupPath := testhelper.TempDir(t) + + tc.setup(t, ctx, backupPath) + + sink := NewFilesystemSink(backupPath) + var l Locator = PointerLocator{ + Sink: sink, + } + l = ManifestLocator{ + Sink: sink, + Fallback: l, + } + + backup, err := l.FindLatest(ctx, tc.repo) + require.NoError(t, err) + + require.Equal(t, tc.expectedBackup, backup) + }) + } +} -- cgit v1.2.3