From c9e015e45ac2fdfeb6c762565f06f62b393e2c0f Mon Sep 17 00:00:00 2001 From: Todd Date: Fri, 18 Mar 2022 17:18:18 -0500 Subject: [PATCH] fix issue regarding not pre-filtering instance types by provider constraints (#1543) --- pkg/cloudprovider/fake/cloudprovider.go | 2 ++ pkg/controllers/provisioning/scheduling/node.go | 13 +++++++++++++ .../provisioning/scheduling/suite_test.go | 14 ++++++++++++++ 3 files changed, 29 insertions(+) diff --git a/pkg/cloudprovider/fake/cloudprovider.go b/pkg/cloudprovider/fake/cloudprovider.go index 4231b16075a5..4bc65216472f 100644 --- a/pkg/cloudprovider/fake/cloudprovider.go +++ b/pkg/cloudprovider/fake/cloudprovider.go @@ -109,6 +109,8 @@ func (c *CloudProvider) GetInstanceTypes(_ context.Context, _ *v1alpha5.Provider NewInstanceType(InstanceTypeOptions{ Name: "arm-instance-type", Architecture: "arm64", + CPU: resource.MustParse("16"), + Memory: resource.MustParse("128Gi"), }), }, nil } diff --git a/pkg/controllers/provisioning/scheduling/node.go b/pkg/controllers/provisioning/scheduling/node.go index dd5fc6889055..c885e9b4425d 100644 --- a/pkg/controllers/provisioning/scheduling/node.go +++ b/pkg/controllers/provisioning/scheduling/node.go @@ -19,6 +19,8 @@ import ( v1 "k8s.io/api/core/v1" + "github.com/aws/karpenter/pkg/utils/sets" + "github.com/aws/karpenter/pkg/apis/provisioning/v1alpha5" "github.com/aws/karpenter/pkg/cloudprovider" "github.com/aws/karpenter/pkg/utils/resources" @@ -41,6 +43,16 @@ func NewNode(constraints *v1alpha5.Constraints, instanceTypeOptions []cloudprovi if !instanceTypes.Has(it.Name()) { continue } + + // provisioner constraints can't exclude the instance type by arch or OS + if !constraints.Requirements.Get(v1.LabelArchStable).Has(it.Architecture()) { + continue + } + if constraints.Requirements.Get(v1.LabelOSStable).Intersection( + sets.NewSet(it.OperatingSystems().List()...)).Len() == 0 { + continue + } + included := false // and the instance type must have some valid offering combination per the provisioner constraints for _, off := range it.Offerings() { @@ -61,6 +73,7 @@ func NewNode(constraints *v1alpha5.Constraints, instanceTypeOptions []cloudprovi } return n } + func (n Node) Compatible(pod *v1.Pod) error { podRequirements := v1alpha5.NewPodRequirements(pod) if err := n.Constraints.Requirements.Compatible(podRequirements); err != nil { diff --git a/pkg/controllers/provisioning/scheduling/suite_test.go b/pkg/controllers/provisioning/scheduling/suite_test.go index 676f594a37b5..711687c0ec40 100644 --- a/pkg/controllers/provisioning/scheduling/suite_test.go +++ b/pkg/controllers/provisioning/scheduling/suite_test.go @@ -1585,6 +1585,20 @@ var _ = Describe("Instance Type Compatibility", func() { } Expect(nodeNames.Len()).To(Equal(2)) }) + It("should exclude instance types that are not supported by the provider constraints (arch)", func() { + provisioner.Spec.Requirements.Requirements = []v1.NodeSelectorRequirement{ + { + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{v1alpha5.ArchitectureAmd64}, + }, + } + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, + test.UnschedulablePod(test.PodOptions{ResourceRequirements: v1.ResourceRequirements{ + Limits: map[v1.ResourceName]resource.Quantity{v1.ResourceCPU: resource.MustParse("14")}}})) + // only the ARM instance has enough CPU, but it's not allowed per the provisioner + ExpectNotScheduled(ctx, env.Client, pod[0]) + }) It("should launch pods with different operating systems on different instances", func() { provisioner.Spec.Requirements.Requirements = []v1.NodeSelectorRequirement{ {