Skip to content

Commit

Permalink
feat(csi): allow setting annotation on StorageClass to determine Volu…
Browse files Browse the repository at this point in the history 
…meSnapshotClass

Signed-off-by: Carlo Field <[email protected]>
  • Loading branch information
@cfi2017
cfi2017 committed Jan 23, 2025
1 parent a9031eb commit 87f393d
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 60 deletions.
1 change: 1 addition & 0 deletions pkg/apis/velero/v1/labels_annotations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/backup/actions/csi/pvc_action.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
37 changes: 32 additions & 5 deletions pkg/util/csi/volume_snapshot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -307,7 +308,7 @@ func patchVSC(
}

func GetVolumeSnapshotClass(
provisioner string,
storageClass *storagev1api.StorageClass,
backup *velerov1api.Backup,
pvc *corev1api.PersistentVolumeClaim,
log logrus.FieldLogger,
Expand All @@ -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)
Expand All @@ -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)
}
Expand All @@ -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")
}
Expand Down Expand Up @@ -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.
Expand Down
150 changes: 96 additions & 54 deletions pkg/util/csi/volume_snapshot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 87f393d

Please sign in to comment.