Use GA topoogy labels for EBS

This commit is contained in:
Ayberk Yilmaz 2021-02-16 18:37:14 +00:00
parent f24b87ec8a
commit 339b8b450f
8 changed files with 212 additions and 34 deletions

View File

@ -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,
},
},
},

View File

@ -176,7 +176,7 @@ func Test_PVLAdmission(t *testing.T) {
pvlabeler: mockVolumeLabels(map[string]string{
"a": "1",
"b": "2",
v1.LabelFailureDomainBetaZone: "1__2__3",
v1.LabelTopologyZone: "1__2__3",
}),
preAdmissionPV: &api.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{Name: "awsebs", Namespace: "myns"},
@ -195,7 +195,7 @@ func Test_PVLAdmission(t *testing.T) {
Labels: map[string]string{
"a": "1",
"b": "2",
v1.LabelFailureDomainBetaZone: "1__2__3",
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"},
},
@ -632,7 +632,7 @@ func Test_PVLAdmission(t *testing.T) {
pvlabeler: mockVolumeLabels(map[string]string{
"a": "1",
"b": "2",
v1.LabelFailureDomainBetaZone: "1__2__3",
v1.LabelTopologyZone: "1__2__3",
}),
preAdmissionPV: &api.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
@ -657,7 +657,7 @@ func Test_PVLAdmission(t *testing.T) {
Labels: map[string]string{
"a": "1",
"b": "2",
v1.LabelFailureDomainBetaZone: "1__2__3",
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"},
},

View File

@ -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
}

View File

@ -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)
}
})
}
}

View File

@ -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",

View File

@ -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
}

View File

@ -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,
},

View File

@ -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,
},
}
}