Skip to content

Commit

Permalink
Dynamically filter out invalid instance types (#517)
Browse files Browse the repository at this point in the history
* Dynamically filter out invalid instance types

* Clean up error messages and return values

Co-authored-by: Carter <[email protected]>

---------

Co-authored-by: Carter <[email protected]>
  • Loading branch information
mselim00 and cartermckinnon authored Dec 9, 2024
1 parent 71aa002 commit bd9412a
Showing 1 changed file with 44 additions and 6 deletions.
50 changes: 44 additions & 6 deletions kubetest2/internal/deployers/eksapi/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}

0 comments on commit bd9412a

Please sign in to comment.