From 52252b00c03894cddd55071a10a39a8f967f4aeb Mon Sep 17 00:00:00 2001 From: Brian Dwyer Date: Sun, 5 Dec 2021 15:20:47 -0500 Subject: [PATCH] Try a different approach --- pkg/cloudprovider/aws/instance.go | 17 +++++------------ pkg/cloudprovider/aws/instancetype.go | 7 +++++-- pkg/cloudprovider/aws/suite_test.go | 5 ++++- pkg/cloudprovider/fake/cloudprovider.go | 2 +- pkg/cloudprovider/fake/instancetype.go | 6 +++--- pkg/cloudprovider/types.go | 2 +- .../provisioning/binpacking/packable.go | 9 +++++---- 7 files changed, 24 insertions(+), 24 deletions(-) diff --git a/pkg/cloudprovider/aws/instance.go b/pkg/cloudprovider/aws/instance.go index c0d12e538874..7108d67ddce2 100644 --- a/pkg/cloudprovider/aws/instance.go +++ b/pkg/cloudprovider/aws/instance.go @@ -34,7 +34,6 @@ import ( "github.com/aws/karpenter/pkg/cloudprovider" "github.com/aws/karpenter/pkg/cloudprovider/aws/apis/v1alpha1" "github.com/aws/karpenter/pkg/utils/injection" - "github.com/aws/karpenter/pkg/utils/resources" ) type InstanceProvider struct { @@ -237,17 +236,11 @@ func (p *InstanceProvider) instanceToNode(instance *ec2.Instance, instanceTypes return &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: aws.StringValue(instance.PrivateDnsName), - Labels: func() (labels map[string]string) { - labels = map[string]string{ - v1.LabelTopologyZone: aws.StringValue(instance.Placement.AvailabilityZone), - v1.LabelInstanceTypeStable: aws.StringValue(instance.InstanceType), - v1alpha5.LabelCapacityType: getCapacityType(instance), - } - if instanceType.AWSPodENI() { - labels[resources.AWSPodENI] = "true" - } - return - }(), + Labels: map[string]string{ + v1.LabelTopologyZone: aws.StringValue(instance.Placement.AvailabilityZone), + v1.LabelInstanceTypeStable: aws.StringValue(instance.InstanceType), + v1alpha5.LabelCapacityType: getCapacityType(instance), + }, }, Spec: v1.NodeSpec{ ProviderID: fmt.Sprintf("aws:///%s/%s", aws.StringValue(instance.Placement.AvailabilityZone), aws.StringValue(instance.InstanceId)), diff --git a/pkg/cloudprovider/aws/instancetype.go b/pkg/cloudprovider/aws/instancetype.go index 0b8d528a7058..986e1a2117a4 100644 --- a/pkg/cloudprovider/aws/instancetype.go +++ b/pkg/cloudprovider/aws/instancetype.go @@ -71,10 +71,13 @@ func (i *InstanceType) Pods() *resource.Quantity { return resources.Quantity(fmt.Sprint(*i.NetworkInfo.MaximumNetworkInterfaces*(*i.NetworkInfo.Ipv4AddressesPerInterface-1) + 2)) } -func (i *InstanceType) AWSPodENI() bool { +func (i *InstanceType) AWSPodENI() *resource.Quantity { // Pod ENI is supported by Bare Metal & all Nitro except T3 // https://docs.aws.amazon.com/eks/latest/userguide/security-groups-for-pods.html#supported-instance-types - return (i.BareMetal != nil && *i.BareMetal) || (i.Hypervisor != nil && *i.Hypervisor == "nitro" && !strings.HasPrefix(*i.InstanceType, "t3")) + if (i.BareMetal != nil && *i.BareMetal) || (i.Hypervisor != nil && *i.Hypervisor == "nitro" && !strings.HasPrefix(*i.InstanceType, "t3")) { + return i.Pods() + } + return resources.Quantity("0") } func (i *InstanceType) NvidiaGPUs() *resource.Quantity { diff --git a/pkg/cloudprovider/aws/suite_test.go b/pkg/cloudprovider/aws/suite_test.go index e669dfe0392d..36f9202ed907 100644 --- a/pkg/cloudprovider/aws/suite_test.go +++ b/pkg/cloudprovider/aws/suite_test.go @@ -126,7 +126,10 @@ var _ = Describe("Allocation", func() { test.UnschedulablePod(test.PodOptions{ NodeSelector: map[string]string{ v1.LabelInstanceTypeStable: "t3.large", - resources.AWSPodENI: "true", + }, + ResourceRequirements: v1.ResourceRequirements{ + Requests: v1.ResourceList{resources.AWSPodENI: resource.MustParse("1")}, + Limits: v1.ResourceList{resources.AWSPodENI: resource.MustParse("1")}, }, })) { ExpectNotScheduled(ctx, env.Client, pod) diff --git a/pkg/cloudprovider/fake/cloudprovider.go b/pkg/cloudprovider/fake/cloudprovider.go index 8a20821eb09a..ab8ef950ec8c 100644 --- a/pkg/cloudprovider/fake/cloudprovider.go +++ b/pkg/cloudprovider/fake/cloudprovider.go @@ -84,7 +84,7 @@ func (c *CloudProvider) GetInstanceTypes(_ context.Context, _ *v1alpha5.Constrai }), NewInstanceType(InstanceTypeOptions{ name: "pod-eni-instance-type", - awsPodENI: true, + awsPodENI: resource.MustParse("1"), }), NewInstanceType(InstanceTypeOptions{ name: "small-instance-type", diff --git a/pkg/cloudprovider/fake/instancetype.go b/pkg/cloudprovider/fake/instancetype.go index ba31b8f203ee..7510f8c1e35c 100644 --- a/pkg/cloudprovider/fake/instancetype.go +++ b/pkg/cloudprovider/fake/instancetype.go @@ -68,7 +68,7 @@ type InstanceTypeOptions struct { nvidiaGPUs resource.Quantity amdGPUs resource.Quantity awsNeurons resource.Quantity - awsPodENI bool + awsPodENI resource.Quantity } type InstanceType struct { @@ -111,8 +111,8 @@ func (i *InstanceType) AWSNeurons() *resource.Quantity { return &i.awsNeurons } -func (i *InstanceType) AWSPodENI() bool { - return i.awsPodENI +func (i *InstanceType) AWSPodENI() *resource.Quantity { + return &i.awsPodENI } func (i *InstanceType) Overhead() v1.ResourceList { diff --git a/pkg/cloudprovider/types.go b/pkg/cloudprovider/types.go index d43dc93bb553..46f216d2e821 100644 --- a/pkg/cloudprovider/types.go +++ b/pkg/cloudprovider/types.go @@ -61,7 +61,7 @@ type InstanceType interface { NvidiaGPUs() *resource.Quantity AMDGPUs() *resource.Quantity AWSNeurons() *resource.Quantity - AWSPodENI() bool + AWSPodENI() *resource.Quantity Overhead() v1.ResourceList } diff --git a/pkg/controllers/provisioning/binpacking/packable.go b/pkg/controllers/provisioning/binpacking/packable.go index 21d444c00fcc..a4069d2fd0c6 100644 --- a/pkg/controllers/provisioning/binpacking/packable.go +++ b/pkg/controllers/provisioning/binpacking/packable.go @@ -88,6 +88,7 @@ func PackableFor(i cloudprovider.InstanceType) *Packable { resources.NvidiaGPU: *i.NvidiaGPUs(), resources.AMDGPU: *i.AMDGPUs(), resources.AWSNeuron: *i.AWSNeurons(), + resources.AWSPodENI: *i.AWSPodENI(), v1.ResourcePods: *i.Pods(), }, } @@ -231,17 +232,17 @@ func (p *Packable) validateAWSNeurons(schedule *scheduling.Schedule) error { } func (p *Packable) validateAWSPodENI(schedule *scheduling.Schedule) error { - if p.InstanceType.AWSPodENI() { - return nil - } for _, pod := range schedule.Pods { for _, container := range pod.Spec.Containers { if _, ok := container.Resources.Requests[resources.AWSPodENI]; ok { + if p.InstanceType.AWSPodENI().IsZero() { + return fmt.Errorf("aws pod eni is required") + } return nil } } } - return fmt.Errorf("aws pod eni is not required") + return nil } func packableNames(instanceTypes []*Packable) []string {