From 8639bb423f8da45605586f8e07662a4d8b34c945 Mon Sep 17 00:00:00 2001 From: Abhinav Dahiya Date: Tue, 2 Jul 2019 10:37:40 -0700 Subject: [PATCH 1/2] platformtest/aws: update tests to allow more than one candidate for instance class multiple reserved instance classes can use used to satisfy the test, and sometimes less preferred instance class might need to be chosen when AWS allows higher priority in reserved and not in on-demand EC2 instances. ```console go test ./platformtests/... -v === RUN TestGetDefaultInstanceClass === RUN TestGetDefaultInstanceClass/Asia_Pacific_(Tokyo) === RUN TestGetDefaultInstanceClass/EU_(Paris) === RUN TestGetDefaultInstanceClass/Asia_Pacific_(Sydney) === RUN TestGetDefaultInstanceClass/US_East_(Ohio) === RUN TestGetDefaultInstanceClass/EU_(Stockholm) === RUN TestGetDefaultInstanceClass/EU_(London) === RUN TestGetDefaultInstanceClass/Asia_Pacific_(Osaka-Local) === RUN TestGetDefaultInstanceClass/EU_(Frankfurt) === RUN TestGetDefaultInstanceClass/Asia_Pacific_(Seoul) === RUN TestGetDefaultInstanceClass/US_West_(N._California) === RUN TestGetDefaultInstanceClass/Canada_(Central) === RUN TestGetDefaultInstanceClass/US_West_(Oregon) === RUN TestGetDefaultInstanceClass/Asia_Pacific_(Mumbai) === RUN TestGetDefaultInstanceClass/South_America_(Sao_Paulo) === RUN TestGetDefaultInstanceClass/Asia_Pacific_(Singapore) === RUN TestGetDefaultInstanceClass/US_East_(N._Virginia) === RUN TestGetDefaultInstanceClass/AWS_GovCloud_(US) === RUN TestGetDefaultInstanceClass/Asia_Pacific_(Hong_Kong) === RUN TestGetDefaultInstanceClass/AWS_GovCloud_(US-East) === RUN TestGetDefaultInstanceClass/EU_(Ireland) --- PASS: TestGetDefaultInstanceClass (71.20s) --- PASS: TestGetDefaultInstanceClass/Asia_Pacific_(Tokyo) (2.75s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[ap-northeast-1a:{} ap-northeast-1c:{} ap-northeast-1d:{}] m5:map[ap-northeast-1a:{} ap-northeast-1c:{} --- PASS: TestGetDefaultInstanceClass/EU_(Paris) (1.44s) default_instance_class_test.go:134: m4 default_instance_class_test.go:136: skip the unpriced m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m5:map[eu-west-3a:{} eu-west-3b:{} eu-west-3c:{}]] --- PASS: TestGetDefaultInstanceClass/Asia_Pacific_(Sydney) (2.79s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[ap-southeast-2a:{} ap-southeast-2b:{} ap-southeast-2c:{}] m5:map[ap-southeast-2a:{} ap-southeast-2c:{}] --- PASS: TestGetDefaultInstanceClass/US_East_(Ohio) (1.35s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[us-east-2a:{} us-east-2b:{} us-east-2c:{}] m5:map[us-east-2a:{} us-east-2b:{} us-east-2c:{}]] --- PASS: TestGetDefaultInstanceClass/EU_(Stockholm) (1.86s) default_instance_class_test.go:134: m4 default_instance_class_test.go:136: skip the unpriced m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m5:map[eu-north-1a:{} eu-north-1b:{} eu-north-1c:{}]] --- PASS: TestGetDefaultInstanceClass/EU_(London) (2.23s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[eu-west-2a:{} eu-west-2b:{} eu-west-2c:{}] m5:map[eu-west-2a:{} eu-west-2b:{} eu-west-2c:{}]] --- PASS: TestGetDefaultInstanceClass/Asia_Pacific_(Osaka-Local) (0.70s) default_instance_class_test.go:106: no direct access to region, assuming full support: OptInRequired: You are not subscribed to this service. Please g status code: 401, request id: 94786b2a-85c2-4b83-a928-a5b2a72d803b default_instance_class_test.go:120: map[m3:{} m4:{} m5:{} m5d:{} t2:{} t3:{}] --- PASS: TestGetDefaultInstanceClass/EU_(Frankfurt) (2.08s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[eu-central-1a:{} eu-central-1b:{} eu-central-1c:{}] m5:map[eu-central-1a:{} eu-central-1b:{} eu-central --- PASS: TestGetDefaultInstanceClass/Asia_Pacific_(Seoul) (2.14s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[ap-northeast-2a:{} ap-northeast-2b:{} ap-northeast-2c:{}] m5:map[ap-northeast-2a:{} ap-northeast-2b:{} --- PASS: TestGetDefaultInstanceClass/US_West_(N._California) (0.88s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[us-west-1a:{} us-west-1b:{}] m5:map[us-west-1a:{} us-west-1b:{}]] --- PASS: TestGetDefaultInstanceClass/Canada_(Central) (1.48s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[ca-central-1a:{} ca-central-1b:{}] m5:map[ca-central-1a:{} ca-central-1b:{}]] --- PASS: TestGetDefaultInstanceClass/US_West_(Oregon) (1.29s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[us-west-2a:{} us-west-2b:{} us-west-2c:{}] m5:map[us-west-2a:{} us-west-2b:{} us-west-2c:{} us-west-2d: --- PASS: TestGetDefaultInstanceClass/Asia_Pacific_(Mumbai) (3.44s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[ap-south-1a:{} ap-south-1b:{} ap-south-1c:{}] m5:map[ap-south-1a:{} ap-south-1b:{} ap-south-1c:{}]] --- PASS: TestGetDefaultInstanceClass/South_America_(Sao_Paulo) (2.60s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[sa-east-1a:{} sa-east-1c:{}] m5:map[sa-east-1a:{} sa-east-1c:{}]] --- PASS: TestGetDefaultInstanceClass/Asia_Pacific_(Singapore) (2.32s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[ap-southeast-1a:{} ap-southeast-1b:{} ap-southeast-1c:{}] m5:map[ap-southeast-1a:{} ap-southeast-1b:{} --- PASS: TestGetDefaultInstanceClass/US_East_(N._Virginia) (1.58s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[us-east-1a:{} us-east-1b:{} us-east-1c:{} us-east-1d:{} us-east-1e:{} us-east-1f:{}] m5:map[us-east-1a: --- PASS: TestGetDefaultInstanceClass/AWS_GovCloud_(US) (0.17s) default_instance_class_test.go:106: no direct access to region, assuming full support: AuthFailure: AWS was not able to validate the provided access c status code: 401, request id: 510b245a-0d50-438d-894f-4ec2ea6c8dcc default_instance_class_test.go:120: map[m1:{} m3:{} m4:{} m5:{} m5d:{} t2:{} t3:{}] --- PASS: TestGetDefaultInstanceClass/Asia_Pacific_(Hong_Kong) (0.84s) default_instance_class_test.go:106: no direct access to region, assuming full support: AuthFailure: AWS was not able to validate the provided access c status code: 401, request id: 90b6acb2-779c-4c1e-a270-1ee530180090 default_instance_class_test.go:120: map[m5:{} m5d:{} t3:{}] --- PASS: TestGetDefaultInstanceClass/AWS_GovCloud_(US-East) (0.35s) default_instance_class_test.go:106: no direct access to region, assuming full support: AuthFailure: AWS was not able to validate the provided access c status code: 401, request id: f85d4b7f-632f-4bf4-a167-0b0611d841b3 default_instance_class_test.go:120: map[m5:{} m5d:{} t3:{}] --- PASS: TestGetDefaultInstanceClass/EU_(Ireland) (2.02s) default_instance_class_test.go:134: m4 default_instance_class_test.go:134: m5 default_instance_class_test.go:176: map[m4:map[eu-west-1a:{} eu-west-1b:{} eu-west-1c:{}] m5:map[eu-west-1a:{} eu-west-1b:{} eu-west-1c:{}]] PASS ok github.com/openshift/installer/platformtests/aws 71.212s ``` --- platformtests/aws/default_instance_class_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/platformtests/aws/default_instance_class_test.go b/platformtests/aws/default_instance_class_test.go index ce1d9cacc4b..1b16faab2f6 100644 --- a/platformtests/aws/default_instance_class_test.go +++ b/platformtests/aws/default_instance_class_test.go @@ -124,7 +124,7 @@ func TestGetDefaultInstanceClass(t *testing.T) { } available := make(map[string]map[string]struct{}, len(preferredInstanceClasses)) - var match string + var allowed []string for _, instanceClass := range preferredInstanceClasses { if _, ok := classes[instanceClass]; !ok { @@ -160,17 +160,16 @@ func TestGetDefaultInstanceClass(t *testing.T) { } if reflect.DeepEqual(available[instanceClass], zones) { - match = instanceClass - break + allowed = append(allowed, instanceClass) } } - if match == "" { + if len(allowed) == 0 { t.Fatalf("none of the preferred instance classes are fully supported: %v", available) } t.Log(available) - assert.Equal(t, defaults.InstanceClass(region), match) + assert.Contains(t, allowed, defaults.InstanceClass(region)) }) } } From b4acdf8d58cf7f3fcb787cd4a450015f0af13593 Mon Sep 17 00:00:00 2001 From: Abhinav Dahiya Date: Tue, 2 Jul 2019 10:40:00 -0700 Subject: [PATCH 2/2] types/aws/default: move ap-northeast-2 to m5 instance class AWS allows m4 class for reserved instances in all AZs for ap-northeast-2 ```console aws --region ap-northeast-2 ec2 describe-reserved-instances-offerings --instance-tenancy default --instance-type m4.large --product-description 'Linux/UNIX' --filters Name=scope,Values='Availability Zone' | jq -r '[.ReservedInstancesOfferings[].AvailabilityZone] | sort | unique[]' ap-northeast-2a ap-northeast-2b ap-northeast-2c ``` But the on-demand instances in ap-norteast-2b was failing with error ``` Unsupported: Your requested instance type (m4.xlarge) is not supported in your requested Availability Zone (ap-northeast-2b). Please retry your request by not specifying an Availability Zone or choosing ap-northeast-2a, ap-northeast-2c. ``` So moving to m5 class for ap-northeast-2 should allow creating control plane instances in all Zones. (the default configuration) This commit cherry-picks 3085753d39 (Bug 1725524: types/aws/default: move ap-northeast-2 to m5 instance class, 2019-07-02, #1935). It's not a clean cherry-pick, because we decided to not backport the ap-east-1 addition from cdd668952a (pkg/types/aws/defaults/platform: Default to m5 in ap-east-1, 2019-05-16, #1755) to 4.1.z [1]. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1734136#c4 --- pkg/types/aws/defaults/platform.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/types/aws/defaults/platform.go b/pkg/types/aws/defaults/platform.go index 953beea6708..6d9498a053a 100644 --- a/pkg/types/aws/defaults/platform.go +++ b/pkg/types/aws/defaults/platform.go @@ -6,10 +6,11 @@ import ( var ( defaultMachineClass = map[string]string{ - "eu-north-1": "m5", - "eu-west-3": "m5", - "us-gov-east-1": "m5", - "us-west-2": "m5", + "eu-north-1": "m5", + "ap-northeast-2": "m5", + "eu-west-3": "m5", + "us-gov-east-1": "m5", + "us-west-2": "m5", } )