Inspect changes in Gitaly 5763976d9d8ec1559fc17b1b236c82089c369c4c
Upstream merge commit 2ffd76c3d604 changes Gitaly server version from 8e9a11bede2d946964dc9a58c5e28952dc78ea86 to 5763976d9d8ec1559fc17b1b236c82089c369c4c, 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 8e9a11bede2d946964dc9a58c5e28952dc78ea86..5763976d9d8ec1559fc17b1b236c82089c369c4c -- proto/*.proto internal/backup
. Here is a preview, truncated at 500 lines:
diff --git a/internal/backup/locator.go b/internal/backup/locator.go
index da789011d..1e1c42550 100644
--- a/internal/backup/locator.go
+++ b/internal/backup/locator.go
@@ -294,7 +294,7 @@ func (l ManifestLocator) Commit(ctx context.Context, backup *Backup) (returnErr
return err
}
- f, err := l.Sink.GetWriter(ctx, manifestPath(backup))
+ f, err := l.Sink.GetWriter(ctx, manifestPath(backup.Repository, backup.ID))
if err != nil {
return fmt.Errorf("manifest: commit: %w", err)
}
@@ -316,16 +316,35 @@ func (l ManifestLocator) FindLatest(ctx context.Context, repo storage.Repository
return l.Fallback.FindLatest(ctx, repo)
}
-// Find passes through to Fallback
+// 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) {
- return l.Fallback.Find(ctx, repo, backupID)
+ f, err := l.Sink.GetReader(ctx, manifestPath(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)
+ }
+ defer f.Close()
+
+ var backup Backup
+
+ if err := toml.NewDecoder(f).Decode(&backup); err != nil {
+ return nil, fmt.Errorf("manifest: find: %w", err)
+ }
+
+ backup.ID = backupID
+ backup.Repository = repo
+
+ return &backup, nil
}
-func manifestPath(backup *Backup) string {
- storageName := backup.Repository.GetStorageName()
+func manifestPath(repo storage.Repository, backupID string) string {
+ storageName := repo.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")
+ relativePath := repo.GetRelativePath()
+ return path.Join("manifests", storageName, relativePath, backupID+".toml")
}
diff --git a/internal/backup/locator_test.go b/internal/backup/locator_test.go
index 7aed83608..90cac3664 100644
--- a/internal/backup/locator_test.go
+++ b/internal/backup/locator_test.go
@@ -11,9 +11,11 @@ import (
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitaly/v16/internal/git"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
+ "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
)
func TestLegacyLocator(t *testing.T) {
@@ -483,3 +485,123 @@ custom_hooks_path = '%[1]s/%[2]s/002.custom_hooks.tar'
`, repo.RelativePath, backupID), string(manifest))
})
}
+
+func TestManifestLocator_Find(t *testing.T) {
+ t.Parallel()
+
+ for _, tc := range []struct {
+ desc string
+ repo storage.Repository
+ backupID string
+ setup func(t *testing.T, ctx context.Context, backupPath string)
+ expectedBackup *Backup
+ }{
+ {
+ desc: "finds manifest",
+ repo: &gitalypb.Repository{
+ StorageName: "default",
+ RelativePath: "vanity/repo.git",
+ },
+ backupID: "abc123",
+ 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/abc123.toml": `object_format = 'sha1'
+
+[[steps]]
+bundle_path = 'path/to/001.bundle'
+ref_path = 'path/to/001.refs'
+custom_hooks_path = 'path/to/001.custom_hooks.tar'
+
+[[steps]]
+bundle_path = 'path/to/002.bundle'
+ref_path = 'path/to/002.refs'
+previous_ref_path = 'path/to/001.refs'
+custom_hooks_path = 'path/to/002.custom_hooks.tar'
+`,
+ })
+ },
+ expectedBackup: &Backup{
+ ID: "abc123",
+ Repository: &gitalypb.Repository{
+ StorageName: "default",
+ RelativePath: "vanity/repo.git",
+ },
+ ObjectFormat: "sha1",
+ Steps: []Step{
+ {
+ BundlePath: "path/to/001.bundle",
+ RefPath: "path/to/001.refs",
+ CustomHooksPath: "path/to/001.custom_hooks.tar",
+ },
+ {
+ BundlePath: "path/to/002.bundle",
+ RefPath: "path/to/002.refs",
+ PreviousRefPath: "path/to/001.refs",
+ CustomHooksPath: "path/to/002.custom_hooks.tar",
+ },
+ },
+ },
+ },
+ {
+ desc: "fallback",
+ repo: &gitalypb.Repository{
+ StorageName: "default",
+ RelativePath: "vanity/repo.git",
+ },
+ backupID: "abc123",
+ 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.Find(ctx, tc.repo, tc.backupID)
+ require.NoError(t, err)
+
+ require.Equal(t, tc.expectedBackup, backup)
+ })
+ }
+}
diff --git a/internal/backup/pipeline.go b/internal/backup/pipeline.go
index e370e030b..8e4e15784 100644
--- a/internal/backup/pipeline.go
+++ b/internal/backup/pipeline.go
@@ -175,7 +175,7 @@ func (p *LoggingPipeline) Handle(ctx context.Context, cmd Command) {
if err := cmd.Execute(ctx); err != nil {
if errors.Is(err, ErrSkipped) {
- log.WithError(err).Warn(fmt.Sprintf("skipped %s", cmd.Name()))
+ log.Warn(fmt.Sprintf("skipped %s", cmd.Name()))
} else {
log.WithError(err).Error(fmt.Sprintf("%s failed", cmd.Name()))
p.addError(cmd.Repository(), err)
diff --git a/internal/backup/pipeline_test.go b/internal/backup/pipeline_test.go
index 62c371352..fe3c82616 100644
--- a/internal/backup/pipeline_test.go
+++ b/internal/backup/pipeline_test.go
@@ -7,8 +7,10 @@ import (
"testing"
"time"
+ "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/log"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
)
@@ -123,37 +125,88 @@ func (s MockStrategy) RemoveAllRepositories(ctx context.Context, req *RemoveAllR
}
func testPipeline(t *testing.T, init func() Pipeline) {
- t.Run("create command", func(t *testing.T) {
- t.Parallel()
-
- strategy := MockStrategy{
- CreateFunc: func(_ context.Context, req *CreateRequest) error {
- switch req.Repository.StorageName {
- case "normal":
- return nil
- case "skip":
- return ErrSkipped
- case "error":
- return assert.AnError
- }
- require.Failf(t, "unexpected call to Create", "StorageName = %q", req.Repository.StorageName)
+ strategy := MockStrategy{
+ CreateFunc: func(_ context.Context, req *CreateRequest) error {
+ switch req.Repository.StorageName {
+ case "normal":
return nil
+ case "skip":
+ return ErrSkipped
+ case "error":
+ return assert.AnError
+ }
+ require.Failf(t, "unexpected call to Create", "StorageName = %q", req.Repository.StorageName)
+ return nil
+ },
+ }
+
+ for _, tc := range []struct {
+ desc string
+ command Command
+ level logrus.Level
+ expectedFields log.Fields
+ }{
+ {
+ desc: "Create command. Normal repository",
+ command: NewCreateCommand(strategy, CreateRequest{Repository: &gitalypb.Repository{RelativePath: "a.git", StorageName: "normal"}}),
+ level: logrus.InfoLevel,
+ expectedFields: log.Fields{
+ "command": "create",
+ "gl_project_path": "",
+ "relative_path": "a.git",
+ "storage_name": "normal",
},
- }
- p := init()
- ctx := testhelper.Context(t)
+ },
+ {
+ desc: "Create command. Skipped repository",
+ command: NewCreateCommand(strategy, CreateRequest{Repository: &gitalypb.Repository{RelativePath: "b.git", StorageName: "skip"}}),
+ level: logrus.WarnLevel,
+ expectedFields: log.Fields{
+ "command": "create",
+ "gl_project_path": "",
+ "relative_path": "b.git",
+ "storage_name": "skip",
+ },
+ },
+ {
+ desc: "Create command. Error creating repository",
+ command: NewCreateCommand(strategy, CreateRequest{Repository: &gitalypb.Repository{RelativePath: "c.git", StorageName: "error"}}),
+ level: logrus.ErrorLevel,
+ expectedFields: log.Fields{
+ "command": "create",
+ "gl_project_path": "",
+ "relative_path": "c.git",
+ "storage_name": "error",
+ },
+ },
+ } {
+ tc := tc
- commands := []Command{
- NewCreateCommand(strategy, CreateRequest{Repository: &gitalypb.Repository{RelativePath: "a.git", StorageName: "normal"}}),
- NewCreateCommand(strategy, CreateRequest{Repository: &gitalypb.Repository{RelativePath: "b.git", StorageName: "skip"}}),
- NewCreateCommand(strategy, CreateRequest{Repository: &gitalypb.Repository{RelativePath: "c.git", StorageName: "error"}}),
- }
- for _, cmd := range commands {
- p.Handle(ctx, cmd)
- }
- err := p.Done()
- require.EqualError(t, err, "pipeline: 1 failures encountered:\n - c.git: assert.AnError general error for testing\n")
- })
+ t.Run(tc.desc, func(t *testing.T) {
+ logger := testhelper.SharedLogger(t)
+ loggerHook := testhelper.AddLoggerHook(logger)
+
+ t.Parallel()
+
+ p := init()
+ ctx := testhelper.Context(t)
+
+ p.Handle(ctx, tc.command)
+
+ logEntries := loggerHook.AllEntries()
+
+ for _, logEntry := range logEntries {
+ require.Equal(t, tc.expectedFields, logEntry.Data)
+ require.Equal(t, tc.level, logEntry.Level)
+ }
+
+ err := p.Done()
+
+ if tc.level == logrus.ErrorLevel {
+ require.EqualError(t, err, "pipeline: 1 failures encountered:\n - c.git: assert.AnError general error for testing\n")
+ }
+ })
+ }
t.Run("restore command", func(t *testing.T) {
t.Parallel()
A merge request may have already been submitted, check that on [merge commit commit](2ffd76c3d604, as usually changes in Gitaly are not breaking, compliance becoming mandatory in the following development cycles.