From bd9412a3599dab6c4eaa85993bfc01ce036f021b Mon Sep 17 00:00:00 2001 From: mselim00 <100049111+mselim00@users.noreply.github.com> Date: Mon, 9 Dec 2024 13:17:25 -0800 Subject: [PATCH] Dynamically filter out invalid instance types (#517) * Dynamically filter out invalid instance types * Clean up error messages and return values Co-authored-by: Carter --------- Co-authored-by: Carter --- .../internal/deployers/eksapi/nodegroup.go | 50 ++++++++++++++++--- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/kubetest2/internal/deployers/eksapi/nodegroup.go b/kubetest2/internal/deployers/eksapi/nodegroup.go index 5095c05df..5f4b5792c 100644 --- a/kubetest2/internal/deployers/eksapi/nodegroup.go +++ b/kubetest2/internal/deployers/eksapi/nodegroup.go @@ -18,6 +18,7 @@ import ( ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" "github.com/aws/aws-sdk-go-v2/service/eks" ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" + "github.com/aws/smithy-go" "k8s.io/klog/v2" "github.com/aws/aws-k8s-tester/kubetest2/internal/deployers/eksapi/templates" @@ -78,9 +79,9 @@ func (m *NodegroupManager) createNodegroup(infra *Infrastructure, cluster *Clust return fmt.Errorf("failed to describe AMI when populating default instance types: %s: %v", opts.AMI, err) } else { amiArch := out.Images[0].Architecture - defaultInstanceTypes, ok := defaultInstanceTypesByEC2ArchitectureValues[amiArch] - if !ok { - return fmt.Errorf("no default instance types known for AMI architecture: %v", amiArch) + defaultInstanceTypes, err := m.getValidDefaultInstanceTypesByEC2Arch(amiArch) + if err != nil { + return fmt.Errorf("failed to get default instance types for AmiArch: %s: %v", amiArch, err) } opts.InstanceTypes = defaultInstanceTypes klog.V(2).Infof("Using default instance types for AMI architecture: %v: %v", amiArch, opts.InstanceTypes) @@ -116,9 +117,9 @@ func (m *NodegroupManager) createManagedNodegroup(infra *Infrastructure, cluster } else { // managed nodegroups uses a t3.medium by default at the time of writing // this only supports 17 pods, which can cause some flakes in the k8s e2e suite - defaultInstanceTypes, ok := defaultInstanceTypesByEKSAMITypes[input.AmiType] - if !ok { - return fmt.Errorf("no default instance types known for AmiType: %v", input.AmiType) + defaultInstanceTypes, err := m.getValidDefaultInstanceTypesByEKSAMIType(input.AmiType) + if err != nil { + return fmt.Errorf("failed to get default instance types for AmiType: %s: %v", input.AmiType, err) } input.InstanceTypes = defaultInstanceTypes } @@ -532,3 +533,40 @@ func (m *NodegroupManager) getSubnetWithCapacity(infra *Infrastructure, opts *de klog.Infof("Using subnet: %s", subnetId) return subnetId, capacityReservationId, nil } + +func (m *NodegroupManager) getValidDefaultInstanceTypesByEKSAMIType(amiType ekstypes.AMITypes) ([]string, error) { + defaults, ok := defaultInstanceTypesByEKSAMITypes[amiType] + if !ok { + return nil, fmt.Errorf("no default instance types known for AmiType: %v", amiType) + } + return m.getValidInstanceTypesFromList(defaults) +} + +func (m *NodegroupManager) getValidDefaultInstanceTypesByEC2Arch(arch ec2types.ArchitectureValues) ([]string, error) { + defaults, ok := defaultInstanceTypesByEC2ArchitectureValues[arch] + if !ok { + return nil, fmt.Errorf("no default instance types known for AMI architecture: %v", arch) + } + return m.getValidInstanceTypesFromList(defaults) +} + +func (m *NodegroupManager) getValidInstanceTypesFromList(desiredInstanceTypes []string) ([]string, error) { + var validInstanceTypes []string + for _, instanceType := range desiredInstanceTypes { + ec2InstanceType := ec2types.InstanceType(instanceType) + _, err := m.clients.EC2().DescribeInstanceTypes(context.TODO(), &ec2.DescribeInstanceTypesInput{ + InstanceTypes: []ec2types.InstanceType{ec2InstanceType}, + }) + if err != nil { + var apierr smithy.APIError + if errors.As(err, &apierr) && apierr.ErrorCode() == "InvalidInstanceType" { + klog.Infof("Eliminating instance type %s as an option", instanceType) + } else { + return nil, fmt.Errorf("failed to describe instance type: %s: %v", instanceType, err) + } + } else { + validInstanceTypes = append(validInstanceTypes, instanceType) + } + } + return validInstanceTypes, nil +}