From 67fed317a1b6ffc58d21afc32730a5084575a0c9 Mon Sep 17 00:00:00 2001 From: Jiawei Wang Date: Thu, 7 Jan 2021 16:39:30 -0800 Subject: [PATCH] Prepare for Topology migration to GA from CSI migration This also includes a change on CSI migration TranslateCSIToInTree where we remove the CSI topology and add Kubernetes Topology to the NodeAffinity --- .../persistentvolume/label/admission.go | 22 +- .../persistentvolume/label/admission_test.go | 90 ++- .../cloud-provider/volume/helpers/zones.go | 13 +- .../volume/helpers/zones_test.go | 118 ++-- .../csi-translation-lib/plugins/aws_ebs.go | 2 +- .../csi-translation-lib/plugins/gce_pd.go | 45 +- .../plugins/in_tree_volume.go | 263 ++++++++- .../plugins/in_tree_volume_test.go | 538 ++++++++++++++++++ .../plugins/openstack_cinder.go | 2 +- .../csi-translation-lib/translate_test.go | 76 ++- 10 files changed, 1067 insertions(+), 102 deletions(-) diff --git a/plugin/pkg/admission/storage/persistentvolume/label/admission.go b/plugin/pkg/admission/storage/persistentvolume/label/admission.go index 36235e21792..7a7fe94dca7 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/admission.go +++ b/plugin/pkg/admission/storage/persistentvolume/label/admission.go @@ -130,7 +130,7 @@ func (l *persistentVolumeLabel) Admit(ctx context.Context, a admission.Attribute // Set NodeSelectorRequirements based on the labels var values []string - if k == v1.LabelFailureDomainBetaZone { + if k == v1.LabelTopologyZone || k == v1.LabelFailureDomainBetaZone { zones, err := volumehelpers.LabelZonesToSet(v) if err != nil { return admission.NewForbidden(a, fmt.Errorf("failed to convert label string for Zone: %s to a Set", v)) @@ -172,15 +172,31 @@ func (l *persistentVolumeLabel) findVolumeLabels(volume *api.PersistentVolume) ( existingLabels := volume.Labels // All cloud providers set only these two labels. - domain, domainOK := existingLabels[v1.LabelFailureDomainBetaZone] - region, regionOK := existingLabels[v1.LabelFailureDomainBetaRegion] + topologyLabelGA := true + domain, domainOK := existingLabels[v1.LabelTopologyZone] + region, regionOK := existingLabels[v1.LabelTopologyRegion] + // If they dont have GA labels we should check for failuredomain beta labels + // TODO: remove this once all the cloud provider change to GA topology labels + if !domainOK || !regionOK { + topologyLabelGA = false + domain, domainOK = existingLabels[v1.LabelFailureDomainBetaZone] + region, regionOK = existingLabels[v1.LabelFailureDomainBetaRegion] + } + isDynamicallyProvisioned := metav1.HasAnnotation(volume.ObjectMeta, persistentvolume.AnnDynamicallyProvisioned) if isDynamicallyProvisioned && domainOK && regionOK { // PV already has all the labels and we can trust the dynamic provisioning that it provided correct values. + if topologyLabelGA { + return map[string]string{ + v1.LabelTopologyZone: domain, + v1.LabelTopologyRegion: region, + }, nil + } return map[string]string{ v1.LabelFailureDomainBetaZone: domain, v1.LabelFailureDomainBetaRegion: region, }, nil + } // Either missing labels or we don't trust the user provided correct values. diff --git a/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go b/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go index f46a7b7b312..398a53c5f4c 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go +++ b/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go @@ -66,9 +66,9 @@ func Test_PVLAdmission(t *testing.T) { name: "non-cloud PV ignored", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - "a": "1", - "b": "2", - v1.LabelFailureDomainBetaZone: "1__2__3", + "a": "1", + "b": "2", + v1.LabelTopologyZone: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "noncloud", Namespace: "myns"}, @@ -234,7 +234,7 @@ func Test_PVLAdmission(t *testing.T) { err: nil, }, { - name: "existing labels from dynamic provisioning are not changed", + name: "existing Beta labels from dynamic provisioning are not changed", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ v1.LabelFailureDomainBetaZone: "domain1", @@ -301,6 +301,74 @@ func Test_PVLAdmission(t *testing.T) { }, err: nil, }, + { + name: "existing GA labels from dynamic provisioning are not changed", + handler: newPersistentVolumeLabel(), + pvlabeler: mockVolumeLabels(map[string]string{ + v1.LabelTopologyZone: "domain1", + v1.LabelTopologyRegion: "region1", + }), + preAdmissionPV: &api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awsebs", Namespace: "myns", + Labels: map[string]string{ + v1.LabelTopologyZone: "existingDomain", + v1.LabelTopologyRegion: "existingRegion", + }, + Annotations: map[string]string{ + persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs", + }, + }, + Spec: api.PersistentVolumeSpec{ + PersistentVolumeSource: api.PersistentVolumeSource{ + AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{ + VolumeID: "123", + }, + }, + }, + }, + postAdmissionPV: &api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awsebs", + Namespace: "myns", + Labels: map[string]string{ + v1.LabelTopologyZone: "existingDomain", + v1.LabelTopologyRegion: "existingRegion", + }, + Annotations: map[string]string{ + persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs", + }, + }, + Spec: api.PersistentVolumeSpec{ + PersistentVolumeSource: api.PersistentVolumeSource{ + AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{ + VolumeID: "123", + }, + }, + NodeAffinity: &api.VolumeNodeAffinity{ + Required: &api.NodeSelector{ + NodeSelectorTerms: []api.NodeSelectorTerm{ + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: v1.LabelTopologyRegion, + Operator: api.NodeSelectorOpIn, + Values: []string{"existingRegion"}, + }, + { + Key: v1.LabelTopologyZone, + Operator: api.NodeSelectorOpIn, + Values: []string{"existingDomain"}, + }, + }, + }, + }, + }, + }, + }, + }, + err: nil, + }, { name: "existing labels from user are changed", handler: newPersistentVolumeLabel(), @@ -367,9 +435,9 @@ func Test_PVLAdmission(t *testing.T) { name: "GCE PD PV labeled correctly", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - "a": "1", - "b": "2", - v1.LabelFailureDomainBetaZone: "1__2__3", + "a": "1", + "b": "2", + v1.LabelTopologyZone: "1__2__3", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "gcepd", Namespace: "myns"}, @@ -386,9 +454,9 @@ func Test_PVLAdmission(t *testing.T) { Name: "gcepd", Namespace: "myns", Labels: map[string]string{ - "a": "1", - "b": "2", - v1.LabelFailureDomainBetaZone: "1__2__3", + "a": "1", + "b": "2", + v1.LabelTopologyZone: "1__2__3", }, }, Spec: api.PersistentVolumeSpec{ @@ -413,7 +481,7 @@ func Test_PVLAdmission(t *testing.T) { Values: []string{"2"}, }, { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Operator: api.NodeSelectorOpIn, Values: []string{"1", "2", "3"}, }, diff --git a/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go b/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go index a3a7375d1b5..ff3a392846f 100644 --- a/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go +++ b/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go @@ -23,7 +23,7 @@ import ( "strconv" "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" cloudvolume "k8s.io/cloud-provider/volume" "k8s.io/klog/v2" @@ -118,9 +118,12 @@ func SelectZonesForVolume(zoneParameterPresent, zonesParameterPresent bool, zone // pick node's zone for one of the replicas var ok bool - zoneFromNode, ok = node.ObjectMeta.Labels[v1.LabelFailureDomainBetaZone] + zoneFromNode, ok = node.ObjectMeta.Labels[v1.LabelTopologyZone] if !ok { - return nil, fmt.Errorf("%s Label for node missing", v1.LabelFailureDomainBetaZone) + zoneFromNode, ok = node.ObjectMeta.Labels[v1.LabelFailureDomainBetaZone] + if !ok { + return nil, fmt.Errorf("Either %s or %s Label for node missing", v1.LabelTopologyZone, v1.LabelFailureDomainBetaZone) + } } // if single replica volume and node with zone found, return immediately if numReplicas == 1 { @@ -135,7 +138,7 @@ func SelectZonesForVolume(zoneParameterPresent, zonesParameterPresent bool, zone } if (len(allowedTopologies) > 0) && (allowedZones.Len() == 0) { - return nil, fmt.Errorf("no matchLabelExpressions with %s key found in allowedTopologies. Please specify matchLabelExpressions with %s key", v1.LabelFailureDomainBetaZone, v1.LabelFailureDomainBetaZone) + return nil, fmt.Errorf("no matchLabelExpressions with %s key found in allowedTopologies. Please specify matchLabelExpressions with %s key", v1.LabelTopologyZone, v1.LabelTopologyZone) } if allowedZones.Len() > 0 { @@ -185,7 +188,7 @@ func ZonesFromAllowedTopologies(allowedTopologies []v1.TopologySelectorTerm) (se zones := make(sets.String) for _, term := range allowedTopologies { for _, exp := range term.MatchLabelExpressions { - if exp.Key == v1.LabelFailureDomainBetaZone { + if exp.Key == v1.LabelTopologyZone || exp.Key == v1.LabelFailureDomainBetaZone { for _, value := range exp.Values { zones.Insert(value) } diff --git a/staging/src/k8s.io/cloud-provider/volume/helpers/zones_test.go b/staging/src/k8s.io/cloud-provider/volume/helpers/zones_test.go index e07a5af458f..e31a7224b2b 100644 --- a/staging/src/k8s.io/cloud-provider/volume/helpers/zones_test.go +++ b/staging/src/k8s.io/cloud-provider/volume/helpers/zones_test.go @@ -20,7 +20,7 @@ import ( "hash/fnv" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" ) @@ -416,8 +416,11 @@ func TestChooseZonesForVolume(t *testing.T) { func TestSelectZoneForVolume(t *testing.T) { - nodeWithZoneLabels := &v1.Node{} - nodeWithZoneLabels.Labels = map[string]string{v1.LabelFailureDomainBetaZone: "zoneX"} + nodeWithGATopologyLabels := &v1.Node{} + nodeWithGATopologyLabels.Labels = map[string]string{v1.LabelTopologyZone: "zoneX"} + + nodeWithBetaTopologyLabels := &v1.Node{} + nodeWithBetaTopologyLabels.Labels = map[string]string{v1.LabelFailureDomainBetaZone: "zoneY"} nodeWithNoLabels := &v1.Node{} @@ -468,7 +471,7 @@ func TestSelectZoneForVolume(t *testing.T) { // [3] AllowedTopologies irrelevant { Name: "Node_with_Zone_labels_and_Zone_parameter_present", - Node: nodeWithZoneLabels, + Node: nodeWithGATopologyLabels, ZonePresent: true, Zone: "zoneX", Reject: true, @@ -480,7 +483,7 @@ func TestSelectZoneForVolume(t *testing.T) { // [3] AllowedTopologies irrelevant { Name: "Node_with_Zone_labels_and_Zones_parameter_present", - Node: nodeWithZoneLabels, + Node: nodeWithGATopologyLabels, ZonesPresent: true, Zones: "zoneX,zoneY", Reject: true, @@ -499,7 +502,7 @@ func TestSelectZoneForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX"}, }, }, @@ -521,7 +524,7 @@ func TestSelectZoneForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX"}, }, }, @@ -530,7 +533,7 @@ func TestSelectZoneForVolume(t *testing.T) { Reject: true, }, - // Key specified in AllowedTopologies is not LabelFailureDomainBetaZone [Fail] + // Key specified in AllowedTopologies is not LabelTopologyZone [Fail] // [1] nil Node // [2] no Zone/Zones parameter // [3] AllowedTopologies with invalid key specified @@ -545,7 +548,7 @@ func TestSelectZoneForVolume(t *testing.T) { Values: []string{"zoneX"}, }, { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneY"}, }, }, @@ -554,7 +557,7 @@ func TestSelectZoneForVolume(t *testing.T) { Reject: true, }, - // AllowedTopologies without keys specifying LabelFailureDomainBetaZone [Fail] + // AllowedTopologies without keys specifying LabelTopologyZone [Fail] // [1] nil Node // [2] no Zone/Zones parameter // [3] Invalid AllowedTopologies @@ -610,30 +613,42 @@ func TestSelectZoneForVolume(t *testing.T) { ExpectedZones: "zoneX,zoneY", }, - // Select zone from node label [Pass] + // Select zone from node ga label [Pass] // [1] Node with zone labels // [2] no zone/zones parameters // [3] no AllowedTopology { - Name: "Node_with_Zone_labels_and_VolumeScheduling_enabled", - Node: nodeWithZoneLabels, + Name: "Node_with_GA_Zone_labels_and_VolumeScheduling_enabled", + Node: nodeWithGATopologyLabels, Reject: false, ExpectSpecificZone: true, ExpectedZone: "zoneX", }, + // Select zone from node beta label [Pass] + // [1] Node with zone labels + // [2] no zone/zones parameters + // [3] no AllowedTopology + { + Name: "Node_with_Beta_Zone_labels_and_VolumeScheduling_enabled", + Node: nodeWithBetaTopologyLabels, + Reject: false, + ExpectSpecificZone: true, + ExpectedZone: "zoneY", + }, + // Select zone from node label [Pass] // [1] Node with zone labels // [2] no Zone/Zones parameters // [3] AllowedTopology with single term with multiple values specified (ignored) { Name: "Node_with_Zone_labels_and_Multiple_Allowed_Topology_values_and_VolumeScheduling_enabled", - Node: nodeWithZoneLabels, + Node: nodeWithGATopologyLabels, AllowedTopologies: []v1.TopologySelectorTerm{ { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneZ", "zoneY"}, }, }, @@ -651,6 +666,27 @@ func TestSelectZoneForVolume(t *testing.T) { { Name: "Nil_Node_with_Multiple_Allowed_Topology_values_and_VolumeScheduling_enabled", Node: nil, + AllowedTopologies: []v1.TopologySelectorTerm{ + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelTopologyZone, + Values: []string{"zoneX", "zoneY"}, + }, + }, + }, + }, + Reject: false, + ExpectedZones: "zoneX,zoneY", + }, + + // Select Zone from AllowedTopologies using Beta labels [Pass] + // [1] nil Node + // [2] no Zone/Zones parametes specified + // [3] AllowedTopologies with single term with multiple values specified + { + Name: "Nil_Node_with_Multiple_Allowed_Topology_values_Beta_label_and_VolumeScheduling_enabled", + Node: nil, AllowedTopologies: []v1.TopologySelectorTerm{ { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ @@ -676,7 +712,7 @@ func TestSelectZoneForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX"}, }, }, @@ -684,7 +720,7 @@ func TestSelectZoneForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneY"}, }, }, @@ -706,7 +742,7 @@ func TestSelectZoneForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX"}, }, }, @@ -771,7 +807,7 @@ func TestSelectZoneForVolume(t *testing.T) { func TestSelectZonesForVolume(t *testing.T) { nodeWithZoneLabels := &v1.Node{} - nodeWithZoneLabels.Labels = map[string]string{v1.LabelFailureDomainBetaZone: "zoneX"} + nodeWithZoneLabels.Labels = map[string]string{v1.LabelTopologyZone: "zoneX"} nodeWithNoLabels := &v1.Node{} @@ -865,7 +901,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX"}, }, }, @@ -889,7 +925,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX"}, }, }, @@ -898,7 +934,7 @@ func TestSelectZonesForVolume(t *testing.T) { Reject: true, }, - // Key specified in AllowedTopologies is not LabelFailureDomainBetaZone [Fail] + // Key specified in AllowedTopologies is not LabelTopologyZone [Fail] // [1] nil Node // [2] no Zone/Zones parameter // [3] AllowedTopologies with invalid key specified @@ -915,7 +951,7 @@ func TestSelectZonesForVolume(t *testing.T) { Values: []string{"zoneX"}, }, { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneY"}, }, }, @@ -924,7 +960,7 @@ func TestSelectZonesForVolume(t *testing.T) { Reject: true, }, - // AllowedTopologies without keys specifying LabelFailureDomainBetaZone [Fail] + // AllowedTopologies without keys specifying LabelTopologyZone [Fail] // [1] nil Node // [2] no Zone/Zones parameter // [3] Invalid AllowedTopologies @@ -999,7 +1035,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX"}, }, }, @@ -1123,7 +1159,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneV", "zoneW", "zoneX", "zoneY", "zoneZ"}, }, }, @@ -1149,7 +1185,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX", "zoneY"}, }, }, @@ -1176,7 +1212,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX", "zoneY", "zoneZ"}, }, }, @@ -1200,7 +1236,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX", "zoneY"}, }, }, @@ -1228,7 +1264,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneV"}, }, }, @@ -1236,7 +1272,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneW"}, }, }, @@ -1244,7 +1280,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX"}, }, }, @@ -1252,7 +1288,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneY"}, }, }, @@ -1260,7 +1296,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneZ"}, }, }, @@ -1286,7 +1322,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX"}, }, }, @@ -1294,7 +1330,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneY"}, }, }, @@ -1321,7 +1357,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX"}, }, }, @@ -1329,7 +1365,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneY"}, }, }, @@ -1337,7 +1373,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneZ"}, }, }, @@ -1361,7 +1397,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneX"}, }, }, @@ -1369,7 +1405,7 @@ func TestSelectZonesForVolume(t *testing.T) { { MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Values: []string{"zoneY"}, }, }, 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 5db1e921f8d..843da7d867d 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 @@ -142,7 +142,7 @@ func (t *awsElasticBlockStoreCSITranslator) TranslateInTreePVToCSI(pv *v1.Persis }, } - if err := translateTopology(pv, AWSEBSTopologyKey); err != nil { + if err := translateTopologyFromInTreeToCSI(pv, AWSEBSTopologyKey); err != nil { return nil, fmt.Errorf("failed to translate topology: %v", err) } 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 1a1541019ce..ca02aac507c 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 @@ -215,7 +215,12 @@ func (g *gcePersistentDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.Persisten return nil, fmt.Errorf("pv is nil or GCE Persistent Disk source not defined on pv") } + // depend on which version it migrates from, the label could be failuredomain beta or topology GA version zonesLabel := pv.Labels[v1.LabelFailureDomainBetaZone] + if zonesLabel == "" { + zonesLabel = pv.Labels[v1.LabelTopologyZone] + } + zones := strings.Split(zonesLabel, labelMultiZoneDelimiter) if len(zones) == 1 && len(zones[0]) != 0 { // Zonal @@ -249,7 +254,7 @@ func (g *gcePersistentDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.Persisten }, } - if err := translateTopology(pv, GCEPDTopologyKey); err != nil { + if err := translateTopologyFromInTreeToCSI(pv, GCEPDTopologyKey); err != nil { return nil, fmt.Errorf("failed to translate topology: %v", err) } @@ -286,7 +291,10 @@ func (g *gcePersistentDiskCSITranslator) TranslateCSIPVToInTree(pv *v1.Persisten gceSource.Partition = int32(partInt) } - // TODO: Take the zone/regional information and stick it into the label. + // translate CSI topology to In-tree topology for rollback compatibility + if err := translateTopologyFromCSIToInTree(pv, GCEPDTopologyKey, gceRegionParser); err != nil { + return nil, fmt.Errorf("failed to translate topology. PV:%+v. Error:%v", *pv, err) + } pv.Spec.CSI = nil pv.Spec.GCEPersistentDisk = gceSource @@ -369,6 +377,39 @@ func pdNameFromVolumeID(id string) (string, error) { return splitID[volIDDiskNameValue], nil } +func gceRegionParser(pv *v1.PersistentVolume) (string, error) { + _, zoneLabel, regionLabel := getTopologyLabel(pv) + + regionVal := getTopologyValues(pv, regionLabel) + // if we found the val in topology directly + if len(regionVal) != 0 { + if len(regionVal) > 1 { + return "", fmt.Errorf("multiple regions found: %v", regionVal) + } + return regionVal[0], nil + } + + // No region info found in NodeAffinity, generate it from Zones NodeAffinity + zonesVal := getTopologyValues(pv, zoneLabel) + if len(zonesVal) == 0 { + // No kubernetes NodeAffinity found, generate it from CSI Topology + zonesVal = getTopologyValues(pv, GCEPDTopologyKey) + } + if len(zonesVal) != 0 { + return getRegionFromZones(zonesVal) + } + + // Not found anything in NodeAffinity, check region labels + regionValFromLabel, regionOK := pv.Labels[regionLabel] + if regionOK { + return regionValFromLabel, nil + } + // Not found anything in region labels, check zone labels + zonesVal = strings.Split(pv.Labels[zoneLabel], labelMultiZoneDelimiter) + + return getRegionFromZones(zonesVal) +} + // TODO: Replace this with the imported one from GCE PD CSI Driver when // the driver removes all k8s/k8s dependencies func getRegionFromZones(zones []string) (string, error) { 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 9d22b80aad7..c2a9e8956de 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 @@ -19,6 +19,7 @@ package plugins import ( "errors" "fmt" + "sort" "strings" v1 "k8s.io/api/core/v1" @@ -88,23 +89,30 @@ func replaceTopology(pv *v1.PersistentVolume, oldKey, newKey string) error { return nil } -// getTopologyZones returns all topology zones with the given key found in the PV. -func getTopologyZones(pv *v1.PersistentVolume, key string) []string { +// getTopologyValues returns all unique topology values with the given key found in the PV. +func getTopologyValues(pv *v1.PersistentVolume, key string) []string { if pv.Spec.NodeAffinity == nil || pv.Spec.NodeAffinity.Required == nil || len(pv.Spec.NodeAffinity.Required.NodeSelectorTerms) < 1 { return nil } - var values []string + values := make(map[string]bool) for i := range pv.Spec.NodeAffinity.Required.NodeSelectorTerms { for _, r := range pv.Spec.NodeAffinity.Required.NodeSelectorTerms[i].MatchExpressions { if r.Key == key { - values = append(values, r.Values...) + for _, v := range r.Values { + values[v] = true + } } } } - return values + var re []string + for k := range values { + re = append(re, k) + } + sort.Strings(re) + return re } // addTopology appends the topology to the given PV. @@ -124,9 +132,17 @@ func addTopology(pv *v1.PersistentVolume, topologyKey string, zones []string) er } // Make sure the necessary fields exist - pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity) - pv.Spec.NodeAffinity.Required = new(v1.NodeSelector) - pv.Spec.NodeAffinity.Required.NodeSelectorTerms = make([]v1.NodeSelectorTerm, 1) + if pv.Spec.NodeAffinity == nil { + pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity) + } + + if pv.Spec.NodeAffinity.Required == nil { + pv.Spec.NodeAffinity.Required = new(v1.NodeSelector) + } + + if len(pv.Spec.NodeAffinity.Required.NodeSelectorTerms) == 0 { + pv.Spec.NodeAffinity.Required.NodeSelectorTerms = make([]v1.NodeSelectorTerm, 1) + } topology := v1.NodeSelectorRequirement{ Key: topologyKey, @@ -134,28 +150,86 @@ func addTopology(pv *v1.PersistentVolume, topologyKey string, zones []string) er Values: zones, } - pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions = append( - pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions, - topology, - ) + // add the CSI topology to each term + for i := range pv.Spec.NodeAffinity.Required.NodeSelectorTerms { + pv.Spec.NodeAffinity.Required.NodeSelectorTerms[i].MatchExpressions = append( + pv.Spec.NodeAffinity.Required.NodeSelectorTerms[i].MatchExpressions, + topology, + ) + } return nil } -// translateTopology converts existing zone labels or in-tree topology to CSI topology. +// removeTopology removes the topology from the given PV. Return false +// if the topology key is not found +func removeTopology(pv *v1.PersistentVolume, topologyKey string) bool { + // Make sure the necessary fields exist + if pv == nil || pv.Spec.NodeAffinity == nil || pv.Spec.NodeAffinity.Required == nil || + pv.Spec.NodeAffinity.Required.NodeSelectorTerms == nil || len(pv.Spec.NodeAffinity.Required.NodeSelectorTerms) == 0 { + return false + } + + found := true + succeed := false + for found == true { + found = false + var termIndexRemoved []int + for termIndex, nodeSelectorTerms := range pv.Spec.NodeAffinity.Required.NodeSelectorTerms { + nsrequirements := nodeSelectorTerms.MatchExpressions + + index := -1 + for i, nodeSelectorRequirement := range nsrequirements { + if nodeSelectorRequirement.Key == topologyKey { + index = i + break + } + } + // We found the key that need to be removed + if index != -1 { + nsrequirements[len(nsrequirements)-1], nsrequirements[index] = nsrequirements[index], nsrequirements[len(nsrequirements)-1] + pv.Spec.NodeAffinity.Required.NodeSelectorTerms[termIndex].MatchExpressions = nsrequirements[:len(nsrequirements)-1] + if len(nsrequirements)-1 == 0 { + // No other expression left, the whole term should be removed + termIndexRemoved = append(termIndexRemoved, termIndex) + } + succeed = true + found = true + } + } + + if len(termIndexRemoved) > 0 { + for i, index := range termIndexRemoved { + index = index - i + nodeSelectorTerms := pv.Spec.NodeAffinity.Required.NodeSelectorTerms + nodeSelectorTerms[len(nodeSelectorTerms)-1], nodeSelectorTerms[index] = nodeSelectorTerms[index], nodeSelectorTerms[len(nodeSelectorTerms)-1] + pv.Spec.NodeAffinity.Required.NodeSelectorTerms = nodeSelectorTerms[:len(nodeSelectorTerms)-1] + } + } + } + + return succeed +} + +// translateTopologyFromInTreeToCSI converts existing zone labels or in-tree topology to CSI topology. // In-tree topology has precedence over zone labels. -func translateTopology(pv *v1.PersistentVolume, topologyKey string) error { +func translateTopologyFromInTreeToCSI(pv *v1.PersistentVolume, topologyKey string) error { // If topology is already set, assume the content is accurate - if len(getTopologyZones(pv, topologyKey)) > 0 { + if len(getTopologyValues(pv, topologyKey)) > 0 { return nil } - zones := getTopologyZones(pv, v1.LabelFailureDomainBetaZone) + _, zoneLabel, _ := getTopologyLabel(pv) + + // migrate topology node affinity + zones := getTopologyValues(pv, zoneLabel) if len(zones) > 0 { - return replaceTopology(pv, v1.LabelFailureDomainBetaZone, topologyKey) + return replaceTopology(pv, zoneLabel, topologyKey) } - if label, ok := pv.Labels[v1.LabelFailureDomainBetaZone]; ok { + // if nothing is in the NodeAffinity, try to fetch the topology from PV labels + label, ok := pv.Labels[zoneLabel] + if ok { zones = strings.Split(label, labelMultiZoneDelimiter) if len(zones) > 0 { return addTopology(pv, topologyKey, zones) @@ -165,6 +239,157 @@ func translateTopology(pv *v1.PersistentVolume, topologyKey string) error { return nil } +// getTopologyLabel checks if the kubernetes topology used in this PV is GA +// and return the zone/region label used. +// It first check the NodeAffinity to find topology. If nothing is found, +// It checks the PV labels. If it is empty in both places, it will return +// GA label by default +func getTopologyLabel(pv *v1.PersistentVolume) (bool, string, string) { + + // Check the NodeAffinity first for the topology version + zoneGA := TopologyKeyExist(v1.LabelTopologyZone, pv.Spec.NodeAffinity) + regionGA := TopologyKeyExist(v1.LabelTopologyRegion, pv.Spec.NodeAffinity) + if zoneGA || regionGA { + // GA NodeAffinity exists + return true, v1.LabelTopologyZone, v1.LabelTopologyRegion + } + + // If no GA topology in NodeAffinity, check the beta one + zoneBeta := TopologyKeyExist(v1.LabelFailureDomainBetaZone, pv.Spec.NodeAffinity) + regionBeta := TopologyKeyExist(v1.LabelFailureDomainBetaRegion, pv.Spec.NodeAffinity) + if zoneBeta || regionBeta { + // Beta NodeAffinity exists, GA NodeAffinity not exist + return false, v1.LabelFailureDomainBetaZone, v1.LabelFailureDomainBetaRegion + } + + // If nothing is in the NodeAfinity, we should check pv labels + _, zoneGA = pv.Labels[v1.LabelTopologyZone] + _, regionGA = pv.Labels[v1.LabelTopologyRegion] + if zoneGA || regionGA { + // NodeAffinity not exist, GA label exists + return true, v1.LabelTopologyZone, v1.LabelTopologyRegion + } + + // If GA label not exists, check beta version + _, zoneBeta = pv.Labels[v1.LabelFailureDomainBetaZone] + _, regionBeta = pv.Labels[v1.LabelFailureDomainBetaRegion] + if zoneBeta || regionBeta { + // Beta label exists, NodeAffinity not exist, GA label not exists + return false, v1.LabelFailureDomainBetaZone, v1.LabelFailureDomainBetaRegion + } + + // No labels or NodeAffinity exist, default to GA version + return true, v1.LabelTopologyZone, v1.LabelTopologyRegion +} + +// TopologyKeyExist checks if a certain key exists in a VolumeNodeAffinity +func TopologyKeyExist(key string, vna *v1.VolumeNodeAffinity) bool { + if vna == nil || vna.Required == nil || vna.Required.NodeSelectorTerms == nil || len(vna.Required.NodeSelectorTerms) == 0 { + return false + } + + for _, nodeSelectorTerms := range vna.Required.NodeSelectorTerms { + nsrequirements := nodeSelectorTerms.MatchExpressions + for _, nodeSelectorRequirement := range nsrequirements { + if nodeSelectorRequirement.Key == key { + return true + } + } + } + return false +} + +type regionParser func(pv *v1.PersistentVolume) (string, error) + +// translateTopologyFromCSIToInTree translate a CSI PV topology to +// Kubernetes topology and add labels to it. Note that this function +// will only work for plugin with single topologyKey. If a plugin has +// more than one topologyKey, it will need to be processed separately +// by the plugin. +// regionParser is a function to generate region val based on PV +// if the function is not set, we will not set region topology. +// 1. Remove any existing CSI topologyKey from NodeAffinity +// 2. Add Kubernetes Topology in the NodeAffinity if it does not exist +// 2.1 Try to use CSI topology values to recover the Kubernetes topology first +// 2.2 If CSI topology values does not exist, try to use PV label +// 3. Add Kubernetes Topology labels(zone and region) +func translateTopologyFromCSIToInTree(pv *v1.PersistentVolume, topologyKey string, regionParser regionParser) error { + + csiTopologyZoneValues := getTopologyValues(pv, topologyKey) + + // 1. Frist remove the CSI topology Key + removeTopology(pv, topologyKey) + + _, zoneLabel, regionLabel := getTopologyLabel(pv) + zoneLabelVal, zoneOK := pv.Labels[zoneLabel] + regionLabelVal, regionOK := pv.Labels[regionLabel] + + // 2. Add Kubernetes Topology in the NodeAffinity if it does not exist + // Check if Kubernetes Zone Topology already exist + topologyZoneValue := getTopologyValues(pv, zoneLabel) + if len(topologyZoneValue) == 0 { + // No Kubernetes Topology exist in the current PV, we need to add it + // 2.1 Let's try to use CSI topology to recover the zone topology first + if len(csiTopologyZoneValues) > 0 { + err := addTopology(pv, zoneLabel, csiTopologyZoneValues) + if err != nil { + return fmt.Errorf("Failed to add Kubernetes topology zone to PV NodeAffinity. %v", err) + } + } else if zoneOK { + // 2.2 If no CSI topology values exist, try to search PV labels for zone labels + zones := strings.Split(zoneLabelVal, labelMultiZoneDelimiter) + if len(zones) > 0 { + err := addTopology(pv, zoneLabel, zones) + if err != nil { + return fmt.Errorf("Failed to add Kubernetes topology zone to PV NodeAffinity. %v", err) + } + } + } + } + + // Check if Kubernetes Region Topology already exist + topologyRegionValue := getTopologyValues(pv, regionLabel) + if len(topologyRegionValue) == 0 { + // 2.1 Let's try to use CSI topology to recover the region topology first + if len(csiTopologyZoneValues) > 0 && regionParser != nil { + regionVal, err := regionParser(pv) + if err != nil { + return fmt.Errorf("Failed to parse zones value(%v) to region label %v", csiTopologyZoneValues, err) + } + err = addTopology(pv, regionLabel, []string{regionVal}) + if err != nil { + return fmt.Errorf("Failed to add Kubernetes topology region to PV NodeAffinity. %v", err) + } + } else if regionOK { + // 2.2 If no CSI topology values exist, try to search PV labels for region labels + err := addTopology(pv, regionLabel, []string{regionLabelVal}) + if err != nil { + return fmt.Errorf("Failed to add Kubernetes topology region to PV NodeAffinity. %v", err) + } + } + } + + // 3. Add Kubernetes Topology labels, if it already exists, we trust it + if len(csiTopologyZoneValues) > 0 { + if pv.Labels == nil { + pv.Labels = make(map[string]string) + } + if !zoneOK { + csiTopologyZoneValStr := strings.Join(csiTopologyZoneValues, labelMultiZoneDelimiter) + pv.Labels[zoneLabel] = csiTopologyZoneValStr + } + if !regionOK && regionParser != nil { + regionVal, err := regionParser(pv) + if err != nil { + return fmt.Errorf("Failed to parse zones value(%v) to region label %v", csiTopologyZoneValues, err) + } + pv.Labels[regionLabel] = regionVal + } + } + + return nil +} + // translateAllowedTopologies translates allowed topologies within storage class // from legacy failure domain to given CSI topology key func translateAllowedTopologies(terms []v1.TopologySelectorTerm, key string) ([]v1.TopologySelectorTerm, error) { @@ -177,7 +402,7 @@ func translateAllowedTopologies(terms []v1.TopologySelectorTerm, key string) ([] newTerm := v1.TopologySelectorTerm{} for _, exp := range term.MatchLabelExpressions { var newExp v1.TopologySelectorLabelRequirement - if exp.Key == v1.LabelFailureDomainBetaZone { + if exp.Key == v1.LabelFailureDomainBetaZone || exp.Key == v1.LabelTopologyZone { newExp = v1.TopologySelectorLabelRequirement{ Key: key, Values: exp.Values, diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume_test.go index db5685c1a5d..5c2efb1c4c9 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume_test.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume_test.go @@ -21,8 +21,403 @@ import ( "testing" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +var ( + useast1aGALabels = map[string]string{ + v1.LabelTopologyZone: "us-east1-a", + v1.LabelTopologyRegion: "us-east1", + } + useast1aGANodeSelectorTerm = []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelTopologyZone, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + { + Key: v1.LabelTopologyRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1"}, + }, + }, + }, + } + + uswest2bBetaLabels = map[string]string{ + v1.LabelFailureDomainBetaZone: "us-west2-b", + v1.LabelFailureDomainBetaRegion: "us-west2", + } + + uswest2bBetaNodeSelectorTerm = []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelFailureDomainBetaZone, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-west2-b"}, + }, + { + Key: v1.LabelFailureDomainBetaRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-west2"}, + }, + }, + }, + } +) + +func TestTranslateTopologyFromCSIToInTree(t *testing.T) { + testCases := []struct { + name string + key string + expErr bool + regionParser regionParser + pv *v1.PersistentVolume + expectedNodeSelectorTerms []v1.NodeSelectorTerm + expectedLabels map[string]string + }{ + { + name: "Remove CSI Topology Key and do not change existing GA Kubernetes topology", + key: GCEPDTopologyKey, + expErr: false, + regionParser: gceRegionParser, + pv: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gcepd", Namespace: "myns", + Labels: useast1aGALabels, + }, + Spec: v1.PersistentVolumeSpec{ + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelTopologyZone, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + { + Key: v1.LabelTopologyRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1"}, + }, + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"whatever"}, + }, + }, + }, + }, + }, + }, + }, + }, + expectedNodeSelectorTerms: useast1aGANodeSelectorTerm, + expectedLabels: useast1aGALabels, + }, + { + name: "Remove CSI Topology Key and do not change existing Beta Kubernetes topology", + key: GCEPDTopologyKey, + expErr: false, + regionParser: gceRegionParser, + pv: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gcepd", Namespace: "myns", + Labels: uswest2bBetaLabels, + }, + Spec: v1.PersistentVolumeSpec{ + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelFailureDomainBetaZone, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-west2-b"}, + }, + { + Key: v1.LabelFailureDomainBetaRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-west2"}, + }, + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"whatever"}, + }, + }, + }, + }, + }, + }, + }, + }, + expectedNodeSelectorTerms: uswest2bBetaNodeSelectorTerm, + expectedLabels: uswest2bBetaLabels, + }, + { + name: "Remove CSI Topology Key and add Kubernetes topology from NodeAffinity, ignore labels", + key: GCEPDTopologyKey, + expErr: false, + regionParser: gceRegionParser, + pv: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gcepd", Namespace: "myns", + Labels: map[string]string{ + v1.LabelTopologyZone: "existingZone", + v1.LabelTopologyRegion: "existingRegion", + }, + }, + Spec: v1.PersistentVolumeSpec{ + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + }, + }, + }, + }, + }, + }, + }, + expectedNodeSelectorTerms: useast1aGANodeSelectorTerm, + expectedLabels: map[string]string{ + v1.LabelTopologyRegion: "existingRegion", + v1.LabelTopologyZone: "existingZone", + }, + }, + { + name: "Add GA Kubernetes topology from labels", + key: GCEPDTopologyKey, + expErr: false, + regionParser: gceRegionParser, + pv: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gcepd", Namespace: "myns", + Labels: map[string]string{ + v1.LabelTopologyZone: "existingZone", + v1.LabelTopologyRegion: "existingRegion", + }, + }, + }, + expectedNodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelTopologyZone, + Operator: v1.NodeSelectorOpIn, + Values: []string{"existingZone"}, + }, + { + Key: v1.LabelTopologyRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"existingRegion"}, + }, + }, + }, + }, + expectedLabels: map[string]string{ + v1.LabelTopologyZone: "existingZone", + v1.LabelTopologyRegion: "existingRegion", + }, + }, + { + name: "Generate GA labels and kubernetes topology only from CSI topology", + key: GCEPDTopologyKey, + expErr: false, + regionParser: gceRegionParser, + pv: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gcepd", Namespace: "myns", + }, + Spec: v1.PersistentVolumeSpec{ + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + }, + }, + }, + }, + }, + }, + }, + expectedNodeSelectorTerms: useast1aGANodeSelectorTerm, + expectedLabels: useast1aGALabels, + }, + { + name: "Generate Beta labels and kubernetes topology from CSI topology with partial Beta NodeAffinity", + key: GCEPDTopologyKey, + expErr: false, + regionParser: gceRegionParser, + pv: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gcepd", Namespace: "myns", + }, + Spec: v1.PersistentVolumeSpec{ + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-west2-b"}, + }, + { + Key: v1.LabelFailureDomainBetaZone, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-west2-b"}, + }, + }, + }, + }, + }, + }, + }, + }, + expectedNodeSelectorTerms: uswest2bBetaNodeSelectorTerm, + expectedLabels: uswest2bBetaLabels, + }, + { + name: "regionParser is missing and only zone labels get generated", + key: GCEPDTopologyKey, + expErr: false, + regionParser: nil, + pv: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gcepd", Namespace: "myns", + }, + Spec: v1.PersistentVolumeSpec{ + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + }, + }, + }, + }, + }, + }, + }, + expectedNodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelTopologyZone, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + }, + }, + }, + expectedLabels: map[string]string{ + v1.LabelTopologyZone: "us-east1-a", + }, + }, + { + name: "Remove multi-term CSI Topology Key and add GA Kubernetes topology", + key: GCEPDTopologyKey, + expErr: false, + regionParser: gceRegionParser, + pv: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gcepd", Namespace: "myns", + }, + Spec: v1.PersistentVolumeSpec{ + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-c"}, + }, + }, + }, + }, + }, + }, + }, + }, + expectedNodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelTopologyZone, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a", "us-east1-c"}, + }, + { + Key: v1.LabelTopologyRegion, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1"}, + }, + }, + }, + }, + expectedLabels: map[string]string{ + v1.LabelTopologyZone: "us-east1-a__us-east1-c", + v1.LabelTopologyRegion: "us-east1", + }, + }, + } + + for _, tc := range testCases { + t.Logf("Running test: %v", tc.name) + err := translateTopologyFromCSIToInTree(tc.pv, tc.key, tc.regionParser) + 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(tc.pv.Spec.NodeAffinity.Required.NodeSelectorTerms, tc.expectedNodeSelectorTerms) { + t.Errorf("Expected topology: %v, but got: %v", tc.expectedNodeSelectorTerms, tc.pv.Spec.NodeAffinity.Required.NodeSelectorTerms) + } + if !reflect.DeepEqual(tc.pv.Labels, tc.expectedLabels) { + t.Errorf("Expected labels: %v, but got: %v", tc.expectedLabels, tc.pv.Labels) + } + } +} + func TestTranslateAllowedTopologies(t *testing.T) { testCases := []struct { name string @@ -212,3 +607,146 @@ func TestAddTopology(t *testing.T) { } } } + +func TestRemoveTopology(t *testing.T) { + testCases := []struct { + name string + topologyKey string + pv *v1.PersistentVolume + expOk bool + expectedAffinity *v1.VolumeNodeAffinity + }{ + { + name: "Remove single csi topology from PV", + topologyKey: GCEPDTopologyKey, + pv: makePVWithNodeSelectorTerms([]v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + }, + }, + }), + expOk: true, + expectedAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{}, + }, + }, + }, + { + name: "Not found the topology key so do nothing", + topologyKey: GCEPDTopologyKey, + pv: makePVWithNodeSelectorTerms([]v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelTopologyZone, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + }, + }, + }), + expOk: false, + expectedAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelTopologyZone, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + }, + }, + }, + }, + }, + }, + { + name: "Remove the topology key from multiple terms", + topologyKey: GCEPDTopologyKey, + pv: makePVWithNodeSelectorTerms([]v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-c"}, + }, + }, + }, + }), + expOk: true, + expectedAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{}, + }, + }, + }, + { + name: "Remove the topology key from single term duplicate expression", + topologyKey: GCEPDTopologyKey, + pv: makePVWithNodeSelectorTerms([]v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + { + Key: GCEPDTopologyKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{"us-east1-a"}, + }, + }, + }, + }), + expOk: true, + expectedAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{}, + }, + }, + }, + } + + for _, tc := range testCases { + t.Logf("Running test: %v", tc.name) + ok := removeTopology(tc.pv, tc.topologyKey) + if tc.expOk != ok { + t.Errorf("Expected ok: %v, but got: %v", tc.expOk, ok) + } + if !reflect.DeepEqual(tc.pv.Spec.NodeAffinity, tc.expectedAffinity) { + t.Errorf("Expected affinity: %v, but got: %v", tc.expectedAffinity, tc.pv.Spec.NodeAffinity) + } + } +} + +func makePVWithNodeSelectorTerms(nodeSelectorTerms []v1.NodeSelectorTerm) *v1.PersistentVolume { + return &v1.PersistentVolume{ + Spec: v1.PersistentVolumeSpec{ + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: nodeSelectorTerms, + }, + }, + }, + } + +} 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 f3e21bc760a..c506e6acefc 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 @@ -95,7 +95,7 @@ func (t *osCinderCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume) VolumeAttributes: map[string]string{}, } - if err := translateTopology(pv, CinderTopologyKey); err != nil { + if err := translateTopologyFromInTreeToCSI(pv, CinderTopologyKey); err != nil { return nil, fmt.Errorf("failed to translate topology: %v", err) } diff --git a/staging/src/k8s.io/csi-translation-lib/translate_test.go b/staging/src/k8s.io/csi-translation-lib/translate_test.go index da0e60416ef..fc32cf250b7 100644 --- a/staging/src/k8s.io/csi-translation-lib/translate_test.go +++ b/staging/src/k8s.io/csi-translation-lib/translate_test.go @@ -28,13 +28,20 @@ import ( ) var ( - defaultZoneLabels = map[string]string{ + kubernetesBetaTopologyLabels = map[string]string{ v1.LabelFailureDomainBetaZone: "us-east-1a", v1.LabelFailureDomainBetaRegion: "us-east-1", } - regionalPDLabels = map[string]string{ + kubernetesGATopologyLabels = map[string]string{ + v1.LabelTopologyZone: "us-east-1a", + v1.LabelTopologyRegion: "us-east-1", + } + regionalBetaPDLabels = map[string]string{ v1.LabelFailureDomainBetaZone: "europe-west1-b__europe-west1-c", } + regionalGAPDLabels = map[string]string{ + v1.LabelTopologyZone: "europe-west1-b__europe-west1-c", + } ) func TestTranslationStability(t *testing.T) { @@ -94,12 +101,20 @@ func TestTranslationStability(t *testing.T) { func TestTopologyTranslation(t *testing.T) { testCases := []struct { name string + key string pv *v1.PersistentVolume expectedNodeAffinity *v1.VolumeNodeAffinity }{ { - name: "GCE PD with zone labels", - pv: makeGCEPDPV(defaultZoneLabels, nil /*topology*/), + name: "GCE PD with beta zone labels", + key: plugins.GCEPDTopologyKey, + pv: makeGCEPDPV(kubernetesBetaTopologyLabels, nil /*topology*/), + expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.GCEPDTopologyKey, "us-east-1a"), + }, + { + name: "GCE PD with GA kubernetes zone labels", + key: plugins.GCEPDTopologyKey, + pv: makeGCEPDPV(kubernetesGATopologyLabels, nil /*topology*/), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.GCEPDTopologyKey, "us-east-1a"), }, { @@ -109,35 +124,46 @@ func TestTopologyTranslation(t *testing.T) { }, { name: "GCE PD with existing topology (CSI keys)", + key: plugins.GCEPDTopologyKey, pv: makeGCEPDPV(nil /*labels*/, makeTopology(plugins.GCEPDTopologyKey, "us-east-2a")), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.GCEPDTopologyKey, "us-east-2a"), }, { name: "GCE PD with zone labels and topology", - pv: makeGCEPDPV(defaultZoneLabels, makeTopology(v1.LabelFailureDomainBetaZone, "us-east-2a")), + pv: makeGCEPDPV(kubernetesBetaTopologyLabels, makeTopology(v1.LabelFailureDomainBetaZone, "us-east-2a")), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.GCEPDTopologyKey, "us-east-2a"), }, { name: "GCE PD with regional zones", - pv: makeGCEPDPV(regionalPDLabels, nil /*topology*/), + key: plugins.GCEPDTopologyKey, + pv: makeGCEPDPV(regionalBetaPDLabels, nil /*topology*/), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.GCEPDTopologyKey, "europe-west1-b", "europe-west1-c"), }, { name: "GCE PD with regional topology", - pv: makeGCEPDPV(nil /*labels*/, makeTopology(v1.LabelFailureDomainBetaZone, "europe-west1-b", "europe-west1-c")), + key: plugins.GCEPDTopologyKey, + pv: makeGCEPDPV(nil /*labels*/, makeTopology(v1.LabelTopologyZone, "europe-west1-b", "europe-west1-c")), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.GCEPDTopologyKey, "europe-west1-b", "europe-west1-c"), }, { - name: "GCE PD with regional zone and topology", - pv: makeGCEPDPV(regionalPDLabels, makeTopology(v1.LabelFailureDomainBetaZone, "europe-west1-f", "europe-west1-g")), + name: "GCE PD with Beta regional zone and topology", + key: plugins.GCEPDTopologyKey, + pv: makeGCEPDPV(regionalBetaPDLabels, makeTopology(v1.LabelFailureDomainBetaZone, "europe-west1-f", "europe-west1-g")), + expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.GCEPDTopologyKey, "europe-west1-f", "europe-west1-g"), + }, + { + name: "GCE PD with GA regional zone and topology", + key: plugins.GCEPDTopologyKey, + pv: makeGCEPDPV(regionalGAPDLabels, makeTopology(v1.LabelTopologyZone, "europe-west1-f", "europe-west1-g")), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.GCEPDTopologyKey, "europe-west1-f", "europe-west1-g"), }, { name: "GCE PD with multiple node selector terms", + key: plugins.GCEPDTopologyKey, pv: makeGCEPDPVMultTerms( nil, /*labels*/ - makeTopology(v1.LabelFailureDomainBetaZone, "europe-west1-f"), - makeTopology(v1.LabelFailureDomainBetaZone, "europe-west1-g")), + makeTopology(v1.LabelTopologyZone, "europe-west1-f"), + makeTopology(v1.LabelTopologyZone, "europe-west1-g")), expectedNodeAffinity: makeNodeAffinity( true, /*multiTerms*/ plugins.GCEPDTopologyKey, "europe-west1-f", "europe-west1-g"), @@ -145,23 +171,23 @@ func TestTopologyTranslation(t *testing.T) { // EBS test cases: test mostly topology key, i.e., don't repeat testing done with GCE { name: "AWS EBS with zone labels", - pv: makeAWSEBSPV(defaultZoneLabels, nil /*topology*/), + pv: makeAWSEBSPV(kubernetesBetaTopologyLabels, nil /*topology*/), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.AWSEBSTopologyKey, "us-east-1a"), }, { name: "AWS EBS with zone labels and topology", - pv: makeAWSEBSPV(defaultZoneLabels, makeTopology(v1.LabelFailureDomainBetaZone, "us-east-2a")), + pv: makeAWSEBSPV(kubernetesBetaTopologyLabels, makeTopology(v1.LabelFailureDomainBetaZone, "us-east-2a")), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.AWSEBSTopologyKey, "us-east-2a"), }, // Cinder test cases: test mosty topology key, i.e., don't repeat testing done with GCE { name: "OpenStack Cinder with zone labels", - pv: makeCinderPV(defaultZoneLabels, nil /*topology*/), + pv: makeCinderPV(kubernetesBetaTopologyLabels, nil /*topology*/), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.CinderTopologyKey, "us-east-1a"), }, { name: "OpenStack Cinder with zone labels and topology", - pv: makeCinderPV(defaultZoneLabels, makeTopology(v1.LabelFailureDomainBetaZone, "us-east-2a")), + pv: makeCinderPV(kubernetesBetaTopologyLabels, makeTopology(v1.LabelFailureDomainBetaZone, "us-east-2a")), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.CinderTopologyKey, "us-east-2a"), }, } @@ -181,15 +207,27 @@ func TestTopologyTranslation(t *testing.T) { t.Errorf("Expected node affinity %v, got %v", *test.expectedNodeAffinity, *nodeAffinity) } - // Translate back to in-tree and make sure node affinity is still set + // Translate back to in-tree and make sure node affinity has been removed newInTreePV, err := ctl.TranslateCSIPVToInTree(newCSIPV) if err != nil { t.Errorf("Error when translating to in-tree: %v", err) } - nodeAffinity = newInTreePV.Spec.NodeAffinity - if !reflect.DeepEqual(nodeAffinity, test.expectedNodeAffinity) { - t.Errorf("Expected node affinity %v, got %v", *test.expectedNodeAffinity, *nodeAffinity) + // For now, non-pd cloud should stay the old behavior which is still have the CSI topology. + if test.key != "" { + nodeAffinity = newInTreePV.Spec.NodeAffinity + if plugins.TopologyKeyExist(test.key, nodeAffinity) { + t.Errorf("Expected node affinity key %v being removed, got %v", test.key, *nodeAffinity) + } + // verify that either beta or GA kubernetes topology key should exist + if !(plugins.TopologyKeyExist(v1.LabelFailureDomainBetaZone, nodeAffinity) || plugins.TopologyKeyExist(v1.LabelTopologyZone, nodeAffinity)) { + t.Errorf("Expected node affinity kuberenetes topology label exist, got %v", *nodeAffinity) + } + } else { + nodeAffinity := newCSIPV.Spec.NodeAffinity + if !reflect.DeepEqual(nodeAffinity, test.expectedNodeAffinity) { + t.Errorf("Expected node affinity %v, got %v", *test.expectedNodeAffinity, *nodeAffinity) + } } } }