Added topology translation and backward compatible access modes

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

@@ -40,5 +40,8 @@ go_test(
         "gce_pd_test.go",
     ],
     embed = [":go_default_library"],
-    deps = ["//staging/src/k8s.io/api/storage/v1: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

@@ -45,7 +45,7 @@ func NewAWSElasticBlockStoreCSITranslator() InTreePlugin {
 }
 
 // TranslateInTreeStorageClassParametersToCSI translates InTree EBS storage class parameters to CSI storage class
-func (t *awsElasticBlockStoreCSITranslator) TranslateInTreeVolumeOptionsToCSI(sc storage.StorageClass) (storage.StorageClass, error) {
+func (t *awsElasticBlockStoreCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.StorageClass) (*storage.StorageClass, error) {
 	return sc, nil
 }
 

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

@@ -33,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"
@@ -56,24 +59,104 @@ 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) TranslateInTreeVolumeOptionsToCSI(sc storage.StorageClass) (storage.StorageClass, error) {
+func (g *gcePersistentDiskCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.StorageClass) (*storage.StorageClass, error) {
+	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
 
-	// TODO(#77235): Translate AccessModes and zone/zones to AccessibleTopologies
-
 	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
 // and converts the GCEPersistentDisk source to a CSIPersistentVolumeSource
 func (g *gcePersistentDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume) (*v1.PersistentVolume, error) {
@@ -119,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

@@ -24,49 +24,186 @@ import (
 	storage "k8s.io/api/storage/v1"
 )
 
-func NewStorageClass(params map[string]string) storage.StorageClass {
+func NewStorageClass(params map[string]string, allowedTopologies []v1.TopologySelectorTerm) *storage.StorageClass {
-	return storage.StorageClass{
+	return &storage.StorageClass{
-		Parameters: params,
+		Parameters:        params,
+		AllowedTopologies: allowedTopologies,
 	}
 }
 
-func TestTranslatePDInTreeVolumeOptionsToCSI(t *testing.T) {
+func TestTranslatePDInTreeStorageClassToCSI(t *testing.T) {
 	g := NewGCEPersistentDiskCSITranslator()
 
 	tcs := []struct {
 		name       string
-		options    storage.StorageClass
+		options    *storage.StorageClass
-		expOptions storage.StorageClass
+		expOptions *storage.StorageClass
+		expErr     bool
 	}{
 		{
 			name:       "nothing special",
-			options:    NewStorageClass(map[string]string{"foo": "bar"}),
+			options:    NewStorageClass(map[string]string{"foo": "bar"}, nil),
-			expOptions: NewStorageClass(map[string]string{"foo": "bar"}),
+			expOptions: NewStorageClass(map[string]string{"foo": "bar"}, nil),
 		},
 		{
 			name:       "fstype",
-			options:    NewStorageClass(map[string]string{"fstype": "myfs"}),
+			options:    NewStorageClass(map[string]string{"fstype": "myfs"}, nil),
-			expOptions: NewStorageClass(map[string]string{"csi.storage.k8s.io/fstype": "myfs"}),
+			expOptions: NewStorageClass(map[string]string{"csi.storage.k8s.io/fstype": "myfs"}, nil),
 		},
 		{
 			name:       "empty params",
-			options:    NewStorageClass(map[string]string{}),
+			options:    NewStorageClass(map[string]string{}, nil),
-			expOptions: NewStorageClass(map[string]string{}),
+			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.TranslateInTreeVolumeOptionsToCSI(tc.options)
+		gotOptions, err := g.TranslateInTreeStorageClassToCSI(tc.options)
-		if err != nil {
+		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

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

@@ -24,9 +24,9 @@ import (
 // InTreePlugin handles translations between CSI and in-tree sources in a PV
 type InTreePlugin interface {
 
-	// TranslateInTreeVolumeOptionsToCSI takes in-tree volume options
+	// TranslateInTreeStorageClassToCSI takes in-tree volume options
 	// and translates them to a volume options consumable by CSI plugin
-	TranslateInTreeVolumeOptionsToCSI(sc storage.StorageClass) (storage.StorageClass, 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

@@ -41,7 +41,7 @@ func NewOpenStackCinderCSITranslator() InTreePlugin {
 }
 
 // TranslateInTreeStorageClassParametersToCSI translates InTree Cinder storage class parameters to CSI storage class
-func (t *osCinderCSITranslator) TranslateInTreeVolumeOptionsToCSI(sc storage.StorageClass) (storage.StorageClass, error) {
+func (t *osCinderCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.StorageClass) (*storage.StorageClass, error) {
 	return sc, nil
 }
 

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

@@ -33,15 +33,16 @@ var (
 	}
 )
 
-// TranslateInTreeVolumeOptionsToCSI takes in-tree volume options
+// TranslateInTreeStorageClassToCSI takes in-tree Storage Class
-// 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 TranslateInTreeVolumeOptionsToCSI(inTreePluginName string, sc storage.StorageClass) (storage.StorageClass, error) {
+func TranslateInTreeStorageClassToCSI(inTreePluginName string, sc *storage.StorageClass) (*storage.StorageClass, error) {
+	newSC := sc.DeepCopy()
 	for _, curPlugin := range inTreePlugins {
 		if inTreePluginName == curPlugin.GetInTreePluginName() {
-			return curPlugin.TranslateInTreeVolumeOptionsToCSI(sc)
+			return curPlugin.TranslateInTreeStorageClassToCSI(newSC)
 		}
 	}
-	return storage.StorageClass{}, fmt.Errorf("could not find in-tree storage class parameter translation logic for %#v", inTreePluginName)
+	return nil, fmt.Errorf("could not find in-tree storage class parameter translation logic for %#v", inTreePluginName)
 }
 
 // TranslateInTreePVToCSI takes a persistent volume and will translate