diff --git a/pkg/apis/velero/v1/labels_annotations.go b/pkg/apis/velero/v1/labels_annotations.go index c86b4e91bf..f8b9f099c6 100644 --- a/pkg/apis/velero/v1/labels_annotations.go +++ b/pkg/apis/velero/v1/labels_annotations.go @@ -126,6 +126,7 @@ const ( VolumeSnapshotClassSelectorLabel = "velero.io/csi-volumesnapshot-class" VolumeSnapshotClassDriverBackupAnnotationPrefix = "velero.io/csi-volumesnapshot-class" VolumeSnapshotClassDriverPVCAnnotation = "velero.io/csi-volumesnapshot-class" + VolumeSnapshotClassDriverStorageClassAnnotation = "velero.io/csi-volumesnapshot-class" // There is no release w/ these constants exported. Using the strings for now. // CSI Annotation volumesnapshotclass diff --git a/pkg/backup/actions/csi/pvc_action.go b/pkg/backup/actions/csi/pvc_action.go index 589cd61d78..5f7cddfdad 100644 --- a/pkg/backup/actions/csi/pvc_action.go +++ b/pkg/backup/actions/csi/pvc_action.go @@ -145,7 +145,7 @@ func (p *pvcBackupItemAction) createVolumeSnapshot( p.log.Debugf("Fetching VolumeSnapshotClass for %s", storageClass.Provisioner) vsClass, err := csi.GetVolumeSnapshotClass( - storageClass.Provisioner, + storageClass, backup, &pvc, p.log, diff --git a/pkg/util/csi/volume_snapshot.go b/pkg/util/csi/volume_snapshot.go index 53796ab972..7052115ba9 100644 --- a/pkg/util/csi/volume_snapshot.go +++ b/pkg/util/csi/volume_snapshot.go @@ -29,6 +29,7 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" corev1api "k8s.io/api/core/v1" + storagev1api "k8s.io/api/storage/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -307,7 +308,7 @@ func patchVSC( } func GetVolumeSnapshotClass( - provisioner string, + storageClass *storagev1api.StorageClass, backup *velerov1api.Backup, pvc *corev1api.PersistentVolumeClaim, log logrus.FieldLogger, @@ -320,7 +321,7 @@ func GetVolumeSnapshotClass( } // If a snapshot class is set for provider in PVC annotations, use that snapshotClass, err := GetVolumeSnapshotClassFromPVCAnnotationsForDriver( - pvc, provisioner, snapshotClasses, + pvc, storageClass.Provisioner, snapshotClasses, ) if err != nil { log.Debugf("Didn't find VolumeSnapshotClass from PVC annotations: %v", err) @@ -329,9 +330,18 @@ func GetVolumeSnapshotClass( return snapshotClass, nil } - // If there is no annotation in PVC, attempt to fetch it from backup annotations + // If there is no annotation in PVC, attempt to fetch it from storageClass annotations + snapshotClass, err = GetVolumeSnapshotClassFromStorageClassAnnotationsForDriver(storageClass, snapshotClasses) + if err != nil { + log.Debugf("Didn't find VolumeSnapshotClass from StorageClass annotations: %v", err) + } + if snapshotClass != nil { + return snapshotClass, nil + } + + // If there is no annotation in storageClass, attempt to fetch it from backup annotations snapshotClass, err = GetVolumeSnapshotClassFromBackupAnnotationsForDriver( - backup, provisioner, snapshotClasses) + backup, storageClass.Provisioner, snapshotClasses) if err != nil { log.Debugf("Didn't find VolumeSnapshotClass from Backup annotations: %v", err) } @@ -341,7 +351,7 @@ func GetVolumeSnapshotClass( // fallback to default behavior of fetching snapshot class based on label snapshotClass, err = GetVolumeSnapshotClassForStorageClass( - provisioner, snapshotClasses) + storageClass.Provisioner, snapshotClasses) if err != nil || snapshotClass == nil { return nil, errors.Wrap(err, "error getting VolumeSnapshotClass") } @@ -376,6 +386,23 @@ func GetVolumeSnapshotClassFromPVCAnnotationsForDriver( ) } +func GetVolumeSnapshotClassFromStorageClassAnnotationsForDriver(storageClass *storagev1api.StorageClass, snapshotClasses *snapshotv1api.VolumeSnapshotClassList) (*snapshotv1api.VolumeSnapshotClass, error) { + annotationKey := velerov1api.VolumeSnapshotClassDriverStorageClassAnnotation + snapshotClassName, ok := storageClass.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, storageClass.Provisioner) { + return nil, errors.Errorf("Incorrect volumesnapshotclass, snapshot class %s is not for driver %s", sc.ObjectMeta.Name, storageClass.Provisioner) + } + return &sc, nil + } + } + return nil, errors.Errorf("No CSI VolumeSnapshotClass found with name %s for provisioner %s for StorageClass %s", snapshotClassName, storageClass.Provisioner, storageClass.Name) +} + // GetVolumeSnapshotClassFromAnnotationsForDriver returns a // VolumeSnapshotClass for the supplied volume provisioner/driver // name from the annotation of the backup. diff --git a/pkg/util/csi/volume_snapshot_test.go b/pkg/util/csi/volume_snapshot_test.go index e5a87c2bdd..2c44833a5b 100644 --- a/pkg/util/csi/volume_snapshot_test.go +++ b/pkg/util/csi/volume_snapshot_test.go @@ -28,6 +28,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" + storagev1api "k8s.io/api/storage/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -951,80 +952,121 @@ func TestGetVolumeSnapshotClass(t *testing.T) { Driver: "bar.csi.k8s.io", } + // storageclasses + fooStorageClass := &storagev1api.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Labels: map[string]string{ + "velero.io/csi-volumesnapshot-class": "foo", + }, + }, + Provisioner: "foo.csi.k8s.io", + } + + barStorageClass := &storagev1api.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Labels: map[string]string{ + "velero.io/csi-volumesnapshot-class": "foo", + }, + }, + Provisioner: "bar.csi.k8s.io", + } + + hostpathStorageClass := &storagev1api.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hostpath", + Labels: map[string]string{ + "velero.io/csi-volumesnapshot-class": "foo", + }, + }, + Provisioner: "hostpath.csi.k8s.io", + } + + blahStorageClass := &storagev1api.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "blah", + Labels: map[string]string{ + "velero.io/csi-volumesnapshot-class": "foo", + }, + }, + Provisioner: "blah.csi.k8s.io", + } + objs := []runtime.Object{hostpathClass, fooClass, barClass, fooClassWithoutLabel, barClass2} fakeClient := velerotest.NewFakeControllerRuntimeClient(t, objs...) testCases := []struct { - name string - driverName string - pvc *v1.PersistentVolumeClaim - backup *velerov1api.Backup - expectedVSC *snapshotv1api.VolumeSnapshotClass - expectError bool + name string + storageClass *storagev1api.StorageClass + pvc *v1.PersistentVolumeClaim + backup *velerov1api.Backup + expectedVSC *snapshotv1api.VolumeSnapshotClass + expectError bool }{ { - name: "no annotations on pvc and backup, should find hostpath volumesnapshotclass using default behavior of labels", - driverName: "hostpath.csi.k8s.io", - pvc: pvcNone, - backup: backupNone, - expectedVSC: hostpathClass, - expectError: false, + name: "no annotations on pvc and backup, should find hostpath volumesnapshotclass using default behavior of labels", + storageClass: hostpathStorageClass, + 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", + storageClass: fooStorageClass, + 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 behavior 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, no annotation on backup so fallback to default behavior of labels", + storageClass: barStorageClass, + 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 pvc, but csi driver does not match so fallback to fetch from backupAnnotations ", + storageClass: barStorageClass, + 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 foo.csi.k8s.io", + storageClass: fooStorageClass, + 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 behavior of labels", - driverName: "bar.csi.k8s.io", - pvc: pvcNone, - backup: backupFoo, - expectedVSC: barClass, - 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 behavior of labels", + storageClass: barStorageClass, + 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: "no snapshotClass for given driver", + storageClass: blahStorageClass, + pvc: pvcNone, + backup: backupNone, + expectedVSC: nil, + expectError: true, }, { - name: "foo2 VSC annotations on pvc, but doesn't exist in cluster, fallback to default behavior of labels", - driverName: "foo.csi.k8s.io", - pvc: pvcFoo2, - backup: backupFoo2, - expectedVSC: fooClass, - expectError: false, + name: "foo2 VSC annotations on pvc, but doesn't exist in cluster, fallback to default behavior of labels", + storageClass: fooStorageClass, + pvc: pvcFoo2, + backup: backupFoo2, + expectedVSC: fooClass, + expectError: false, }, } for _, tc := range testCases {