Skip to content

Commit

Permalink
Merge pull request #7544 from blackpiglet/refactor_native_snapshot
Browse files Browse the repository at this point in the history
Refactor the native snapshot definition code.
  • Loading branch information
ywk253100 authored Mar 21, 2024
2 parents 67c06e6 + efb94ae commit 13f4efd
Show file tree
Hide file tree
Showing 22 changed files with 76 additions and 89 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/7544-blackpiglet
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Move the native snapshot definition code into internal directory
File renamed without changes.
3 changes: 1 addition & 2 deletions internal/volume/volumes_information.go
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
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
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
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
5 changes: 2 additions & 3 deletions pkg/backup/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,11 @@ import (

"github.com/vmware-tanzu/velero/internal/hook"
"github.com/vmware-tanzu/velero/internal/resourcepolicies"
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/framework"
"github.com/vmware-tanzu/velero/pkg/util/collections"
"github.com/vmware-tanzu/velero/pkg/volume"
)

type itemKey struct {
Expand All @@ -53,7 +52,7 @@ type Request struct {
itemOperationsList *[]*itemoperation.BackupOperation
ResPolicies *resourcepolicies.Policies
SkippedPVTracker *skipPVTracker
VolumesInformation internalVolume.VolumesInformation
VolumesInformation volume.VolumesInformation
}

// VolumesInformation contains the information needs by generating
Expand Down
3 changes: 1 addition & 2 deletions pkg/cmd/util/output/backup_describer.go
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
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
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
4 changes: 2 additions & 2 deletions pkg/controller/backup_deletion_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (

"github.com/vmware-tanzu/velero/internal/credentials"
"github.com/vmware-tanzu/velero/internal/delete"
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"
velerov2alpha1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v2alpha1"
"github.com/vmware-tanzu/velero/pkg/discovery"
Expand Down Expand Up @@ -456,7 +456,7 @@ func (r *backupDeletionReconciler) volumeSnapshottersForVSL(
}

// add credential to config
err := internalVolume.UpdateVolumeSnapshotLocationWithCredentialConfig(vsl, r.credentialStore)
err := volume.UpdateVolumeSnapshotLocationWithCredentialConfig(vsl, r.credentialStore)
if err != nil {
return nil, errors.WithStack(err)
}
Expand Down
7 changes: 3 additions & 4 deletions pkg/controller/backup_deletion_controller_test.go
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
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
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
10 changes: 5 additions & 5 deletions pkg/controller/restore_finalizer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/clock"

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/metrics"
"github.com/vmware-tanzu/velero/pkg/persistence"
Expand Down Expand Up @@ -238,7 +238,7 @@ type finalizerContext struct {
logger logrus.FieldLogger
restore *velerov1api.Restore
crClient client.Client
volumeInfo []*internalVolume.VolumeInfo
volumeInfo []*volume.VolumeInfo
restoredPVCList map[string]struct{}
}

Expand All @@ -263,7 +263,7 @@ func (ctx *finalizerContext) patchDynamicPVWithVolumeInfo() (errs results.Result
semaphore := make(chan struct{}, maxConcurrency)

for _, volumeItem := range ctx.volumeInfo {
if (volumeItem.BackupMethod == internalVolume.PodVolumeBackup || volumeItem.BackupMethod == internalVolume.CSISnapshot) && volumeItem.PVInfo != nil {
if (volumeItem.BackupMethod == volume.PodVolumeBackup || volumeItem.BackupMethod == volume.CSISnapshot) && volumeItem.PVInfo != nil {
// Determine restored PVC namespace
restoredNamespace := volumeItem.PVCNamespace
if remapped, ok := ctx.restore.Spec.NamespaceMapping[restoredNamespace]; ok {
Expand All @@ -277,7 +277,7 @@ func (ctx *finalizerContext) patchDynamicPVWithVolumeInfo() (errs results.Result
}

pvWaitGroup.Add(1)
go func(volInfo internalVolume.VolumeInfo, restoredNamespace string) {
go func(volInfo volume.VolumeInfo, restoredNamespace string) {
defer pvWaitGroup.Done()

semaphore <- struct{}{}
Expand Down Expand Up @@ -375,7 +375,7 @@ func getRestoredPVCFromRestoredResourceList(restoredResourceList map[string][]st
return pvcList
}

func needPatch(newPV *v1.PersistentVolume, pvInfo *internalVolume.PVInfo) bool {
func needPatch(newPV *v1.PersistentVolume, pvInfo *volume.PVInfo) bool {
if newPV.Spec.PersistentVolumeReclaimPolicy != v1.PersistentVolumeReclaimPolicy(pvInfo.ReclaimPolicy) {
return true
}
Expand Down
11 changes: 5 additions & 6 deletions pkg/persistence/mocks/backup_store.go

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
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
Loading

0 comments on commit 13f4efd

Please sign in to comment.