From f705b23bbd2941da4ab4301539a96ec009121719 Mon Sep 17 00:00:00 2001 From: Chris Henzie Date: Fri, 11 Sep 2020 15:28:45 -0700 Subject: [PATCH] Pass snapshot metadata to CSI driver --- cmd/csi-snapshotter/main.go | 2 ++ pkg/sidecar-controller/content_create_test.go | 18 +++++++++++- pkg/sidecar-controller/csi_handler.go | 7 +---- pkg/sidecar-controller/framework_test.go | 1 + pkg/sidecar-controller/snapshot_controller.go | 12 +++++++- .../snapshot_controller_base.go | 29 ++++++++++--------- pkg/utils/util.go | 8 ++++- pkg/utils/util_test.go | 6 ++-- 8 files changed, 59 insertions(+), 24 deletions(-) diff --git a/cmd/csi-snapshotter/main.go b/cmd/csi-snapshotter/main.go index 8c606da2..97e831f3 100644 --- a/cmd/csi-snapshotter/main.go +++ b/cmd/csi-snapshotter/main.go @@ -62,6 +62,7 @@ var ( showVersion = flag.Bool("version", false, "Show version.") threads = flag.Int("worker-threads", 10, "Number of worker threads.") csiTimeout = flag.Duration("timeout", defaultCSITimeout, "The timeout for any RPCs to the CSI driver. Default is 1 minute.") + extraCreateMetadata = flag.Bool("extra-create-metadata", false, "If set, add snapshot metadata to plugin snapshot requests as parameters.") leaderElection = flag.Bool("leader-election", false, "Enables leader election.") leaderElectionNamespace = flag.String("leader-election-namespace", "", "The namespace where the leader election resource exists. Defaults to the pod namespace if not set.") @@ -173,6 +174,7 @@ func main() { *resyncPeriod, *snapshotNamePrefix, *snapshotNameUUIDLength, + *extraCreateMetadata, ) run := func(context.Context) { diff --git a/pkg/sidecar-controller/content_create_test.go b/pkg/sidecar-controller/content_create_test.go index 9e1aa04b..9c0b066b 100644 --- a/pkg/sidecar-controller/content_create_test.go +++ b/pkg/sidecar-controller/content_create_test.go @@ -41,6 +41,11 @@ func TestSyncContent(t *testing.T) { snapshotName: "snapshot-snapuid1-1", driverName: mockDriverName, snapshotId: "snapuid1-1", + parameters: map[string]string{ + utils.PrefixedVolumeSnapshotNameKey: "snap1-1", + utils.PrefixedVolumeSnapshotNamespaceKey: "default", + utils.PrefixedVolumeSnapshotContentNameKey: "content1-1", + }, creationTime: timeNow, readyToUse: true, }, @@ -63,6 +68,11 @@ func TestSyncContent(t *testing.T) { snapshotName: "snapshot-snapuid1-2", driverName: mockDriverName, snapshotId: "snapuid1-2", + parameters: map[string]string{ + utils.PrefixedVolumeSnapshotNameKey: "snap1-2", + utils.PrefixedVolumeSnapshotNamespaceKey: "default", + utils.PrefixedVolumeSnapshotContentNameKey: "content1-2", + }, creationTime: timeNow, readyToUse: true, size: defaultSize, @@ -114,7 +124,13 @@ func TestSyncContent(t *testing.T) { { volumeHandle: "volume-handle-1-4", snapshotName: "snapshot-snapuid1-4", - parameters: class5Parameters, + parameters: map[string]string{ + utils.AnnDeletionSecretRefName: "secret", + utils.AnnDeletionSecretRefNamespace: "default", + utils.PrefixedVolumeSnapshotNameKey: "snap1-4", + utils.PrefixedVolumeSnapshotNamespaceKey: "default", + utils.PrefixedVolumeSnapshotContentNameKey: "content1-4", + }, secrets: map[string]string{ "foo": "bar", }, diff --git a/pkg/sidecar-controller/csi_handler.go b/pkg/sidecar-controller/csi_handler.go index 5ff94ebf..96628200 100644 --- a/pkg/sidecar-controller/csi_handler.go +++ b/pkg/sidecar-controller/csi_handler.go @@ -24,7 +24,6 @@ import ( crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/snapshotter" - "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" ) // Handler is responsible for handling VolumeSnapshot events from informer. @@ -74,11 +73,7 @@ func (handler *csiHandler) CreateSnapshot(content *crdv1.VolumeSnapshotContent, if err != nil { return "", "", time.Time{}, 0, false, err } - newParameters, err := utils.RemovePrefixedParameters(parameters) - if err != nil { - return "", "", time.Time{}, 0, false, fmt.Errorf("failed to remove CSI Parameters of prefixed keys: %v", err) - } - return handler.snapshotter.CreateSnapshot(ctx, snapshotName, *content.Spec.Source.VolumeHandle, newParameters, snapshotterCredentials) + return handler.snapshotter.CreateSnapshot(ctx, snapshotName, *content.Spec.Source.VolumeHandle, parameters, snapshotterCredentials) } func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error { diff --git a/pkg/sidecar-controller/framework_test.go b/pkg/sidecar-controller/framework_test.go index 675d24bf..652c15a2 100644 --- a/pkg/sidecar-controller/framework_test.go +++ b/pkg/sidecar-controller/framework_test.go @@ -521,6 +521,7 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte 60*time.Second, "snapshot", -1, + true, ) ctrl.eventRecorder = record.NewFakeRecorder(1000) diff --git a/pkg/sidecar-controller/snapshot_controller.go b/pkg/sidecar-controller/snapshot_controller.go index 8b0d51f5..51ad1041 100644 --- a/pkg/sidecar-controller/snapshot_controller.go +++ b/pkg/sidecar-controller/snapshot_controller.go @@ -284,7 +284,17 @@ func (ctrl *csiSnapshotSideCarController) createSnapshotWrapper(content *crdv1.V return nil, fmt.Errorf("failed to add VolumeSnapshotBeingCreated annotation on the content %s: %q", content.Name, err) } - driverName, snapshotID, creationTime, size, readyToUse, err := ctrl.handler.CreateSnapshot(content, class.Parameters, snapshotterCredentials) + parameters, err := utils.RemovePrefixedParameters(class.Parameters) + if err != nil { + return nil, fmt.Errorf("failed to remove CSI Parameters of prefixed keys: %v", err) + } + if ctrl.extraCreateMetadata { + parameters[utils.PrefixedVolumeSnapshotNameKey] = content.Spec.VolumeSnapshotRef.Name + parameters[utils.PrefixedVolumeSnapshotNamespaceKey] = content.Spec.VolumeSnapshotRef.Namespace + parameters[utils.PrefixedVolumeSnapshotContentNameKey] = content.Name + } + + driverName, snapshotID, creationTime, size, readyToUse, err := ctrl.handler.CreateSnapshot(content, parameters, snapshotterCredentials) if err != nil { // NOTE(xyang): handle create timeout // If it is a final error, remove annotation to indicate diff --git a/pkg/sidecar-controller/snapshot_controller_base.go b/pkg/sidecar-controller/snapshot_controller_base.go index cb68d2e3..92a80057 100644 --- a/pkg/sidecar-controller/snapshot_controller_base.go +++ b/pkg/sidecar-controller/snapshot_controller_base.go @@ -40,11 +40,12 @@ import ( ) type csiSnapshotSideCarController struct { - clientset clientset.Interface - client kubernetes.Interface - driverName string - eventRecorder record.EventRecorder - contentQueue workqueue.RateLimitingInterface + clientset clientset.Interface + client kubernetes.Interface + driverName string + eventRecorder record.EventRecorder + contentQueue workqueue.RateLimitingInterface + extraCreateMetadata bool contentLister storagelisters.VolumeSnapshotContentLister contentListerSynced cache.InformerSynced @@ -70,6 +71,7 @@ func NewCSISnapshotSideCarController( resyncPeriod time.Duration, snapshotNamePrefix string, snapshotNameUUIDLength int, + extraCreateMetadata bool, ) *csiSnapshotSideCarController { broadcaster := record.NewBroadcaster() broadcaster.StartLogging(klog.Infof) @@ -78,14 +80,15 @@ func NewCSISnapshotSideCarController( eventRecorder = broadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: fmt.Sprintf("csi-snapshotter %s", driverName)}) ctrl := &csiSnapshotSideCarController{ - clientset: clientset, - client: client, - driverName: driverName, - eventRecorder: eventRecorder, - handler: NewCSIHandler(snapshotter, timeout, snapshotNamePrefix, snapshotNameUUIDLength), - resyncPeriod: resyncPeriod, - contentStore: cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc), - contentQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "csi-snapshotter-content"), + clientset: clientset, + client: client, + driverName: driverName, + eventRecorder: eventRecorder, + handler: NewCSIHandler(snapshotter, timeout, snapshotNamePrefix, snapshotNameUUIDLength), + resyncPeriod: resyncPeriod, + contentStore: cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc), + contentQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "csi-snapshotter-content"), + extraCreateMetadata: extraCreateMetadata, } volumeSnapshotContentInformer.Informer().AddEventHandlerWithResyncPeriod( diff --git a/pkg/utils/util.go b/pkg/utils/util.go index c7c52086..c4744855 100644 --- a/pkg/utils/util.go +++ b/pkg/utils/util.go @@ -49,7 +49,9 @@ const ( // CSI Parameters prefixed with csiParameterPrefix are not passed through // to the driver on CreateSnapshotRequest calls. Instead they are intended // to be used by the CSI external-snapshotter and maybe used to populate - // fields in subsequent CSI calls or Kubernetes API objects. + // fields in subsequent CSI calls or Kubernetes API objects. An exception + // exists for the volume snapshot and volume snapshot content keys, which are + // passed as parameters on CreateSnapshotRequest calls. csiParameterPrefix = "csi.storage.k8s.io/" PrefixedSnapshotterSecretNameKey = csiParameterPrefix + "snapshotter-secret-name" // Prefixed name key for DeleteSnapshot secret @@ -58,6 +60,10 @@ const ( PrefixedSnapshotterListSecretNameKey = csiParameterPrefix + "snapshotter-list-secret-name" // Prefixed name key for ListSnapshots secret PrefixedSnapshotterListSecretNamespaceKey = csiParameterPrefix + "snapshotter-list-secret-namespace" // Prefixed namespace key for ListSnapshots secret + PrefixedVolumeSnapshotNameKey = csiParameterPrefix + "volumesnapshot/name" // Prefixed VolumeSnapshot name key + PrefixedVolumeSnapshotNamespaceKey = csiParameterPrefix + "volumesnapshot/namespace" // Prefixed VolumeSnapshot namespace key + PrefixedVolumeSnapshotContentNameKey = csiParameterPrefix + "volumesnapshotcontent/name" // Prefixed VolumeSnapshotContent name key + // Name of finalizer on VolumeSnapshotContents that are bound by VolumeSnapshots VolumeSnapshotContentFinalizer = "snapshot.storage.kubernetes.io/volumesnapshotcontent-bound-protection" // Name of finalizer on VolumeSnapshot that is being used as a source to create a PVC diff --git a/pkg/utils/util_test.go b/pkg/utils/util_test.go index 148f62a4..a25d456a 100644 --- a/pkg/utils/util_test.go +++ b/pkg/utils/util_test.go @@ -169,8 +169,10 @@ func TestRemovePrefixedCSIParams(t *testing.T) { { name: "all known prefixed", params: map[string]string{ - PrefixedSnapshotterSecretNameKey: "csiBar", - PrefixedSnapshotterSecretNamespaceKey: "csiBar", + PrefixedSnapshotterSecretNameKey: "csiBar", + PrefixedSnapshotterSecretNamespaceKey: "csiBar", + PrefixedSnapshotterListSecretNameKey: "csiBar", + PrefixedSnapshotterListSecretNamespaceKey: "csiBar", }, expectedParams: map[string]string{}, },