[manila-csi-plugin] Add csi-provisioner's --extra-create-metadata as share metadata (kubernetes#2023)

* manila-csi-plugin: Add volume parameters passed from csi-provisioner as metadata

* charts: Add extraCreateMetadata value to run csi-provisioner with --extra-create-metadata

Author: gman0

charts/manila-csi-plugin/templates/controllerplugin-statefulset.yaml

@@ -26,6 +26,9 @@ spec:
             {{- if $.Values.csimanila.topologyAwarenessEnabled }}
             - "--feature-gates=Topology=true"
             {{- end }}
+            {{- if $.Values.controllerplugin.provisioner.extraCreateMetadata }}
+            - "--extra-create-metadata"
+            {{- end }}
           env:
             - name: ADDRESS
               value: "unix:///var/lib/kubelet/plugins/{{ printf "%s.%s" .protocolSelector $.Values.driverName | lower }}/csi-controllerplugin.sock"

charts/manila-csi-plugin/values.yaml

@@ -79,6 +79,8 @@ controllerplugin:
       tag: v3.0.0
       pullPolicy: IfNotPresent
     resources: {}
+    # Whether to pass --extra-create-metadata flag to csi-provisioner.
+    extraCreateMetadata: false
   # CSI external-snapshotter container spec
   snapshotter:
     image:

pkg/csi/manila/controllerserver.go

@@ -43,6 +43,15 @@ type controllerServer struct {
 var (
 	pendingVolumes   = sync.Map{}
 	pendingSnapshots = sync.Map{}
+
+	// Recognized volume parameters passed by Kubernetes csi-provisioner sidecar
+	// when run with --extra-create-metadata flag. These are added to metadata
+	// of newly created shares if present.
+	recognizedCSIProvisionerParams = []string{
+		"csi.storage.k8s.io/pvc/name",
+		"csi.storage.k8s.io/pvc/namespace",
+		"csi.storage.k8s.io/pv/name",
+	}
 )
 
 func getVolumeCreator(source *csi.VolumeContentSource, shareOpts *options.ControllerVolumeContext, compatOpts *options.CompatibilityOptions) (volumeCreator, error) {
@@ -92,7 +101,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		return nil, status.Errorf(codes.InvalidArgument, "invalid volume parameters: %v", err)
 	}
 
-	shareMetadata, err := prepareShareMetadata(shareOpts.AppendShareMetadata, cs.d.clusterID)
+	shareMetadata, err := prepareShareMetadata(shareOpts.AppendShareMetadata, cs.d.clusterID, params)
 	if err != nil {
 		return nil, err
 	}
@@ -485,16 +494,34 @@ func parseStringMapFromJSON(data string) (m map[string]string, err error) {
 	return
 }
 
-func prepareShareMetadata(appendShareMetadata, clusterID string) (map[string]string, error) {
-	shareMetadata, err := parseStringMapFromJSON(appendShareMetadata)
+func prepareShareMetadata(appendShareMetadata, clusterID string, volumeParams map[string]string) (map[string]string, error) {
+	shareMetadata := make(map[string]string)
+
+	// Get extra metadata provided by csi-provisioner sidecar if present.
+	for _, k := range recognizedCSIProvisionerParams {
+		if v, ok := volumeParams[k]; ok {
+			shareMetadata[k] = v
+		}
+	}
+
+	// Get metadata from appendShareMetadata volume parameter.
+	// It will not overwrite keys defined in recognizedCSIProvisionerParams.
+
+	appendShareMetadataMap, err := parseStringMapFromJSON(appendShareMetadata)
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "failed to parse appendShareMetadata field: %v", err)
 	}
 
-	if clusterID != "" {
-		if shareMetadata == nil {
-			shareMetadata = make(map[string]string)
+	for k, v := range appendShareMetadataMap {
+		if existingValue, ok := shareMetadata[k]; ok {
+			klog.Warningf("skip adding share metadata key %s from appendShareMetadata because it already exists with value %s", k, existingValue)
+		} else {
+			shareMetadata[k] = v
 		}
+	}
+
+	// Get cluster ID.
+	if clusterID != "" {
 		if val, ok := shareMetadata[clusterMetadataKey]; ok && val != clusterID {
 			klog.Warningf("skip adding cluster ID %v to share metadata because appended metadata already defines it as %v", clusterID, val)
 		} else {

pkg/csi/manila/controllerserver_test.go

@@ -20,21 +20,24 @@ import (
 
 func TestPrepareShareMetadata(t *testing.T) {
 	ts := []struct {
+		allVolumeParams     map[string]string
 		appendShareMetadata string
 		cluster             string
 		expectedResult      map[string]string
 		expectedError       bool
 	}{
 		{
 			// Empty metadata and cluster
+			allVolumeParams:     map[string]string{},
 			appendShareMetadata: "",
 			cluster:             "",
 			expectedResult:      nil,
 			expectedError:       false,
 		},
 		{
 			// Existing metadata and empty cluster
-			appendShareMetadata: "{\"keyA\": \"valueA\", \"keyB\": \"valueB\"}",
+			allVolumeParams:     map[string]string{"appendShareMetadata": `{"keyA": "valueA", "keyB": "valueB"}`},
+			appendShareMetadata: `{"keyA": "valueA", "keyB": "valueB"}`,
 			cluster:             "",
 			expectedResult:      map[string]string{"keyA": "valueA", "keyB": "valueB"},
 			expectedError:       false,
@@ -48,29 +51,66 @@ func TestPrepareShareMetadata(t *testing.T) {
 		},
 		{
 			// Both metadata and cluster
+			allVolumeParams:     map[string]string{"appendShareMetadata": `{"keyA": "valueA", "keyB": "valueB"}`},
 			appendShareMetadata: "{\"keyA\": \"valueA\", \"keyB\": \"valueB\"}",
 			cluster:             "MyCluster",
 			expectedResult:      map[string]string{"keyA": "valueA", "keyB": "valueB", clusterMetadataKey: "MyCluster"},
 			expectedError:       false,
 		},
 		{
 			// Overwrite cluster
+			allVolumeParams:     map[string]string{"appendShareMetadata": "{\"keyA\": \"valueA\", \"" + clusterMetadataKey + "\": \"SomeValue\"}"},
 			appendShareMetadata: "{\"keyA\": \"valueA\", \"" + clusterMetadataKey + "\": \"SomeValue\"}",
 			cluster:             "MyCluster",
 			expectedResult:      map[string]string{"keyA": "valueA", clusterMetadataKey: "SomeValue"},
 			expectedError:       false,
 		},
 		{
 			// Incorrect metadata
+			allVolumeParams:     map[string]string{"appendShareMetadata": "INVALID"},
 			appendShareMetadata: "INVALID",
 			cluster:             "MyCluster",
 			expectedResult:      nil,
 			expectedError:       true,
 		},
+		{
+			// csi-provisioner PV/PVC metadata
+			allVolumeParams: map[string]string{
+				"csi.storage.k8s.io/pvc/name":      "pvc-name",
+				"csi.storage.k8s.io/pvc/namespace": "pvc-namespace",
+				"csi.storage.k8s.io/pv/name":       "pv-name",
+			},
+			cluster: "",
+			expectedResult: map[string]string{
+				"csi.storage.k8s.io/pvc/name":      "pvc-name",
+				"csi.storage.k8s.io/pvc/namespace": "pvc-namespace",
+				"csi.storage.k8s.io/pv/name":       "pv-name",
+			},
+			appendShareMetadata: "",
+			expectedError:       false,
+		},
+		{
+			// csi-provisioner PV/PVC metadata with conflicting appendShareMetadata
+			allVolumeParams: map[string]string{
+				"csi.storage.k8s.io/pvc/name":      "pvc-name",
+				"csi.storage.k8s.io/pvc/namespace": "pvc-namespace",
+				"csi.storage.k8s.io/pv/name":       "pv-name",
+				"appendShareMetadata":              `{"csi.storage.k8s.io/pvc/name": "SomeValue", "keyX": "valueX"}`,
+			},
+			appendShareMetadata: `{"csi.storage.k8s.io/pvc/name": "SomeValue", "keyX": "valueX"}`,
+			cluster:             "",
+			expectedResult: map[string]string{
+				"csi.storage.k8s.io/pvc/name":      "pvc-name",
+				"csi.storage.k8s.io/pvc/namespace": "pvc-namespace",
+				"csi.storage.k8s.io/pv/name":       "pv-name",
+				"keyX":                             "valueX",
+			},
+			expectedError: false,
+		},
 	}
 
 	for i := range ts {
-		result, err := prepareShareMetadata(ts[i].appendShareMetadata, ts[i].cluster)
+		result, err := prepareShareMetadata(ts[i].appendShareMetadata, ts[i].cluster, ts[i].allVolumeParams)
 
 		if err != nil && !ts[i].expectedError {
 			t.Errorf("test %d: unexpected error: %v", i, err)