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

Adding Support For VolumeAttributes in Resource Policy #8383

Merged
merged 14 commits into from
Nov 28, 2024
1 change: 1 addition & 0 deletions changelogs/unreleased/8383-mayankagg9722
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Adding support in velero Resource Policies for filtering PVs based on additional VolumeAttributes properties under CSI PVs
84 changes: 84 additions & 0 deletions design/Implemented/supporting-volumeattributes-resource-policy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# Adding Support For VolumeAttributes in Resource Policy

## Abstract
Currently [Velero Resource policies](https://velero.io/docs/main/resource-filtering/#creating-resource-policies) are only supporting "Driver" to be filtered for [CSI volume conditions](https://github.com/vmware-tanzu/velero/blob/8e23752a6ea83f101bd94a69dcf17f519a805388/internal/resourcepolicies/volume_resources_validator.go#L28)

If user want to skip certain CSI volumes based on other volume attributes like protocol or SKU, etc, they can't do it with the current Velero resource policies. It would be convenient if Velero resource policies could be extended to filter on volume attributes along with existing driver filter in the resource policies `conditions` to handle the backup of volumes just by `some specific volumes attributes conditions`.

## Background
As of Today, Velero resource policy already provides us the way to filter volumes based on the `driver` name. But it's not enough to handle the volumes based on other volume attributes like protocol, SKU, etc.

## Example:
- Provision Azure NFS: Define the Storage class with `protocol: nfs` under storage class parameters to provision [CSI NFS Azure File Shares](https://learn.microsoft.com/en-us/azure/aks/azure-files-csi#nfs-file-shares).
- User wants to back up AFS (Azure file shares) but only want to backup `SMB` type of file share volumes and not `NFS` file share volumes.

## Goals
- We are only bringing additional support in the resource policy to only handle volumes during backup.
- Introducing support for `VolumeAttributes` filter along with `driver` filter in CSI volume conditions to handle volumes.

## Non-Goals
- Currently, only handles volumes, and does not support other resources.

## Use-cases/Scenarios
### Skip backup volumes by some volume attributes:
Users want to skip PV with the requirements:
- option to skip specified PV on volume attributes type (like Protocol as NFS, SMB, etc)

### Sample Storage Class Used to create such Volumes
```
apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
name: azurefile-csi-nfs
provisioner: file.csi.azure.com
allowVolumeExpansion: true
parameters:
protocol: nfs
```

## High-Level Design
Modifying the existing Resource Policies code for [csiVolumeSource](https://github.com/vmware-tanzu/velero/blob/8e23752a6ea83f101bd94a69dcf17f519a805388/internal/resourcepolicies/volume_resources_validator.go#L28C6-L28C22) to add the new `VolumeAttributes` filter for CSI volumes and adding validations in existing [csiCondition](https://github.com/vmware-tanzu/velero/blob/8e23752a6ea83f101bd94a69dcf17f519a805388/internal/resourcepolicies/volume_resources.go#L150) to match with volume attributes in the conditions from Resource Policy config map and original persistent volume.

## Detailed Design
The volume resources policies should contain a list of policies which is the combination of conditions and related `action`, when target volumes meet the conditions, the related `action` will take effection.

Below is the API Design for the user configuration:

### API Design
```go
type csiVolumeSource struct {
Driver string `yaml:"driver,omitempty"`
// [NEW] CSI volume attributes
VolumeAttributes map[string]string `yaml:"volumeAttributes,omitempty"`
}
```

The policies YAML config file would look like this:
```yaml
version: v1
volumePolicies:
- conditions:
csi:
driver: disk.csi.azure.com
action:
type: skip
- conditions:
csi:
driver: file.csi.azure.com
volumeAttributes:
protocol: nfs
action:
type: skip`
```

### New Supported Conditions
#### VolumeAttributes
Existing CSI Volume Condition can now add `volumeAttributes` which will be key and value pairs.

Specify details for the related volume source (currently only csi driver is supported filter)
```yaml
csi: // match volume using `file.csi.azure.com` and with volumeAttributes protocol as nfs
driver: file.csi.azure.com
volumeAttributes:
protocol: nfs
```
252 changes: 236 additions & 16 deletions internal/resourcepolicies/resource_policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,20 +93,32 @@ func TestLoadResourcePolicies(t *testing.T) {
wantErr: true,
},
{
name: "supported formart volume policies",
name: "supported format volume policies",
yamlData: `version: v1
volumePolicies:
- conditions:
capacity: "0,100Gi"
csi:
driver: aws.efs.csi.driver
nfs: {}
storageClass:
- gp2
- ebs-sc
action:
type: skip`,
wantErr: true,
volumePolicies:
- conditions:
capacity: '0,100Gi'
csi:
driver: aws.efs.csi.driver
action:
type: skip
`,
wantErr: false,
},
{
name: "supported format csi driver with volumeAttributes for volume policies",
yamlData: `version: v1
volumePolicies:
- conditions:
capacity: '0,100Gi'
csi:
driver: aws.efs.csi.driver
volumeAttributes:
key1: value1
action:
type: skip
`,
wantErr: false,
},
}
for _, tc := range testCases {
Expand Down Expand Up @@ -135,6 +147,16 @@ func TestGetResourceMatchedAction(t *testing.T) {
}),
},
},
{
Action: Action{Type: "skip"},
Conditions: map[string]interface{}{
"csi": interface{}(
map[string]interface{}{
"driver": "files.csi.driver",
"volumeAttributes": map[string]string{"protocol": "nfs"},
}),
},
},
{
Action: Action{Type: "snapshot"},
Conditions: map[string]interface{}{
Expand Down Expand Up @@ -172,6 +194,24 @@ func TestGetResourceMatchedAction(t *testing.T) {
},
expectedAction: &Action{Type: "skip"},
},
{
name: "match policy AFS NFS",
volume: &structuredVolume{
capacity: *resource.NewQuantity(5<<30, resource.BinarySI),
storageClass: "afs-nfs",
csi: &csiVolumeSource{Driver: "files.csi.driver", VolumeAttributes: map[string]string{"protocol": "nfs"}},
},
expectedAction: &Action{Type: "skip"},
},
{
name: "match policy AFS SMB",
volume: &structuredVolume{
capacity: *resource.NewQuantity(5<<30, resource.BinarySI),
storageClass: "afs-smb",
csi: &csiVolumeSource{Driver: "files.csi.driver"},
},
expectedAction: nil,
},
{
name: "both matches return the first policy",
volume: &structuredVolume{
Expand Down Expand Up @@ -226,7 +266,7 @@ func TestGetResourcePoliciesFromConfig(t *testing.T) {
Namespace: "test-namespace",
},
Data: map[string]string{
"test-data": "version: v1\nvolumePolicies:\n- conditions:\n capacity: '0,10Gi'\n action:\n type: skip",
"test-data": "version: v1\nvolumePolicies:\n - conditions:\n capacity: '0,10Gi'\n csi:\n driver: disks.csi.driver\n action:\n type: skip\n - conditions:\n csi:\n driver: files.csi.driver\n volumeAttributes:\n protocol: nfs\n action:\n type: skip",
},
}

Expand All @@ -236,13 +276,27 @@ func TestGetResourcePoliciesFromConfig(t *testing.T) {

// Check that the returned resourcePolicies object contains the expected data
assert.Equal(t, "v1", resPolicies.version)
assert.Len(t, resPolicies.volumePolicies, 1)
assert.Len(t, resPolicies.volumePolicies, 2)
policies := ResourcePolicies{
Version: "v1",
VolumePolicies: []VolumePolicy{
{
Conditions: map[string]interface{}{
"capacity": "0,10Gi",
"csi": map[string]interface{}{
"driver": "disks.csi.driver",
},
},
Action: Action{
Type: Skip,
},
},
{
Conditions: map[string]interface{}{
"csi": map[string]interface{}{
"driver": "files.csi.driver",
"volumeAttributes": map[string]string{"protocol": "nfs"},
},
},
Action: Action{
Type: Skip,
Expand Down Expand Up @@ -298,7 +352,173 @@ volumePolicies:
skip: false,
},
{
name: "csi not configured",
name: "Skip AFS CSI condition with Disk volumes",
yamlData: `version: v1
volumePolicies:
- conditions:
csi:
driver: files.csi.driver
action:
type: skip`,
vol: &v1.PersistentVolume{
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{Driver: "disks.csi.driver"},
}},
},
skip: false,
},
{
name: "Skip AFS CSI condition with AFS volumes",
yamlData: `version: v1
volumePolicies:
- conditions:
csi:
driver: files.csi.driver
action:
type: skip`,
vol: &v1.PersistentVolume{
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{Driver: "files.csi.driver"},
}},
},
skip: true,
},
{
name: "Skip AFS NFS CSI condition with Disk volumes",
yamlData: `version: v1
volumePolicies:
- conditions:
csi:
driver: files.csi.driver
volumeAttributes:
protocol: nfs
action:
type: skip
`,
vol: &v1.PersistentVolume{
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{Driver: "disks.csi.driver"},
}},
},
skip: false,
},
{
name: "Skip AFS NFS CSI condition with AFS SMB volumes",
yamlData: `version: v1
volumePolicies:
- conditions:
csi:
driver: files.csi.driver
volumeAttributes:
protocol: nfs
action:
type: skip
`,
vol: &v1.PersistentVolume{
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{Driver: "files.csi.driver", VolumeAttributes: map[string]string{"key1": "val1"}},
}},
},
skip: false,
},
{
name: "Skip AFS NFS CSI condition with AFS NFS volumes",
yamlData: `version: v1
volumePolicies:
- conditions:
csi:
driver: files.csi.driver
volumeAttributes:
protocol: nfs
action:
type: skip
`,
vol: &v1.PersistentVolume{
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{Driver: "files.csi.driver", VolumeAttributes: map[string]string{"protocol": "nfs"}},
}},
},
skip: true,
},
{
name: "Skip Disk and AFS NFS CSI condition with Disk volumes",
yamlData: `version: v1
volumePolicies:
- conditions:
csi:
driver: disks.csi.driver
action:
type: skip
- conditions:
csi:
driver: files.csi.driver
volumeAttributes:
protocol: nfs
action:
type: skip`,
vol: &v1.PersistentVolume{
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{Driver: "disks.csi.driver", VolumeAttributes: map[string]string{"key1": "val1"}},
}},
},
skip: true,
},
{
name: "Skip Disk and AFS NFS CSI condition with AFS SMB volumes",
yamlData: `version: v1
volumePolicies:
- conditions:
csi:
driver: disks.csi.driver
action:
type: skip
- conditions:
csi:
driver: files.csi.driver
volumeAttributes:
protocol: nfs
action:
type: skip`,
vol: &v1.PersistentVolume{
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{Driver: "files.csi.driver", VolumeAttributes: map[string]string{"key1": "val1"}},
}},
},
skip: false,
},
{
name: "Skip Disk and AFS NFS CSI condition with AFS NFS volumes",
yamlData: `version: v1
volumePolicies:
- conditions:
csi:
driver: disks.csi.driver
action:
type: skip
- conditions:
csi:
driver: files.csi.driver
volumeAttributes:
protocol: nfs
action:
type: skip`,
vol: &v1.PersistentVolume{
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{Driver: "files.csi.driver", VolumeAttributes: map[string]string{"key1": "val1", "protocol": "nfs"}},
}},
},
skip: true,
},
{
name: "csi not configured and testing capacity condition",
yamlData: `version: v1
volumePolicies:
- conditions:
Expand Down
Loading