Skip to content

Commit

Permalink
fix: Pass security groups and subnets into RunInstances dry-run (#7844)
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathan-innis authored Mar 5, 2025
1 parent 56e8015 commit dd9dc93
Showing 1 changed file with 28 additions and 3 deletions.
31 changes: 28 additions & 3 deletions pkg/controllers/nodeclass/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,18 +189,26 @@ func (v *Validation) validateRunInstancesAuthorization(
nodeClaim *karpv1.NodeClaim,
tags map[string]string,
) (reason string, err error) {
// NOTE: Since we've already validated the AMI status condition is true, this should never occur
// NOTE: Since we've already validated the status conditions are true, these should never occur
if len(nodeClass.Status.AMIs) == 0 {
return "", fmt.Errorf("no resolved amis in status")
}
if len(nodeClass.Status.Subnets) == 0 {
return "", fmt.Errorf("no resolved subnets in status")
}
if len(nodeClass.Status.SecurityGroups) == 0 {
return "", fmt.Errorf("no resolved security groups in status")
}
if nodeClass.Status.InstanceProfile == "" {
return "", fmt.Errorf("no instance profile in status")
}

var instanceType ec2types.InstanceType
requirements := scheduling.NewNodeSelectorRequirements(nodeClass.Status.AMIs[0].Requirements...)

instanceType = ec2types.InstanceTypeM6gLarge
if requirements.Get(corev1.LabelArchStable).Has(karpv1.ArchitectureAmd64) {
instanceType = ec2types.InstanceTypeM5Large
} else if requirements.Get(corev1.LabelArchStable).Has(karpv1.ArchitectureArm64) {
instanceType = ec2types.InstanceTypeM6gLarge
}

runInstancesInput := &ec2.RunInstancesInput{
Expand All @@ -216,6 +224,10 @@ func (v *Validation) validateRunInstancesAuthorization(
//nolint: gosec
HttpPutResponseHopLimit: lo.ToPtr(int32(lo.FromPtr(nodeClass.Spec.MetadataOptions.HTTPPutResponseHopLimit))),
},
Monitoring: &ec2types.RunInstancesMonitoringEnabled{
// Default Enabled to False if not specified
Enabled: lo.ToPtr(lo.FromPtr(nodeClass.Spec.DetailedMonitoring)),
},
TagSpecifications: []ec2types.TagSpecification{
{
ResourceType: ec2types.ResourceTypeInstance,
Expand All @@ -231,6 +243,19 @@ func (v *Validation) validateRunInstancesAuthorization(
},
},
ImageId: lo.ToPtr(nodeClass.Status.AMIs[0].ID),
IamInstanceProfile: &ec2types.IamInstanceProfileSpecification{
Name: lo.ToPtr(nodeClass.Status.InstanceProfile),
},
// EC2 dry-run doesn't validate the number of IPs, so it's safe to take the first subnet here
// even if that subnet has no more IPv4 or IPv6 addresses to give out
NetworkInterfaces: []ec2types.InstanceNetworkInterfaceSpecification{
{
AssociatePublicIpAddress: nodeClass.Spec.AssociatePublicIPAddress,
DeviceIndex: lo.ToPtr[int32](0),
Groups: lo.Map(nodeClass.Status.SecurityGroups, func(s v1.SecurityGroup, _ int) string { return s.ID }),
SubnetId: lo.ToPtr(nodeClass.Status.Subnets[0].ID),
},
},
}

if _, err := v.ec2api.RunInstances(ctx, runInstancesInput); awserrors.IgnoreDryRunError(err) != nil {
Expand Down

0 comments on commit dd9dc93

Please sign in to comment.