CSI - Extract volume attributes from PV annotations"

csi - code review fixes

pkg/volume/csi/csi_client.go

@@ -39,6 +39,7 @@ type csiClient interface {
 		targetPath string,
 		accessMode api.PersistentVolumeAccessMode,
 		volumeInfo map[string]string,
+		volumeAttribs map[string]string,
 		fsType string,
 	) error
 	NodeUnpublishVolume(ctx grpctx.Context, volID string, targetPath string) error
@@ -141,6 +142,7 @@ func (c *csiDriverClient) NodePublishVolume(
 	targetPath string,
 	accessMode api.PersistentVolumeAccessMode,
 	volumeInfo map[string]string,
+	volumeAttribs map[string]string,
 	fsType string,
 ) error {
 
@@ -161,6 +163,7 @@ func (c *csiDriverClient) NodePublishVolume(
 		TargetPath:        targetPath,
 		Readonly:          readOnly,
 		PublishVolumeInfo: volumeInfo,
+		VolumeAttributes:  volumeAttribs,
 
 		VolumeCapability: &csipb.VolumeCapability{
 			AccessMode: &csipb.VolumeCapability_AccessMode{

pkg/volume/csi/csi_client_test.go

@@ -90,6 +90,7 @@ func TestClientNodePublishVolume(t *testing.T) {
 			tc.targetPath,
 			api.ReadWriteOnce,
 			map[string]string{"device": "/dev/null"},
+			map[string]string{"attr0": "val0"},
 			tc.fsType,
 		)
 

pkg/volume/csi/csi_mounter.go

@@ -17,6 +17,7 @@ limitations under the License.
 package csi
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"path"
@@ -111,6 +112,15 @@ func (c *csiMountMgr) SetUpAt(dir string, fsGroup *int64) error {
 		c.volumeInfo = attachment.Status.AttachmentMetadata
 	}
 
+	// get volume attributes
+	// TODO: for alpha vol atttributes are passed via PV.Annotations
+	// Beta will fix that
+	attribs, err := getVolAttribsFromSpec(c.spec)
+	if err != nil {
+		glog.Error(log("mounter.SetUpAt failed to extract volume attributes from PV annotations: %v", err))
+		return err
+	}
+
 	//TODO (vladimirvivien) implement better AccessModes mapping between k8s and CSI
 	accessMode := api.ReadWriteOnce
 	if c.spec.PersistentVolume.Spec.AccessModes != nil {
@@ -124,6 +134,7 @@ func (c *csiMountMgr) SetUpAt(dir string, fsGroup *int64) error {
 		dir,
 		accessMode,
 		c.volumeInfo,
+		attribs,
 		"ext4", //TODO needs to be sourced from PV or somewhere else
 	)
 
@@ -187,3 +198,26 @@ func (c *csiMountMgr) TearDownAt(dir string) error {
 
 	return nil
 }
+
+// getVolAttribsFromSpec exracts CSI VolumeAttributes information from PV.Annotations
+// using key csi.kubernetes.io/volume-attributes.  The annotation value is expected
+// to be a JSON-encoded object of form {"key0":"val0",...,"keyN":"valN"}
+func getVolAttribsFromSpec(spec *volume.Spec) (map[string]string, error) {
+	if spec == nil {
+		return nil, errors.New("missing volume spec")
+	}
+	annotations := spec.PersistentVolume.GetAnnotations()
+	if annotations == nil {
+		return nil, nil // no annotations found
+	}
+	jsonAttribs := annotations[csiVolAttribsAnnotationKey]
+	if jsonAttribs == "" {
+		return nil, nil // csi annotation not found
+	}
+	attribs := map[string]string{}
+	if err := json.Unmarshal([]byte(jsonAttribs), &attribs); err != nil {
+		glog.Error(log("error parsing csi PV.Annotation [%s]=%s: %v", csiVolAttribsAnnotationKey, jsonAttribs, err))
+		return nil, err
+	}
+	return attribs, nil
+}

pkg/volume/csi/csi_mounter_test.go

@@ -161,3 +161,50 @@ func TestUnmounterTeardown(t *testing.T) {
 	}
 
 }
+
+func TestGetVolAttribsFromSpec(t *testing.T) {
+	testCases := []struct {
+		name        string
+		annotations map[string]string
+		attribs     map[string]string
+		shouldFail  bool
+	}{
+		{
+			name:        "attribs ok",
+			annotations: map[string]string{"key0": "val0", csiVolAttribsAnnotationKey: `{"k0":"attr0","k1":"attr1","k2":"attr2"}`, "keyN": "valN"},
+			attribs:     map[string]string{"k0": "attr0", "k1": "attr1", "k2": "attr2"},
+		},
+
+		{
+			name:        "missing attribs",
+			annotations: map[string]string{"key0": "val0", "keyN": "valN"},
+		},
+		{
+			name: "missing annotations",
+		},
+		{
+			name:        "bad json",
+			annotations: map[string]string{"key0": "val0", csiVolAttribsAnnotationKey: `{"k0""attr0","k1":"attr1,"k2":"attr2"`, "keyN": "valN"},
+			attribs:     map[string]string{"k0": "attr0", "k1": "attr1", "k2": "attr2"},
+			shouldFail:  true,
+		},
+	}
+	spec := volume.NewSpecFromPersistentVolume(makeTestPV("test-pv", 10, testDriver, testVol), false)
+	for _, tc := range testCases {
+		t.Log("test case:", tc.name)
+		spec.PersistentVolume.Annotations = tc.annotations
+		attribs, err := getVolAttribsFromSpec(spec)
+		if !tc.shouldFail && err != nil {
+			t.Error("test case should not fail, but err != nil", err)
+		}
+		eq := true
+		for k, v := range attribs {
+			if tc.attribs[k] != v {
+				eq = false
+			}
+		}
+		if !eq {
+			t.Errorf("expecting attribs %#v, but got %#v", tc.attribs, attribs)
+		}
+	}
+}

pkg/volume/csi/csi_plugin.go

@@ -34,7 +34,8 @@ import (
 )
 
 const (
-	csiPluginName = "kubernetes.io/csi"
+	csiPluginName              = "kubernetes.io/csi"
+	csiVolAttribsAnnotationKey = "csi.volume.kubernetes.io/volume-attributes"
 
 	// TODO (vladimirvivien) implement a more dynamic way to discover
 	// the unix domain socket path for each installed csi driver.