From 06acb88ff7129395ee39cb9fd80b765126b00f8d Mon Sep 17 00:00:00 2001 From: Jason Deal Date: Wed, 29 Nov 2023 00:52:43 -0500 Subject: [PATCH] gh comments --- pkg/cloudprovider/cloudprovider.go | 9 ++- pkg/providers/amifamily/resolver.go | 79 +++++++++++-------- pkg/providers/instance/types.go | 2 +- .../launchtemplate/launchtemplate.go | 3 +- .../integration/extended_resources_test.go | 7 +- 5 files changed, 60 insertions(+), 40 deletions(-) diff --git a/pkg/cloudprovider/cloudprovider.go b/pkg/cloudprovider/cloudprovider.go index 7bac2deec2e9..ba11bc8a732e 100644 --- a/pkg/cloudprovider/cloudprovider.go +++ b/pkg/cloudprovider/cloudprovider.go @@ -308,10 +308,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) diff --git a/pkg/providers/amifamily/resolver.go b/pkg/providers/amifamily/resolver.go index ae8d5f9c3c4c..0aa8d02de922 100644 --- a/pkg/providers/amifamily/resolver.go +++ b/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 +} diff --git a/pkg/providers/instance/types.go b/pkg/providers/instance/types.go index d4b7df473653..67aeea8f688f 100644 --- a/pkg/providers/instance/types.go +++ b/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 }), } diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go index 89db8682906b..7cfe4ca34460 100644 --- a/pkg/providers/launchtemplate/launchtemplate.go +++ b/pkg/providers/launchtemplate/launchtemplate.go @@ -292,8 +292,9 @@ func (p *Provider) generateNetworkInterface(options *amifamily.LaunchTemplate) [ // 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)), + // 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("efa"), + InterfaceType: lo.ToPtr(ec2.NetworkInterfaceTypeEfa), Groups: lo.Map(options.SecurityGroups, func(s v1beta1.SecurityGroup, _ int) *string { return aws.String(s.ID) }), } }) diff --git a/test/suites/integration/extended_resources_test.go b/test/suites/integration/extended_resources_test.go index 17da53d5178f..106dd60fa2ff 100644 --- a/test/suites/integration/extended_resources_test.go +++ b/test/suites/integration/extended_resources_test.go @@ -221,6 +221,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), @@ -236,10 +239,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"), }, }, },