From f38bc20a0aec2a9e7dac3506061698b298236c0a Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Thu, 25 Jun 2026 17:01:17 +0800 Subject: [PATCH] block uploader snapshot implementation Signed-off-by: Lyndon-Li --- pkg/uploader/block/dev_linux.go | 2 +- pkg/uploader/block/dev_other.go | 2 +- pkg/uploader/block/snapshot.go | 18 +++++++++--------- pkg/uploader/block/snapshot_test.go | 19 ++++++++++++------- pkg/uploader/block/uploader.go | 10 +++++----- pkg/uploader/block/uploader_test.go | 4 ++-- pkg/uploader/provider/block_test.go | 10 +++++++--- 7 files changed, 37 insertions(+), 28 deletions(-) diff --git a/pkg/uploader/block/dev_linux.go b/pkg/uploader/block/dev_linux.go index 6383060fb..85b378c55 100644 --- a/pkg/uploader/block/dev_linux.go +++ b/pkg/uploader/block/dev_linux.go @@ -22,7 +22,7 @@ package block import ( "os" - "github.com/pkg/errors" + "github.com/cockroachdb/errors" ) // implement in following PRs diff --git a/pkg/uploader/block/dev_other.go b/pkg/uploader/block/dev_other.go index 60689a3d6..c8a55cab2 100644 --- a/pkg/uploader/block/dev_other.go +++ b/pkg/uploader/block/dev_other.go @@ -25,5 +25,5 @@ import ( ) func openBlockDevice(_ string, _ bool) (*os.File, error) { - return nil, fmt.Errorf("block mode is not supported for Windows") + return nil, fmt.Errorf("block mode is not supported for non-linux platforms") } diff --git a/pkg/uploader/block/snapshot.go b/pkg/uploader/block/snapshot.go index 41d42ba36..30626da53 100644 --- a/pkg/uploader/block/snapshot.go +++ b/pkg/uploader/block/snapshot.go @@ -23,7 +23,7 @@ import ( "path/filepath" "time" - "github.com/pkg/errors" + "github.com/cockroachdb/errors" "github.com/sirupsen/logrus" "github.com/vmware-tanzu/velero/pkg/cbtservice" @@ -41,9 +41,9 @@ type parentBackupInfo struct { } // Backup backup specific sourcePath and update progress -func Backup(ctx context.Context, blkup Uploader, repoWriter udmrepo.BackupRepo, sourcePath string, realSource string, cbtSource cbtservice.SourceInfo, - forceFull bool, parentSnapshot string, cbtservice cbtservice.Service, uploaderCfg map[string]string, tags map[string]string, log logrus.FieldLogger) (uploader.SnapshotInfo, bool, error) { - if blkup == nil { +func Backup(ctx context.Context, blkUp Uploader, repoWriter udmrepo.BackupRepo, sourcePath string, realSource string, cbtSource cbtservice.SourceInfo, + forceFull bool, parentSnapshot string, cbtService cbtservice.Service, uploaderCfg map[string]string, tags map[string]string, log logrus.FieldLogger) (uploader.SnapshotInfo, bool, error) { + if blkUp == nil { return uploader.SnapshotInfo{}, false, errors.New("get empty block uploader") } @@ -77,7 +77,7 @@ func Backup(ctx context.Context, blkup Uploader, repoWriter udmrepo.BackupRepo, return uploader.SnapshotInfo{}, false, errors.Wrapf(err, "error reset pos of block device %s", source) } - snapID, backupSize, err := snapshotSource(ctx, repoWriter, blkup, sourceInfo, forceFull, parentSnapshot, cbtSource, cbtservice, tags, uploaderCfg, log, "Block Uploader") + snapID, backupSize, err := snapshotSource(ctx, repoWriter, blkUp, sourceInfo, forceFull, parentSnapshot, cbtSource, cbtService, tags, uploaderCfg, log, "Block Uploader") snapshotInfo := uploader.SnapshotInfo{ ID: snapID, Size: sourceInfo.size, @@ -95,7 +95,7 @@ func snapshotSource( forceFull bool, parentSnapshot string, cbtSource cbtservice.SourceInfo, - cbtservice cbtservice.Service, + cbtService cbtservice.Service, snapshotTags map[string]string, uploaderCfg map[string]string, log logrus.FieldLogger, @@ -108,7 +108,7 @@ func snapshotSource( bitmap := cbt.NewBitmap(blockSize, uint64(source.size), cbtSource.Snapshot, parentBackup.changeID, parentBackup.volumeID) - err := cbt.SetBitmapOrFull(ctx, cbtservice, bitmap) + err := cbt.SetBitmapOrFull(ctx, cbtService, bitmap) if err != nil { parentBackup.parentObject = "" log.WithError(err).Warnf("Failed to create CBT with source %v, fallback to real full backup", cbtSource) @@ -193,7 +193,7 @@ func getParentBackupInfo(ctx context.Context, rep udmrepo.BackupRepo, forceFull } // Restore restore specific sourcePath with given snapshotID and update progress -func Restore(ctx context.Context, blkup Uploader, rep udmrepo.BackupRepo, snapshotID, dest string, uploaderCfg map[string]string, log logrus.FieldLogger) (int64, error) { +func Restore(ctx context.Context, blkUp Uploader, rep udmrepo.BackupRepo, snapshotID, dest string, uploaderCfg map[string]string, log logrus.FieldLogger) (int64, error) { log.Info("Start to restore...") snapshot, err := rep.GetSnapshot(ctx, udmrepo.ID(snapshotID)) @@ -218,7 +218,7 @@ func Restore(ctx context.Context, blkup Uploader, rep udmrepo.BackupRepo, snapsh return 0, errors.Wrapf(err, "error opening block device '%s'", destPath) } - size, err := blkup.Restore(snapshot, destInfo{dev: destDev, path: destPath}, bitmap.Iterator(), uploaderCfg) + size, err := blkUp.Restore(snapshot, destInfo{dev: destDev, path: destPath}, bitmap.Iterator(), uploaderCfg) if err != nil { return 0, errors.Wrapf(err, "error restoring to block dev %s", destPath) } diff --git a/pkg/uploader/block/snapshot_test.go b/pkg/uploader/block/snapshot_test.go index 5d17ee0f4..8f6338311 100644 --- a/pkg/uploader/block/snapshot_test.go +++ b/pkg/uploader/block/snapshot_test.go @@ -24,7 +24,7 @@ import ( "testing" "time" - "github.com/pkg/errors" + "github.com/cockroachdb/errors" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -59,7 +59,7 @@ func testLog() logrus.FieldLogger { func tempFile(t *testing.T, content string) *os.File { t.Helper() - f, err := os.CreateTemp("", "blktest-*") + f, err := os.CreateTemp(t.TempDir(), "blktest-*") require.NoError(t, err) if content != "" { _, err = f.WriteString(content) @@ -93,6 +93,7 @@ func TestBackup(t *testing.T) { { name: "SnapshotSource error propagates", setupOpenDev: func(t *testing.T) *os.File { + t.Helper() return tempFile(t, "") }, setupMocks: func(blkup *mockUploader, _ *udmrepomocks.BackupRepo) { @@ -104,6 +105,7 @@ func TestBackup(t *testing.T) { { name: "success returns correct SnapshotInfo", setupOpenDev: func(t *testing.T) *os.File { + t.Helper() return tempFile(t, "test-block-data") }, setupMocks: func(blkup *mockUploader, repo *udmrepomocks.BackupRepo) { @@ -113,9 +115,10 @@ func TestBackup(t *testing.T) { repo.On("Flush", mock.Anything).Return(nil) }, checkInfo: func(t *testing.T, info uploader.SnapshotInfo) { + t.Helper() assert.Equal(t, "snap-001", info.ID) assert.Equal(t, int64(8), info.IncrementalSize) - assert.Greater(t, info.Size, int64(0)) + assert.Positive(t, info.Size) }, }, } @@ -157,7 +160,7 @@ func TestBackup(t *testing.T) { if tc.expectedErrStr != "" { require.Error(t, err) - assert.ErrorContains(t, err, tc.expectedErrStr) + require.ErrorContains(t, err, tc.expectedErrStr) } else { require.NoError(t, err) assert.False(t, isEmpty) @@ -260,7 +263,7 @@ func TestSnapshotSource(t *testing.T) { if tc.expectedErrStr != "" { require.Error(t, err) - assert.ErrorContains(t, err, tc.expectedErrStr) + require.ErrorContains(t, err, tc.expectedErrStr) } else { require.NoError(t, err) assert.Equal(t, tc.expectedSnapID, snapID) @@ -530,7 +533,7 @@ func TestFindPreviousSnapshot(t *testing.T) { if tc.expectedErrStr != "" { require.Error(t, err) - assert.ErrorContains(t, err, tc.expectedErrStr) + require.ErrorContains(t, err, tc.expectedErrStr) } else { require.NoError(t, err) assert.Equal(t, udmrepo.ID(tc.expectedID), snap.RootObject.ID) @@ -574,6 +577,7 @@ func TestRestore(t *testing.T) { Return(int64(0), errors.New("restore I/O error")) }, setupOpenDev: func(t *testing.T) *os.File { + t.Helper() return tempFile(t, "") }, expectedErrStr: "error restoring to block dev", @@ -587,6 +591,7 @@ func TestRestore(t *testing.T) { Return(int64(4096), nil) }, setupOpenDev: func(t *testing.T) *os.File { + t.Helper() return tempFile(t, "") }, expectedSize: 4096, @@ -616,7 +621,7 @@ func TestRestore(t *testing.T) { if tc.expectedErrStr != "" { require.Error(t, err) - assert.ErrorContains(t, err, tc.expectedErrStr) + require.ErrorContains(t, err, tc.expectedErrStr) assert.Equal(t, int64(0), size) } else { require.NoError(t, err) diff --git a/pkg/uploader/block/uploader.go b/pkg/uploader/block/uploader.go index 7f089bd01..233d72a17 100644 --- a/pkg/uploader/block/uploader.go +++ b/pkg/uploader/block/uploader.go @@ -20,7 +20,7 @@ import ( "context" "os" - "github.com/pkg/errors" + "github.com/cockroachdb/errors" "github.com/sirupsen/logrus" "github.com/vmware-tanzu/velero/pkg/repository/udmrepo" @@ -60,14 +60,14 @@ func loadObjectFromSnapshot(ctx context.Context, rep udmrepo.BackupRepo, snapsho return "", errors.New("snapshot is empty") } - parentMeta, err := rep.ReadMetadata(ctx, snapshot.RootObject.ID) + meta, err := rep.ReadMetadata(ctx, snapshot.RootObject.ID) if err != nil { return "", errors.Wrapf(err, "error reading snapshot metadata for %s", snapshot.Description) } - if len(parentMeta.SubObjects) != 1 { - return "", errors.Errorf("unexpected number of bdev object (%d) for snapshot %s", len(parentMeta.SubObjects), snapshot.Description) + if len(meta.SubObjects) != 1 { + return "", errors.Errorf("unexpected number of bdev object (%d) for snapshot %s", len(meta.SubObjects), snapshot.Description) } - return parentMeta.SubObjects[0].ID, nil + return meta.SubObjects[0].ID, nil } diff --git a/pkg/uploader/block/uploader_test.go b/pkg/uploader/block/uploader_test.go index ea1986197..8209569e1 100644 --- a/pkg/uploader/block/uploader_test.go +++ b/pkg/uploader/block/uploader_test.go @@ -20,7 +20,7 @@ import ( "context" "testing" - "github.com/pkg/errors" + "github.com/cockroachdb/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -101,7 +101,7 @@ func TestLoadObjectFromSnapshot(t *testing.T) { if tc.expectedErrStr != "" { require.Error(t, err) - assert.ErrorContains(t, err, tc.expectedErrStr) + require.ErrorContains(t, err, tc.expectedErrStr) assert.Empty(t, id) } else { require.NoError(t, err) diff --git a/pkg/uploader/provider/block_test.go b/pkg/uploader/provider/block_test.go index e7af93855..8ec445168 100644 --- a/pkg/uploader/provider/block_test.go +++ b/pkg/uploader/provider/block_test.go @@ -280,6 +280,7 @@ func TestBlockProviderRunBackup(t *testing.T) { mockBackupResult: uploader.SnapshotInfo{ID: "snap-tags"}, expectedID: "snap-tags", checkCaptures: func(t *testing.T, _ string, tags map[string]string) { + t.Helper() assert.Equal(t, requestorType, tags[uploader.SnapshotRequesterTag]) assert.Equal(t, uploader.BlockType, tags[uploader.SnapshotUploaderTag]) }, @@ -292,6 +293,7 @@ func TestBlockProviderRunBackup(t *testing.T) { mockBackupResult: uploader.SnapshotInfo{ID: "snap-source"}, expectedID: "snap-source", checkCaptures: func(t *testing.T, realSource string, _ map[string]string) { + t.Helper() assert.Equal(t, requestorType+"/"+uploader.BlockType+"/my-volume", realSource) }, }, @@ -303,7 +305,8 @@ func TestBlockProviderRunBackup(t *testing.T) { mockBackupResult: uploader.SnapshotInfo{ID: "snap-nosource"}, expectedID: "snap-nosource", checkCaptures: func(t *testing.T, realSource string, _ map[string]string) { - assert.Equal(t, "", realSource) + t.Helper() + assert.Empty(t, realSource) }, }, } @@ -349,7 +352,7 @@ func TestBlockProviderRunBackup(t *testing.T) { if tc.expectError { require.Error(t, err) if tc.expectedErrStr != "" { - assert.ErrorContains(t, err, tc.expectedErrStr) + require.ErrorContains(t, err, tc.expectedErrStr) } } else { require.NoError(t, err) @@ -416,6 +419,7 @@ func TestBlockProviderRunRestore(t *testing.T) { mockRestoreSize: 512, expectedSize: 512, checkCaptures: func(t *testing.T, snapshotID, volumePath string) { + t.Helper() assert.Equal(t, "snap-fwd", snapshotID) assert.Equal(t, "/dev/sdc", volumePath) }, @@ -452,7 +456,7 @@ func TestBlockProviderRunRestore(t *testing.T) { if tc.expectError { require.Error(t, err) if tc.expectedErrStr != "" { - assert.ErrorContains(t, err, tc.expectedErrStr) + require.ErrorContains(t, err, tc.expectedErrStr) } assert.Equal(t, int64(0), size) } else {