From 8e604b17b2ea881573a66519ef1978db74e16d78 Mon Sep 17 00:00:00 2001 From: Yonghui Li Date: Tue, 8 Sep 2026 17:43:27 +0800 Subject: [PATCH] add ID to repo snapshot Signed-off-by: Yonghui Li --- changelogs/unreleased/10500-Lyndon-Li | 1 + pkg/repository/udmrepo/kopialib/lib_repo.go | 2 ++ .../udmrepo/kopialib/lib_repo_test.go | 2 ++ pkg/repository/udmrepo/repo.go | 1 + pkg/uploader/block/snapshot.go | 20 +++++++------------ pkg/uploader/block/snapshot_test.go | 4 +++- pkg/uploader/block/uploader.go | 10 +++++----- 7 files changed, 21 insertions(+), 19 deletions(-) create mode 100644 changelogs/unreleased/10500-Lyndon-Li diff --git a/changelogs/unreleased/10500-Lyndon-Li b/changelogs/unreleased/10500-Lyndon-Li new file mode 100644 index 000000000..458d50b45 --- /dev/null +++ b/changelogs/unreleased/10500-Lyndon-Li @@ -0,0 +1 @@ +Add ID to repo snapshot and modify block uploader to use snapshot ID in logs \ No newline at end of file diff --git a/pkg/repository/udmrepo/kopialib/lib_repo.go b/pkg/repository/udmrepo/kopialib/lib_repo.go index e60128358..35f6db402 100644 --- a/pkg/repository/udmrepo/kopialib/lib_repo.go +++ b/pkg/repository/udmrepo/kopialib/lib_repo.go @@ -712,6 +712,7 @@ func (kr *kopiaRepository) GetSnapshot(ctx context.Context, id udmrepo.ID) (udmr } return udmrepo.Snapshot{ + ID: udmrepo.ID(snap.ID), Source: snap.Source.Path, Description: snap.Description, StartTime: snap.StartTime.ToTime(), @@ -751,6 +752,7 @@ func (kr *kopiaRepository) ListSnapshot(ctx context.Context, source string) ([]u snapshots := []udmrepo.Snapshot{} for _, snap := range mani { snapshots = append(snapshots, udmrepo.Snapshot{ + ID: udmrepo.ID(snap.ID), Source: snap.Source.Path, Description: snap.Description, StartTime: snap.StartTime.ToTime(), diff --git a/pkg/repository/udmrepo/kopialib/lib_repo_test.go b/pkg/repository/udmrepo/kopialib/lib_repo_test.go index b4d487c43..cef533863 100644 --- a/pkg/repository/udmrepo/kopialib/lib_repo_test.go +++ b/pkg/repository/udmrepo/kopialib/lib_repo_test.go @@ -1609,6 +1609,7 @@ func TestGetSnapshot(t *testing.T) { snapshotID: udmrepo.ID("fake-id"), setRepoMock: true, expectedSnap: udmrepo.Snapshot{ + ID: "fake-id", Source: "fake-source", Description: "fake-desc", StartTime: mockMani.StartTime.ToTime(), @@ -1805,6 +1806,7 @@ func TestListSnapshot(t *testing.T) { setRepoMock: true, expectedSnaps: []udmrepo.Snapshot{ { + ID: "fake-id", Source: "fake-source", Description: "fake-desc", StartTime: mockMani.StartTime.ToTime(), diff --git a/pkg/repository/udmrepo/repo.go b/pkg/repository/udmrepo/repo.go index 5873db743..a2c3a7b05 100644 --- a/pkg/repository/udmrepo/repo.go +++ b/pkg/repository/udmrepo/repo.go @@ -98,6 +98,7 @@ type Metadata struct { } type Snapshot struct { + ID ID Source string Description string StartTime time.Time diff --git a/pkg/uploader/block/snapshot.go b/pkg/uploader/block/snapshot.go index 566ebad4f..370737bd6 100644 --- a/pkg/uploader/block/snapshot.go +++ b/pkg/uploader/block/snapshot.go @@ -150,11 +150,6 @@ func snapshotSource( func getParentBackupInfo(ctx context.Context, rep udmrepo.BackupRepo, forceFull bool, parentSnapshot string, volumeID string, realSource string, snapshotTags map[string]string, log logrus.FieldLogger) parentBackupInfo { var previous *udmrepo.Snapshot - // parentID names whichever snapshot ended up being the parent. On the discovery - // branch the parentSnapshot parameter is empty by definition, so logging it there - // produces messages that describe a decision without naming the object it was about. - parentID := parentSnapshot - if !forceFull { if parentSnapshot != "" { snap, err := rep.GetSnapshot(ctx, udmrepo.ID(parentSnapshot)) @@ -172,8 +167,7 @@ func getParentBackupInfo(ctx context.Context, rep udmrepo.BackupRepo, forceFull log.WithError(err).Warn("Failed to search previous snapshot, fallback to full backup") } else { previous = &snap - parentID = string(snap.RootObject.ID) - log.Infof("Using previous snapshot %s", snap.RootObject.ID) + log.Infof("Using previous snapshot %s", snap.ID) } } } else { @@ -183,21 +177,21 @@ func getParentBackupInfo(ctx context.Context, rep udmrepo.BackupRepo, forceFull parentInfo := parentBackupInfo{} if previous != nil { if previous.Tags == nil { - log.Warnf("No tag from parent snapshot %s, fallback to full backup", parentID) + log.Warnf("No tag from parent snapshot %s, fallback to full backup", previous.ID) } else if previous.Tags[uploader.CBTChangeIDTag] == "" { - log.Warnf("No ChangeID tag from parent snapshot %s, fallback to full backup", parentID) + log.Warnf("No ChangeID tag from parent snapshot %s, fallback to full backup", previous.ID) } else if previous.Tags[uploader.CBTVolumeIDTag] == "" { - log.Warnf("No VolumeID tag from parent snapshot %s, fallback to full backup", parentID) + log.Warnf("No VolumeID tag from parent snapshot %s, fallback to full backup", previous.ID) } else if previous.Tags[uploader.CBTVolumeIDTag] != volumeID { - log.Warnf("VolumeID %s from parent snapshot %s is not expected as %s, fallback to full backup", previous.Tags[uploader.CBTVolumeIDTag], parentID, volumeID) + log.Warnf("VolumeID %s from parent snapshot %s is not expected as %s, fallback to full backup", previous.Tags[uploader.CBTVolumeIDTag], previous.ID, volumeID) } else if obj, err := loadObjectFromSnapshot(ctx, rep, previous); err != nil { - log.WithError(err).Warnf("Failed to load object from parent snapshot %s, fallback to full backup", parentID) + log.WithError(err).Warnf("Failed to load object from parent snapshot %s, fallback to full backup", previous.ID) } else { parentInfo.parentObject = obj parentInfo.changeID = previous.Tags[uploader.CBTChangeIDTag] parentInfo.volumeID = previous.Tags[uploader.CBTVolumeIDTag] - log.Infof("Using parent snapshot %s, start time %v, end time %v, description %s", parentID, previous.StartTime, previous.EndTime, previous.Description) + log.Infof("Using parent snapshot %s, start time %v, end time %v, description %s", previous.ID, previous.StartTime, previous.EndTime, previous.Description) } } diff --git a/pkg/uploader/block/snapshot_test.go b/pkg/uploader/block/snapshot_test.go index ae95f12ef..260f94b82 100644 --- a/pkg/uploader/block/snapshot_test.go +++ b/pkg/uploader/block/snapshot_test.go @@ -351,6 +351,7 @@ func TestSnapshotSource(t *testing.T) { func TestGetParentBackupInfoLogsDiscoveredParentID(t *testing.T) { const volumeID = "vol-123" const realSource = "/test/source" + const parentSnapID = "snap-parent-42" const rootObj = "root-obj-42" snapshotTags := map[string]string{ @@ -364,6 +365,7 @@ func TestGetParentBackupInfoLogsDiscoveredParentID(t *testing.T) { repo := udmrepomocks.NewBackupRepo(t) repo.On("ListSnapshot", mock.Anything, realSource). Return([]udmrepo.Snapshot{{ + ID: parentSnapID, RootObject: udmrepo.ObjectMetadata{ID: rootObj}, Tags: map[string]string{ uploader.CBTChangeIDTag: "cid-abc", @@ -389,7 +391,7 @@ func TestGetParentBackupInfoLogsDiscoveredParentID(t *testing.T) { for _, entry := range hook.AllEntries() { if strings.HasPrefix(entry.Message, "Using parent snapshot ") { found = true - assert.Contains(t, entry.Message, rootObj, + assert.Contains(t, entry.Message, parentSnapID, "parent-selection message must name the discovered snapshot, got %q", entry.Message) } } diff --git a/pkg/uploader/block/uploader.go b/pkg/uploader/block/uploader.go index e2d464872..beccb3972 100644 --- a/pkg/uploader/block/uploader.go +++ b/pkg/uploader/block/uploader.go @@ -155,17 +155,17 @@ func (blkup *blockUploader) Restore(snapshot udmrepo.Snapshot, dest destInfo, bi meta, err := blkup.repoWriter.ReadMetadata(blkup.ctx, snapshot.RootObject.ID) if err != nil { - return 0, 0, errors.Wrapf(err, "error reading snapshot metadata for %s", snapshot.Description) + return 0, 0, errors.Wrapf(err, "error reading snapshot metadata for %s", snapshot.ID) } if len(meta.SubObjects) != 1 { - return 0, 0, errors.Errorf("unexpected number of bdev object (%d) for snapshot %s", len(meta.SubObjects), snapshot.Description) + return 0, 0, errors.Errorf("unexpected number of bdev object (%d) for snapshot %s", len(meta.SubObjects), snapshot.ID) } sourceSize, err := getSourceSize(snapshot) if err != nil { sourceSize = meta.SubObjects[0].Size - blkup.log.Warnf("Failed to get source size from snapshot %s, use backup size %v", snapshot.Description, sourceSize) + blkup.log.Warnf("Failed to get source size from snapshot %s, use backup size %v", snapshot.ID, sourceSize) } if sourceSize > meta.SubObjects[0].Size { @@ -655,11 +655,11 @@ func loadObjectFromSnapshot(ctx context.Context, rep udmrepo.BackupRepo, snapsho meta, err := rep.ReadMetadata(ctx, snapshot.RootObject.ID) if err != nil { - return "", errors.Wrapf(err, "error reading snapshot metadata for %s", snapshot.Description) + return "", errors.Wrap(err, "error reading snapshot metadata") } if len(meta.SubObjects) != 1 { - return "", errors.Errorf("unexpected number of bdev object (%d) for snapshot %s", len(meta.SubObjects), snapshot.Description) + return "", errors.Errorf("unexpected number of bdev object (%d)", len(meta.SubObjects)) } return meta.SubObjects[0].ID, nil