Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamically filter out invalid instance types #517

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

mselim00
Copy link
Contributor

@mselim00 mselim00 commented Dec 4, 2024

Description of changes:
Leverage EC2:DescribeInstanceTypes API to dynamically scope down default instance types list to only those that exist in the test region.

Relies on error format:

operation error EC2: DescribeInstanceTypes, https response error StatusCode: 400, RequestID: <request_id>, api error InvalidInstanceType: The following supplied instance types do not exist: [<instance_type(s)>]

Specifically, the error handler relies on the presence of InvalidInstanceType in the error string, a printed list of the instance types being discoverable by indexing the last occurrence of a forward square bracket ([), and multiple types being separated by a comma and a space (, ).

Tested this by appending ec2types.InstanceTypeTrn1n32xlarge to the defaultInstanceTypes_x86_64 array and then creating a test deployment in us-west-1. Verified that the 'Eliminating instance type' log line was hit only for trn1n.32xlarge and that node groups create.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

kubetest2/internal/deployers/eksapi/nodegroup.go Outdated Show resolved Hide resolved
}

func (m *NodegroupManager) getValidInstanceTypesFromList(desiredInstanceTypes []ec2types.InstanceType) []string {
_, err := m.clients.EC2().DescribeInstanceTypes(context.TODO(), &ec2.DescribeInstanceTypesInput{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to check the error code here like:

if err != nil {
	var apierr smithy.APIError
	if errors.As(err, &apierr) {
		// do something with apierr.ErrorCode()
    }
}

instead of doing the parsing yourself

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pushed a change to parse the error code in this format and pass just the error message to the invalid instance types parser rather than the whole error.

@mselim00 mselim00 force-pushed the autodiscover-types branch 5 times, most recently from c4c1f06 to f902cc2 Compare December 5, 2024 19:58
Copy link
Member

@cartermckinnon cartermckinnon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nits 👍

kubetest2/internal/deployers/eksapi/nodegroup.go Outdated Show resolved Hide resolved
kubetest2/internal/deployers/eksapi/nodegroup.go Outdated Show resolved Hide resolved
kubetest2/internal/deployers/eksapi/nodegroup.go Outdated Show resolved Hide resolved
kubetest2/internal/deployers/eksapi/nodegroup.go Outdated Show resolved Hide resolved
@cartermckinnon cartermckinnon merged commit bd9412a into aws:main Dec 9, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants