From 196bbaa964b5e2dc298a4f930a4379eecae3ec5c Mon Sep 17 00:00:00 2001 From: David Zhu Date: Mon, 13 May 2019 13:20:06 -0700 Subject: [PATCH 1/3] Translate fstype storage class parameter to prefixed stripped parameter in the gce pd translation library. Change storage class translation library to operate on StorageClass instead of parameters only. --- staging/src/k8s.io/csi-translation-lib/BUILD | 1 + .../k8s.io/csi-translation-lib/plugins/BUILD | 7 +- .../csi-translation-lib/plugins/aws_ebs.go | 5 +- .../csi-translation-lib/plugins/gce_pd.go | 18 ++++- .../plugins/gce_pd_test.go | 67 +++++++++++++++++++ .../plugins/in_tree_volume.go | 11 +-- .../plugins/openstack_cinder.go | 6 +- .../k8s.io/csi-translation-lib/translate.go | 11 +-- 8 files changed, 110 insertions(+), 16 deletions(-) create mode 100644 staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go diff --git a/staging/src/k8s.io/csi-translation-lib/BUILD b/staging/src/k8s.io/csi-translation-lib/BUILD index 19624f2108e..64333e4fba2 100644 --- a/staging/src/k8s.io/csi-translation-lib/BUILD +++ b/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", ], ) diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/BUILD b/staging/src/k8s.io/csi-translation-lib/plugins/BUILD index 965bbd0175b..5aa90077f73 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/BUILD +++ b/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,10 @@ 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/storage/v1:go_default_library"], ) diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go b/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go index 9cb61768493..5d4cd74f365 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go +++ b/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) { - return scParameters, nil +func (t *awsElasticBlockStoreCSITranslator) TranslateInTreeVolumeOptionsToCSI(sc storage.StorageClass) (storage.StorageClass, error) { + return sc, nil } // TranslateInTreePVToCSI takes a PV with AWSElasticBlockStore set from in-tree diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go index 103a62279de..5f7b82d7d5e 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go +++ b/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" ) @@ -56,8 +57,21 @@ func NewGCEPersistentDiskCSITranslator() InTreePlugin { } // TranslateInTreeStorageClassParametersToCSI translates InTree GCE storage class parameters to CSI storage class -func (g *gcePersistentDiskCSITranslator) TranslateInTreeStorageClassParametersToCSI(scParameters map[string]string) (map[string]string, error) { - return scParameters, nil +func (g *gcePersistentDiskCSITranslator) TranslateInTreeVolumeOptionsToCSI(sc storage.StorageClass) (storage.StorageClass, error) { + np := map[string]string{} + for k, v := range sc.Parameters { + switch strings.ToLower(k) { + case "fstype": + np["csi.storage.k8s.io/fstype"] = v + default: + np[k] = v + } + } + sc.Parameters = np + + // TODO(#77235): Translate AccessModes and zone/zones to AccessibleTopologies + + return sc, nil } // TranslateInTreePVToCSI takes a PV with GCEPersistentDisk set from in-tree diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go new file mode 100644 index 00000000000..e92e1db9636 --- /dev/null +++ b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go @@ -0,0 +1,67 @@ +/* +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" + + storage "k8s.io/api/storage/v1" +) + +func NewStorageClass(params map[string]string) storage.StorageClass { + return storage.StorageClass{ + Parameters: params, + } +} + +func TestTranslatePDInTreeVolumeOptionsToCSI(t *testing.T) { + g := NewGCEPersistentDiskCSITranslator() + + tcs := []struct { + name string + options storage.StorageClass + expOptions storage.StorageClass + }{ + { + name: "nothing special", + options: NewStorageClass(map[string]string{"foo": "bar"}), + expOptions: NewStorageClass(map[string]string{"foo": "bar"}), + }, + { + name: "fstype", + options: NewStorageClass(map[string]string{"fstype": "myfs"}), + expOptions: NewStorageClass(map[string]string{"csi.storage.k8s.io/fstype": "myfs"}), + }, + { + name: "empty params", + options: NewStorageClass(map[string]string{}), + expOptions: NewStorageClass(map[string]string{}), + }, + } + + for _, tc := range tcs { + t.Logf("Testing %v", tc.name) + gotOptions, err := g.TranslateInTreeVolumeOptionsToCSI(tc.options) + if err != nil { + t.Errorf("Did not expect error but got: %v", err) + } + if !reflect.DeepEqual(gotOptions, tc.expOptions) { + t.Errorf("Got parameters: %v, expected :%v", gotOptions, tc.expOptions) + } + } +} diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume.go b/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume.go index 6d5afd9f1f5..29a0ef5f8cd 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume.go +++ b/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 - // parameters and translates them to a set of parameters consumable by CSI plugin - TranslateInTreeStorageClassParametersToCSI(scParameters map[string]string) (map[string]string, error) + // TranslateInTreeVolumeOptionsToCSI takes in-tree volume options + // and translates them to a volume options consumable by CSI plugin + TranslateInTreeVolumeOptionsToCSI(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 diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go b/staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go index abd204a5575..5b22b5e6951 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go +++ b/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) { - return scParameters, nil +func (t *osCinderCSITranslator) TranslateInTreeVolumeOptionsToCSI(sc storage.StorageClass) (storage.StorageClass, error) { + return sc, nil } // TranslateInTreePVToCSI takes a PV with Cinder set from in-tree diff --git a/staging/src/k8s.io/csi-translation-lib/translate.go b/staging/src/k8s.io/csi-translation-lib/translate.go index ac1a4dd3d86..d22c0c2c619 100644 --- a/staging/src/k8s.io/csi-translation-lib/translate.go +++ b/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,15 +33,15 @@ var ( } ) -// TranslateInTreeStorageClassParametersToCSI takes in-tree storage class -// parameters and translates them to a set of parameters consumable by CSI plugin -func TranslateInTreeStorageClassParametersToCSI(inTreePluginName string, scParameters map[string]string) (map[string]string, error) { +// TranslateInTreeVolumeOptionsToCSI takes in-tree volume options +// and translates them to a set of parameters consumable by CSI plugin +func TranslateInTreeVolumeOptionsToCSI(inTreePluginName string, sc storage.StorageClass) (storage.StorageClass, error) { for _, curPlugin := range inTreePlugins { if inTreePluginName == curPlugin.GetInTreePluginName() { - return curPlugin.TranslateInTreeStorageClassParametersToCSI(scParameters) + return curPlugin.TranslateInTreeVolumeOptionsToCSI(sc) } } - return nil, fmt.Errorf("could not find in-tree storage class parameter translation logic for %#v", inTreePluginName) + return storage.StorageClass{}, fmt.Errorf("could not find in-tree storage class parameter translation logic for %#v", inTreePluginName) } // TranslateInTreePVToCSI takes a persistent volume and will translate From 101c6298ce3167e04c0568e80f621456197eabdf Mon Sep 17 00:00:00 2001 From: David Zhu Date: Wed, 1 May 2019 18:10:18 -0700 Subject: [PATCH 2/3] Add tests for backwardCompatibleAccessModes --- .../plugins/gce_pd_test.go | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go index e92e1db9636..e5d970fb220 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go @@ -20,6 +20,7 @@ import ( "reflect" "testing" + "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" ) @@ -65,3 +66,53 @@ func TestTranslatePDInTreeVolumeOptionsToCSI(t *testing.T) { } } } + +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) + } + } +} From 6aea7fcd8349872ef102c4317a392b5662dd6492 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Mon, 13 May 2019 18:39:55 -0700 Subject: [PATCH 3/3] Added topology translation and backward compatible access modes --- .../k8s.io/csi-translation-lib/plugins/BUILD | 5 +- .../csi-translation-lib/plugins/aws_ebs.go | 2 +- .../csi-translation-lib/plugins/gce_pd.go | 90 +++++++++- .../plugins/gce_pd_test.go | 165 ++++++++++++++++-- .../plugins/in_tree_volume.go | 4 +- .../plugins/openstack_cinder.go | 2 +- .../k8s.io/csi-translation-lib/translate.go | 11 +- 7 files changed, 252 insertions(+), 27 deletions(-) diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/BUILD b/staging/src/k8s.io/csi-translation-lib/plugins/BUILD index 5aa90077f73..88c1a91f080 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/BUILD +++ b/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", + ], ) diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go b/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go index 5d4cd74f365..6a9a8f6a6c8 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go +++ b/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 } diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go index 5f7b82d7d5e..95ada8d227d 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go +++ b/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 } diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go index e5d970fb220..cd64ec92275 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go +++ b/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 { - return storage.StorageClass{ - Parameters: params, +func NewStorageClass(params map[string]string, allowedTopologies []v1.TopologySelectorTerm) *storage.StorageClass { + return &storage.StorageClass{ + Parameters: params, + AllowedTopologies: allowedTopologies, } } -func TestTranslatePDInTreeVolumeOptionsToCSI(t *testing.T) { +func TestTranslatePDInTreeStorageClassToCSI(t *testing.T) { g := NewGCEPersistentDiskCSITranslator() tcs := []struct { name string - options storage.StorageClass - expOptions storage.StorageClass + options *storage.StorageClass + expOptions *storage.StorageClass + expErr bool }{ { name: "nothing special", - options: NewStorageClass(map[string]string{"foo": "bar"}), - expOptions: NewStorageClass(map[string]string{"foo": "bar"}), + 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"}), - expOptions: NewStorageClass(map[string]string{"csi.storage.k8s.io/fstype": "myfs"}), + 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{}), - expOptions: NewStorageClass(map[string]string{}), + 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.TranslateInTreeVolumeOptionsToCSI(tc.options) - if err != nil { + 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 diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume.go b/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume.go index 29a0ef5f8cd..d50316743c9 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume.go +++ b/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 diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go b/staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go index 5b22b5e6951..de283453a4b 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go +++ b/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 } diff --git a/staging/src/k8s.io/csi-translation-lib/translate.go b/staging/src/k8s.io/csi-translation-lib/translate.go index d22c0c2c619..a13ac422df2 100644 --- a/staging/src/k8s.io/csi-translation-lib/translate.go +++ b/staging/src/k8s.io/csi-translation-lib/translate.go @@ -33,15 +33,16 @@ var ( } ) -// TranslateInTreeVolumeOptionsToCSI takes in-tree volume options -// and translates them to a set of parameters consumable by CSI plugin -func TranslateInTreeVolumeOptionsToCSI(inTreePluginName string, sc storage.StorageClass) (storage.StorageClass, error) { +// TranslateInTreeStorageClassToCSI takes in-tree Storage Class +// and translates it to a set of parameters consumable by CSI plugin +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