Skip to content

Commit

Permalink
fix issue regarding not pre-filtering instance types by provider cons…
Browse files Browse the repository at this point in the history
…traints (#1543)
  • Loading branch information
tzneal authored Mar 18, 2022
1 parent 640573f commit c9e015e
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 0 deletions.
2 changes: 2 additions & 0 deletions pkg/cloudprovider/fake/cloudprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
13 changes: 13 additions & 0 deletions pkg/controllers/provisioning/scheduling/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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() {
Expand All @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions pkg/controllers/provisioning/scheduling/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down

0 comments on commit c9e015e

Please sign in to comment.