Merge pull request #77837 from davidz627/fix/pdFSType

Translate StorageClass object instead of parameters. Add GCE PD Storage class translation logic.

staging/src/k8s.io/csi-translation-lib/BUILD

@@ -8,6 +8,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//staging/src/k8s.io/api/storage/v1:go_default_library",
         "//staging/src/k8s.io/csi-translation-lib/plugins:go_default_library",
     ],
 )

staging/src/k8s.io/csi-translation-lib/plugins/BUILD

@@ -13,6 +13,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//staging/src/k8s.io/api/storage/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/cloud-provider/volume:go_default_library",
     ],
@@ -34,6 +35,13 @@ filegroup(
 
 go_test(
     name = "go_default_test",
-    srcs = ["aws_ebs_test.go"],
+    srcs = [
+        "aws_ebs_test.go",
+        "gce_pd_test.go",
+    ],
     embed = [":go_default_library"],
+    deps = [
+        "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//staging/src/k8s.io/api/storage/v1:go_default_library",
+    ],
 )

staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go

@@ -24,6 +24,7 @@ import (
 	"strings"
 
 	"k8s.io/api/core/v1"
+	storage "k8s.io/api/storage/v1"
 )
 
 const (
@@ -44,8 +45,8 @@ func NewAWSElasticBlockStoreCSITranslator() InTreePlugin {
 }
 
 // TranslateInTreeStorageClassParametersToCSI translates InTree EBS storage class parameters to CSI storage class
-func (t *awsElasticBlockStoreCSITranslator) TranslateInTreeStorageClassParametersToCSI(scParameters map[string]string) (map[string]string, error) {
+func (t *awsElasticBlockStoreCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.StorageClass) (*storage.StorageClass, error) {
-	return scParameters, nil
+	return sc, nil
 }
 
 // TranslateInTreePVToCSI takes a PV with AWSElasticBlockStore set from in-tree

staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go

@@ -22,6 +22,7 @@ import (
 	"strings"
 
 	"k8s.io/api/core/v1"
+	storage "k8s.io/api/storage/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	cloudvolume "k8s.io/cloud-provider/volume"
 )
@@ -32,6 +33,9 @@ const (
 	// GCEPDInTreePluginName is the name of the intree plugin for GCE PD
 	GCEPDInTreePluginName = "kubernetes.io/gce-pd"
 
+	// GCEPDTopologyKey is the zonal topology key for GCE PD CSI Driver
+	GCEPDTopologyKey = "topology.gke.io/zone"
+
 	// Volume ID Expected Format
 	// "projects/{projectName}/zones/{zoneName}/disks/{diskName}"
 	volIDZonalFmt = "projects/%s/zones/%s/disks/%s"
@@ -55,9 +59,102 @@ func NewGCEPersistentDiskCSITranslator() InTreePlugin {
 	return &gcePersistentDiskCSITranslator{}
 }
 
+func translateAllowedTopologies(terms []v1.TopologySelectorTerm) ([]v1.TopologySelectorTerm, error) {
+	if terms == nil {
+		return nil, nil
+	}
+
+	newTopologies := []v1.TopologySelectorTerm{}
+	for _, term := range terms {
+		newTerm := v1.TopologySelectorTerm{}
+		for _, exp := range term.MatchLabelExpressions {
+			var newExp v1.TopologySelectorLabelRequirement
+			if exp.Key == v1.LabelZoneFailureDomain {
+				newExp = v1.TopologySelectorLabelRequirement{
+					Key:    GCEPDTopologyKey,
+					Values: exp.Values,
+				}
+			} else if exp.Key == GCEPDTopologyKey {
+				newExp = exp
+			} else {
+				return nil, fmt.Errorf("unknown topology key: %v", exp.Key)
+			}
+			newTerm.MatchLabelExpressions = append(newTerm.MatchLabelExpressions, newExp)
+		}
+		newTopologies = append(newTopologies, newTerm)
+	}
+	return newTopologies, nil
+}
+
+func generateToplogySelectors(key string, values []string) []v1.TopologySelectorTerm {
+	return []v1.TopologySelectorTerm{
+		{
+			MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+				{
+					Key:    key,
+					Values: values,
+				},
+			},
+		},
+	}
+}
+
 // TranslateInTreeStorageClassParametersToCSI translates InTree GCE storage class parameters to CSI storage class
-func (g *gcePersistentDiskCSITranslator) TranslateInTreeStorageClassParametersToCSI(scParameters map[string]string) (map[string]string, error) {
+func (g *gcePersistentDiskCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.StorageClass) (*storage.StorageClass, error) {
-	return scParameters, nil
+	var generatedTopologies []v1.TopologySelectorTerm
+
+	np := map[string]string{}
+	for k, v := range sc.Parameters {
+		switch strings.ToLower(k) {
+		case "fstype":
+			// prefixed fstype parameter is stripped out by external provisioner
+			np["csi.storage.k8s.io/fstype"] = v
+		// Strip out zone and zones parameters and translate them into topologies instead
+		case "zone":
+			generatedTopologies = generateToplogySelectors(GCEPDTopologyKey, []string{v})
+		case "zones":
+			generatedTopologies = generateToplogySelectors(GCEPDTopologyKey, strings.Split(v, ","))
+		default:
+			np[k] = v
+		}
+	}
+
+	if len(generatedTopologies) > 0 && len(sc.AllowedTopologies) > 0 {
+		return nil, fmt.Errorf("cannot simultaneously set allowed topologies and zone/zones parameters")
+	} else if len(generatedTopologies) > 0 {
+		sc.AllowedTopologies = generatedTopologies
+	} else if len(sc.AllowedTopologies) > 0 {
+		newTopologies, err := translateAllowedTopologies(sc.AllowedTopologies)
+		if err != nil {
+			return nil, fmt.Errorf("failed translating allowed topologies: %v", err)
+		}
+		sc.AllowedTopologies = newTopologies
+	}
+
+	sc.Parameters = np
+
+	return sc, nil
+}
+
+// backwardCompatibleAccessModes translates all instances of ReadWriteMany
+// access mode from the in-tree plugin to ReadWriteOnce. This is because in-tree
+// plugin never supported ReadWriteMany but also did not validate or enforce
+// this access mode for pre-provisioned volumes. The GCE PD CSI Driver validates
+// and enforces (fails) ReadWriteMany. Therefore we treat all in-tree
+// ReadWriteMany as ReadWriteOnce volumes to not break legacy volumes.
+func backwardCompatibleAccessModes(ams []v1.PersistentVolumeAccessMode) []v1.PersistentVolumeAccessMode {
+	if ams == nil {
+		return nil
+	}
+	newAM := []v1.PersistentVolumeAccessMode{}
+	for _, am := range ams {
+		if am == v1.ReadWriteMany {
+			newAM = append(newAM, v1.ReadWriteOnce)
+		} else {
+			newAM = append(newAM, am)
+		}
+	}
+	return newAM
 }
 
 // TranslateInTreePVToCSI takes a PV with GCEPersistentDisk set from in-tree
@@ -105,6 +202,7 @@ func (g *gcePersistentDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.Persisten
 
 	pv.Spec.PersistentVolumeSource.GCEPersistentDisk = nil
 	pv.Spec.PersistentVolumeSource.CSI = csiSource
+	pv.Spec.AccessModes = backwardCompatibleAccessModes(pv.Spec.AccessModes)
 
 	return pv, nil
 }

staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go

@@ -0,0 +1,255 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package plugins
+
+import (
+	"reflect"
+	"testing"
+
+	"k8s.io/api/core/v1"
+	storage "k8s.io/api/storage/v1"
+)
+
+func NewStorageClass(params map[string]string, allowedTopologies []v1.TopologySelectorTerm) *storage.StorageClass {
+	return &storage.StorageClass{
+		Parameters:        params,
+		AllowedTopologies: allowedTopologies,
+	}
+}
+
+func TestTranslatePDInTreeStorageClassToCSI(t *testing.T) {
+	g := NewGCEPersistentDiskCSITranslator()
+
+	tcs := []struct {
+		name       string
+		options    *storage.StorageClass
+		expOptions *storage.StorageClass
+		expErr     bool
+	}{
+		{
+			name:       "nothing special",
+			options:    NewStorageClass(map[string]string{"foo": "bar"}, nil),
+			expOptions: NewStorageClass(map[string]string{"foo": "bar"}, nil),
+		},
+		{
+			name:       "fstype",
+			options:    NewStorageClass(map[string]string{"fstype": "myfs"}, nil),
+			expOptions: NewStorageClass(map[string]string{"csi.storage.k8s.io/fstype": "myfs"}, nil),
+		},
+		{
+			name:       "empty params",
+			options:    NewStorageClass(map[string]string{}, nil),
+			expOptions: NewStorageClass(map[string]string{}, nil),
+		},
+		{
+			name:       "zone",
+			options:    NewStorageClass(map[string]string{"zone": "foo"}, nil),
+			expOptions: NewStorageClass(map[string]string{}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo"})),
+		},
+		{
+			name:       "zones",
+			options:    NewStorageClass(map[string]string{"zones": "foo,bar,baz"}, nil),
+			expOptions: NewStorageClass(map[string]string{}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo", "bar", "baz"})),
+		},
+		{
+			name:       "some normal topology",
+			options:    NewStorageClass(map[string]string{}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo"})),
+			expOptions: NewStorageClass(map[string]string{}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo"})),
+		},
+		{
+			name:       "some translated topology",
+			options:    NewStorageClass(map[string]string{}, generateToplogySelectors(v1.LabelZoneFailureDomain, []string{"foo"})),
+			expOptions: NewStorageClass(map[string]string{}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo"})),
+		},
+		{
+			name:    "zone and topology",
+			options: NewStorageClass(map[string]string{"zone": "foo"}, generateToplogySelectors(GCEPDTopologyKey, []string{"foo"})),
+			expErr:  true,
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Logf("Testing %v", tc.name)
+		gotOptions, err := g.TranslateInTreeStorageClassToCSI(tc.options)
+		if err != nil && !tc.expErr {
+			t.Errorf("Did not expect error but got: %v", err)
+		}
+		if err == nil && tc.expErr {
+			t.Errorf("Expected error, but did not get one.")
+		}
+		if !reflect.DeepEqual(gotOptions, tc.expOptions) {
+			t.Errorf("Got parameters: %v, expected :%v", gotOptions, tc.expOptions)
+		}
+	}
+}
+
+func TestTranslateAllowedTopologies(t *testing.T) {
+	testCases := []struct {
+		name            string
+		topology        []v1.TopologySelectorTerm
+		expectedToplogy []v1.TopologySelectorTerm
+		expErr          bool
+	}{
+		{
+			name:     "no translation",
+			topology: generateToplogySelectors(GCEPDTopologyKey, []string{"foo", "bar"}),
+			expectedToplogy: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    GCEPDTopologyKey,
+							Values: []string{"foo", "bar"},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "translate",
+			topology: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    "failure-domain.beta.kubernetes.io/zone",
+							Values: []string{"foo", "bar"},
+						},
+					},
+				},
+			},
+			expectedToplogy: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    GCEPDTopologyKey,
+							Values: []string{"foo", "bar"},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "combo",
+			topology: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    "failure-domain.beta.kubernetes.io/zone",
+							Values: []string{"foo", "bar"},
+						},
+						{
+							Key:    GCEPDTopologyKey,
+							Values: []string{"boo", "baz"},
+						},
+					},
+				},
+			},
+			expectedToplogy: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    GCEPDTopologyKey,
+							Values: []string{"foo", "bar"},
+						},
+						{
+							Key:    GCEPDTopologyKey,
+							Values: []string{"boo", "baz"},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "some other key",
+			topology: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    "test",
+							Values: []string{"foo", "bar"},
+						},
+					},
+				},
+			},
+			expErr: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Logf("Running test: %v", tc.name)
+		gotTop, err := translateAllowedTopologies(tc.topology)
+		if err != nil && !tc.expErr {
+			t.Errorf("Did not expect an error, got: %v", err)
+		}
+		if err == nil && tc.expErr {
+			t.Errorf("Expected an error but did not get one")
+		}
+
+		if !reflect.DeepEqual(gotTop, tc.expectedToplogy) {
+			t.Errorf("Expected topology: %v, but got: %v", tc.expectedToplogy, gotTop)
+		}
+	}
+}
+
+func TestBackwardCompatibleAccessModes(t *testing.T) {
+	testCases := []struct {
+		name           string
+		accessModes    []v1.PersistentVolumeAccessMode
+		expAccessModes []v1.PersistentVolumeAccessMode
+	}{
+		{
+			name: "multiple normals",
+			accessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadOnlyMany,
+				v1.ReadWriteOnce,
+			},
+			expAccessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadOnlyMany,
+				v1.ReadWriteOnce,
+			},
+		},
+		{
+			name: "one normal",
+			accessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadWriteOnce,
+			},
+			expAccessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadWriteOnce,
+			},
+		},
+		{
+			name: "some readwritemany",
+			accessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadWriteOnce,
+				v1.ReadWriteMany,
+			},
+			expAccessModes: []v1.PersistentVolumeAccessMode{
+				v1.ReadWriteOnce,
+				v1.ReadWriteOnce,
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Logf("running test: %v", tc.name)
+
+		got := backwardCompatibleAccessModes(tc.accessModes)
+
+		if !reflect.DeepEqual(tc.expAccessModes, got) {
+			t.Fatalf("Expected access modes: %v, instead got: %v", tc.expAccessModes, got)
+		}
+	}
+}

staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume.go

@@ -16,14 +16,17 @@ limitations under the License.
 
 package plugins
 
-import "k8s.io/api/core/v1"
+import (
+	"k8s.io/api/core/v1"
+	storage "k8s.io/api/storage/v1"
+)
 
 // InTreePlugin handles translations between CSI and in-tree sources in a PV
 type InTreePlugin interface {
 
-	// TranslateInTreeStorageClassParametersToCSI takes in-tree storage class
+	// TranslateInTreeStorageClassToCSI takes in-tree volume options
-	// parameters and translates them to a set of parameters consumable by CSI plugin
+	// and translates them to a volume options consumable by CSI plugin
-	TranslateInTreeStorageClassParametersToCSI(scParameters map[string]string) (map[string]string, error)
+	TranslateInTreeStorageClassToCSI(sc *storage.StorageClass) (*storage.StorageClass, error)
 
 	// TranslateInTreePVToCSI takes a persistent volume and will translate
 	// the in-tree source to a CSI Source. The input persistent volume can be modified

staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go

@@ -18,7 +18,9 @@ package plugins
 
 import (
 	"fmt"
+
 	"k8s.io/api/core/v1"
+	storage "k8s.io/api/storage/v1"
 )
 
 const (
@@ -39,8 +41,8 @@ func NewOpenStackCinderCSITranslator() InTreePlugin {
 }
 
 // TranslateInTreeStorageClassParametersToCSI translates InTree Cinder storage class parameters to CSI storage class
-func (t *osCinderCSITranslator) TranslateInTreeStorageClassParametersToCSI(scParameters map[string]string) (map[string]string, error) {
+func (t *osCinderCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.StorageClass) (*storage.StorageClass, error) {
-	return scParameters, nil
+	return sc, nil
 }
 
 // TranslateInTreePVToCSI takes a PV with Cinder set from in-tree

staging/src/k8s.io/csi-translation-lib/translate.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	"k8s.io/api/core/v1"
+	storage "k8s.io/api/storage/v1"
 	"k8s.io/csi-translation-lib/plugins"
 )
 
@@ -32,12 +33,13 @@ var (
 	}
 )
 
-// TranslateInTreeStorageClassParametersToCSI takes in-tree storage class
+// TranslateInTreeStorageClassToCSI takes in-tree Storage Class
-// parameters and translates them to a set of parameters consumable by CSI plugin
+// and translates it to a set of parameters consumable by CSI plugin
-func TranslateInTreeStorageClassParametersToCSI(inTreePluginName string, scParameters map[string]string) (map[string]string, error) {
+func TranslateInTreeStorageClassToCSI(inTreePluginName string, sc *storage.StorageClass) (*storage.StorageClass, error) {
+	newSC := sc.DeepCopy()
 	for _, curPlugin := range inTreePlugins {
 		if inTreePluginName == curPlugin.GetInTreePluginName() {
-			return curPlugin.TranslateInTreeStorageClassParametersToCSI(scParameters)
+			return curPlugin.TranslateInTreeStorageClassToCSI(newSC)
 		}
 	}
 	return nil, fmt.Errorf("could not find in-tree storage class parameter translation logic for %#v", inTreePluginName)