feat(csi): allow setting annotation on StorageClass to determine VolumeSnapshotClass

changelogs/unreleased/8646-cfi2017

@@ -0,0 +1 @@
+Add a way to specify the desired VolumeSnapshotClass for CSI snapshot backups using an annotation in the Volumes StorageClass.

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

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,

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.

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,86 +952,143 @@ func TestGetVolumeSnapshotClass(t *testing.T) {
 		Driver: "bar.csi.k8s.io",
 	}
 
+	sctestClass := &snapshotv1api.VolumeSnapshotClass{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "sctest",
+		},
+		Driver: "foo.csi.k8s.io",
+	}
+
+	// storageclasses
+	fooStorageClass := &storagev1api.StorageClass{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:   "foo",
+			Labels: map[string]string{},
+		},
+		Provisioner: "foo.csi.k8s.io",
+	}
+
+	foolabelStorageClass := &storagev1api.StorageClass{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "foo",
+			Labels: map[string]string{
+				"velero.io/csi-volumesnapshot-class": "sctest",
+			},
+		},
+		Provisioner: "foo.csi.k8s.io",
+	}
+
+	barStorageClass := &storagev1api.StorageClass{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:   "bar",
+			Labels: map[string]string{},
+		},
+		Provisioner: "bar.csi.k8s.io",
+	}
+
+	hostpathStorageClass := &storagev1api.StorageClass{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:   "hostpath",
+			Labels: map[string]string{},
+		},
+		Provisioner: "hostpath.csi.k8s.io",
+	}
+
+	blahStorageClass := &storagev1api.StorageClass{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:   "blah",
+			Labels: map[string]string{},
+		},
+		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:         "footwithoutlabel VSC annotations on StorageClass",
+			storageClass: foolabelStorageClass,
+			pvc:          pvcNone,
+			backup:       backupNone,
+			expectedVSC:  sctestClass,
 		},
 		{
-			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, 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 backup for foo.csi.k8s.io",
-			driverName:  "foo.csi.k8s.io",
-			pvc:         pvcNone,
-			backup:      backupFoo,
-			expectedVSC: fooClassWithoutLabel,
-			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 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 foo.csi.k8s.io",
+			storageClass: fooStorageClass,
+			pvc:          pvcNone,
+			backup:       backupFoo,
+			expectedVSC:  fooClassWithoutLabel,
+			expectError:  false,
 		},
 		{
-			name:        "no snapshotClass for given driver",
-			driverName:  "blah.csi.k8s.io",
-			pvc:         pvcNone,
-			backup:      backupNone,
-			expectedVSC: nil,
-			expectError: true,
+			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:        "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:         "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",
+			storageClass: fooStorageClass,
+			pvc:          pvcFoo2,
+			backup:       backupFoo2,
+			expectedVSC:  fooClass,
+			expectError:  false,
 		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			actualSnapshotClass, actualError := GetVolumeSnapshotClass(
-				tc.driverName, tc.backup, tc.pvc, logrus.New(), fakeClient)
+				tc.storageClass, tc.backup, tc.pvc, logrus.New(), fakeClient)
 			if tc.expectError {
 				assert.Error(t, actualError)
 				assert.Nil(t, actualSnapshotClass)

site/content/docs/main/csi.md

@@ -73,7 +73,18 @@ This section documents some of the choices made during implementing the CSI snap
         ```
         Note: Please ensure all your annotations are in lowercase. And follow the following format: `velero.io/csi-volumesnapshot-class_<driver name> = <VolumeSnapshotClass Name>`
 
-    3. **Choosing VolumeSnapshotClass for a particular PVC:**
+    3. **Choose VolumeSnapshotClass for a particular StorageClass:**
+    If you want to use a particular VolumeSnapshotClass for a particular StorageClass, you can add an annotation to the StorageClass 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 a given StorageClass, you can annotate it:
+        ```yaml
+        apiVersion: storage/v1
+        kind: StorageClass
+        metadata:
+          name: test-storageclass
+          annotations:
+            velero.io/csi-volumesnapshot-class: "test-snapclass"
+        ```
+
+    4. **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