From ebbf92d8f300e8d95e4d4aa0662bfe348a79b499 Mon Sep 17 00:00:00 2001 From: Anshul Ahuja Date: Mon, 29 May 2023 15:37:15 +0530 Subject: [PATCH 1/4] Add support for Multiple VolumeSnapshotClasses Signed-off-by: Anshul Ahuja --- README.md | 51 +++++++ internal/backup/pvc_action.go | 2 +- internal/util/labels_annotations.go | 18 +-- internal/util/util.go | 68 ++++++++- internal/util/util_test.go | 216 +++++++++++++++++++++++++++- 5 files changed, 339 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 7a2cb5d3..799b226f 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,57 @@ Below is a listing of plugin versions and respective Velero versions that are co | v0.3.0 | v1.9.x | | v0.2.0 | v1.7.x, v1.8.x | +### Choosing VolumeSnapshotClass For snapshotting (>=0.6.0) +#### Default Behavior +You can simply create a VolumeSnapshotClass for a particular driver and put a label on it to indicate that it is the default VolumeSnapshotClass for that driver. For example, if you want to create a VolumeSnapshotClass for the CSI driver `disk.csi.cloud.com` for taking snapshots of disks created with `disk.csi.cloud.com` based storage classes, you can create a VolumeSnapshotClass like this: +```yaml +apiVersion: snapshot.storage.k8s.io/v1 +kind: VolumeSnapshotClass +metadata: + name: test-snapclass + labels: + velero.io/csi-volumesnapshot-class: "true" +driver: disk.csi.cloud.com +``` + +> Note: For each driver type, there should only be 1 VolumeSnapshotClass with the label `velero.io/csi-volumesnapshot-class: "true"`. + +#### Choose VolumeSnapshotClass for a particular Backup Or Schedule +If you want to use a particular VolumeSnapshotClass for a particular backup or schedule, you can add a annotation to the backup or schedule to indicate which VolumeSnapshotClass to use. For example, if you want to use the VolumeSnapshotClass `test-snapclass` for a particular backup for snapshotting PVCs of `disk.csi.cloud.com`, you can create a backup like this: +```yaml +apiVersion: velero.io/v1 +kind: Backup +metadata: + name: test-backup + annotations: + velero.io/csi-volumesnapshot-class/disk.csi.cloud.com: "test-snapclass" +spec: + includedNamespaces: + - default +``` + +> Note: Please ensure all your annotations are in lowercase. And follow the following format: `velero.io/csi-volumesnapshot-class/ = ` + +#### Choosing VolumeSnapshotClass for a particular PVC +If you want to use a particular VolumeSnapshotClass for a particular PVC, you can add a annotation to the PVC to indicate which VolumeSnapshotClass to use. This overrides any annotation added to backup or schedule. For example, if you want to use the VolumeSnapshotClass `test-snapclass` for a particular PVC, you can create a PVC like this: +```yaml +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: test-pvc + annotations: + velero.io/csi-volumesnapshot-class: "test-snapclass" +spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + storageClassName: disk.csi.cloud.com +``` + +> Note: Please ensure all your annotations are in lowercase. And follow the following format: `velero.io/csi-volumesnapshot-class = ` + ## Filing issues If you would like to file a GitHub issue for the plugin, please open the issue on the [core Velero repo][103] diff --git a/internal/backup/pvc_action.go b/internal/backup/pvc_action.go index 276dd6ac..fc5bc90b 100644 --- a/internal/backup/pvc_action.go +++ b/internal/backup/pvc_action.go @@ -105,7 +105,7 @@ func (p *PVCBackupItemAction) Execute(item runtime.Unstructured, backup *velerov return nil, nil, errors.Wrap(err, "error getting storage class") } p.Log.Debugf("Fetching volumesnapshot class for %s", storageClass.Provisioner) - snapshotClass, err := util.GetVolumeSnapshotClassForStorageClass(storageClass.Provisioner, snapshotClient.SnapshotV1()) + snapshotClass, err := util.GetVolumeSnapshotClass(&pvc, storageClass.Provisioner, backup, p.Log, snapshotClient.SnapshotV1()) if err != nil { return nil, nil, errors.Wrapf(err, "failed to get volumesnapshotclass for storageclass %s", storageClass.Name) } diff --git a/internal/util/labels_annotations.go b/internal/util/labels_annotations.go index e721d2e7..0d25d0f6 100644 --- a/internal/util/labels_annotations.go +++ b/internal/util/labels_annotations.go @@ -17,14 +17,16 @@ limitations under the License. package util const ( - VolumeSnapshotLabel = "velero.io/volume-snapshot-name" - VolumeSnapshotHandleAnnotation = "velero.io/csi-volumesnapshot-handle" - VolumeSnapshotRestoreSize = "velero.io/vsi-volumesnapshot-restore-size" - CSIDriverNameAnnotation = "velero.io/csi-driver-name" - CSIDeleteSnapshotSecretName = "velero.io/csi-deletesnapshotsecret-name" - CSIDeleteSnapshotSecretNamespace = "velero.io/csi-deletesnapshotsecret-namespace" - CSIVSCDeletionPolicy = "velero.io/csi-vsc-deletion-policy" - VolumeSnapshotClassSelectorLabel = "velero.io/csi-volumesnapshot-class" + VolumeSnapshotLabel = "velero.io/volume-snapshot-name" + VolumeSnapshotHandleAnnotation = "velero.io/csi-volumesnapshot-handle" + VolumeSnapshotRestoreSize = "velero.io/vsi-volumesnapshot-restore-size" + CSIDriverNameAnnotation = "velero.io/csi-driver-name" + CSIDeleteSnapshotSecretName = "velero.io/csi-deletesnapshotsecret-name" + CSIDeleteSnapshotSecretNamespace = "velero.io/csi-deletesnapshotsecret-namespace" + CSIVSCDeletionPolicy = "velero.io/csi-vsc-deletion-policy" + VolumeSnapshotClassSelectorLabel = "velero.io/csi-volumesnapshot-class" + VolumeSnapshotClassDriverBackupAnnotationPrefix = "velero.io/csi-volumesnapshot-class" + VolumeSnapshotClassDriverPVCAnnotation = "velero.io/csi-volumesnapshot-class" // There is no release w/ these constants exported. Using the strings for now. // CSI Labels volumesnapshotclass diff --git a/internal/util/util.go b/internal/util/util.go index 24bd7ff4..17661e5a 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -114,13 +114,73 @@ func IsPVCDefaultToFSBackup(pvcNamespace, pvcName string, podClient corev1client return false, nil } - -// GetVolumeSnapshotClassForStorageClass returns a VolumeSnapshotClass for the supplied volume provisioner/ driver name. -func GetVolumeSnapshotClassForStorageClass(provisioner string, snapshotClient snapshotter.SnapshotV1Interface) (*snapshotv1api.VolumeSnapshotClass, error) { +func GetVolumeSnapshotClass(pvc *corev1api.PersistentVolumeClaim, provisioner string, backup *velerov1api.Backup, log logrus.FieldLogger, snapshotClient snapshotter.SnapshotV1Interface) (*snapshotv1api.VolumeSnapshotClass, error) { snapshotClasses, err := snapshotClient.VolumeSnapshotClasses().List(context.TODO(), metav1.ListOptions{}) if err != nil { return nil, errors.Wrap(err, "error listing volumesnapshot classes") } + // If a snapshot class is sent for provider in PVC annotations, use that + snapshotClass, err := GetVolumeSnapshotClassFromPVCAnnotationsForDriver(pvc, provisioner, snapshotClasses) + if err != nil { + // log error + log.Debugf("Didn't find VolumeSnapshotClass from PVC annotations: %v", err) + } + if snapshotClass != nil { + return snapshotClass, nil + } + + // If there is no annotation in PVC, attempt to fetch it from backup annotations + snapshotClass, err = GetVolumeSnapshotClassFromBackupAnnotationsForDriver(backup, provisioner, snapshotClasses) + if err != nil { + log.Debugf("Didn't find VolumeSnapshotClass from Backup annotations: %v", err) + } + if snapshotClass != nil { + return snapshotClass, nil + } + + // fallback to default behaviour of fetching snapshot class based on label + snapshotClass, err = GetVolumeSnapshotClassForStorageClass(provisioner, snapshotClasses) + if err != nil || snapshotClass == nil { + return nil, errors.Wrap(err, "error getting volumesnapshotclass") + } + + return snapshotClass, nil +} + +func GetVolumeSnapshotClassFromPVCAnnotationsForDriver(pvc *corev1api.PersistentVolumeClaim, provisioner string, snapshotClasses *snapshotv1api.VolumeSnapshotClassList) (*snapshotv1api.VolumeSnapshotClass, error) { + annotationKey := VolumeSnapshotClassDriverPVCAnnotation + snapshotClassName, ok := pvc.ObjectMeta.Annotations[annotationKey] + if !ok { + return nil, nil + } + for _, sc := range snapshotClasses.Items { + if strings.EqualFold(snapshotClassName, sc.ObjectMeta.Name) { + if !strings.EqualFold(sc.Driver, provisioner) { + return nil, errors.Errorf("snapshot class %s is not for driver %s", sc.ObjectMeta.Name, provisioner) + } + return &sc, nil + } + } + return nil, errors.Errorf("No CSI VolumeSnapshotClass found with name %s for provisioner %s for PVC %s", snapshotClassName, provisioner, pvc.Name) +} + +// GetVolumeSnapshotClassFromAnnotationsForStorageClass returns a VolumeSnapshotClass for the supplied volume provisioner/ driver name from the annotation of the backup +func GetVolumeSnapshotClassFromBackupAnnotationsForDriver(backup *velerov1api.Backup, provisioner string, snapshotClasses *snapshotv1api.VolumeSnapshotClassList) (*snapshotv1api.VolumeSnapshotClass, error) { + annotationKey := fmt.Sprintf("%s/%s", VolumeSnapshotClassDriverBackupAnnotationPrefix, strings.ToLower(provisioner)) + snapshotClassName, ok := backup.ObjectMeta.Annotations[annotationKey] + if !ok { + return nil, nil + } + for _, sc := range snapshotClasses.Items { + if strings.EqualFold(snapshotClassName, sc.ObjectMeta.Name) { + return &sc, nil + } + } + return nil, errors.Errorf("No CSI VolumeSnapshotClass found with name %s for provisioner %s for backup %s", snapshotClassName, provisioner, backup.Name) +} + +// GetVolumeSnapshotClassForStorageClass returns a VolumeSnapshotClass for the supplied volume provisioner/ driver name. +func GetVolumeSnapshotClassForStorageClass(provisioner string, snapshotClasses *snapshotv1api.VolumeSnapshotClassList) (*snapshotv1api.VolumeSnapshotClass, error) { n := 0 var vsclass snapshotv1api.VolumeSnapshotClass // We pick the volumesnapshotclass that matches the CSI driver name and has a 'velero.io/csi-volumesnapshot-class' @@ -325,4 +385,4 @@ func CleanupVolumeSnapshot(volSnap *snapshotv1api.VolumeSnapshot, snapshotClient } else { log.Info("Deleted volumesnapshot with volumesnapshotContent %s/%s", vs.Namespace, vs.Name) } -} \ No newline at end of file +} diff --git a/internal/util/util_test.go b/internal/util/util_test.go index 247403e8..7812ebaa 100644 --- a/internal/util/util_test.go +++ b/internal/util/util_test.go @@ -25,6 +25,7 @@ import ( snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1" snapshotFake "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned/fake" "github.com/sirupsen/logrus" + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -619,6 +620,213 @@ func TestIsPVCDefaultToFSBackup(t *testing.T) { } } +func TestGetVolumeSnapshotClass(t *testing.T) { + // backups + backupFoo := &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Annotations: map[string]string{ + "velero.io/csi-volumesnapshot-class/foo.csi.k8s.io": "foowithoutlabel", + }, + }, + Spec: velerov1api.BackupSpec{ + IncludedNamespaces: []string{"ns1", "ns2"}, + }, + } + backupFoo2 := &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo2", + Annotations: map[string]string{ + "velero.io/csi-volumesnapshot-class/foo.csi.k8s.io": "foo2", + }, + }, + Spec: velerov1api.BackupSpec{ + IncludedNamespaces: []string{"ns1", "ns2"}, + }, + } + + backupBar2 := &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Annotations: map[string]string{ + "velero.io/csi-volumesnapshot-class/bar.csi.k8s.io": "bar2", + }, + }, + Spec: velerov1api.BackupSpec{ + IncludedNamespaces: []string{"ns1", "ns2"}, + }, + } + + backupNone := &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "none", + }, + Spec: velerov1api.BackupSpec{ + IncludedNamespaces: []string{"ns1", "ns2"}, + }, + } + + // pvcs + pvcFoo := &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Annotations: map[string]string{ + "velero.io/csi-volumesnapshot-class": "foowithoutlabel", + }, + }, + Spec: v1.PersistentVolumeClaimSpec{}, + } + pvcFoo2 := &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Annotations: map[string]string{ + "velero.io/csi-volumesnapshot-class": "foo2", + }, + }, + Spec: v1.PersistentVolumeClaimSpec{}, + } + pvcNone := &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "none", + }, + Spec: v1.PersistentVolumeClaimSpec{}, + } + + // vsclasses + hostpathClass := &snapshotv1api.VolumeSnapshotClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hostpath", + Labels: map[string]string{ + VolumeSnapshotClassSelectorLabel: "foo", + }, + }, + Driver: "hostpath.csi.k8s.io", + } + + fooClass := &snapshotv1api.VolumeSnapshotClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Labels: map[string]string{ + VolumeSnapshotClassSelectorLabel: "foo", + }, + }, + Driver: "foo.csi.k8s.io", + } + fooClassWithoutLabel := &snapshotv1api.VolumeSnapshotClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foowithoutlabel", + }, + Driver: "foo.csi.k8s.io", + } + + barClass := &snapshotv1api.VolumeSnapshotClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Labels: map[string]string{ + VolumeSnapshotClassSelectorLabel: "true", + }, + }, + Driver: "bar.csi.k8s.io", + } + + barClass2 := &snapshotv1api.VolumeSnapshotClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar2", + Labels: map[string]string{ + VolumeSnapshotClassSelectorLabel: "true", + }, + }, + Driver: "bar.csi.k8s.io", + } + + objs := []runtime.Object{hostpathClass, fooClass, barClass, fooClassWithoutLabel, barClass2} + fakeClient := snapshotFake.NewSimpleClientset(objs...) + + testCases := []struct { + name string + driverName string + pvc *v1.PersistentVolumeClaim + backup *velerov1api.Backup + expectedVSC *snapshotv1api.VolumeSnapshotClass + expectError bool + }{ + { + name: "no annotations on pvc and backup, should find hostpath volumesnapshotclass using default behaviour of labels", + driverName: "hostpath.csi.k8s.io", + pvc: pvcNone, + backup: backupNone, + expectedVSC: hostpathClass, + expectError: false, + }, + { + name: "foowithoutlabel VSC annotations on pvc", + driverName: "foo.csi.k8s.io", + pvc: pvcFoo, + backup: backupNone, + expectedVSC: fooClassWithoutLabel, + expectError: false, + }, + { + name: "foowithoutlabel VSC annotations on pvc, but csi driver does not match, no annotation on backup so fallback to default behaviour of labels", + driverName: "bar.csi.k8s.io", + pvc: pvcFoo, + backup: backupNone, + expectedVSC: barClass, + expectError: false, + }, + { + name: "foowithoutlabel VSC annotations on pvc, but csi driver does not match so fallback to fetch from backupAnnotations ", + driverName: "bar.csi.k8s.io", + pvc: pvcFoo, + backup: backupBar2, + expectedVSC: barClass2, + expectError: false, + }, + { + name: "foowithoutlabel VSC annotations on backup for foo.csi.k8s.io", + driverName: "foo.csi.k8s.io", + pvc: pvcNone, + backup: backupFoo, + expectedVSC: fooClassWithoutLabel, + expectError: false, + }, + { + name: "foowithoutlabel VSC annotations on backup for bar.csi.k8s.io, no annotation corresponding to foo.csi.k8s.io, so fallback to default behaviour of labels", + driverName: "bar.csi.k8s.io", + pvc: pvcNone, + backup: backupFoo, + expectedVSC: barClass, + expectError: false, + }, + { + name: "no snapshotClass for given driver", + driverName: "blah.csi.k8s.io", + pvc: pvcNone, + backup: backupNone, + expectedVSC: nil, + expectError: true, + }, + { + name: "foo2 VSC annotations on pvc, but doesn't exist in cluster, fallback to default behaviour of labels", + driverName: "foo.csi.k8s.io", + pvc: pvcFoo2, + backup: backupFoo2, + expectedVSC: fooClass, + expectError: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actualSnapshotClass, actualError := GetVolumeSnapshotClass(tc.pvc, tc.driverName, tc.backup, logrus.New(), fakeClient.SnapshotV1()) + if tc.expectError { + assert.NotNil(t, actualError) + assert.Nil(t, actualSnapshotClass) + return + } + assert.Equal(t, tc.expectedVSC, actualSnapshotClass) + }) + } +} func TestGetVolumeSnapshotClassForStorageClass(t *testing.T) { hostpathClass := &snapshotv1api.VolumeSnapshotClass{ ObjectMeta: metav1.ObjectMeta{ @@ -671,8 +879,10 @@ func TestGetVolumeSnapshotClassForStorageClass(t *testing.T) { Driver: "amb.csi.k8s.io", } - objs := []runtime.Object{hostpathClass, fooClass, barClass, bazClass, ambClass1, ambClass2} - fakeClient := snapshotFake.NewSimpleClientset(objs...) + snapshotClasses := &snapshotv1api.VolumeSnapshotClassList{ + Items: []snapshotv1api.VolumeSnapshotClass{ + *hostpathClass, *fooClass, *barClass, *bazClass, *ambClass1, *ambClass2}, + } testCases := []struct { name string @@ -720,7 +930,7 @@ func TestGetVolumeSnapshotClassForStorageClass(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - actualVSC, actualError := GetVolumeSnapshotClassForStorageClass(tc.driverName, fakeClient.SnapshotV1()) + actualVSC, actualError := GetVolumeSnapshotClassForStorageClass(tc.driverName, snapshotClasses) if tc.expectError { assert.NotNil(t, actualError) From a9b66fc6f1e0dd3c6e6336e450a0f01590b83cb7 Mon Sep 17 00:00:00 2001 From: Anshul Ahuja Date: Mon, 5 Jun 2023 10:25:35 +0530 Subject: [PATCH 2/4] Add provisioner sanity check in backup Signed-off-by: Anshul Ahuja --- internal/util/util.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/util/util.go b/internal/util/util.go index 17661e5a..ac272dce 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -122,7 +122,6 @@ func GetVolumeSnapshotClass(pvc *corev1api.PersistentVolumeClaim, provisioner st // If a snapshot class is sent for provider in PVC annotations, use that snapshotClass, err := GetVolumeSnapshotClassFromPVCAnnotationsForDriver(pvc, provisioner, snapshotClasses) if err != nil { - // log error log.Debugf("Didn't find VolumeSnapshotClass from PVC annotations: %v", err) } if snapshotClass != nil { @@ -156,7 +155,7 @@ func GetVolumeSnapshotClassFromPVCAnnotationsForDriver(pvc *corev1api.Persistent for _, sc := range snapshotClasses.Items { if strings.EqualFold(snapshotClassName, sc.ObjectMeta.Name) { if !strings.EqualFold(sc.Driver, provisioner) { - return nil, errors.Errorf("snapshot class %s is not for driver %s", sc.ObjectMeta.Name, provisioner) + return nil, errors.Errorf("Incorrect volumesnapshotclass, snapshot class %s is not for driver %s", sc.ObjectMeta.Name, provisioner) } return &sc, nil } @@ -173,10 +172,13 @@ func GetVolumeSnapshotClassFromBackupAnnotationsForDriver(backup *velerov1api.Ba } for _, sc := range snapshotClasses.Items { if strings.EqualFold(snapshotClassName, sc.ObjectMeta.Name) { + if !strings.EqualFold(sc.Driver, provisioner) { + return nil, errors.Errorf("Incorrect volumesnapshotclass, snapshot class %s is not for driver %s for backup %s", sc.ObjectMeta.Name, provisioner, backup.Name) + } return &sc, nil } } - return nil, errors.Errorf("No CSI VolumeSnapshotClass found with name %s for provisioner %s for backup %s", snapshotClassName, provisioner, backup.Name) + return nil, errors.Errorf("No CSI VolumeSnapshotClass found with name %s for driver %s for backup %s", snapshotClassName, provisioner, backup.Name) } // GetVolumeSnapshotClassForStorageClass returns a VolumeSnapshotClass for the supplied volume provisioner/ driver name. From 383d02eaf9d5a552324251a4a56d9193967e8aaa Mon Sep 17 00:00:00 2001 From: Anshul Ahuja Date: Wed, 28 Jun 2023 10:10:52 +0530 Subject: [PATCH 3/4] PR comments Signed-off-by: Anshul Ahuja --- internal/backup/pvc_action.go | 2 +- internal/util/util.go | 4 ++-- internal/util/util_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/backup/pvc_action.go b/internal/backup/pvc_action.go index fc5bc90b..4e60b02b 100644 --- a/internal/backup/pvc_action.go +++ b/internal/backup/pvc_action.go @@ -105,7 +105,7 @@ func (p *PVCBackupItemAction) Execute(item runtime.Unstructured, backup *velerov return nil, nil, errors.Wrap(err, "error getting storage class") } p.Log.Debugf("Fetching volumesnapshot class for %s", storageClass.Provisioner) - snapshotClass, err := util.GetVolumeSnapshotClass(&pvc, storageClass.Provisioner, backup, p.Log, snapshotClient.SnapshotV1()) + snapshotClass, err := util.GetVolumeSnapshotClass(storageClass.Provisioner, backup, &pvc, p.Log, snapshotClient.SnapshotV1()) if err != nil { return nil, nil, errors.Wrapf(err, "failed to get volumesnapshotclass for storageclass %s", storageClass.Name) } diff --git a/internal/util/util.go b/internal/util/util.go index ac272dce..dca13cf2 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -114,7 +114,7 @@ func IsPVCDefaultToFSBackup(pvcNamespace, pvcName string, podClient corev1client return false, nil } -func GetVolumeSnapshotClass(pvc *corev1api.PersistentVolumeClaim, provisioner string, backup *velerov1api.Backup, log logrus.FieldLogger, snapshotClient snapshotter.SnapshotV1Interface) (*snapshotv1api.VolumeSnapshotClass, error) { +func GetVolumeSnapshotClass(provisioner string, backup *velerov1api.Backup, pvc *corev1api.PersistentVolumeClaim, log logrus.FieldLogger, snapshotClient snapshotter.SnapshotV1Interface) (*snapshotv1api.VolumeSnapshotClass, error) { snapshotClasses, err := snapshotClient.VolumeSnapshotClasses().List(context.TODO(), metav1.ListOptions{}) if err != nil { return nil, errors.Wrap(err, "error listing volumesnapshot classes") @@ -163,7 +163,7 @@ func GetVolumeSnapshotClassFromPVCAnnotationsForDriver(pvc *corev1api.Persistent return nil, errors.Errorf("No CSI VolumeSnapshotClass found with name %s for provisioner %s for PVC %s", snapshotClassName, provisioner, pvc.Name) } -// GetVolumeSnapshotClassFromAnnotationsForStorageClass returns a VolumeSnapshotClass for the supplied volume provisioner/ driver name from the annotation of the backup +// GetVolumeSnapshotClassFromAnnotationsForDriver returns a VolumeSnapshotClass for the supplied volume provisioner/ driver name from the annotation of the backup func GetVolumeSnapshotClassFromBackupAnnotationsForDriver(backup *velerov1api.Backup, provisioner string, snapshotClasses *snapshotv1api.VolumeSnapshotClassList) (*snapshotv1api.VolumeSnapshotClass, error) { annotationKey := fmt.Sprintf("%s/%s", VolumeSnapshotClassDriverBackupAnnotationPrefix, strings.ToLower(provisioner)) snapshotClassName, ok := backup.ObjectMeta.Annotations[annotationKey] diff --git a/internal/util/util_test.go b/internal/util/util_test.go index 7812ebaa..c15e536c 100644 --- a/internal/util/util_test.go +++ b/internal/util/util_test.go @@ -817,7 +817,7 @@ func TestGetVolumeSnapshotClass(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - actualSnapshotClass, actualError := GetVolumeSnapshotClass(tc.pvc, tc.driverName, tc.backup, logrus.New(), fakeClient.SnapshotV1()) + actualSnapshotClass, actualError := GetVolumeSnapshotClass(tc.driverName, tc.backup, tc.pvc, logrus.New(), fakeClient.SnapshotV1()) if tc.expectError { assert.NotNil(t, actualError) assert.Nil(t, actualSnapshotClass) From 56ccb24a6dbe28702d7a539257c43c96e5ea9d7d Mon Sep 17 00:00:00 2001 From: Anshul Ahuja Date: Wed, 28 Jun 2023 10:18:36 +0530 Subject: [PATCH 4/4] fix merge related mistakes Signed-off-by: Anshul Ahuja --- internal/backup/pvc_action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/backup/pvc_action.go b/internal/backup/pvc_action.go index df0c7162..d17559e8 100644 --- a/internal/backup/pvc_action.go +++ b/internal/backup/pvc_action.go @@ -123,7 +123,7 @@ func (p *PVCBackupItemAction) Execute(item runtime.Unstructured, backup *velerov return nil, nil, "", nil, errors.Wrap(err, "error getting storage class") } p.Log.Debugf("Fetching volumesnapshot class for %s", storageClass.Provisioner) - snapshotClass, err := util.GetVolumeSnapshotClass(storageClass.Provisioner, backup, &pvc, p.Log, snapshotClient.SnapshotV1()) + snapshotClass, err := util.GetVolumeSnapshotClass(storageClass.Provisioner, backup, &pvc, p.Log, p.SnapshotClient.SnapshotV1()) if err != nil { return nil, nil, "", nil, errors.Wrapf(err, "failed to get volumesnapshotclass for storageclass %s", storageClass.Name) }