Skip to content

Commit

Permalink
feat: Add minValues flexibility to the NodePool requirement. (#5640)
Browse files Browse the repository at this point in the history
Co-authored-by: nikmohan <nikmohan>
  • Loading branch information
nikmohan123 authored Feb 22, 2024
1 parent 65d848a commit 2e3a0c5
Show file tree
Hide file tree
Showing 34 changed files with 1,164 additions and 540 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ require (
k8s.io/utils v0.0.0-20230726121419-3b25d923346b
knative.dev/pkg v0.0.0-20231010144348-ca8c009405dd
sigs.k8s.io/controller-runtime v0.17.2
sigs.k8s.io/karpenter v0.34.1-0.20240220171136-46d3d646ea37
sigs.k8s.io/karpenter v0.34.1-0.20240221213555-0fea7cee3459
)

require (
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -760,8 +760,8 @@ sigs.k8s.io/controller-runtime v0.17.2 h1:FwHwD1CTUemg0pW2otk7/U5/i5m2ymzvOXdbeG
sigs.k8s.io/controller-runtime v0.17.2/go.mod h1:+MngTvIQQQhfXtwfdGw/UOQ/aIaqsYywfCINOtwMO/s=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
sigs.k8s.io/karpenter v0.34.1-0.20240220171136-46d3d646ea37 h1:9BGNR9+y6wBZCUyNtouAmE6IKZ7OTWp0Pbah4sdQnsc=
sigs.k8s.io/karpenter v0.34.1-0.20240220171136-46d3d646ea37/go.mod h1:xHMNckVQTSXN56es2BHr3s4ehyo39tOkcUU3OeUsK8U=
sigs.k8s.io/karpenter v0.34.1-0.20240221213555-0fea7cee3459 h1:WgQezuhG/D1zKHhcPDScd9Fqqe2Y7ZFduVOMvGznZH0=
sigs.k8s.io/karpenter v0.34.1-0.20240221213555-0fea7cee3459/go.mod h1:xHMNckVQTSXN56es2BHr3s4ehyo39tOkcUU3OeUsK8U=
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=
sigs.k8s.io/structured-merge-diff/v4 v4.4.1/go.mod h1:N8hJocpFajUSSeSJ9bOZ77VzejKZaXsTtZo4/u7Io08=
sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
Expand Down
11 changes: 9 additions & 2 deletions pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -482,12 +482,19 @@ spec:
type
items:
description: |-
A node selector requirement is a selector that contains values, a key, and an operator
that relates the key and values.
A node selector requirement with min values is a selector that contains values, a key, an operator that relates the key and values
and minValues that represent the requirement to have at least that many values.
properties:
key:
description: The label key that the selector applies to.
type: string
minValues:
description: |-
This field is ALPHA and can be dropped or replaced at any time
MinValues is the minimum number of unique values required to define the flexibility of the specific requirement.
maximum: 50
minimum: 1
type: integer
operator:
description: |-
Represents a key's relationship to a set of values.
Expand Down
13 changes: 11 additions & 2 deletions pkg/apis/crds/karpenter.sh_nodeclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ spec:
description: Requirements are layered with GetLabels and applied to every node.
items:
description: |-
A node selector requirement is a selector that contains values, a key, and an operator
that relates the key and values.
A node selector requirement with min values is a selector that contains values, a key, an operator that relates the key and values
and minValues that represent the requirement to have at least that many values.
properties:
key:
description: The label key that the selector applies to.
Expand All @@ -227,6 +227,13 @@ spec:
rule: self != "kubernetes.io/hostname"
- message: label domain "karpenter.k8s.aws" is restricted
rule: self in ["karpenter.k8s.aws/instance-encryption-in-transit-supported", "karpenter.k8s.aws/instance-category", "karpenter.k8s.aws/instance-hypervisor", "karpenter.k8s.aws/instance-family", "karpenter.k8s.aws/instance-generation", "karpenter.k8s.aws/instance-local-nvme", "karpenter.k8s.aws/instance-size", "karpenter.k8s.aws/instance-cpu","karpenter.k8s.aws/instance-memory", "karpenter.k8s.aws/instance-network-bandwidth", "karpenter.k8s.aws/instance-gpu-name", "karpenter.k8s.aws/instance-gpu-manufacturer", "karpenter.k8s.aws/instance-gpu-count", "karpenter.k8s.aws/instance-gpu-memory", "karpenter.k8s.aws/instance-accelerator-name", "karpenter.k8s.aws/instance-accelerator-manufacturer", "karpenter.k8s.aws/instance-accelerator-count"] || !self.find("^([^/]+)").endsWith("karpenter.k8s.aws")
minValues:
description: |-
This field is ALPHA and can be dropped or replaced at any time
MinValues is the minimum number of unique values required to define the flexibility of the specific requirement.
maximum: 50
minimum: 1
type: integer
operator:
description: |-
Represents a key's relationship to a set of values.
Expand Down Expand Up @@ -262,6 +269,8 @@ spec:
rule: 'self.all(x, x.operator == ''In'' ? x.values.size() != 0 : true)'
- message: requirements operator 'Gt' or 'Lt' must have a single positive integer value
rule: 'self.all(x, (x.operator == ''Gt'' || x.operator == ''Lt'') ? (x.values.size() == 1 && int(x.values[0]) >= 0) : true)'
- message: requirements with 'minValues' must have at least that many values specified in the 'values' field
rule: 'self.all(x, (x.operator == ''In'' && has(x.minValues)) ? x.values.size() >= x.minValues : true)'
resources:
description: Resources models the resource requirements for the NodeClaim to launch
properties:
Expand Down
13 changes: 11 additions & 2 deletions pkg/apis/crds/karpenter.sh_nodepools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,8 @@ spec:
description: Requirements are layered with GetLabels and applied to every node.
items:
description: |-
A node selector requirement is a selector that contains values, a key, and an operator
that relates the key and values.
A node selector requirement with min values is a selector that contains values, a key, an operator that relates the key and values
and minValues that represent the requirement to have at least that many values.
properties:
key:
description: The label key that the selector applies to.
Expand All @@ -355,6 +355,13 @@ spec:
rule: self != "kubernetes.io/hostname"
- message: label domain "karpenter.k8s.aws" is restricted
rule: self in ["karpenter.k8s.aws/instance-encryption-in-transit-supported", "karpenter.k8s.aws/instance-category", "karpenter.k8s.aws/instance-hypervisor", "karpenter.k8s.aws/instance-family", "karpenter.k8s.aws/instance-generation", "karpenter.k8s.aws/instance-local-nvme", "karpenter.k8s.aws/instance-size", "karpenter.k8s.aws/instance-cpu","karpenter.k8s.aws/instance-memory", "karpenter.k8s.aws/instance-network-bandwidth", "karpenter.k8s.aws/instance-gpu-name", "karpenter.k8s.aws/instance-gpu-manufacturer", "karpenter.k8s.aws/instance-gpu-count", "karpenter.k8s.aws/instance-gpu-memory", "karpenter.k8s.aws/instance-accelerator-name", "karpenter.k8s.aws/instance-accelerator-manufacturer", "karpenter.k8s.aws/instance-accelerator-count"] || !self.find("^([^/]+)").endsWith("karpenter.k8s.aws")
minValues:
description: |-
This field is ALPHA and can be dropped or replaced at any time
MinValues is the minimum number of unique values required to define the flexibility of the specific requirement.
maximum: 50
minimum: 1
type: integer
operator:
description: |-
Represents a key's relationship to a set of values.
Expand Down Expand Up @@ -390,6 +397,8 @@ spec:
rule: 'self.all(x, x.operator == ''In'' ? x.values.size() != 0 : true)'
- message: requirements operator 'Gt' or 'Lt' must have a single positive integer value
rule: 'self.all(x, (x.operator == ''Gt'' || x.operator == ''Lt'') ? (x.values.size() == 1 && int(x.values[0]) >= 0) : true)'
- message: requirements with 'minValues' must have at least that many values specified in the 'values' field
rule: 'self.all(x, (x.operator == ''In'' && has(x.minValues)) ? x.values.size() >= x.minValues : true)'
resources:
description: Resources models the resource requirements for the NodeClaim to launch
properties:
Expand Down
6 changes: 4 additions & 2 deletions pkg/apis/v1beta1/ec2nodeclass_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ limitations under the License.

package v1beta1

import v1 "k8s.io/api/core/v1"
import (
corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1"
)

// Subnet contains resolved Subnet selector values utilized for node launch
type Subnet struct {
Expand Down Expand Up @@ -46,7 +48,7 @@ type AMI struct {
Name string `json:"name,omitempty"`
// Requirements of the AMI to be utilized on an instance type
// +required
Requirements []v1.NodeSelectorRequirement `json:"requirements"`
Requirements []corev1beta1.NodeSelectorRequirementWithMinValues `json:"requirements"`
}

// EC2NodeClassStatus contains the resolved state of the EC2NodeClass
Expand Down
16 changes: 9 additions & 7 deletions pkg/apis/v1beta1/nodepool_validation_cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,12 @@ var _ = Describe("CEL/Validation", func() {
Kind: "NodeClaim",
Name: "default",
},
Requirements: []v1.NodeSelectorRequirement{
Requirements: []v1beta1.NodeSelectorRequirementWithMinValues{
{
Key: v1beta1.CapacityTypeLabelKey,
Operator: v1.NodeSelectorOpExists,
NodeSelectorRequirement: v1.NodeSelectorRequirement{
Key: v1beta1.CapacityTypeLabelKey,
Operator: v1.NodeSelectorOpExists,
},
},
},
},
Expand All @@ -58,8 +60,8 @@ var _ = Describe("CEL/Validation", func() {
It("should allow restricted domains exceptions", func() {
oldNodePool := nodePool.DeepCopy()
for label := range v1beta1.LabelDomainExceptions {
nodePool.Spec.Template.Spec.Requirements = []v1.NodeSelectorRequirement{
{Key: label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}},
nodePool.Spec.Template.Spec.Requirements = []v1beta1.NodeSelectorRequirementWithMinValues{
{NodeSelectorRequirement: v1.NodeSelectorRequirement{Key: label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}},
}
Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
Expect(nodePool.RuntimeValidate()).To(Succeed())
Expand All @@ -70,8 +72,8 @@ var _ = Describe("CEL/Validation", func() {
It("should allow well known label exceptions", func() {
oldNodePool := nodePool.DeepCopy()
for label := range v1beta1.WellKnownLabels.Difference(sets.New(v1beta1.NodePoolLabelKey)) {
nodePool.Spec.Template.Spec.Requirements = []v1.NodeSelectorRequirement{
{Key: label, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}},
nodePool.Spec.Template.Spec.Requirements = []v1beta1.NodeSelectorRequirementWithMinValues{
{NodeSelectorRequirement: v1.NodeSelectorRequirement{Key: label, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}},
}
Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
Expect(nodePool.RuntimeValidate()).To(Succeed())
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/cloudprovider/cloudprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ func (c *CloudProvider) resolveInstanceTypes(ctx context.Context, nodeClaim *cor
if err != nil {
return nil, fmt.Errorf("getting instance types, %w", err)
}
reqs := scheduling.NewNodeSelectorRequirements(nodeClaim.Spec.Requirements...)
reqs := scheduling.NewNodeSelectorRequirementsWithMinValues(nodeClaim.Spec.Requirements...)
return lo.Filter(instanceTypes, func(i *cloudprovider.InstanceType, _ int) bool {
return reqs.Compatible(i.Requirements, scheduling.AllowUndefinedWellKnownLabels) == nil &&
len(i.Offerings.Compatible(reqs).Available()) > 0 &&
Expand Down
Loading

0 comments on commit 2e3a0c5

Please sign in to comment.