diff --git a/pkg/volume/awsebs/aws_ebs_test.go b/pkg/volume/awsebs/aws_ebs_test.go index f6f7b165f94..a74a70039d3 100644 --- a/pkg/volume/awsebs/aws_ebs_test.go +++ b/pkg/volume/awsebs/aws_ebs_test.go @@ -396,7 +396,7 @@ func TestGetCandidateZone(t *testing.T) { node: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - v1.LabelFailureDomainBetaZone: testZone, + v1.LabelTopologyZone: testZone, }, }, }, diff --git a/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go b/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go index 398a53c5f4c..708d1c84f79 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go +++ b/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go @@ -174,9 +174,9 @@ func Test_PVLAdmission(t *testing.T) { name: "AWS EBS 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: "awsebs", Namespace: "myns"}, @@ -193,9 +193,9 @@ func Test_PVLAdmission(t *testing.T) { Name: "awsebs", 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{ @@ -220,7 +220,7 @@ func Test_PVLAdmission(t *testing.T) { Values: []string{"2"}, }, { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Operator: api.NodeSelectorOpIn, Values: []string{"1", "2", "3"}, }, @@ -373,15 +373,15 @@ func Test_PVLAdmission(t *testing.T) { name: "existing labels from user are changed", handler: newPersistentVolumeLabel(), pvlabeler: mockVolumeLabels(map[string]string{ - v1.LabelFailureDomainBetaZone: "domain1", - v1.LabelFailureDomainBetaRegion: "region1", + v1.LabelTopologyZone: "domain1", + v1.LabelTopologyRegion: "region1", }), preAdmissionPV: &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - v1.LabelFailureDomainBetaZone: "existingDomain", - v1.LabelFailureDomainBetaRegion: "existingRegion", + v1.LabelTopologyZone: "existingDomain", + v1.LabelTopologyRegion: "existingRegion", }, }, Spec: api.PersistentVolumeSpec{ @@ -397,8 +397,8 @@ func Test_PVLAdmission(t *testing.T) { Name: "awsebs", Namespace: "myns", Labels: map[string]string{ - v1.LabelFailureDomainBetaZone: "domain1", - v1.LabelFailureDomainBetaRegion: "region1", + v1.LabelTopologyZone: "domain1", + v1.LabelTopologyRegion: "region1", }, }, Spec: api.PersistentVolumeSpec{ @@ -413,12 +413,12 @@ func Test_PVLAdmission(t *testing.T) { { MatchExpressions: []api.NodeSelectorRequirement{ { - Key: v1.LabelFailureDomainBetaRegion, + Key: v1.LabelTopologyRegion, Operator: api.NodeSelectorOpIn, Values: []string{"region1"}, }, { - Key: v1.LabelFailureDomainBetaZone, + Key: v1.LabelTopologyZone, Operator: api.NodeSelectorOpIn, Values: []string{"domain1"}, }, @@ -630,9 +630,9 @@ func Test_PVLAdmission(t *testing.T) { name: "AWS EBS PV overrides user applied labels", 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{ @@ -655,9 +655,9 @@ func Test_PVLAdmission(t *testing.T) { Name: "awsebs", 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{ @@ -682,7 +682,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/csi-translation-lib/plugins/aws_ebs.go b/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go index 5acc8529834..483248dcc2b 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 @@ -26,6 +26,7 @@ import ( v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" ) const ( @@ -174,6 +175,11 @@ func (t *awsElasticBlockStoreCSITranslator) TranslateCSIPVToInTree(pv *v1.Persis ebsSource.Partition = int32(partValue) } + // translate CSI topology to In-tree topology for rollback compatibility + if err := translateTopologyFromCSIToInTree(pv, AWSEBSTopologyKey, awsRegionTopologyHandler); err != nil { + return nil, fmt.Errorf("failed to translate topology. PV:%+v. Error:%v", *pv, err) + } + pv.Spec.CSI = nil pv.Spec.AWSElasticBlockStore = ebsSource return pv, nil @@ -253,3 +259,90 @@ func KubernetesVolumeIDToEBSVolumeID(kubernetesID string) (string, error) { return awsID, nil } + +// This code is copied over from gce_pd as is. However, we're passing our own getRegionFromZones implementation. +// It might be worth moving this to a common place and pass that function as a parameter. +func awsRegionTopologyHandler(pv *v1.PersistentVolume) error { + + // 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 nil + } + + zoneLabel, regionLabel := getTopologyLabel(pv) + + // process each term + for index, nodeSelectorTerm := range pv.Spec.NodeAffinity.Required.NodeSelectorTerms { + // In the first loop, see if regionLabel already exist + regionExist := false + var zoneVals []string + for _, nsRequirement := range nodeSelectorTerm.MatchExpressions { + if nsRequirement.Key == regionLabel { + regionExist = true + break + } else if nsRequirement.Key == zoneLabel { + zoneVals = append(zoneVals, nsRequirement.Values...) + } + } + if regionExist { + // Regionlabel already exist in this term, skip it + continue + } + // If no regionLabel found, generate region label from the zoneLabel we collect from this term + regionVal, err := getAwsRegionFromZones(zoneVals) + if err != nil { + return err + } + // Add the regionVal to this term + pv.Spec.NodeAffinity.Required.NodeSelectorTerms[index].MatchExpressions = + append(pv.Spec.NodeAffinity.Required.NodeSelectorTerms[index].MatchExpressions, v1.NodeSelectorRequirement{ + Key: regionLabel, + Operator: v1.NodeSelectorOpIn, + Values: []string{regionVal}, + }) + + } + + // Add region label + regionVals := getTopologyValues(pv, regionLabel) + if len(regionVals) == 1 { + // We should only have exactly 1 region value + if pv.Labels == nil { + pv.Labels = make(map[string]string) + } + _, regionOK := pv.Labels[regionLabel] + if !regionOK { + pv.Labels[regionLabel] = regionVals[0] + } + } + + return nil +} + +func getAwsRegionFromZones(zones []string) (string, error) { + regions := sets.String{} + if len(zones) < 1 { + return "", fmt.Errorf("no zones specified") + } + + // AWS zones can be in four forms: + // us-west-2a, us-gov-east-1a, us-west-2-lax-1a (local zone) and us-east-1-wl1-bos-wlz-1 (wavelength). + for _, zone := range zones { + splitZone := strings.Split(zone, "-") + if (len(splitZone) == 3 || len(splitZone) == 4) && len(splitZone[len(splitZone)-1]) == 2 { + // this would break if we ever have a location with more than 9 regions, ie us-west-10. + splitZone[len(splitZone)-1] = splitZone[len(splitZone)-1][:1] + regions.Insert(strings.Join(splitZone, "-")) + } else if len(splitZone) == 5 || len(splitZone) == 7 { + // local zone or wavelength + regions.Insert(strings.Join(splitZone[:3], "-")) + } else { + return "", fmt.Errorf("Unexpected zone format: %v is not a valid AWS zone", zone) + } + } + if regions.Len() != 1 { + return "", fmt.Errorf("multiple or no regions gotten from zones, got: %v", regions) + } + return regions.UnsortedList()[0], nil +} diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs_test.go index 38eac7863db..c02ff54660e 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs_test.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs_test.go @@ -17,10 +17,11 @@ limitations under the License. package plugins import ( - v1 "k8s.io/api/core/v1" "reflect" "testing" + v1 "k8s.io/api/core/v1" + storage "k8s.io/api/storage/v1" ) @@ -198,3 +199,77 @@ func TestTranslateInTreeInlineVolumeToCSI(t *testing.T) { }) } } + +func TestGetAwsRegionFromZones(t *testing.T) { + + cases := []struct { + name string + zones []string + expRegion string + expErr bool + }{ + { + name: "Commercial zone", + zones: []string{"us-west-2a", "us-west-2b"}, + expRegion: "us-west-2", + }, + { + name: "Govcloud zone", + zones: []string{"us-gov-east-1a"}, + expRegion: "us-gov-east-1", + }, + { + name: "Wavelength zone", + zones: []string{"us-east-1-wl1-bos-wlz-1"}, + expRegion: "us-east-1", + }, + { + name: "Local zone", + zones: []string{"us-west-2-lax-1a"}, + expRegion: "us-west-2", + }, + { + name: "Invalid: empty zones", + zones: []string{}, + expErr: true, + }, + { + name: "Invalid: multiple regions", + zones: []string{"us-west-2a", "us-east-1a"}, + expErr: true, + }, + { + name: "Invalid: region name only", + zones: []string{"us-west-2"}, + expErr: true, + }, + { + name: "Invalid: invalid suffix", + zones: []string{"us-west-2ab"}, + expErr: true, + }, + { + name: "Invalid: not enough fields", + zones: []string{"us-west"}, + expErr: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Logf("Testing %v", tc.name) + got, err := getAwsRegionFromZones(tc.zones) + if err != nil && !tc.expErr { + t.Fatalf("Did not expect error but got: %v", err) + } + + if err == nil && tc.expErr { + t.Fatalf("Expected error, but did not get one.") + } + + if err == nil && !reflect.DeepEqual(got, tc.expRegion) { + t.Errorf("Got PV name: %v, expected :%v", got, tc.expRegion) + } + }) + } +} 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 fc32cf250b7..2a97180efdf 100644 --- a/staging/src/k8s.io/csi-translation-lib/translate_test.go +++ b/staging/src/k8s.io/csi-translation-lib/translate_test.go @@ -170,15 +170,25 @@ 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", + name: "AWS EBS with beta zone labels", pv: makeAWSEBSPV(kubernetesBetaTopologyLabels, nil /*topology*/), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.AWSEBSTopologyKey, "us-east-1a"), }, { - name: "AWS EBS with zone labels and topology", + name: "AWS EBS with beta zone labels and topology", pv: makeAWSEBSPV(kubernetesBetaTopologyLabels, makeTopology(v1.LabelFailureDomainBetaZone, "us-east-2a")), expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.AWSEBSTopologyKey, "us-east-2a"), }, + { + name: "AWS EBS with GA zone labels", + pv: makeAWSEBSPV(kubernetesGATopologyLabels, nil /*topology*/), + expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.AWSEBSTopologyKey, "us-east-1a"), + }, + { + name: "AWS EBS with GA zone labels and topology", + pv: makeAWSEBSPV(kubernetesGATopologyLabels, makeTopology(v1.LabelTopologyZone, "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", diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go index fbbaa6567d0..3588f38cb4e 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go @@ -2749,12 +2749,12 @@ func (c *Cloud) GetVolumeLabels(volumeName KubernetesVolumeID) (map[string]strin return nil, fmt.Errorf("volume did not have AZ information: %q", aws.StringValue(info.VolumeId)) } - labels[v1.LabelFailureDomainBetaZone] = az + labels[v1.LabelTopologyZone] = az region, err := azToRegion(az) if err != nil { return nil, err } - labels[v1.LabelFailureDomainBetaRegion] = region + labels[v1.LabelTopologyRegion] = region return labels, nil } diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go index cee17b26bf1..a6cd5d29a6c 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go @@ -1576,8 +1576,8 @@ func TestGetVolumeLabels(t *testing.T) { assert.Nil(t, err, "Error creating Volume %v", err) assert.Equal(t, map[string]string{ - v1.LabelFailureDomainBetaZone: "us-east-1a", - v1.LabelFailureDomainBetaRegion: "us-east-1"}, labels) + v1.LabelTopologyZone: "us-east-1a", + v1.LabelTopologyRegion: "us-east-1"}, labels) awsServices.ec2.(*MockedFakeEC2).AssertExpectations(t) } @@ -1650,8 +1650,8 @@ func TestGetLabelsForVolume(t *testing.T) { AvailabilityZone: aws.String("us-east-1a"), }}, map[string]string{ - v1.LabelFailureDomainBetaZone: "us-east-1a", - v1.LabelFailureDomainBetaRegion: "us-east-1", + v1.LabelTopologyZone: "us-east-1a", + v1.LabelTopologyRegion: "us-east-1", }, nil, }, diff --git a/test/e2e/storage/drivers/in_tree.go b/test/e2e/storage/drivers/in_tree.go index 04e38cc48a5..e4c8559c89f 100644 --- a/test/e2e/storage/drivers/in_tree.go +++ b/test/e2e/storage/drivers/in_tree.go @@ -1683,7 +1683,7 @@ func InitAwsDriver() storageframework.TestDriver { "ntfs", ), SupportedMountOption: sets.NewString("debug", "nouid32"), - TopologyKeys: []string{v1.LabelFailureDomainBetaZone}, + TopologyKeys: []string{v1.LabelTopologyZone}, Capabilities: map[storageframework.Capability]bool{ storageframework.CapPersistence: true, storageframework.CapFsGroup: true, @@ -1775,7 +1775,7 @@ func (a *awsDriver) CreateVolume(config *storageframework.PerTestConfig, volType // so pods should be also scheduled there. config.ClientNodeSelection = e2epod.NodeSelection{ Selector: map[string]string{ - v1.LabelFailureDomainBetaZone: zone, + v1.LabelTopologyZone: zone, }, } }