Skip to content

Commit

Permalink
Refactor the native snapshot definition code.
Browse files Browse the repository at this point in the history 
Signed-off-by: Xun Jiang <[email protected]>
  • Loading branch information
@blackpiglet
blackpiglet committed Mar 20, 2024
1 parent 67c06e6 commit 94b9e0d
Show file tree
Hide file tree
Showing 19 changed files with 67 additions and 80 deletions.
0 pkg/volume/snapshot.go → internal/volume/native_snapshot.go
View file
File renamed without changes.
3 changes: 1 addition & 2 deletions internal/volume/volumes_information.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"github.com/vmware-tanzu/velero/pkg/itemoperation"
"github.com/vmware-tanzu/velero/pkg/kuberesource"
"github.com/vmware-tanzu/velero/pkg/plugin/velero"
"github.com/vmware-tanzu/velero/pkg/volume"
)

type VolumeBackupMethod string
Expand Down Expand Up @@ -188,7 +187,7 @@ type VolumesInformation struct {
volumeSnapshotContents []snapshotv1api.VolumeSnapshotContent
volumeSnapshotClasses []snapshotv1api.VolumeSnapshotClass
SkippedPVs map[string]string
NativeSnapshots []*volume.Snapshot
NativeSnapshots []*Snapshot
PodVolumeBackups []*velerov1api.PodVolumeBackup
BackupOperations []*itemoperation.BackupOperation
BackupName string
Expand Down
23 changes: 11 additions & 12 deletions internal/volume/volumes_information_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (
"github.com/vmware-tanzu/velero/pkg/plugin/velero"
velerotest "github.com/vmware-tanzu/velero/pkg/test"
"github.com/vmware-tanzu/velero/pkg/util/logging"
"github.com/vmware-tanzu/velero/pkg/volume"
)

func TestGenerateVolumeInfoForSkippedPV(t *testing.T) {
Expand Down Expand Up @@ -142,14 +141,14 @@ func TestGenerateVolumeInfoForSkippedPV(t *testing.T) {
func TestGenerateVolumeInfoForVeleroNativeSnapshot(t *testing.T) {
tests := []struct {
name string
nativeSnapshot volume.Snapshot
nativeSnapshot Snapshot
pvMap map[string]pvcPvInfo
expectedVolumeInfos []*VolumeInfo
}{
{
name: "Native snapshot's IPOS pointer is nil",
nativeSnapshot: volume.Snapshot{
Spec: volume.SnapshotSpec{
nativeSnapshot: Snapshot{
Spec: SnapshotSpec{
PersistentVolumeName: "testPV",
VolumeIOPS: nil,
},
Expand All @@ -158,8 +157,8 @@ func TestGenerateVolumeInfoForVeleroNativeSnapshot(t *testing.T) {
},
{
name: "Cannot find info for the PV",
nativeSnapshot: volume.Snapshot{
Spec: volume.SnapshotSpec{
nativeSnapshot: Snapshot{
Spec: SnapshotSpec{
PersistentVolumeName: "testPV",
VolumeIOPS: int64Ptr(100),
},
Expand All @@ -183,14 +182,14 @@ func TestGenerateVolumeInfoForVeleroNativeSnapshot(t *testing.T) {
},
},
},
nativeSnapshot: volume.Snapshot{
Spec: volume.SnapshotSpec{
nativeSnapshot: Snapshot{
Spec: SnapshotSpec{
PersistentVolumeName: "testPV",
VolumeIOPS: int64Ptr(100),
VolumeType: "ssd",
VolumeAZ: "us-central1-a",
},
Status: volume.SnapshotStatus{
Status: SnapshotStatus{
ProviderSnapshotID: "pvc-b31e3386-4bbb-4937-95d-7934cd62-b0a1-494b-95d7-0687440e8d0c",
},
},
Expand All @@ -213,14 +212,14 @@ func TestGenerateVolumeInfoForVeleroNativeSnapshot(t *testing.T) {
},
},
},
nativeSnapshot: volume.Snapshot{
Spec: volume.SnapshotSpec{
nativeSnapshot: Snapshot{
Spec: SnapshotSpec{
PersistentVolumeName: "testPV",
VolumeIOPS: int64Ptr(100),
VolumeType: "ssd",
VolumeAZ: "us-central1-a",
},
Status: volume.SnapshotStatus{
Status: SnapshotStatus{
ProviderSnapshotID: "pvc-b31e3386-4bbb-4937-95d-7934cd62-b0a1-494b-95d7-0687440e8d0c",
},
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/backup/backup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"

"github.com/vmware-tanzu/velero/internal/resourcepolicies"
"github.com/vmware-tanzu/velero/internal/volume"
velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/builder"
"github.com/vmware-tanzu/velero/pkg/client"
Expand All @@ -55,7 +56,6 @@ import (
"github.com/vmware-tanzu/velero/pkg/podvolume"
"github.com/vmware-tanzu/velero/pkg/test"
kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube"
"github.com/vmware-tanzu/velero/pkg/volume"
)

func TestBackedUpItemsMatchesTarballContents(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/backup/item_backupper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (

"github.com/vmware-tanzu/velero/internal/hook"
"github.com/vmware-tanzu/velero/internal/resourcepolicies"
"github.com/vmware-tanzu/velero/internal/volume"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/archive"
"github.com/vmware-tanzu/velero/pkg/client"
Expand All @@ -54,7 +55,6 @@ import (
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
csiutil "github.com/vmware-tanzu/velero/pkg/util/csi"
pdvolumeutil "github.com/vmware-tanzu/velero/pkg/util/podvolume"
"github.com/vmware-tanzu/velero/pkg/volume"
)

const (
Expand Down
2 changes: 1 addition & 1 deletion pkg/backup/request.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import (

"github.com/vmware-tanzu/velero/internal/hook"
"github.com/vmware-tanzu/velero/internal/resourcepolicies"
"github.com/vmware-tanzu/velero/internal/volume"
internalVolume "github.com/vmware-tanzu/velero/internal/volume"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/itemoperation"
"github.com/vmware-tanzu/velero/pkg/plugin/framework"
"github.com/vmware-tanzu/velero/pkg/util/collections"
"github.com/vmware-tanzu/velero/pkg/volume"
)

type itemKey struct {
Expand Down
3 changes: 1 addition & 2 deletions pkg/cmd/util/output/backup_describer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import (
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
"github.com/vmware-tanzu/velero/pkg/util/collections"
"github.com/vmware-tanzu/velero/pkg/util/results"
nativesnap "github.com/vmware-tanzu/velero/pkg/volume"
)

// DescribeBackup describes a backup in human-readable format.
Expand Down Expand Up @@ -502,7 +501,7 @@ func retrieveNativeSnapshotLegacy(ctx context.Context, kbClient kbclient.Client,
return nativeSnapshots, errors.Wrapf(err, "error to download native snapshot info")
}

var snapshots []*nativesnap.Snapshot
var snapshots []*volume.Snapshot

Check warning on line 504 in pkg/cmd/util/output/backup_describer.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/util/output/backup_describer.go#L504 

Added line #L504 was not covered by tests
if err := json.NewDecoder(buf).Decode(&snapshots); err != nil {
return nativeSnapshots, errors.Wrapf(err, "error to decode native snapshot info")
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/controller/backup_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import (
"github.com/vmware-tanzu/velero/internal/credentials"
"github.com/vmware-tanzu/velero/internal/resourcepolicies"
"github.com/vmware-tanzu/velero/internal/storage"
internalVolume "github.com/vmware-tanzu/velero/internal/volume"
"github.com/vmware-tanzu/velero/internal/volume"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
pkgbackup "github.com/vmware-tanzu/velero/pkg/backup"
"github.com/vmware-tanzu/velero/pkg/discovery"
Expand All @@ -56,7 +56,6 @@ import (
kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube"
"github.com/vmware-tanzu/velero/pkg/util/logging"
"github.com/vmware-tanzu/velero/pkg/util/results"
"github.com/vmware-tanzu/velero/pkg/volume"
)

const (
Expand Down Expand Up @@ -588,7 +587,7 @@ func (b *backupReconciler) validateAndGetSnapshotLocations(backup *velerov1api.B

// add credential to config for each location
for _, location := range providerLocations {
err = internalVolume.UpdateVolumeSnapshotLocationWithCredentialConfig(location, b.credentialFileStore)
err = volume.UpdateVolumeSnapshotLocationWithCredentialConfig(location, b.credentialFileStore)
if err != nil {
errors = append(errors, fmt.Sprintf("error adding credentials to volume snapshot location named %s: %v", location.Name, err))
continue
Expand Down
7 changes: 3 additions & 4 deletions pkg/controller/backup_deletion_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,16 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/vmware-tanzu/velero/pkg/plugin/velero"
"github.com/vmware-tanzu/velero/pkg/plugin/velero/mocks"
"github.com/vmware-tanzu/velero/pkg/volume"

"github.com/vmware-tanzu/velero/internal/volume"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
pkgbackup "github.com/vmware-tanzu/velero/pkg/backup"
"github.com/vmware-tanzu/velero/pkg/builder"
"github.com/vmware-tanzu/velero/pkg/metrics"
persistencemocks "github.com/vmware-tanzu/velero/pkg/persistence/mocks"
"github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt"
pluginmocks "github.com/vmware-tanzu/velero/pkg/plugin/mocks"
"github.com/vmware-tanzu/velero/pkg/plugin/velero"
"github.com/vmware-tanzu/velero/pkg/plugin/velero/mocks"
"github.com/vmware-tanzu/velero/pkg/repository"
velerotest "github.com/vmware-tanzu/velero/pkg/test"
)
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/restore_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import (

"github.com/vmware-tanzu/velero/internal/hook"
"github.com/vmware-tanzu/velero/internal/resourcemodifiers"
internalVolume "github.com/vmware-tanzu/velero/internal/volume"
"github.com/vmware-tanzu/velero/internal/volume"
api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/itemoperation"
"github.com/vmware-tanzu/velero/pkg/label"
Expand Down Expand Up @@ -521,7 +521,7 @@ func (r *restoreReconciler) runValidatedRestore(restore *api.Restore, info backu
return errors.Wrap(err, "fail to fetch CSI VolumeSnapshots metadata")
}

backupVolumeInfoMap := make(map[string]internalVolume.VolumeInfo)
backupVolumeInfoMap := make(map[string]volume.VolumeInfo)
volumeInfos, err := backupStore.GetBackupVolumeInfos(restore.Spec.BackupName)
if err != nil {
restoreLog.WithError(err).Errorf("fail to get VolumeInfos metadata file for backup %s", restore.Spec.BackupName)
Expand Down
5 changes: 2 additions & 3 deletions pkg/controller/restore_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/vmware-tanzu/velero/internal/resourcemodifiers"
internalVolume "github.com/vmware-tanzu/velero/internal/volume"
"github.com/vmware-tanzu/velero/internal/volume"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/builder"
"github.com/vmware-tanzu/velero/pkg/metrics"
Expand All @@ -50,7 +50,6 @@ import (
velerotest "github.com/vmware-tanzu/velero/pkg/test"
"github.com/vmware-tanzu/velero/pkg/util/logging"
"github.com/vmware-tanzu/velero/pkg/util/results"
"github.com/vmware-tanzu/velero/pkg/volume"
)

func TestFetchBackupInfo(t *testing.T) {
Expand Down Expand Up @@ -506,7 +505,7 @@ func TestRestoreReconcile(t *testing.T) {
if test.emptyVolumeInfo == true {
backupStore.On("GetBackupVolumeInfos", test.backup.Name).Return(nil, nil)
} else {
backupStore.On("GetBackupVolumeInfos", test.backup.Name).Return([]*internalVolume.VolumeInfo{}, nil)
backupStore.On("GetBackupVolumeInfos", test.backup.Name).Return([]*volume.VolumeInfo{}, nil)
}

volumeSnapshots := []*volume.Snapshot{
Expand Down
11 changes: 5 additions & 6 deletions pkg/persistence/mocks/backup_store.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 4 additions & 5 deletions pkg/persistence/object_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,12 @@ import (
kerrors "k8s.io/apimachinery/pkg/util/errors"

"github.com/vmware-tanzu/velero/internal/credentials"
internalVolume "github.com/vmware-tanzu/velero/internal/volume"
"github.com/vmware-tanzu/velero/internal/volume"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/itemoperation"
"github.com/vmware-tanzu/velero/pkg/plugin/velero"
"github.com/vmware-tanzu/velero/pkg/util"
"github.com/vmware-tanzu/velero/pkg/util/results"
"github.com/vmware-tanzu/velero/pkg/volume"
)

type BackupInfo struct {
Expand Down Expand Up @@ -75,7 +74,7 @@ type BackupStore interface {
GetCSIVolumeSnapshots(name string) ([]*snapshotv1api.VolumeSnapshot, error)
GetCSIVolumeSnapshotContents(name string) ([]*snapshotv1api.VolumeSnapshotContent, error)
GetCSIVolumeSnapshotClasses(name string) ([]*snapshotv1api.VolumeSnapshotClass, error)
GetBackupVolumeInfos(name string) ([]*internalVolume.VolumeInfo, error)
GetBackupVolumeInfos(name string) ([]*volume.VolumeInfo, error)
GetRestoreResults(name string) (map[string]results.Result, error)

// BackupExists checks if the backup metadata file exists in object storage.
Expand Down Expand Up @@ -498,8 +497,8 @@ func (s *objectBackupStore) GetPodVolumeBackups(name string) ([]*velerov1api.Pod
return podVolumeBackups, nil
}

func (s *objectBackupStore) GetBackupVolumeInfos(name string) ([]*internalVolume.VolumeInfo, error) {
volumeInfos := make([]*internalVolume.VolumeInfo, 0)
func (s *objectBackupStore) GetBackupVolumeInfos(name string) ([]*volume.VolumeInfo, error) {
volumeInfos := make([]*volume.VolumeInfo, 0)

res, err := tryGet(s.objectStore, s.bucket, s.layout.getBackupVolumeInfoKey(name))
if err != nil {
Expand Down
13 changes: 6 additions & 7 deletions pkg/persistence/object_store_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"

"github.com/vmware-tanzu/velero/internal/credentials"
internalVolume "github.com/vmware-tanzu/velero/internal/volume"
"github.com/vmware-tanzu/velero/internal/volume"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/builder"
"github.com/vmware-tanzu/velero/pkg/itemoperation"
Expand All @@ -44,7 +44,6 @@ import (
velerotest "github.com/vmware-tanzu/velero/pkg/test"
"github.com/vmware-tanzu/velero/pkg/util/encode"
"github.com/vmware-tanzu/velero/pkg/util/results"
"github.com/vmware-tanzu/velero/pkg/volume"
)

type objectBackupStoreTestHarness struct {
Expand Down Expand Up @@ -1069,25 +1068,25 @@ func TestNewObjectBackupStoreGetterConfig(t *testing.T) {
func TestGetBackupVolumeInfos(t *testing.T) {
tests := []struct {
name string
volumeInfo []*internalVolume.VolumeInfo
volumeInfo []*volume.VolumeInfo
volumeInfoStr string
expectedErr string
expectedResult []*internalVolume.VolumeInfo
expectedResult []*volume.VolumeInfo
}{
{
name: "No VolumeInfos, expect no error.",
},
{
name: "Valid VolumeInfo, should pass.",
volumeInfo: []*internalVolume.VolumeInfo{
volumeInfo: []*volume.VolumeInfo{
{
PVCName: "pvcName",
PVName: "pvName",
Skipped: true,
SnapshotDataMoved: false,
},
},
expectedResult: []*internalVolume.VolumeInfo{
expectedResult: []*volume.VolumeInfo{
{
PVCName: "pvcName",
PVName: "pvName",
Expand All @@ -1099,7 +1098,7 @@ func TestGetBackupVolumeInfos(t *testing.T) {
{
name: "Invalid VolumeInfo string, should also pass.",
volumeInfoStr: `[{"abc": "123", "def": "456", "pvcName": "pvcName"}]`,
expectedResult: []*internalVolume.VolumeInfo{
expectedResult: []*volume.VolumeInfo{
{
PVCName: "pvcName",
},
Expand Down
5 changes: 2 additions & 3 deletions pkg/restore/pv_restorer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/vmware-tanzu/velero/internal/credentials"
internalVolume "github.com/vmware-tanzu/velero/internal/volume"
"github.com/vmware-tanzu/velero/internal/volume"
api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
"github.com/vmware-tanzu/velero/pkg/volume"
)

type PVRestorer interface {
Expand Down Expand Up @@ -133,7 +132,7 @@ func getSnapshotInfo(pvName string, backup *api.Backup, volumeSnapshots []*volum
}

// add credential to config
err = internalVolume.UpdateVolumeSnapshotLocationWithCredentialConfig(snapshotLocation, credentialStore)
err = volume.UpdateVolumeSnapshotLocationWithCredentialConfig(snapshotLocation, credentialStore)
if err != nil {
return nil, errors.WithStack(err)
}
Expand Down
Loading

0 comments on commit 94b9e0d

Please sign in to comment.