gh comments

pkg/cloudprovider/cloudprovider.go

@@ -312,10 +312,15 @@ func (c *CloudProvider) instanceToNodeClaim(i *instance.Instance, instanceType *
 			}
 		}
 		resourceFilter := func(n v1.ResourceName, v resource.Quantity) bool {
-			if !i.EFAEnabled && n == v1beta1.ResourceEFA {
+			if resources.IsZero(v) {
 				return false
 			}
-			return !resources.IsZero(v)
+			// The nodeclaim should only advertise an EFA resource if it was requested. EFA network interfaces are only
+			// added to the launch template if they're requested, otherwise the instance is launched with a normal ENI.
+			if n == v1beta1.ResourceEFA {
+				return i.EFAEnabled
+			}
+			return true
 		}
 		nodeClaim.Status.Capacity = functional.FilterMap(instanceType.Capacity, resourceFilter)
 		nodeClaim.Status.Allocatable = functional.FilterMap(instanceType.Allocatable(), resourceFilter)

pkg/providers/amifamily/resolver.go

@@ -131,6 +131,11 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass,
 	}
 	var resolvedTemplates []*LaunchTemplate
 	for amiID, instanceTypes := range mappedAMIs {
+		// In order to support reserved ENIs for CNI custom networking setups,
+		// we need to pass down the max-pods calculation to the kubelet.
+		// This requires that we resolve a unique launch template per max-pods value.
+		// Similarly, instance types configured with EfAs require unique launch templates depending on the number of
+		// EFAs they support.
 		type launchTemplateParams struct {
 			efaCount int
 			maxPods  int
@@ -145,41 +150,10 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass,
 				maxPods: int(instanceType.Capacity.Pods().Value()),
 			}
 		})
-		// In order to support reserved ENIs for CNI custom networking setups,
-		// we need to pass down the max-pods calculation to the kubelet.
-		// This requires that we resolve a unique launch template per max-pods value.
 		for params, instanceTypes := range paramsToInstanceTypes {
-			kubeletConfig := &corev1beta1.KubeletConfiguration{}
-			if nodeClaim.Spec.Kubelet != nil {
-				if err := mergo.Merge(kubeletConfig, nodeClaim.Spec.Kubelet); err != nil {
-					return nil, err
-				}
-			}
-			if kubeletConfig.MaxPods == nil {
-				kubeletConfig.MaxPods = lo.ToPtr(int32(params.maxPods))
-			}
-			resolved := &LaunchTemplate{
-				Options: options,
-				UserData: amiFamily.UserData(
-					r.defaultClusterDNS(options, kubeletConfig),
-					append(nodeClaim.Spec.Taints, nodeClaim.Spec.StartupTaints...),
-					options.Labels,
-					options.CABundle,
-					instanceTypes,
-					nodeClass.Spec.UserData,
-				),
-				BlockDeviceMappings: nodeClass.Spec.BlockDeviceMappings,
-				MetadataOptions:     nodeClass.Spec.MetadataOptions,
-				DetailedMonitoring:  aws.BoolValue(nodeClass.Spec.DetailedMonitoring),
-				AMIID:               amiID,
-				InstanceTypes:       instanceTypes,
-				EFACount:            params.efaCount,
-			}
-			if len(resolved.BlockDeviceMappings) == 0 {
-				resolved.BlockDeviceMappings = amiFamily.DefaultBlockDeviceMappings()
-			}
-			if resolved.MetadataOptions == nil {
-				resolved.MetadataOptions = amiFamily.DefaultMetadataOptions()
+			resolved, err := r.resolveLaunchTemplate(nodeClass, nodeClaim, instanceTypes, amiFamily, amiID, params.maxPods, params.efaCount, options)
+			if err != nil {
+				return nil, err
 			}
 			resolvedTemplates = append(resolvedTemplates, resolved)
 		}
@@ -229,3 +203,40 @@ func (r Resolver) defaultClusterDNS(opts *Options, kubeletConfig *corev1beta1.Ku
 	newKubeletConfig.ClusterDNS = []string{opts.KubeDNSIP.String()}
 	return newKubeletConfig
 }
+
+func (r Resolver) resolveLaunchTemplate(nodeClass *v1beta1.EC2NodeClass, nodeClaim *corev1beta1.NodeClaim, instanceTypes []*cloudprovider.InstanceType,
+	amiFamily AMIFamily, amiID string, maxPods int, efaCount int, options *Options) (*LaunchTemplate, error) {
+	kubeletConfig := &corev1beta1.KubeletConfiguration{}
+	if nodeClaim.Spec.Kubelet != nil {
+		if err := mergo.Merge(kubeletConfig, nodeClaim.Spec.Kubelet); err != nil {
+			return nil, err
+		}
+	}
+	if kubeletConfig.MaxPods == nil {
+		kubeletConfig.MaxPods = lo.ToPtr(int32(maxPods))
+	}
+	resolved := &LaunchTemplate{
+		Options: options,
+		UserData: amiFamily.UserData(
+			r.defaultClusterDNS(options, kubeletConfig),
+			append(nodeClaim.Spec.Taints, nodeClaim.Spec.StartupTaints...),
+			options.Labels,
+			options.CABundle,
+			instanceTypes,
+			nodeClass.Spec.UserData,
+		),
+		BlockDeviceMappings: nodeClass.Spec.BlockDeviceMappings,
+		MetadataOptions:     nodeClass.Spec.MetadataOptions,
+		DetailedMonitoring:  aws.BoolValue(nodeClass.Spec.DetailedMonitoring),
+		AMIID:               amiID,
+		InstanceTypes:       instanceTypes,
+		EFACount:            efaCount,
+	}
+	if len(resolved.BlockDeviceMappings) == 0 {
+		resolved.BlockDeviceMappings = amiFamily.DefaultBlockDeviceMappings()
+	}
+	if resolved.MetadataOptions == nil {
+		resolved.MetadataOptions = amiFamily.DefaultMetadataOptions()
+	}
+	return resolved, nil
+}

pkg/providers/instance/types.go

@@ -55,7 +55,7 @@ func NewInstance(out *ec2.Instance) *Instance {
 		SubnetID: aws.StringValue(out.SubnetId),
 		Tags:     lo.SliceToMap(out.Tags, func(t *ec2.Tag) (string, string) { return aws.StringValue(t.Key), aws.StringValue(t.Value) }),
 		EFAEnabled: lo.ContainsBy(out.NetworkInterfaces, func(ni *ec2.InstanceNetworkInterface) bool {
-			return ni != nil && lo.FromPtr(ni.InterfaceType) == "efa"
+			return ni != nil && lo.FromPtr(ni.InterfaceType) == ec2.NetworkInterfaceTypeEfa
 		}),
 	}
 

pkg/providers/launchtemplate/launchtemplate.go

@@ -242,7 +242,7 @@ func (p *Provider) createLaunchTemplate(ctx context.Context, capacityType string
 	if capacityType == corev1beta1.CapacityTypeSpot {
 		launchTemplateDataTags = append(launchTemplateDataTags, &ec2.LaunchTemplateTagSpecificationRequest{ResourceType: aws.String(ec2.ResourceTypeSpotInstancesRequest), Tags: utils.MergeTags(options.Tags)})
 	}
-	networkInterfaces := p.generateNetworkInterface(options)
+	networkInterfaces := p.generateNetworkInterfaces(options)
 	output, err := p.ec2api.CreateLaunchTemplateWithContext(ctx, &ec2.CreateLaunchTemplateInput{
 		LaunchTemplateName: aws.String(launchTemplateName(options)),
 		LaunchTemplateData: &ec2.RequestLaunchTemplateData{
@@ -280,24 +280,25 @@ func (p *Provider) createLaunchTemplate(ctx context.Context, capacityType string
 	return output.LaunchTemplate, nil
 }
 
-// generateNetworkInterface generates a network interface for the launch template.
-// If all referenced subnets do not assign public IPv4 addresses to EC2 instances therein, we explicitly set
-// AssociatePublicIpAddress to 'false' in the Launch Template, generated based on this configuration struct.
-// This is done to help comply with AWS account policies that require explicitly setting that field to 'false'.
-// https://github.com/aws/karpenter/issues/3815
-func (p *Provider) generateNetworkInterface(options *amifamily.LaunchTemplate) []*ec2.LaunchTemplateInstanceNetworkInterfaceSpecificationRequest {
+// generateNetworkInterfaces generates network interfaces for the launch template.
+func (p *Provider) generateNetworkInterfaces(options *amifamily.LaunchTemplate) []*ec2.LaunchTemplateInstanceNetworkInterfaceSpecificationRequest {
 	if options.EFACount != 0 {
 		return lo.Times(options.EFACount, func(i int) *ec2.LaunchTemplateInstanceNetworkInterfaceSpecificationRequest {
 			return &ec2.LaunchTemplateInstanceNetworkInterfaceSpecificationRequest{
-				// You may only associate a public IP if there is a single network interface.
-				AssociatePublicIpAddress: lo.Ternary(options.EFACount == 1, options.AssociatePublicIPAddress, nil),
-				NetworkCardIndex:         lo.ToPtr(int64(i)),
-				DeviceIndex:              lo.ToPtr(lo.Ternary[int64](i == 0, 0, 1)),
-				InterfaceType:            lo.ToPtr("efa"),
-				Groups:                   lo.Map(options.SecurityGroups, func(s v1beta1.SecurityGroup, _ int) *string { return aws.String(s.ID) }),
+				NetworkCardIndex: lo.ToPtr(int64(i)),
+				// Some networking magic to ensure that one network card has higher priority than all the others (important if an instance needs a public IP w/o adding an EIP to every network card)
+				DeviceIndex:   lo.ToPtr(lo.Ternary[int64](i == 0, 0, 1)),
+				InterfaceType: lo.ToPtr(ec2.NetworkInterfaceTypeEfa),
+				Groups:        lo.Map(options.SecurityGroups, func(s v1beta1.SecurityGroup, _ int) *string { return aws.String(s.ID) }),
 			}
 		})
 	}
+
+	// If all referenced subnets do not assign public IPv4 addresses to EC2 instances therein, we explicitly set
+	// AssociatePublicIpAddress to 'false' in the Launch Template, generated based on this configuration struct.
+	// This is done to help comply with AWS account policies that require explicitly setting that field to 'false'.
+	// This is ignored for EFA instances since it can't be specified if you launch with multiple network interfaces.
+	// https://github.com/aws/karpenter/issues/3815
 	if options.AssociatePublicIPAddress != nil {
 		return []*ec2.LaunchTemplateInstanceNetworkInterfaceSpecificationRequest{
 			{

test/suites/integration/extended_resources_test.go

@@ -222,6 +222,9 @@ var _ = Describe("Extended Resources", func() {
 				Effect: v1.TaintEffectNoSchedule,
 			},
 		}
+		// Only select private subnets since instances with multiple network instances at launch won't get a public IP.
+		nodeClass.Spec.SubnetSelectorTerms[0].Tags["Name"] = "*Private*"
+
 		numPods := 1
 		dep := test.Deployment(test.DeploymentOptions{
 			Replicas: int32(numPods),
@@ -237,10 +240,10 @@ var _ = Describe("Extended Resources", func() {
 				},
 				ResourceRequirements: v1.ResourceRequirements{
 					Requests: v1.ResourceList{
-						"vpc.amazonaws.com/efa": resource.MustParse("1"),
+						"vpc.amazonaws.com/efa": resource.MustParse("2"),
 					},
 					Limits: v1.ResourceList{
-						"vpc.amazonaws.com/efa": resource.MustParse("1"),
+						"vpc.amazonaws.com/efa": resource.MustParse("2"),
 					},
 				},
 			},