refactor CBT retrievement to report the concrete error

Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
This commit is contained in:
Lyndon-Li
2026-09-09 15:55:01 +08:00
parent 44f09189c2
commit e52bf08c99
8 changed files with 249 additions and 78 deletions
+36 -39
View File
@@ -169,52 +169,49 @@ func getParentBackupInfo(ctx context.Context, rep udmrepo.BackupRepo, forceFull
}
var previous *udmrepo.Snapshot
if parentSnapshot != "" {
log.Infof("Loading provided parent snapshot %s", parentSnapshot)
if !forceFull {
if parentSnapshot != "" {
snap, err := rep.GetSnapshot(ctx, udmrepo.ID(parentSnapshot))
if err != nil {
log.WithError(err).Warn("Failed to load previous snapshot, fallback to full backup")
} else {
previous = &snap
log.Infof("Using provided parent snapshot %s", parentSnapshot)
}
} else {
log.Infof("Searching for parent snapshot")
snap, err := findPreviousSnapshot(ctx, rep, realSource, snapshotTags, nil, log)
if err != nil {
log.WithError(err).Warn("Failed to search previous snapshot, fallback to full backup")
} else {
previous = &snap
log.Infof("Using previous snapshot %s", snap.ID)
}
snap, err := rep.GetSnapshot(ctx, udmrepo.ID(parentSnapshot))
if err != nil {
return parentBackupInfo{}, errors.Wrapf(err, "error loading previous snapshot")
}
previous = &snap
} else {
log.Info("Forcing full snapshot")
}
log.Infof("Searching for parent snapshot")
parentInfo := parentBackupInfo{}
if previous != nil {
if previous.Tags == nil {
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", previous.ID)
} else if previous.Tags[uploader.CBTVolumeIDTag] == "" {
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], 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", 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", previous.ID, previous.StartTime, previous.EndTime, previous.Description)
snap, err := findPreviousSnapshot(ctx, rep, realSource, snapshotTags, nil, log)
if err != nil {
return parentBackupInfo{}, errors.Wrapf(err, "error searching previous snapshot")
}
previous = &snap
}
if previous.Tags == nil {
return parentBackupInfo{}, errors.Errorf("no tag from parent snapshot %s", previous.ID)
}
if previous.Tags[uploader.CBTChangeIDTag] == "" {
return parentBackupInfo{}, errors.Errorf("no ChangeID tag from parent snapshot %s", previous.ID)
}
if previous.Tags[uploader.CBTVolumeIDTag] == "" {
return parentBackupInfo{}, errors.Errorf("no VolumeID tag from parent snapshot %s", previous.ID)
}
if previous.Tags[uploader.CBTVolumeIDTag] != volumeID {
return parentBackupInfo{}, errors.Errorf("VolumeID %s from parent snapshot %s is not expected as %s", previous.Tags[uploader.CBTVolumeIDTag], previous.ID, volumeID)
}
obj, err := loadObjectFromSnapshot(ctx, rep, previous)
if err != nil {
return parentBackupInfo{}, errors.Wrapf(err, "error loading object from parent snapshot %s", previous.ID)
}
log.Infof("Using parent snapshot %s, start time %v, end time %v, description %s", previous.ID, previous.StartTime, previous.EndTime, previous.Description)
return parentBackupInfo{
parentObject: obj,
changeID: previous.Tags[uploader.CBTChangeIDTag],
+161 -13
View File
@@ -379,11 +379,12 @@ func TestGetParentBackupInfoLogsDiscoveredParentID(t *testing.T) {
SubObjects: []udmrepo.ObjectMetadata{{ID: udmrepo.ID("parent-obj")}},
}, nil)
info := getParentBackupInfo(
info, err := getParentBackupInfo(
context.Background(), repo,
false, "", // no explicit parent -> discovery branch
volumeID, realSource, snapshotTags, logger,
)
require.NoError(t, err)
require.Equal(t, udmrepo.ID("parent-obj"), info.parentObject)
@@ -408,6 +409,7 @@ func TestGetParentBackupInfo(t *testing.T) {
}
validSnap := udmrepo.Snapshot{
ID: "snap-valid",
RootObject: udmrepo.ObjectMetadata{ID: "root-obj"},
Tags: map[string]string{
uploader.CBTChangeIDTag: "cid-abc",
@@ -421,7 +423,10 @@ func TestGetParentBackupInfo(t *testing.T) {
name string
forceFull bool
parentSnapshot string
emptyVolID bool
setupMocks func(repo *udmrepomocks.BackupRepo)
expectErr bool
expectedErrStr string
expectEmpty bool
expectedParent udmrepo.ID
expectedCID string
@@ -432,6 +437,13 @@ func TestGetParentBackupInfo(t *testing.T) {
forceFull: true,
expectEmpty: true,
},
{
name: "volumeID not provided",
emptyVolID: true,
expectEmpty: true,
expectErr: true,
expectedErrStr: "volumeID is not provided from the volume snapshot",
},
{
name: "GetSnapshot fails — falls back to full",
parentSnapshot: "snap-parent",
@@ -439,46 +451,69 @@ func TestGetParentBackupInfo(t *testing.T) {
repo.On("GetSnapshot", mock.Anything, udmrepo.ID("snap-parent")).
Return(udmrepo.Snapshot{}, errors.New("not found"))
},
expectEmpty: true,
expectEmpty: true,
expectErr: true,
expectedErrStr: "error loading previous snapshot",
},
{
name: "parent snapshot has nil tags — falls back to full",
parentSnapshot: "snap-notags",
setupMocks: func(repo *udmrepomocks.BackupRepo) {
repo.On("GetSnapshot", mock.Anything, udmrepo.ID("snap-notags")).
Return(udmrepo.Snapshot{Tags: nil}, nil)
Return(udmrepo.Snapshot{ID: "snap-notags", Tags: nil}, nil)
},
expectEmpty: true,
expectEmpty: true,
expectErr: true,
expectedErrStr: "no tag from parent snapshot snap-notags",
},
{
name: "parent snapshot missing ChangeID tag — falls back to full",
parentSnapshot: "snap-nocid",
setupMocks: func(repo *udmrepomocks.BackupRepo) {
repo.On("GetSnapshot", mock.Anything, udmrepo.ID("snap-nocid")).
Return(udmrepo.Snapshot{Tags: map[string]string{uploader.CBTVolumeIDTag: volumeID}}, nil)
Return(udmrepo.Snapshot{ID: "snap-nocid", Tags: map[string]string{uploader.CBTVolumeIDTag: volumeID}}, nil)
},
expectEmpty: true,
expectEmpty: true,
expectErr: true,
expectedErrStr: "no ChangeID tag from parent snapshot snap-nocid",
},
{
name: "parent snapshot missing VolumeID tag — falls back to full",
parentSnapshot: "snap-novid",
setupMocks: func(repo *udmrepomocks.BackupRepo) {
repo.On("GetSnapshot", mock.Anything, udmrepo.ID("snap-novid")).
Return(udmrepo.Snapshot{Tags: map[string]string{uploader.CBTChangeIDTag: "cid"}}, nil)
Return(udmrepo.Snapshot{ID: "snap-novid", Tags: map[string]string{uploader.CBTChangeIDTag: "cid"}}, nil)
},
expectEmpty: true,
expectEmpty: true,
expectErr: true,
expectedErrStr: "no VolumeID tag from parent snapshot snap-novid",
},
{
name: "parent snapshot VolumeID mismatch — falls back to full",
parentSnapshot: "snap-vidmismatch",
setupMocks: func(repo *udmrepomocks.BackupRepo) {
repo.On("GetSnapshot", mock.Anything, udmrepo.ID("snap-vidmismatch")).
Return(udmrepo.Snapshot{Tags: map[string]string{
Return(udmrepo.Snapshot{ID: "snap-vidmismatch", Tags: map[string]string{
uploader.CBTChangeIDTag: "cid",
uploader.CBTVolumeIDTag: "different-vol",
}}, nil)
},
expectEmpty: true,
expectEmpty: true,
expectErr: true,
expectedErrStr: "VolumeID different-vol from parent snapshot snap-vidmismatch is not expected as vol-123",
},
{
name: "loadObjectFromSnapshot fails — falls back to full",
parentSnapshot: "snap-valid",
setupMocks: func(repo *udmrepomocks.BackupRepo) {
repo.On("GetSnapshot", mock.Anything, udmrepo.ID("snap-valid")).
Return(validSnap, nil)
repo.On("ReadMetadata", mock.Anything, udmrepo.ID("root-obj")).
Return(nil, errors.New("read error"))
},
expectEmpty: true,
expectErr: true,
expectedErrStr: "error loading object from parent snapshot snap-valid",
},
{
name: "valid parent snapshot — returns parent info",
@@ -499,7 +534,9 @@ func TestGetParentBackupInfo(t *testing.T) {
repo.On("ListSnapshot", mock.Anything, realSource).
Return(nil, errors.New("list error"))
},
expectEmpty: true,
expectEmpty: true,
expectErr: true,
expectedErrStr: "error searching previous snapshot",
},
{
name: "no parentSnapshot — no matching snapshot — falls back to full",
@@ -507,7 +544,9 @@ func TestGetParentBackupInfo(t *testing.T) {
repo.On("ListSnapshot", mock.Anything, realSource).
Return([]udmrepo.Snapshot{{Tags: map[string]string{"other": "tag"}}}, nil)
},
expectEmpty: true,
expectEmpty: true,
expectErr: true,
expectedErrStr: "error searching previous snapshot",
},
{
name: "no parentSnapshot — matching snapshot found — returns parent info",
@@ -532,7 +571,21 @@ func TestGetParentBackupInfo(t *testing.T) {
tc.setupMocks(mockRepo)
}
info := getParentBackupInfo(ctx, mockRepo, tc.forceFull, tc.parentSnapshot, volumeID, realSource, snapshotTags, testLog())
volID := volumeID
if tc.emptyVolID {
volID = ""
}
info, err := getParentBackupInfo(ctx, mockRepo, tc.forceFull, tc.parentSnapshot, volID, realSource, snapshotTags, testLog())
if tc.expectErr {
require.Error(t, err)
if tc.expectedErrStr != "" {
assert.Contains(t, err.Error(), tc.expectedErrStr)
}
} else {
require.NoError(t, err)
}
if tc.expectEmpty {
assert.Empty(t, info.parentObject)
@@ -547,6 +600,101 @@ func TestGetParentBackupInfo(t *testing.T) {
}
}
func TestGetBackupInfo(t *testing.T) {
const volumeID = "vol-123"
validSnap := udmrepo.Snapshot{
ID: "snap-valid",
Tags: map[string]string{
uploader.CBTChangeIDTag: "cid-abc",
uploader.CBTVolumeIDTag: volumeID,
},
}
testCases := []struct {
name string
snapshot udmrepo.Snapshot
volumeID string
expectErr bool
expectedErrStr string
expectedCID string
}{
{
name: "nil tags",
snapshot: udmrepo.Snapshot{ID: "snap-nil-tags"},
volumeID: volumeID,
expectErr: true,
expectedErrStr: "no tag from snapshot snap-nil-tags",
},
{
name: "missing ChangeID tag",
snapshot: udmrepo.Snapshot{
ID: "snap-no-cid",
Tags: map[string]string{uploader.CBTVolumeIDTag: volumeID},
},
volumeID: volumeID,
expectErr: true,
expectedErrStr: "no ChangeID tag from snapshot snap-no-cid",
},
{
name: "missing VolumeID tag",
snapshot: udmrepo.Snapshot{
ID: "snap-no-vid",
Tags: map[string]string{uploader.CBTChangeIDTag: "cid-abc"},
},
volumeID: volumeID,
expectErr: true,
expectedErrStr: "no VolumeID tag from snapshot snap-no-vid",
},
{
name: "empty volumeID parameter",
snapshot: udmrepo.Snapshot{
ID: "snap-valid",
Tags: map[string]string{
uploader.CBTChangeIDTag: "cid-abc",
uploader.CBTVolumeIDTag: volumeID,
},
},
volumeID: "",
expectErr: true,
expectedErrStr: "no VolumeID tag from the volume snapshot",
},
{
name: "volumeID mismatch",
snapshot: udmrepo.Snapshot{
ID: "snap-vid-mismatch",
Tags: map[string]string{
uploader.CBTChangeIDTag: "cid-abc",
uploader.CBTVolumeIDTag: "other-vol",
},
},
volumeID: volumeID,
expectErr: true,
expectedErrStr: "volumeID other-vol from snapshot snap-vid-mismatch is not expected as vol-123",
},
{
name: "valid snapshot",
snapshot: validSnap,
volumeID: volumeID,
expectErr: false,
expectedCID: "cid-abc",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
info, err := getBackupInfo(tc.snapshot, tc.volumeID)
if tc.expectErr {
require.Error(t, err)
assert.Contains(t, err.Error(), tc.expectedErrStr)
} else {
require.NoError(t, err)
assert.Equal(t, tc.expectedCID, info.changeID)
}
})
}
}
func TestFindPreviousSnapshot(t *testing.T) {
snapshotTags := map[string]string{
uploader.SnapshotRequesterTag: "test-requester",
+7 -8
View File
@@ -295,8 +295,7 @@ func TestBlockUploaderBackup(t *testing.T) {
var iterator cbt.Iterator
if !tc.nilBitmap {
iterMock := cbtmocks.NewIterator(t)
iterMock.On("Error").Return(nil).Maybe()
iterMock.On("Error").Return(nil).Maybe()
iterMock.On("Errors").Return(nil).Maybe()
iterator = iterMock
backupMode := udmrepo.ObjectDataBackupModeInc
@@ -575,7 +574,7 @@ func TestRestoreData(t *testing.T) {
reader := bytes.NewReader(data)
iterMock := cbtmocks.NewIterator(t)
iterMock.On("Error").Return(nil).Maybe()
iterMock.On("Errors").Return(nil).Maybe()
iterMock.On("Count").Return(uint64(1))
iterMock.On("Next").Return(uint64(0), true).Once()
iterMock.On("Next").Return(uint64(0), false)
@@ -606,7 +605,7 @@ func TestRestoreData(t *testing.T) {
reader := &errReader{err: errors.New("read error")}
iterMock := cbtmocks.NewIterator(t)
iterMock.On("Error").Return(nil).Maybe()
iterMock.On("Errors").Return(nil).Maybe()
iterMock.On("Count").Return(uint64(1))
iterMock.On("Next").Return(uint64(0), true).Once()
iterMock.On("Next").Return(uint64(0), false)
@@ -627,7 +626,7 @@ func TestBlockUploaderRestore(t *testing.T) {
repoWriter.On("ReadMetadata", mock.Anything, udmrepo.ID("root-id")).Return(nil, errors.New("meta not found"))
iterMock := cbtmocks.NewIterator(t)
iterMock.On("Error").Return(nil).Maybe()
iterMock.On("Errors").Return(nil).Maybe()
_, _, err := blkup.Restore(udmrepo.Snapshot{RootObject: udmrepo.ObjectMetadata{ID: "root-id"}}, destInfo{}, iterMock, nil)
require.Error(t, err)
assert.Contains(t, err.Error(), "meta not found")
@@ -685,7 +684,7 @@ func TestBlockUploaderRestore(t *testing.T) {
}
iterMock := cbtmocks.NewIterator(t)
iterMock.On("Error").Return(nil).Maybe()
iterMock.On("Errors").Return(nil).Maybe()
iterMock.On("Count").Return(uint64(1))
iterMock.On("Next").Return(uint64(0), true).Once()
iterMock.On("Next").Return(uint64(0), false)
@@ -714,7 +713,7 @@ func TestBlockUploaderRestore(t *testing.T) {
}
dest := destInfo{size: 4194304, path: "/dev/target"}
iterMock := cbtmocks.NewIterator(t)
iterMock.On("Error").Return(nil).Maybe()
iterMock.On("Errors").Return(nil).Maybe()
_, _, err := blkup.Restore(snap, dest, iterMock, nil)
require.Error(t, err)
@@ -739,7 +738,7 @@ func TestBlockUploaderRestore(t *testing.T) {
}
dest := destInfo{size: 512, path: "/dev/small"}
iterMock := cbtmocks.NewIterator(t)
iterMock.On("Error").Return(nil).Maybe()
iterMock.On("Errors").Return(nil).Maybe()
_, _, err := blkup.Restore(snap, dest, iterMock, nil)
require.Error(t, err)
+4
View File
@@ -94,6 +94,10 @@ func (c *bitmapImpl) VolumeID() string {
}
func (c *bitmapImpl) SetError(err error) {
if err == nil {
return
}
c.cbtErrors = append(c.cbtErrors, err)
}
+18 -5
View File
@@ -17,6 +17,7 @@ limitations under the License.
package cbt
import (
"errors"
"testing"
"github.com/stretchr/testify/assert"
@@ -24,10 +25,18 @@ import (
)
func TestBitmapProperties(t *testing.T) {
b := NewBitmap(1024*1024, 10000*1024*1024, "snap-1", "change-1", "vol-1")
b := NewBitmap(1024*1024, 10000*1024*1024, "snap-1", "vol-1")
assert.Equal(t, "snap-1", b.Snapshot())
assert.Equal(t, "change-1", b.ChangeID())
assert.Empty(t, b.ChangeID())
assert.Equal(t, "vol-1", b.VolumeID())
assert.Empty(t, b.Errors())
b.SetChangeID("change-1")
assert.Equal(t, "change-1", b.ChangeID())
err := errors.New("test error")
b.SetError(err)
assert.Equal(t, []error{err}, b.Errors())
}
func TestBitmapSet(t *testing.T) {
@@ -138,7 +147,7 @@ func TestBitmapSet(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
b := NewBitmap(tt.blockSize, tt.totalLength, "snap-1", "change-1", "vol-1")
b := NewBitmap(tt.blockSize, tt.totalLength, "snap-1", "vol-1")
for _, call := range tt.setCalls {
b.Set(call.offset, call.length)
@@ -174,7 +183,7 @@ func TestBitmapSetFull(t *testing.T) {
// block 0: 0 - 1MB
// block 1: 1MB - 2MB
// block 2: 2MB - 3MB
b := NewBitmap(mb, 3*mb, "snap-1", "change-1", "vol-1")
b := NewBitmap(mb, 3*mb, "snap-1", "vol-1")
b.SetFull()
iter := b.Iterator()
@@ -199,7 +208,10 @@ func TestBitmapIterator(t *testing.T) {
const mb = 1024 * 1024
const gb = 1024 * 1024 * 1024
b := NewBitmap(mb, 10*gb, "snap-1", "change-1", "vol-1")
b := NewBitmap(mb, 10*gb, "snap-1", "vol-1")
b.SetChangeID("change-1")
err := errors.New("test error")
b.SetError(err)
// Set multiple ranges to test contiguous iteration
b.Set(mb, 100) // Block 1
@@ -214,6 +226,7 @@ func TestBitmapIterator(t *testing.T) {
assert.Equal(t, "change-1", iter.ChangeID())
assert.Equal(t, "vol-1", iter.VolumeID())
assert.Equal(t, uint(mb), iter.BlockSize())
assert.Equal(t, []error{err}, iter.Errors())
assert.Equal(t, uint64(7), iter.Count()) // 1 + 5 + 1 = 7 blocks
expectedOffsets := []uint64{
+2 -1
View File
@@ -170,7 +170,8 @@ func TestSetBitmapOrFull(t *testing.T) {
svc = svcMock
}
bmp := NewBitmap(mb, 3*mb, tt.snapshotID, tt.changeID, "vol-1")
bmp := NewBitmap(mb, 3*mb, tt.snapshotID, "vol-1")
bmp.SetChangeID(tt.changeID)
err := SetBitmapOrFull(context.Background(), svc, bmp, tt.incOnly)
+13 -6
View File
@@ -292,23 +292,30 @@ func (_c *Bitmap_VolumeID_Call) RunAndReturn(run func() string) *Bitmap_VolumeID
_c.Call.Return(run)
return _c
}
// SetChangeID provides a mock function for the type Bitmap
func (_mock *Bitmap) SetChangeID(id string) {
_mock.Called(id)
}
// SetError provides a mock function for the type Bitmap
func (_mock *Bitmap) SetError(err error) {
_mock.Called(err)
}
// Error provides a mock function for the type Bitmap
func (_mock *Bitmap) Error() error {
// Errors provides a mock function for the type Bitmap
func (_mock *Bitmap) Errors() []error {
ret := _mock.Called()
if len(ret) == 0 {
panic("no return value specified for Error")
panic("no return value specified for Errors")
}
var r0 error
if returnFunc, ok := ret.Get(0).(func() error); ok {
var r0 []error
if returnFunc, ok := ret.Get(0).(func() []error); ok {
r0 = returnFunc()
} else {
r0 = ret.Error(0)
if ret.Get(0) != nil {
r0 = ret.Get(0).([]error)
}
}
return r0
}
+8 -6
View File
@@ -307,18 +307,20 @@ func (_c *Iterator_VolumeID_Call) RunAndReturn(run func() string) *Iterator_Volu
_c.Call.Return(run)
return _c
}
// Error provides a mock function for the type Iterator
func (_mock *Iterator) Error() error {
// Errors provides a mock function for the type Iterator
func (_mock *Iterator) Errors() []error {
ret := _mock.Called()
if len(ret) == 0 {
panic("no return value specified for Error")
panic("no return value specified for Errors")
}
var r0 error
if returnFunc, ok := ret.Get(0).(func() error); ok {
var r0 []error
if returnFunc, ok := ret.Get(0).(func() []error); ok {
r0 = returnFunc()
} else {
r0 = ret.Error(0)
if ret.Get(0) != nil {
r0 = ret.Get(0).([]error)
}
}
return r0
}