Inspect changes in Gitaly 7a80007379240d0c628407b00062cf6f69c6334d
Upstream merge commit e8565895adcc changes Gitaly server version from 9d00c51754d1199119e46d68db3b233115118afa to 7a80007379240d0c628407b00062cf6f69c6334d, with changes in protocol or one of the source subtrees currently under watch.
To further inspect, go to an up to date Gitaly source clone and run
diff 9d00c51754d1199119e46d68db3b233115118afa..7a80007379240d0c628407b00062cf6f69c6334d -- proto/*.proto internal/backup
. Here is a preview, truncated at 500 lines:
diff --git a/internal/backup/backup.go b/internal/backup/backup.go
index a11aa4953..a2b38fc58 100644
--- a/internal/backup/backup.go
+++ b/internal/backup/backup.go
@@ -40,41 +40,48 @@ type Sink interface {
// Backup represents all the information needed to restore a backup for a repository
type Backup struct {
+ // ID is the identifier that uniquely identifies the backup for this repository.
+ ID string `toml:"-"`
+ // Repository is the repository being backed up.
+ Repository storage.Repository `toml:"-"`
// Steps are the ordered list of steps required to restore this backup
- Steps []Step
+ Steps []Step `toml:"steps"`
// ObjectFormat is the name of the object hash used by the repository.
- ObjectFormat string
+ ObjectFormat string `toml:"object_format"`
}
// Step represents an incremental step that makes up a complete backup for a repository
type Step struct {
// BundlePath is the path of the bundle
- BundlePath string
+ BundlePath string `toml:"bundle_path,omitempty"`
// RefPath is the path of the ref file
- RefPath string
+ RefPath string `toml:"ref_path,omitempty"`
// PreviousRefPath is the path of the previous ref file
- PreviousRefPath string
+ PreviousRefPath string `toml:"previous_ref_path,omitempty"`
// CustomHooksPath is the path of the custom hooks archive
- CustomHooksPath string
+ CustomHooksPath string `toml:"custom_hooks_path,omitempty"`
}
// Locator finds sink backup paths for repositories
type Locator interface {
- // BeginFull returns a tentative first step needed to create a new full backup.
- BeginFull(ctx context.Context, repo *gitalypb.Repository, backupID string) *Step
+ // BeginFull returns the tentative backup paths needed to create a full backup.
+ BeginFull(ctx context.Context, repo storage.Repository, backupID string) *Backup
- // BeginIncremental returns a tentative step needed to create a new incremental backup.
- BeginIncremental(ctx context.Context, repo *gitalypb.Repository, backupID string) (*Step, error)
+ // BeginIncremental returns the backup with the last element of Steps being
+ // the tentative step needed to create an incremental backup.
+ BeginIncremental(ctx context.Context, repo storage.Repository, backupID string) (*Backup, error)
- // Commit persists the step so that it can be looked up by FindLatest
- Commit(ctx context.Context, step *Step) error
+ // Commit persists the backup so that it can be looked up by FindLatest. It
+ // is expected that the last element of Steps will be the newly created
+ // backup.
+ Commit(ctx context.Context, backup *Backup) error
// FindLatest returns the latest backup that was written by Commit
- FindLatest(ctx context.Context, repo *gitalypb.Repository) (*Backup, error)
+ FindLatest(ctx context.Context, repo storage.Repository) (*Backup, error)
// Find returns the repository backup at the given backupID. If the backup does
// not exist then the error ErrDoesntExist is returned.
- Find(ctx context.Context, repo *gitalypb.Repository, backupID string) (*Backup, error)
+ Find(ctx context.Context, repo storage.Repository, backupID string) (*Backup, error)
}
// Repository abstracts git access required to make a repository backup
@@ -100,18 +107,25 @@ type Repository interface {
// ResolveLocator returns a locator implementation based on a locator identifier.
func ResolveLocator(layout string, sink Sink) (Locator, error) {
- legacy := LegacyLocator{}
+ var locator Locator = LegacyLocator{}
+
switch layout {
case "legacy":
- return legacy, nil
case "pointer":
- return PointerLocator{
+ locator = PointerLocator{
Sink: sink,
- Fallback: legacy,
- }, nil
+ Fallback: locator,
+ }
default:
return nil, fmt.Errorf("unknown layout: %q", layout)
}
+
+ locator = ManifestLocator{
+ Sink: sink,
+ Fallback: locator,
+ }
+
+ return locator, nil
}
// Manager manages process of the creating/restoring backups.
@@ -197,15 +211,15 @@ func (mgr *Manager) Create(ctx context.Context, req *CreateRequest) error {
return fmt.Errorf("manager: %w", err)
}
- var step *Step
+ var backup *Backup
if req.Incremental {
var err error
- step, err = mgr.locator.BeginIncremental(ctx, req.VanityRepository, req.BackupID)
+ backup, err = mgr.locator.BeginIncremental(ctx, req.VanityRepository, req.BackupID)
if err != nil {
return fmt.Errorf("manager: %w", err)
}
} else {
- step = mgr.locator.BeginFull(ctx, req.VanityRepository, req.BackupID)
+ backup = mgr.locator.BeginFull(ctx, req.VanityRepository, req.BackupID)
}
refs, err := repo.ListRefs(ctx)
@@ -216,6 +230,8 @@ func (mgr *Manager) Create(ctx context.Context, req *CreateRequest) error {
return fmt.Errorf("manager: %w", err)
}
+ step := &backup.Steps[len(backup.Steps)-1]
+
if err := mgr.writeRefs(ctx, step.RefPath, refs); err != nil {
return fmt.Errorf("manager: %w", err)
}
@@ -226,7 +242,7 @@ func (mgr *Manager) Create(ctx context.Context, req *CreateRequest) error {
return fmt.Errorf("manager: %w", err)
}
- if err := mgr.locator.Commit(ctx, step); err != nil {
+ if err := mgr.locator.Commit(ctx, backup); err != nil {
return fmt.Errorf("manager: %w", err)
}
diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go
index ac7b155c0..9b84cec55 100644
--- a/internal/backup/backup_test.go
+++ b/internal/backup/backup_test.go
@@ -173,6 +173,7 @@ func TestManager_Create(t *testing.T) {
StorageName: "some_storage",
}
+ manifestPath := filepath.Join(backupRoot, "manifests", vanityRepo.StorageName, vanityRepo.RelativePath, backupID+".toml")
refsPath := joinBackupPath(t, backupRoot, vanityRepo, backupID, "001.refs")
bundlePath := joinBackupPath(t, backupRoot, vanityRepo, backupID, "001.bundle")
customHooksPath := joinBackupPath(t, backupRoot, vanityRepo, backupID, "001.custom_hooks.tar")
@@ -224,6 +225,12 @@ func TestManager_Create(t *testing.T) {
require.NoFileExists(t, refsPath)
}
+ if tc.createsBundle || tc.createsRefList {
+ require.FileExists(t, manifestPath)
+ } else {
+ require.NoFileExists(t, manifestPath)
+ }
+
if tc.createsCustomHooks {
require.FileExists(t, customHooksPath)
} else {
diff --git a/internal/backup/locator.go b/internal/backup/locator.go
index f2897f2e0..da789011d 100644
--- a/internal/backup/locator.go
+++ b/internal/backup/locator.go
@@ -5,13 +5,15 @@ import (
"errors"
"fmt"
"io"
+ "path"
"path/filepath"
"strconv"
"strings"
+ "github.com/pelletier/go-toml/v2"
"gitlab.com/gitlab-org/gitaly/v16/internal/git"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/helper/text"
- "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
)
// LegacyLocator locates backup paths for historic backups. This is the
@@ -28,42 +30,43 @@ import (
type LegacyLocator struct{}
// BeginFull returns the static paths for a legacy repository backup
-func (l LegacyLocator) BeginFull(ctx context.Context, repo *gitalypb.Repository, backupID string) *Step {
+func (l LegacyLocator) BeginFull(ctx context.Context, repo storage.Repository, backupID string) *Backup {
return l.newFull(repo)
}
// BeginIncremental is not supported for legacy backups
-func (l LegacyLocator) BeginIncremental(ctx context.Context, repo *gitalypb.Repository, backupID string) (*Step, error) {
+func (l LegacyLocator) BeginIncremental(ctx context.Context, repo storage.Repository, backupID string) (*Backup, error) {
return nil, errors.New("legacy layout: begin incremental: not supported")
}
// Commit is unused as the locations are static
-func (l LegacyLocator) Commit(ctx context.Context, full *Step) error {
+func (l LegacyLocator) Commit(ctx context.Context, full *Backup) error {
return nil
}
// FindLatest returns the static paths for a legacy repository backup
-func (l LegacyLocator) FindLatest(ctx context.Context, repo *gitalypb.Repository) (*Backup, error) {
- return &Backup{
- Steps: []Step{
- *l.newFull(repo),
- },
- ObjectFormat: git.ObjectHashSHA1.Format,
- }, nil
+func (l LegacyLocator) FindLatest(ctx context.Context, repo storage.Repository) (*Backup, error) {
+ return l.newFull(repo), nil
}
// Find is not supported for legacy backups.
-func (l LegacyLocator) Find(ctx context.Context, repo *gitalypb.Repository, backupID string) (*Backup, error) {
+func (l LegacyLocator) Find(ctx context.Context, repo storage.Repository, backupID string) (*Backup, error) {
return nil, errors.New("legacy layout: find: not supported")
}
-func (l LegacyLocator) newFull(repo *gitalypb.Repository) *Step {
- backupPath := strings.TrimSuffix(repo.RelativePath, ".git")
+func (l LegacyLocator) newFull(repo storage.Repository) *Backup {
+ backupPath := strings.TrimSuffix(repo.GetRelativePath(), ".git")
- return &Step{
- BundlePath: backupPath + ".bundle",
- RefPath: backupPath + ".refs",
- CustomHooksPath: filepath.Join(backupPath, "custom_hooks.tar"),
+ return &Backup{
+ Repository: repo,
+ ObjectFormat: git.ObjectHashSHA1.Format,
+ Steps: []Step{
+ {
+ BundlePath: backupPath + ".bundle",
+ RefPath: backupPath + ".refs",
+ CustomHooksPath: filepath.Join(backupPath, "custom_hooks.tar"),
+ },
+ },
}
}
@@ -84,13 +87,20 @@ type PointerLocator struct {
}
// BeginFull returns a tentative first step needed to create a new full backup.
-func (l PointerLocator) BeginFull(ctx context.Context, repo *gitalypb.Repository, backupID string) *Step {
- repoPath := strings.TrimSuffix(repo.RelativePath, ".git")
+func (l PointerLocator) BeginFull(ctx context.Context, repo storage.Repository, backupID string) *Backup {
+ repoPath := strings.TrimSuffix(repo.GetRelativePath(), ".git")
- return &Step{
- BundlePath: filepath.Join(repoPath, backupID, "001.bundle"),
- RefPath: filepath.Join(repoPath, backupID, "001.refs"),
- CustomHooksPath: filepath.Join(repoPath, backupID, "001.custom_hooks.tar"),
+ return &Backup{
+ ID: backupID,
+ Repository: repo,
+ ObjectFormat: git.ObjectHashSHA1.Format,
+ Steps: []Step{
+ {
+ BundlePath: filepath.Join(repoPath, backupID, "001.bundle"),
+ RefPath: filepath.Join(repoPath, backupID, "001.refs"),
+ CustomHooksPath: filepath.Join(repoPath, backupID, "001.custom_hooks.tar"),
+ },
+ },
}
}
@@ -98,8 +108,8 @@ func (l PointerLocator) BeginFull(ctx context.Context, repo *gitalypb.Repository
// backup. The incremental backup is always based off of the latest full
// backup. If there is no latest backup, a new full backup step is returned
// using fallbackBackupID
-func (l PointerLocator) BeginIncremental(ctx context.Context, repo *gitalypb.Repository, fallbackBackupID string) (*Step, error) {
- repoPath := strings.TrimSuffix(repo.RelativePath, ".git")
+func (l PointerLocator) BeginIncremental(ctx context.Context, repo storage.Repository, fallbackBackupID string) (*Backup, error) {
+ repoPath := strings.TrimSuffix(repo.GetRelativePath(), ".git")
backupID, err := l.findLatestID(ctx, repoPath)
if err != nil {
if errors.Is(err, ErrDoesntExist) {
@@ -125,16 +135,23 @@ func (l PointerLocator) BeginIncremental(ctx context.Context, repo *gitalypb.Rep
}
id++
- return &Step{
+ backup.ID = fallbackBackupID
+ backup.Steps = append(backup.Steps, Step{
BundlePath: filepath.Join(backupPath, fmt.Sprintf("%03d.bundle", id)),
RefPath: filepath.Join(backupPath, fmt.Sprintf("%03d.refs", id)),
PreviousRefPath: previous.RefPath,
CustomHooksPath: filepath.Join(backupPath, fmt.Sprintf("%03d.custom_hooks.tar", id)),
- }, nil
+ })
+
+ return backup, nil
}
// Commit persists the step so that it can be looked up by FindLatest
-func (l PointerLocator) Commit(ctx context.Context, step *Step) error {
+func (l PointerLocator) Commit(ctx context.Context, backup *Backup) error {
+ if len(backup.Steps) < 1 {
+ return fmt.Errorf("pointer locator: commit: no steps")
+ }
+ step := backup.Steps[len(backup.Steps)-1]
backupPath := filepath.Dir(step.BundlePath)
bundleName := filepath.Base(step.BundlePath)
repoPath := filepath.Dir(backupPath)
@@ -153,8 +170,8 @@ func (l PointerLocator) Commit(ctx context.Context, step *Step) error {
// FindLatest returns the paths committed by the latest call to CommitFull.
//
// If there is no `LATEST` file, the result of the `Fallback` is used.
-func (l PointerLocator) FindLatest(ctx context.Context, repo *gitalypb.Repository) (*Backup, error) {
- repoPath := strings.TrimSuffix(repo.RelativePath, ".git")
+func (l PointerLocator) FindLatest(ctx context.Context, repo storage.Repository) (*Backup, error) {
+ repoPath := strings.TrimSuffix(repo.GetRelativePath(), ".git")
backupID, err := l.findLatestID(ctx, repoPath)
if err != nil {
@@ -173,7 +190,7 @@ func (l PointerLocator) FindLatest(ctx context.Context, repo *gitalypb.Repositor
// Find returns the repository backup at the given backupID. If the backup does
// not exist then the error ErrDoesntExist is returned.
-func (l PointerLocator) Find(ctx context.Context, repo *gitalypb.Repository, backupID string) (*Backup, error) {
+func (l PointerLocator) Find(ctx context.Context, repo storage.Repository, backupID string) (*Backup, error) {
backup, err := l.find(ctx, repo, backupID)
if err != nil {
return nil, fmt.Errorf("pointer locator: %w", err)
@@ -181,8 +198,8 @@ func (l PointerLocator) Find(ctx context.Context, repo *gitalypb.Repository, bac
return backup, nil
}
-func (l PointerLocator) find(ctx context.Context, repo *gitalypb.Repository, backupID string) (*Backup, error) {
- repoPath := strings.TrimSuffix(repo.RelativePath, ".git")
+func (l PointerLocator) find(ctx context.Context, repo storage.Repository, backupID string) (*Backup, error) {
+ repoPath := strings.TrimSuffix(repo.GetRelativePath(), ".git")
backupPath := filepath.Join(repoPath, backupID)
latestIncrementID, err := l.findLatestID(ctx, backupPath)
@@ -196,6 +213,8 @@ func (l PointerLocator) find(ctx context.Context, repo *gitalypb.Repository, bac
}
backup := Backup{
+ ID: backupID,
+ Repository: repo,
ObjectFormat: git.ObjectHashSHA1.Format,
}
@@ -247,3 +266,66 @@ func (l PointerLocator) writeLatest(ctx context.Context, path, target string) (r
return nil
}
+
+// ManifestLocator locates backup paths based on manifest files that are
+// written to a predetermined path:
+//
+// manifests/<repo_storage_name>/<repo_relative_path>/<backup_id>.toml
+//
+// It relies on Fallback to determine paths of new backups.
+type ManifestLocator struct {
+ Sink Sink
+ Fallback Locator
+}
+
+// BeginFull passes through to Fallback
+func (l ManifestLocator) BeginFull(ctx context.Context, repo storage.Repository, backupID string) *Backup {
+ return l.Fallback.BeginFull(ctx, repo, backupID)
+}
+
+// BeginIncremental passes through to Fallback
+func (l ManifestLocator) BeginIncremental(ctx context.Context, repo storage.Repository, backupID string) (*Backup, error) {
+ return l.Fallback.BeginIncremental(ctx, repo, backupID)
+}
+
+// Commit passes through to Fallback, then writes a manifest file for the backup.
+func (l ManifestLocator) Commit(ctx context.Context, backup *Backup) (returnErr error) {
+ if err := l.Fallback.Commit(ctx, backup); err != nil {
+ return err
+ }
+
+ f, err := l.Sink.GetWriter(ctx, manifestPath(backup))
+ if 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 {
+ return fmt.Errorf("manifest: commit: %w", err)
+ }
+
+ return nil
+}
+
+// FindLatest passes through to Fallback
+func (l ManifestLocator) FindLatest(ctx context.Context, repo storage.Repository) (*Backup, error) {
+ return l.Fallback.FindLatest(ctx, repo)
+}
+
+// Find passes through to Fallback
+func (l ManifestLocator) Find(ctx context.Context, repo storage.Repository, backupID string) (*Backup, error) {
+ return l.Fallback.Find(ctx, repo, backupID)
+}
+
+func manifestPath(backup *Backup) string {
+ storageName := backup.Repository.GetStorageName()
+ // Other locators strip the .git suffix off of relative paths. This suffix
+ // is determined by gitlab-rails not gitaly. So here we leave the relative
+ // path as-is so that new backups can be more independent.
+ relativePath := backup.Repository.GetRelativePath()
+ return path.Join("manifests", storageName, relativePath, backup.ID+".toml")
+}
diff --git a/internal/backup/locator_test.go b/internal/backup/locator_test.go
index c9f48fdf1..7aed83608 100644
--- a/internal/backup/locator_test.go
+++ b/internal/backup/locator_test.go
@@ -33,10 +33,17 @@ func TestLegacyLocator(t *testing.T) {
t.Run("Begin/Commit Full", func(t *testing.T) {
t.Parallel()
- expected := &Step{
- BundlePath: repo.RelativePath + ".bundle",
- RefPath: repo.RelativePath + ".refs",
- CustomHooksPath: filepath.Join(repo.RelativePath, "custom_hooks.tar"),
+ expected := &Backup{
+ ID: "", // legacy storage can only store a single backup.
+ Repository: repo,
+ ObjectFormat: git.ObjectHashSHA1.Format,
+ Steps: []Step{
+ {
+ BundlePath: repo.RelativePath + ".bundle",
+ RefPath: repo.RelativePath + ".refs",
+ CustomHooksPath: filepath.Join(repo.RelativePath, "custom_hooks.tar"),
+ },
+ },
}
full := l.BeginFull(ctx, repo, "abc123")
@@ -49,6 +56,8 @@ func TestLegacyLocator(t *testing.T) {
t.Parallel()
expected := &Backup{
+ ID: "", // legacy storage can only store a single backup.
+ Repository: repo,
ObjectFormat: git.ObjectHashSHA1.Format,
Steps: []Step{
{
@@ -89,10 +98,17 @@ func TestPointerLocator(t *testing.T) {
}
const expectedIncrement = "001"
- expected := &Step{
- BundlePath: filepath.Join(repo.RelativePath, backupID, expectedIncrement+".bundle"),
- RefPath: filepath.Join(repo.RelativePath, backupID, expectedIncrement+".refs"),
- CustomHooksPath: filepath.Join(repo.RelativePath, backupID, expectedIncrement+".custom_hooks.tar"),
+ expected := &Backup{
+ ID: backupID,
+ Repository: repo,
+ ObjectFormat: git.ObjectHashSHA1.Format,
+ Steps: []Step{
+ {
+ BundlePath: filepath.Join(repo.RelativePath, backupID, expectedIncrement+".bundle"),
+ RefPath: filepath.Join(repo.RelativePath, backupID, expectedIncrement+".refs"),
+ CustomHooksPath: filepath.Join(repo.RelativePath, backupID, expectedIncrement+".custom_hooks.tar"),
+ },
+ },
}
full := l.BeginFull(ctx, repo, backupID)
@@ -146,19 +162,24 @@ func TestPointerLocator(t *testing.T) {
tc.setup(t, ctx, backupPath)
}
- var expected *Step
+ var expected *Backup
for i := 1; i <= 3; i++ {
- incrementID := i + tc.expectedOffset
- var previousRefPath string
- if incrementID > 1 {
- previousRefPath = filepath.Join(repo.RelativePath, tc.expectedBackupID, fmt.Sprintf("%03d.refs", incrementID-1))
+ var previousRefPath, expectedIncrement string
+ expected = &Backup{
+ ID: fallbackBackupID,
+ Repository: repo,
+ ObjectFormat: git.ObjectHashSHA1.Format,
}
- expectedIncrement := fmt.Sprintf("%03d", incrementID)
- expected = &Step{
- BundlePath: filepath.Join(repo.RelativePath, tc.expectedBackupID, expectedIncrement+".bundle"),
- RefPath: filepath.Join(repo.RelativePath, tc.expectedBackupID, expectedIncrement+".refs"),
- PreviousRefPath: previousRefPath,
- CustomHooksPath: filepath.Join(repo.RelativePath, tc.expectedBackupID, expectedIncrement+".custom_hooks.tar"),
+ for incrementID := 1; incrementID <= i+tc.expectedOffset; incrementID++ {
+ expectedIncrement = fmt.Sprintf("%03d", incrementID)
+ step := Step{
+ BundlePath: filepath.Join(repo.RelativePath, tc.expectedBackupID, expectedIncrement+".bundle"),
+ RefPath: filepath.Join(repo.RelativePath, tc.expectedBackupID, expectedIncrement+".refs"),
+ PreviousRefPath: previousRefPath,
A merge request may have already been submitted, check that on [merge commit commit](e8565895adcc, as usually changes in Gitaly are not breaking, compliance becoming mandatory in the following development cycles.