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