Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Multiple VolumeSnapshotClasses #178

Merged
merged 5 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/<driver name> = <VolumeSnapshotClass Name>`

#### 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 = <VolumeSnapshotClass Name>`

## Filing issues

If you would like to file a GitHub issue for the plugin, please open the issue on the [core Velero repo][103]
Expand Down
2 changes: 1 addition & 1 deletion internal/backup/pvc_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
anshulahuja98 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, nil, errors.Wrapf(err, "failed to get volumesnapshotclass for storageclass %s", storageClass.Name)
}
Expand Down
18 changes: 10 additions & 8 deletions internal/util/labels_annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Contributor

@blackpiglet blackpiglet Jun 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like VolumeSnapshotClassDriverBackupAnnotationPrefix and is duplicated with VolumeSnapshotClassDriverPVCAnnotation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually all 3 variables including VolumeSnapshotClassSelectorLabel have the same string
I kept that intentionally same to make user experience easier
While on the other hand - all are separate 3 variables such that it is easy to change in code than reusing the variables.

Do you have any suggestion for keeping the values different?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that these are logically different parameters. I think it's fine that they have the same value, since they're used in different contexts -- i.e. a PVC annotation shouldn't need to be called "csi-volumesnapshot-class-pvc" since it's already on the PVC.

VolumeSnapshotClassDriverPVCAnnotation = "velero.io/csi-volumesnapshot-class"

// There is no release w/ these constants exported. Using the strings for now.
// CSI Labels volumesnapshotclass
Expand Down
70 changes: 66 additions & 4 deletions internal/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,75 @@ 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) {
anshulahuja98 marked this conversation as resolved.
Show resolved Hide resolved
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.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("Incorrect volumesnapshotclass, 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
anshulahuja98 marked this conversation as resolved.
Show resolved Hide resolved
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) {
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
anshulahuja98 marked this conversation as resolved.
Show resolved Hide resolved
}
}
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.
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'
Expand Down Expand Up @@ -325,4 +387,4 @@ func CleanupVolumeSnapshot(volSnap *snapshotv1api.VolumeSnapshot, snapshotClient
} else {
log.Info("Deleted volumesnapshot with volumesnapshotContent %s/%s", vs.Namespace, vs.Name)
}
}
}
Loading