Skip to content

Commit

Permalink
Implement storage snapshot mapping (#106)
Browse files Browse the repository at this point in the history
Read the mapping from the driver-config ConfigMap.
Use this mapping to determine if a snapshot can be
created with the storage/volume snapshot class combination.
If not possible return an error with a list of the
possible snapshot classes.

Signed-off-by: Alexander Wels <awels@redhat.com>
  • Loading branch information
awels authored Apr 8, 2024
1 parent 701c59c commit 4f94084
Show file tree
Hide file tree
Showing 20 changed files with 2,705 additions and 661 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ data:
infraClusterNamespace: kvcluster
infraClusterLabels: csi-driver/cluster=tenant
```
For more information about the fields available in the `driver-config` ConfigMap see this [documentation](docs/snapshot-driver-config.md)
```yaml
kind: Deployment
apiVersion: apps/v1
Expand Down
7 changes: 6 additions & 1 deletion cmd/kubevirt-csi-driver/kubevirt-csi-driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"k8s.io/client-go/tools/clientcmd"
klog "k8s.io/klog/v2"

snapcli "kubevirt.io/csi-driver/pkg/generated/external-snapshotter/client-go/clientset/versioned"
"kubevirt.io/csi-driver/pkg/kubevirt"
"kubevirt.io/csi-driver/pkg/service"
"kubevirt.io/csi-driver/pkg/util"
Expand Down Expand Up @@ -90,11 +91,15 @@ func handle() {
if err != nil {
klog.Fatalf("Failed to build tenant client set: %v", err)
}
tenantSnapshotClientSet, err := snapcli.NewForConfig(tenantRestConfig)
if err != nil {
klog.Fatalf("Failed to build tenant snapshot client set: %v", err)
}

infraClusterLabelsMap := parseLabels()
storageClassEnforcement := configureStorageClassEnforcement(infraStorageClassEnforcement)

virtClient, err := kubevirt.NewClient(infraRestConfig, infraClusterLabelsMap, storageClassEnforcement, *volumePrefix)
virtClient, err := kubevirt.NewClient(infraRestConfig, infraClusterLabelsMap, tenantClientSet, tenantSnapshotClientSet, storageClassEnforcement, *volumePrefix)
if err != nil {
klog.Fatal(err)
}
Expand Down
4 changes: 4 additions & 0 deletions deploy/tenant/base/deploy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ rules:
resources: ["securitycontextconstraints"]
verbs: ["use"]
resourceNames: ["privileged"]
- apiGroups: ["snapshot.storage.k8s.io"]
resources: ["volumesnapshotclasses"]
verbs: ["list"]

---
kind: ClusterRole
apiVersion: rbac.authorization.k8s.io/v1
Expand Down
97 changes: 97 additions & 0 deletions docs/snapshot-driver-config.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# Configuring infra volume snapshot classes to map to tenant volume snapshot classes
It is possible to map multiple infra storage classes to multiple matching tenant storage classes. For instance if the infra cluster has 2 completely separate storage classes like storage class a and b, we can map them to tenant storage classes x and y. In order to define this mapping one should create a config map in the namespace of the infra storage classes that is used by the tenant storage class. This config map will have a few key value pairs that define the expected behavior.

* allowAll: If allow all is true, then allow all available storage classes in the infra cluster to be mapping to storage classes in the tenant cluster. If false, then use the allowList to limit which storage classes are visible to the tenant.
* allowDefault: If true, then no explicit mapping needs to be defined, and the driver will attempt to use the default storage class and default volume snapshot class of the infra cluster to satisfy requests from the tenant cluster
* allowList: A comma separated string list of all the allowed infra storage classes. Only used if allowAll is false.
* storageSnapshotMapping: Groups lists of infra storage classes and infra volume snapshot classes together. If in the same grouping then creating a snapshot using any of the listed volume snapshot class should work with any of the listed storage classes. Should only contain volume snapshot classes that are compatible with the listed storage classes. This is needed because it is not always possible to determine using the SA of the csi driver controller which volume snapshot classes go together with which storage classes.

## Example driver configs

### allowDefault
The simplest driver config takes the default storage class and default volume snapshot class and uses them, and no storage classes are restricted:

```yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: driver-config
namespace: example-namespace
data:
infraClusterLabels: random-cluster-id #label used to distinguish between tenant clusters, if multiple clusters in same namespace
infraClusterNamespace: example-namespace #Used to tell the tenant cluster which namespace it lives in
infraStorageClassEnforcement: |
allowAll: true
allowDefault: true
```
### allowAll false, with default
The simplest driver config takes the default storage class and default volume snapshot class and uses them, and restricted to storage class a and b. Either storage class a or b should be the default:
```yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: driver-config
namespace: example-namespace
data:
infraClusterLabels: random-cluster-id #label used to distinguish between tenant clusters, if multiple clusters in same namespace
infraClusterNamespace: example-namespace #Used to tell the tenant cluster which namespace it lives in
infraStorageClassEnforcement: |
allowAll: false
allowDefault: true
allowList: [storage_class_a, storage_class_b]
```
Note ensure that the infra cluster has a default snapshot class defined, otherwise creation of the infra cluster snapshots will fail due to a missing snapshot class value.
### Specify which storage class maps to which volume snapshot class, unrelated storage classes
The infra cluster has multiple storage classes and they map to volume snapshot classes. The storage classes are not related and require different volume snapshot classes
```yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: driver-config
namespace: example-namespace
data:
infraClusterLabels: random-cluster-id #label used to distinguish between tenant clusters, if multiple clusters in same namespace
infraClusterNamespace: example-namespace #Used to tell the tenant cluster which namespace it lives in
infraStorageClassEnforcement: |
allowAll: false
allowDefault: true
allowList: [storage_class_a, storage_class_b]
storageSnapshotMapping: - StorageClasses:
- storage_class_a
VolumeSnapshotClasses:
- volumesnapshot_class_a
- StorageClasses:
- storage_class_b
VolumeSnapshotClasses:
- volumesnapshot_class_b
```
If one tries to create a snapshot using volume snapshot class `kubevirt_csi_vsc_y` on a PVC associated with storage class `storage_class_x`. The CSI driver will reject that request and return an error containig a list of valid volume snapshot classes. In this case `kubevirt_csi_vsc_x`.

### Specify which storage class maps to which volume snapshot class, related storage classes
The infra cluster has multiple storage classes and they map to volume snapshot classes. The storage classes are not related and require different volume snapshot classes

```yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: driver-config
namespace: example-namespace
data:
infraClusterLabels: random-cluster-id #label used to distinguish between tenant clusters, if multiple clusters in same namespace
infraClusterNamespace: example-namespace #Used to tell the tenant cluster which namespace it lives in
infraStorageClassEnforcement: |
allowAll: false
allowDefault: true
allowList: [storage_class_a, storage_class_b]
storageSnapshotMapping: - StorageClasses:
- storage_class_a
- storage_class_b
VolumeSnapshotClasses:
- volumesnapshot_class_a
- volumesnapshot_class_b
```
In this case, both storage classes and volumesnapshot classes are in the same `StorageClasses` group, so now trying to create a snapshot using `kubevirt_csi_vsc_y` of a PVC from storage class `storage_class_x` will succeed because that volume snapshot class is part of the group associated with that storage class.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module kubevirt.io/csi-driver

go 1.20
go 1.21

require (
github.com/container-storage-interface/spec v1.6.0
Expand All @@ -25,7 +25,7 @@ require (
k8s.io/klog/v2 v2.120.1
k8s.io/utils v0.0.0-20240102154912-e7106e64919e
kubevirt.io/api v1.1.1
kubevirt.io/containerized-data-importer-api v1.58.1
kubevirt.io/containerized-data-importer-api v1.59.0
)

require (
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1853,6 +1853,8 @@ kubevirt.io/api v1.1.1 h1:vt5bOpACArNFIudx1bcE1VeejQdh5wCd7Oz/uFBIkH8=
kubevirt.io/api v1.1.1/go.mod h1:CJ4vZsaWhVN3jNbyc9y3lIZhw8nUHbWjap0xHABQiqc=
kubevirt.io/containerized-data-importer-api v1.58.1 h1:Zbf0pCvxb4fBvtMR6uI2OIJZ4UfwFxripzOLMO4HPbI=
kubevirt.io/containerized-data-importer-api v1.58.1/go.mod h1:Y/8ETgHS1GjO89bl682DPtQOYEU/1ctPFBz6Sjxm4DM=
kubevirt.io/containerized-data-importer-api v1.59.0 h1:GdDt9BlR0qHejpMaPfASbsG8JWDmBf1s7xZBj5W9qn0=
kubevirt.io/containerized-data-importer-api v1.59.0/go.mod h1:4yOGtCE7HvgKp7wftZZ3TBvDJ0x9d6N6KaRjRYcUFpE=
kubevirt.io/controller-lifecycle-operator-sdk/api v0.0.0-20220329064328-f3cc58c6ed90 h1:QMrd0nKP0BGbnxTqakhDZAUhGKxPiPiN5gSDqKUmGGc=
kubevirt.io/controller-lifecycle-operator-sdk/api v0.0.0-20220329064328-f3cc58c6ed90/go.mod h1:018lASpFYBsYN6XwmA2TIrPCx6e0gviTd/ZNtSitKgc=
lukechampine.com/uint128 v1.1.1/go.mod h1:c4eWIwlEGaxC/+H1VguhU4PHXNWDCDMUlWdIWl2j1gk=
Expand Down
1 change: 0 additions & 1 deletion hack/cluster-sync-split.sh
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ END
mkdir -p ./deploy/controller-infra/dev-overlay
mkdir -p ./deploy/tenant/dev-overlay

cluster::generate_controller_rbac $TENANT_CLUSTER_NAMESPACE
cluster::generate_tenant_dev_kustomization
cluster::generate_controller_dev_kustomization "controller-infra" $TENANT_CLUSTER_NAMESPACE
tenant::deploy_csidriver_namespace $CSI_DRIVER_NAMESPACE
Expand Down
1 change: 0 additions & 1 deletion hack/cluster-sync.sh
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ END
mkdir -p ./deploy/controller-tenant/dev-overlay
mkdir -p ./deploy/tenant/dev-overlay

cluster::generate_controller_rbac $TENANT_CLUSTER_NAMESPACE
cluster::generate_tenant_dev_kustomization
cluster::generate_controller_dev_kustomization "controller-tenant" $CSI_DRIVER_NAMESPACE
tenant::deploy_csidriver_namespace $CSI_DRIVER_NAMESPACE
Expand Down
32 changes: 2 additions & 30 deletions hack/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -131,34 +131,6 @@ function tenant::deploy_snapshotresources() {
./kubevirtci kubectl-tenant apply -f ./deploy/tenant/base/snapshot.storage.k8s.io_volumesnapshotclasses.yaml
./kubevirtci kubectl-tenant apply -f ./deploy/tenant/base/snapshot.storage.k8s.io_volumesnapshotcontents.yaml
./kubevirtci kubectl-tenant apply -f ./deploy/tenant/base/snapshot.storage.k8s.io_volumesnapshots.yaml
# Make sure the infra rbd snapshot class is the default snapshot class
./kubevirtci kubectl patch volumesnapshotclass csi-rbdplugin-snapclass --type merge -p '{"metadata": {"annotations":{"snapshot.storage.kubernetes.io/is-default-class":"true"}}}'
}

function cluster::generate_controller_rbac() {
cat <<- END | ./kubevirtci kubectl apply -f -
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: kubevirt-csi-snapshot
rules:
- apiGroups: ["storage.k8s.io"]
resources: ["storageclasses"]
verbs: ["get"]
- apiGroups: ["snapshot.storage.k8s.io"]
resources: ["volumesnapshotclasses"]
verbs: ["get", "list"]
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: kubevirt-csi-snapshot
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: kubevirt-csi-snapshot
subjects:
- kind: ServiceAccount
name: kubevirt-csi
namespace: $1
END
}
Loading

0 comments on commit 4f94084

Please sign in to comment.