From 167c83a70111190283b9589f26fd0c9a1c1b8440 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Sun, 8 Jun 2025 21:04:32 -0700 Subject: [PATCH 01/11] Add NodeOverlay CRDs --- kwok/charts/crds/_.yaml | 13 ++ .../crds/karpenter.sh_nodeoverlays.yaml | 123 ++++++++++++++++++ pkg/apis/apis.go | 5 +- pkg/apis/crds/_.yaml | 13 ++ pkg/apis/crds/karpenter.sh_nodeoverlays.yaml | 123 ++++++++++++++++++ pkg/apis/v1/nodeclaim_defaults.go | 22 ---- .../nodepool_defaults.go => v1alpha1/doc.go} | 22 +++- pkg/apis/v1alpha1/overlay.go | 86 ++++++++++++ pkg/apis/v1alpha1/zz_generated.deepcopy.go | 118 +++++++++++++++++ 9 files changed, 498 insertions(+), 27 deletions(-) create mode 100644 kwok/charts/crds/_.yaml create mode 100644 kwok/charts/crds/karpenter.sh_nodeoverlays.yaml create mode 100644 pkg/apis/crds/_.yaml create mode 100644 pkg/apis/crds/karpenter.sh_nodeoverlays.yaml delete mode 100644 pkg/apis/v1/nodeclaim_defaults.go rename pkg/apis/{v1/nodepool_defaults.go => v1alpha1/doc.go} (52%) create mode 100644 pkg/apis/v1alpha1/overlay.go create mode 100644 pkg/apis/v1alpha1/zz_generated.deepcopy.go diff --git a/kwok/charts/crds/_.yaml b/kwok/charts/crds/_.yaml new file mode 100644 index 0000000000..2d0f0c1e91 --- /dev/null +++ b/kwok/charts/crds/_.yaml @@ -0,0 +1,13 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.18.0 +spec: + group: "" + names: + kind: "" + plural: "" + scope: "" + versions: null diff --git a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml new file mode 100644 index 0000000000..97985c065b --- /dev/null +++ b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml @@ -0,0 +1,123 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.18.0 + name: nodeoverlays.karpenter.sh +spec: + group: karpenter.sh + names: + categories: + - karpenter + kind: NodeOverlay + listKind: NodeOverlayList + plural: nodeoverlays + singular: nodeoverlay + scope: Cluster + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + properties: + capacity: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: Capacity adds extended resource to instances types based + on the selector provided + type: object + priceAdjustment: + description: |- + UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). + Update Validation + type: string + requirements: + 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. + properties: + key: + description: The label key that the selector applies to. + type: string + operator: + description: |- + Represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists, DoesNotExist. Gt, and Lt. + type: string + values: + description: |- + An array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. If the operator is Gt or Lt, the values + array must have a single element, which will be interpreted as an integer. + This array is replaced during a strategic merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + maxItems: 100 + type: array + x-kubernetes-validations: + - message: requirements with operator 'In' must have a value defined + 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)' + weight: + description: |- + Weight is the priority given to the nodeoverlay while overriding node attributes. A higher + numerical weight indicates that this nodeoverlay will be ordered + ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight + will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, + we will merge them in alphabetical order. + format: int64 + maximum: 100 + minimum: 1 + type: integer + required: + - requirements + type: object + x-kubernetes-validations: + - message: 'need to define a capacity or priceAdjustment field ' + rule: has(self.capacity) || has(self.priceAdjustment) + required: + - spec + type: object + served: true + storage: true diff --git a/pkg/apis/apis.go b/pkg/apis/apis.go index 577f7bf099..c062f7fcb3 100644 --- a/pkg/apis/apis.go +++ b/pkg/apis/apis.go @@ -34,8 +34,11 @@ var ( NodePoolCRD []byte //go:embed crds/karpenter.sh_nodeclaims.yaml NodeClaimCRD []byte - CRDs = []*apiextensionsv1.CustomResourceDefinition{ + //go:embed crds/karpenter.sh_nodeoverlays.yaml + NodeOverlayCRD []byte + CRDs = []*apiextensionsv1.CustomResourceDefinition{ object.Unmarshal[apiextensionsv1.CustomResourceDefinition](NodePoolCRD), object.Unmarshal[apiextensionsv1.CustomResourceDefinition](NodeClaimCRD), + object.Unmarshal[apiextensionsv1.CustomResourceDefinition](NodeOverlayCRD), } ) diff --git a/pkg/apis/crds/_.yaml b/pkg/apis/crds/_.yaml new file mode 100644 index 0000000000..2d0f0c1e91 --- /dev/null +++ b/pkg/apis/crds/_.yaml @@ -0,0 +1,13 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.18.0 +spec: + group: "" + names: + kind: "" + plural: "" + scope: "" + versions: null diff --git a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml new file mode 100644 index 0000000000..97985c065b --- /dev/null +++ b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml @@ -0,0 +1,123 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.18.0 + name: nodeoverlays.karpenter.sh +spec: + group: karpenter.sh + names: + categories: + - karpenter + kind: NodeOverlay + listKind: NodeOverlayList + plural: nodeoverlays + singular: nodeoverlay + scope: Cluster + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + properties: + capacity: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: Capacity adds extended resource to instances types based + on the selector provided + type: object + priceAdjustment: + description: |- + UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). + Update Validation + type: string + requirements: + 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. + properties: + key: + description: The label key that the selector applies to. + type: string + operator: + description: |- + Represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists, DoesNotExist. Gt, and Lt. + type: string + values: + description: |- + An array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. If the operator is Gt or Lt, the values + array must have a single element, which will be interpreted as an integer. + This array is replaced during a strategic merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + maxItems: 100 + type: array + x-kubernetes-validations: + - message: requirements with operator 'In' must have a value defined + 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)' + weight: + description: |- + Weight is the priority given to the nodeoverlay while overriding node attributes. A higher + numerical weight indicates that this nodeoverlay will be ordered + ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight + will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, + we will merge them in alphabetical order. + format: int64 + maximum: 100 + minimum: 1 + type: integer + required: + - requirements + type: object + x-kubernetes-validations: + - message: 'need to define a capacity or priceAdjustment field ' + rule: has(self.capacity) || has(self.priceAdjustment) + required: + - spec + type: object + served: true + storage: true diff --git a/pkg/apis/v1/nodeclaim_defaults.go b/pkg/apis/v1/nodeclaim_defaults.go deleted file mode 100644 index a7f9938907..0000000000 --- a/pkg/apis/v1/nodeclaim_defaults.go +++ /dev/null @@ -1,22 +0,0 @@ -/* -Copyright The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1 - -import "context" - -// SetDefaults for the NodeClaim -func (in *NodeClaim) SetDefaults(_ context.Context) {} diff --git a/pkg/apis/v1/nodepool_defaults.go b/pkg/apis/v1alpha1/doc.go similarity index 52% rename from pkg/apis/v1/nodepool_defaults.go rename to pkg/apis/v1alpha1/doc.go index 98d0990aee..5b35a61c12 100644 --- a/pkg/apis/v1/nodepool_defaults.go +++ b/pkg/apis/v1alpha1/doc.go @@ -14,11 +14,25 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1 +// +k8s:openapi-gen=true +// +k8s:deepcopy-gen=package,register +// +k8s:defaulter-gen=TypeMeta +// +groupName=karpenter.sh +package v1alpha1 // doc.go is discovered by codegen import ( - "context" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes/scheme" + + "sigs.k8s.io/karpenter/pkg/apis" ) -// SetDefaults for the NodePool -func (in *NodePool) SetDefaults(_ context.Context) {} +func init() { + gv := schema.GroupVersion{Group: apis.Group, Version: "v1alpha1"} + v1.AddToGroupVersion(scheme.Scheme, gv) + scheme.Scheme.AddKnownTypes(gv, + &NodeOverlay{}, + &NodeOverlayList{}, + ) +} diff --git a/pkg/apis/v1alpha1/overlay.go b/pkg/apis/v1alpha1/overlay.go new file mode 100644 index 0000000000..17f96e58a0 --- /dev/null +++ b/pkg/apis/v1alpha1/overlay.go @@ -0,0 +1,86 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "sort" + + "github.com/samber/lo" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type NodeOverlaySpec struct { + // Requirements are layered with GetLabels and applied to every node. + // +kubebuilder:validation:XValidation:message="requirements with operator 'In' must have a value defined",rule="self.all(x, x.operator == 'In' ? x.values.size() != 0 : true)" + // +kubebuilder:validation:XValidation: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)" + // +kubebuilder:validation:XValidation: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)" + // +kubebuilder:validation:MaxItems:=100 + // +required + Requirements []v1.NodeSelectorRequirement `json:"requirements,omitempty"` + // UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). + // Update Validation + // +optional + PriceAdjustment string `json:"priceAdjustment,omitempty"` + // Capacity adds extended resource to instances types based on the selector provided + // +optional + Capacity v1.ResourceList `json:"capacity,omitempty"` + // Weight is the priority given to the nodeoverlay while overriding node attributes. A higher + // numerical weight indicates that this nodeoverlay will be ordered + // ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight + // will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, + // we will merge them in alphabetical order. + // +kubebuilder:validation:Minimum:=1 + // +kubebuilder:validation:Maximum:=100 + // +optional + Weight *int64 `json:"weight,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:storageversion +// +kubebuilder:resource:path=nodeoverlays,scope=Cluster,categories=karpenter +type NodeOverlay struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // +kubebuilder:validation:XValidation:message="need to define a capacity or priceAdjustment field ",rule="has(self.capacity) || has(self.priceAdjustment)" + // +required + Spec NodeOverlaySpec `json:"spec"` +} + +// +kubebuilder:object:root=true +type NodeOverlayList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []NodeOverlay `json:"items"` +} + +// OrderByWeight orders the NodeOverlays in the provided slice by their priority weight in-place. This priority evaluates +// the following things in precedence order: +// 1. NodeOverlays that have a larger weight are ordered first +// 2. If two NodeOverlays have the same weight, then the NodePool with the name later in the alphabet will come first +func (nol *NodeOverlayList) OrderByWeight() { + sort.Slice(nol.Items, func(a, b int) bool { + weightA := lo.FromPtr(nol.Items[a].Spec.Weight) + weightB := lo.FromPtr(nol.Items[b].Spec.Weight) + if weightA == weightB { + // Order NodePools by name for a consistent ordering when sorting equal weight + return nol.Items[a].Name > nol.Items[b].Name + } + return weightA > weightB + }) +} diff --git a/pkg/apis/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/v1alpha1/zz_generated.deepcopy.go new file mode 100644 index 0000000000..096ab0536a --- /dev/null +++ b/pkg/apis/v1alpha1/zz_generated.deepcopy.go @@ -0,0 +1,118 @@ +//go:build !ignore_autogenerated + +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + "k8s.io/api/core/v1" + runtime "k8s.io/apimachinery/pkg/runtime" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NodeOverlay) DeepCopyInto(out *NodeOverlay) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeOverlay. +func (in *NodeOverlay) DeepCopy() *NodeOverlay { + if in == nil { + return nil + } + out := new(NodeOverlay) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *NodeOverlay) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NodeOverlayList) DeepCopyInto(out *NodeOverlayList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]NodeOverlay, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeOverlayList. +func (in *NodeOverlayList) DeepCopy() *NodeOverlayList { + if in == nil { + return nil + } + out := new(NodeOverlayList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *NodeOverlayList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NodeOverlaySpec) DeepCopyInto(out *NodeOverlaySpec) { + *out = *in + if in.Requirements != nil { + in, out := &in.Requirements, &out.Requirements + *out = make([]v1.NodeSelectorRequirement, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Capacity != nil { + in, out := &in.Capacity, &out.Capacity + *out = make(v1.ResourceList, len(*in)) + for key, val := range *in { + (*out)[key] = val.DeepCopy() + } + } + if in.Weight != nil { + in, out := &in.Weight, &out.Weight + *out = new(int64) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeOverlaySpec. +func (in *NodeOverlaySpec) DeepCopy() *NodeOverlaySpec { + if in == nil { + return nil + } + out := new(NodeOverlaySpec) + in.DeepCopyInto(out) + return out +} From 06c1cf1232be534ca2653e6c01a0d8c057855a38 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Mon, 9 Jun 2025 13:24:39 -0700 Subject: [PATCH 02/11] Add testing for Node Overlay CRD --- hack/validation/requirements.sh | 18 +- kwok/charts/crds/_.yaml | 13 - .../crds/karpenter.sh_nodeoverlays.yaml | 280 +++++++++++------- pkg/apis/crds/_.yaml | 13 - pkg/apis/crds/karpenter.sh_nodeoverlays.yaml | 280 +++++++++++------- pkg/apis/v1alpha1/nodelay_status.go | 50 ++++ pkg/apis/v1alpha1/nodeoverlay_default_test.go | 60 ++++ pkg/apis/v1alpha1/nodeoverlay_validation.go | 124 ++++++++ .../nodeoverlay_validation_cel_test.go | 274 +++++++++++++++++ pkg/apis/v1alpha1/overlay.go | 5 +- pkg/apis/v1alpha1/suite_test.go | 51 ++++ pkg/apis/v1alpha1/zz_generated.deepcopy.go | 24 ++ 12 files changed, 955 insertions(+), 237 deletions(-) delete mode 100644 kwok/charts/crds/_.yaml delete mode 100644 pkg/apis/crds/_.yaml create mode 100644 pkg/apis/v1alpha1/nodelay_status.go create mode 100644 pkg/apis/v1alpha1/nodeoverlay_default_test.go create mode 100644 pkg/apis/v1alpha1/nodeoverlay_validation.go create mode 100644 pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go create mode 100644 pkg/apis/v1alpha1/suite_test.go diff --git a/hack/validation/requirements.sh b/hack/validation/requirements.sh index 43c53d834f..b3de27e075 100755 --- a/hack/validation/requirements.sh +++ b/hack/validation/requirements.sh @@ -30,4 +30,20 @@ yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.tem yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.requirements.items.properties.operator.enum += ["In","NotIn","Exists","DoesNotExist","Gt","Lt"]' -i pkg/apis/crds/karpenter.sh_nodepools.yaml ## Valid requirement value check yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.requirements.items.properties.values.maxLength = 63' -i pkg/apis/crds/karpenter.sh_nodepools.yaml -yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.requirements.items.properties.values.pattern = "^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$"' -i pkg/apis/crds/karpenter.sh_nodepools.yaml \ No newline at end of file +yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.requirements.items.properties.values.pattern = "^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$"' -i pkg/apis/crds/karpenter.sh_nodepools.yaml + +# NodeOverlay Validation: +## Qualified name for requirement keys +yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.key.maxLength = 316' -i pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.key.pattern = "^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(\/))?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$"' -i pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +## checking for restricted labels while filtering out well-known labels +yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.key.x-kubernetes-validations += [ + {"message": "label domain \"kubernetes.io\" is restricted", "rule": "self in [\"beta.kubernetes.io/instance-type\", \"failure-domain.beta.kubernetes.io/region\", \"beta.kubernetes.io/os\", \"beta.kubernetes.io/arch\", \"failure-domain.beta.kubernetes.io/zone\", \"topology.kubernetes.io/zone\", \"topology.kubernetes.io/region\", \"node.kubernetes.io/instance-type\", \"kubernetes.io/arch\", \"kubernetes.io/os\", \"node.kubernetes.io/windows-build\"] || self.find(\"^([^/]+)\").endsWith(\"node.kubernetes.io\") || self.find(\"^([^/]+)\").endsWith(\"node-restriction.kubernetes.io\") || !self.find(\"^([^/]+)\").endsWith(\"kubernetes.io\")"}, + {"message": "label domain \"k8s.io\" is restricted", "rule": "self.find(\"^([^/]+)\").endsWith(\"kops.k8s.io\") || !self.find(\"^([^/]+)\").endsWith(\"k8s.io\")"}, + {"message": "label domain \"karpenter.sh\" is restricted", "rule": "self in [\"karpenter.sh/capacity-type\", \"karpenter.sh/nodepool\"] || !self.find(\"^([^/]+)\").endsWith(\"karpenter.sh\")"}, + {"message": "label \"kubernetes.io/hostname\" is restricted", "rule": "self != \"kubernetes.io/hostname\""}]' -i pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +## operator enum values +yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.operator.enum += ["In","NotIn","Exists","DoesNotExist","Gt","Lt"]' -i pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +## Valid requirement value check +yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.values.maxLength = 63' -i pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.values.pattern = "^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$"' -i pkg/apis/crds/karpenter.sh_nodeoverlays.yaml \ No newline at end of file diff --git a/kwok/charts/crds/_.yaml b/kwok/charts/crds/_.yaml deleted file mode 100644 index 2d0f0c1e91..0000000000 --- a/kwok/charts/crds/_.yaml +++ /dev/null @@ -1,13 +0,0 @@ ---- -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - annotations: - controller-gen.kubebuilder.io/version: v0.18.0 -spec: - group: "" - names: - kind: "" - plural: "" - scope: "" - versions: null diff --git a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml index 97985c065b..832cf65d90 100644 --- a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml +++ b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml @@ -9,115 +9,187 @@ spec: group: karpenter.sh names: categories: - - karpenter + - karpenter kind: NodeOverlay listKind: NodeOverlayList plural: nodeoverlays singular: nodeoverlay scope: Cluster versions: - - name: v1alpha1 - schema: - openAPIV3Schema: - properties: - apiVersion: - description: |- - APIVersion defines the versioned schema of this representation of an object. - Servers should convert recognized schemas to the latest internal value, and - may reject unrecognized values. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources - type: string - kind: - description: |- - Kind is a string value representing the REST resource this object represents. - Servers may infer this from the endpoint the client submits requests to. - Cannot be updated. - In CamelCase. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds - type: string - metadata: - type: object - spec: - properties: - capacity: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: Capacity adds extended resource to instances types based - on the selector provided - type: object - priceAdjustment: - description: |- - UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). - Update Validation - type: string - requirements: - description: Requirements are layered with GetLabels and applied to - every node. - items: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + properties: + capacity: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: Capacity adds extended resource to instances types based on the selector provided + type: object + priceAdjustment: + default: 100% description: |- - A node selector requirement is a selector that contains values, a key, and an operator - that relates the key and values. - properties: - key: - description: The label key that the selector applies to. - type: string - operator: - description: |- - Represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists, DoesNotExist. Gt, and Lt. - type: string - values: - description: |- - An array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. If the operator is Gt or Lt, the values - array must have a single element, which will be interpreted as an integer. - This array is replaced during a strategic merge patch. - items: + UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). + Update Validation + type: string + requirements: + 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. + properties: + key: + description: The label key that the selector applies to. type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - maxItems: 100 - type: array - x-kubernetes-validations: - - message: requirements with operator 'In' must have a value defined - 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)' - weight: - description: |- - Weight is the priority given to the nodeoverlay while overriding node attributes. A higher - numerical weight indicates that this nodeoverlay will be ordered - ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight - will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, - we will merge them in alphabetical order. - format: int64 - maximum: 100 - minimum: 1 - type: integer - required: - - requirements - type: object - x-kubernetes-validations: - - message: 'need to define a capacity or priceAdjustment field ' - rule: has(self.capacity) || has(self.priceAdjustment) - required: - - spec - type: object - served: true - storage: true + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(\/))?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$ + x-kubernetes-validations: + - message: label domain "kubernetes.io" is restricted + rule: self in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || self.find("^([^/]+)").endsWith("node.kubernetes.io") || self.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !self.find("^([^/]+)").endsWith("kubernetes.io") + - message: label domain "k8s.io" is restricted + rule: self.find("^([^/]+)").endsWith("kops.k8s.io") || !self.find("^([^/]+)").endsWith("k8s.io") + - message: label domain "karpenter.sh" is restricted + rule: self in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !self.find("^([^/]+)").endsWith("karpenter.sh") + - message: label "kubernetes.io/hostname" is restricted + rule: self != "kubernetes.io/hostname" + operator: + description: |- + Represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists, DoesNotExist. Gt, and Lt. + type: string + enum: + - In + - NotIn + - Exists + - DoesNotExist + - Gt + - Lt + values: + description: |- + An array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. If the operator is Gt or Lt, the values + array must have a single element, which will be interpreted as an integer. + This array is replaced during a strategic merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + maxLength: 63 + pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$ + required: + - key + - operator + type: object + maxItems: 100 + type: array + x-kubernetes-validations: + - message: requirements with operator 'In' must have a value defined + 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)' + weight: + description: |- + Weight is the priority given to the nodeoverlay while overriding node attributes. A higher + numerical weight indicates that this nodeoverlay will be ordered + ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight + will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, + we will merge them in alphabetical order. + format: int64 + maximum: 100 + minimum: 1 + type: integer + required: + - requirements + type: object + x-kubernetes-validations: + - message: 'need to define a capacity or priceAdjustment field ' + rule: has(self.capacity) || has(self.priceAdjustment) + status: + description: NodeOverlayStatus defines the observed state of NodeOverlay + properties: + conditions: + description: Conditions contains signals for health and readiness + items: + description: Condition aliases the upstream type and adds additional helper methods + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + type: object + required: + - spec + type: object + served: true + storage: true diff --git a/pkg/apis/crds/_.yaml b/pkg/apis/crds/_.yaml deleted file mode 100644 index 2d0f0c1e91..0000000000 --- a/pkg/apis/crds/_.yaml +++ /dev/null @@ -1,13 +0,0 @@ ---- -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - annotations: - controller-gen.kubebuilder.io/version: v0.18.0 -spec: - group: "" - names: - kind: "" - plural: "" - scope: "" - versions: null diff --git a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml index 97985c065b..832cf65d90 100644 --- a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml @@ -9,115 +9,187 @@ spec: group: karpenter.sh names: categories: - - karpenter + - karpenter kind: NodeOverlay listKind: NodeOverlayList plural: nodeoverlays singular: nodeoverlay scope: Cluster versions: - - name: v1alpha1 - schema: - openAPIV3Schema: - properties: - apiVersion: - description: |- - APIVersion defines the versioned schema of this representation of an object. - Servers should convert recognized schemas to the latest internal value, and - may reject unrecognized values. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources - type: string - kind: - description: |- - Kind is a string value representing the REST resource this object represents. - Servers may infer this from the endpoint the client submits requests to. - Cannot be updated. - In CamelCase. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds - type: string - metadata: - type: object - spec: - properties: - capacity: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: Capacity adds extended resource to instances types based - on the selector provided - type: object - priceAdjustment: - description: |- - UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). - Update Validation - type: string - requirements: - description: Requirements are layered with GetLabels and applied to - every node. - items: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + properties: + capacity: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: Capacity adds extended resource to instances types based on the selector provided + type: object + priceAdjustment: + default: 100% description: |- - A node selector requirement is a selector that contains values, a key, and an operator - that relates the key and values. - properties: - key: - description: The label key that the selector applies to. - type: string - operator: - description: |- - Represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists, DoesNotExist. Gt, and Lt. - type: string - values: - description: |- - An array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. If the operator is Gt or Lt, the values - array must have a single element, which will be interpreted as an integer. - This array is replaced during a strategic merge patch. - items: + UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). + Update Validation + type: string + requirements: + 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. + properties: + key: + description: The label key that the selector applies to. type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - maxItems: 100 - type: array - x-kubernetes-validations: - - message: requirements with operator 'In' must have a value defined - 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)' - weight: - description: |- - Weight is the priority given to the nodeoverlay while overriding node attributes. A higher - numerical weight indicates that this nodeoverlay will be ordered - ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight - will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, - we will merge them in alphabetical order. - format: int64 - maximum: 100 - minimum: 1 - type: integer - required: - - requirements - type: object - x-kubernetes-validations: - - message: 'need to define a capacity or priceAdjustment field ' - rule: has(self.capacity) || has(self.priceAdjustment) - required: - - spec - type: object - served: true - storage: true + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(\/))?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$ + x-kubernetes-validations: + - message: label domain "kubernetes.io" is restricted + rule: self in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || self.find("^([^/]+)").endsWith("node.kubernetes.io") || self.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !self.find("^([^/]+)").endsWith("kubernetes.io") + - message: label domain "k8s.io" is restricted + rule: self.find("^([^/]+)").endsWith("kops.k8s.io") || !self.find("^([^/]+)").endsWith("k8s.io") + - message: label domain "karpenter.sh" is restricted + rule: self in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !self.find("^([^/]+)").endsWith("karpenter.sh") + - message: label "kubernetes.io/hostname" is restricted + rule: self != "kubernetes.io/hostname" + operator: + description: |- + Represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists, DoesNotExist. Gt, and Lt. + type: string + enum: + - In + - NotIn + - Exists + - DoesNotExist + - Gt + - Lt + values: + description: |- + An array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. If the operator is Gt or Lt, the values + array must have a single element, which will be interpreted as an integer. + This array is replaced during a strategic merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + maxLength: 63 + pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$ + required: + - key + - operator + type: object + maxItems: 100 + type: array + x-kubernetes-validations: + - message: requirements with operator 'In' must have a value defined + 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)' + weight: + description: |- + Weight is the priority given to the nodeoverlay while overriding node attributes. A higher + numerical weight indicates that this nodeoverlay will be ordered + ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight + will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, + we will merge them in alphabetical order. + format: int64 + maximum: 100 + minimum: 1 + type: integer + required: + - requirements + type: object + x-kubernetes-validations: + - message: 'need to define a capacity or priceAdjustment field ' + rule: has(self.capacity) || has(self.priceAdjustment) + status: + description: NodeOverlayStatus defines the observed state of NodeOverlay + properties: + conditions: + description: Conditions contains signals for health and readiness + items: + description: Condition aliases the upstream type and adds additional helper methods + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + type: object + required: + - spec + type: object + served: true + storage: true diff --git a/pkg/apis/v1alpha1/nodelay_status.go b/pkg/apis/v1alpha1/nodelay_status.go new file mode 100644 index 0000000000..502764f1f8 --- /dev/null +++ b/pkg/apis/v1alpha1/nodelay_status.go @@ -0,0 +1,50 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "github.com/awslabs/operatorpkg/status" +) + +// Note(Remove after decision point): Only set this up for runtime validation I will need to assess if this is necessary, but I get the feeling it will be + +const ( + // ConditionTypeValidationSucceeded = "ValidationSucceeded" condition indicates that the + // runtime-based configuration is valid for this NodeOverlay + ConditionTypeValidationSucceeded = "ValidationSucceeded" +) + +// NodeOverlayStatus defines the observed state of NodeOverlay +type NodeOverlayStatus struct { + // Conditions contains signals for health and readiness + // +optional + Conditions []status.Condition `json:"conditions,omitempty"` +} + +func (in *NodeOverlay) StatusConditions() status.ConditionSet { + return status.NewReadyConditions( + ConditionTypeValidationSucceeded, + ).For(in) +} + +func (in *NodeOverlay) GetConditions() []status.Condition { + return in.Status.Conditions +} + +func (in *NodeOverlay) SetConditions(conditions []status.Condition) { + in.Status.Conditions = conditions +} diff --git a/pkg/apis/v1alpha1/nodeoverlay_default_test.go b/pkg/apis/v1alpha1/nodeoverlay_default_test.go new file mode 100644 index 0000000000..c816470f33 --- /dev/null +++ b/pkg/apis/v1alpha1/nodeoverlay_default_test.go @@ -0,0 +1,60 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1_test + +import ( + "strings" + + "github.com/Pallinder/go-randomdata" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "sigs.k8s.io/controller-runtime/pkg/client" + + v1 "sigs.k8s.io/karpenter/pkg/apis/v1" + . "sigs.k8s.io/karpenter/pkg/apis/v1alpha1" +) + +var _ = Describe("CEL/Default", func() { + var nodeOverlay *NodeOverlay + + BeforeEach(func() { + nodeOverlay = &NodeOverlay{ + ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}, + Spec: NodeOverlaySpec{ + Requirements: []corev1.NodeSelectorRequirement{ + { + Key: v1.CapacityTypeLabelKey, + Operator: corev1.NodeSelectorOpExists, + }, + }, + }, + } + }) + Context("Defaults/TopLevel", func() { + It("should default the priceAdjustment field when undefined", func() { + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(nodeOverlay), nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.Spec.PriceAdjustment).ToNot(BeNil()) + Expect(nodeOverlay.Spec.PriceAdjustment).To(Equal("100%")) + Expect(nodeOverlay.Spec.Capacity).To(BeNil()) + Expect(nodeOverlay.Spec.Weight).To(BeNil()) + }) + }) +}) diff --git a/pkg/apis/v1alpha1/nodeoverlay_validation.go b/pkg/apis/v1alpha1/nodeoverlay_validation.go new file mode 100644 index 0000000000..38bfe022fa --- /dev/null +++ b/pkg/apis/v1alpha1/nodeoverlay_validation.go @@ -0,0 +1,124 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "context" + "fmt" + "strconv" + + "github.com/samber/lo" + "go.uber.org/multierr" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/validation" + "sigs.k8s.io/controller-runtime/pkg/log" + + v1 "sigs.k8s.io/karpenter/pkg/apis/v1" +) + +// RuntimeValidate will be used to validate any part of the CRD that can not be validated at CRD creation +func (in *NodeOverlay) RuntimeValidate(ctx context.Context) (errs error) { + errs = multierr.Combine(in.Spec.validateRequirements(ctx)) + return errs +} + +// This function is used by the NodeClaim validation webhook to verify the nodepool requirements. +// When this function is called, the nodepool's requirements do not include the requirements from labels. +// NodeClaim requirements only support well known labels. +func (in *NodeOverlaySpec) validateRequirements(ctx context.Context) (errs error) { + for _, requirement := range in.Requirements { + if err := ValidateRequirement(ctx, requirement); err != nil { + errs = multierr.Append(errs, fmt.Errorf("invalid value: %w in requirements, restricted", err)) + } + } + return errs +} + +func ValidateRequirement(ctx context.Context, requirement corev1.NodeSelectorRequirement) error { //nolint:gocyclo + var errs error + if normalized, ok := v1.NormalizedLabels[requirement.Key]; ok { + requirement.Key = normalized + } + if !v1.SupportedNodeSelectorOps.Has(string(requirement.Operator)) { + errs = multierr.Append(errs, fmt.Errorf("key %s has an unsupported operator %s not in %s", requirement.Key, requirement.Operator, v1.SupportedNodeSelectorOps.UnsortedList())) + } + if e := v1.IsRestrictedLabel(requirement.Key); e != nil { + errs = multierr.Append(errs, e) + } + // Validate that at least one value is valid for well-known labels with known values + if err := validateWellKnownValues(ctx, requirement); err != nil { + errs = multierr.Append(errs, err) + } + for _, err := range validation.IsQualifiedName(requirement.Key) { + errs = multierr.Append(errs, fmt.Errorf("key %s is not a qualified name, %s", requirement.Key, err)) + } + for _, value := range requirement.Values { + for _, err := range validation.IsValidLabelValue(value) { + errs = multierr.Append(errs, fmt.Errorf("invalid value %s for key %s, %s", value, requirement.Key, err)) + } + } + if requirement.Operator == corev1.NodeSelectorOpIn && len(requirement.Values) == 0 { + errs = multierr.Append(errs, fmt.Errorf("key %s with operator %s must have a value defined", requirement.Key, requirement.Operator)) + } + + if requirement.Operator == corev1.NodeSelectorOpGt || requirement.Operator == corev1.NodeSelectorOpLt { + if len(requirement.Values) != 1 { + errs = multierr.Append(errs, fmt.Errorf("key %s with operator %s must have a single positive integer value", requirement.Key, requirement.Operator)) + } else { + value, err := strconv.Atoi(requirement.Values[0]) + if err != nil || value < 0 { + errs = multierr.Append(errs, fmt.Errorf("key %s with operator %s must have a single positive integer value", requirement.Key, requirement.Operator)) + } + } + } + return errs +} + +// ValidateWellKnownValues checks if the requirement has well known values. +// An error will cause a NodePool's Readiness to transition to False. +// It returns an error if all values are invalid. +// It returns an error if there are not enough valid values to satisfy min values for a requirement with known values. +// It logs if invalid values are found but valid values can be used. +func validateWellKnownValues(ctx context.Context, requirement corev1.NodeSelectorRequirement) error { + // If the key doesn't have well-known values or the operator is not In, nothing to validate + if !v1.WellKnownLabels.Has(requirement.Key) || requirement.Operator != corev1.NodeSelectorOpIn { + return nil + } + + // If the key doesn't have well-known values defined, nothing to validate + knownValues, exists := v1.WellKnownValuesForRequirements[requirement.Key] + if !exists { + return nil + } + + values, invalidValues := lo.FilterReject(requirement.Values, func(val string, _ int) bool { + return knownValues.Has(val) + }) + + // If there are only invalid values, set an error to transition the nodepool's readiness to false + if len(values) == 0 { + return fmt.Errorf("no valid values found in %v for %s, expected one of: %v, got: %v", + requirement.Values, requirement.Key, knownValues, invalidValues) + } + + // If there are valid and invalid values, log the invalid values and proceed with valid values + if len(invalidValues) > 0 { + log.FromContext(ctx).Error(fmt.Errorf("invalid values found for key"), "please correct found invalid values, proceeding with valid values", "key", requirement.Key, "valid-values", values, "invalid-values", invalidValues) + } + + return nil +} diff --git a/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go b/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go new file mode 100644 index 0000000000..a766442ecc --- /dev/null +++ b/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go @@ -0,0 +1,274 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1_test + +import ( + "fmt" + "strings" + + "github.com/Pallinder/go-randomdata" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + + v1 "sigs.k8s.io/karpenter/pkg/apis/v1" + . "sigs.k8s.io/karpenter/pkg/apis/v1alpha1" +) + +var _ = Describe("CEL/Validation", func() { + var nodeOverlay *NodeOverlay + + BeforeEach(func() { + if env.Version.Minor() < 25 { + Skip("CEL Validation is for 1.25>") + } + nodeOverlay = &NodeOverlay{ + ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}, + Spec: NodeOverlaySpec{ + Requirements: []corev1.NodeSelectorRequirement{ + { + Key: v1.CapacityTypeLabelKey, + Operator: corev1.NodeSelectorOpExists, + }, + }, + }, + } + }) + + It("should define either a priceAdjustment or capacity", func() { + nodeOverlay.Spec.PriceAdjustment = "" + nodeOverlay.Spec.Capacity = nil + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + + nodeOverlay.Spec.PriceAdjustment = "1%" + nodeOverlay.Spec.Capacity = nil + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + + nodeOverlay.Spec.PriceAdjustment = "" + nodeOverlay.Spec.Capacity = corev1.ResourceList{ + corev1.ResourceName("smarter-devices/fuse"): resource.MustParse("1"), + } + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + Context("Requirements", func() { + It("should succeed for valid requirement keys", func() { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: "Test", Operator: corev1.NodeSelectorOpExists}, + {Key: "test.com/Test", Operator: corev1.NodeSelectorOpExists}, + {Key: "test.com.com/test", Operator: corev1.NodeSelectorOpExists}, + {Key: "key-only", Operator: corev1.NodeSelectorOpExists}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) + }) + It("should fail for invalid requirement keys", func() { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{{Key: "test.com.com}", Operator: corev1.NodeSelectorOpExists}} + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{{Key: "Test.com/test", Operator: corev1.NodeSelectorOpExists}} + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{{Key: "test/test/test", Operator: corev1.NodeSelectorOpExists}} + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{{Key: "test/", Operator: corev1.NodeSelectorOpExists}} + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{{Key: "/test", Operator: corev1.NodeSelectorOpExists}} + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + }) + It("should allow for the karpenter.sh/nodepool label", func() { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: v1.NodePoolLabelKey, Operator: corev1.NodeSelectorOpIn, Values: []string{randomdata.SillyName()}}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + }) + It("should fail at runtime for requirement keys that are too long", func() { + oldnodeOverlay := nodeOverlay.DeepCopy() + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{{Key: fmt.Sprintf("test.com.test.%s/test", strings.ToLower(randomdata.Alphanumeric(250))), Operator: corev1.NodeSelectorOpExists}} + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(env.Client.Delete(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + nodeOverlay = oldnodeOverlay.DeepCopy() + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{{Key: fmt.Sprintf("test.com.test/test-%s", strings.ToLower(randomdata.Alphanumeric(250))), Operator: corev1.NodeSelectorOpExists}} + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + }) + It("should allow supported ops", func() { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpIn, Values: []string{"test"}}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpGt, Values: []string{"1"}}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpLt, Values: []string{"1"}}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpNotIn}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpExists}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) + }) + It("should fail for unsupported ops", func() { + for _, op := range []corev1.NodeSelectorOperator{"unknown"} { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: corev1.LabelTopologyZone, Operator: op, Values: []string{"test"}}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + } + }) + It("should fail for restricted domains", func() { + for label := range v1.RestrictedLabelDomains { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: label + "/test", Operator: corev1.NodeSelectorOpIn, Values: []string{"test"}}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + } + }) + It("should allow restricted domains exceptions", func() { + oldnodeOverlay := nodeOverlay.DeepCopy() + for label := range v1.LabelDomainExceptions { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: label + "/test", Operator: corev1.NodeSelectorOpIn, Values: []string{"test"}}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) + Expect(env.Client.Delete(ctx, nodeOverlay)).To(Succeed()) + nodeOverlay = oldnodeOverlay.DeepCopy() + } + }) + It("should allow restricted subdomains exceptions", func() { + oldnodeOverlay := nodeOverlay.DeepCopy() + for label := range v1.LabelDomainExceptions { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: "subdomain." + label + "/test", Operator: corev1.NodeSelectorOpIn, Values: []string{"test"}}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) + Expect(env.Client.Delete(ctx, nodeOverlay)).To(Succeed()) + nodeOverlay = oldnodeOverlay.DeepCopy() + } + }) + It("should allow well known label exceptions", func() { + oldnodeOverlay := nodeOverlay.DeepCopy() + // Capacity Type is runtime validated + for label := range v1.WellKnownLabels.Difference(sets.New(v1.NodePoolLabelKey, v1.CapacityTypeLabelKey)) { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: label, Operator: corev1.NodeSelectorOpIn, Values: []string{"test"}}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) + Expect(env.Client.Delete(ctx, nodeOverlay)).To(Succeed()) + nodeOverlay = oldnodeOverlay.DeepCopy() + } + }) + It("should allow non-empty set after removing overlapped value", func() { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpIn, Values: []string{"test", "foo"}}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpNotIn, Values: []string{"test", "bar"}}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) + }) + It("should allow empty requirements", func() { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{} + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) + }) + It("should fail with invalid GT or LT values", func() { + for _, requirement := range []corev1.NodeSelectorRequirement{ + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpGt, Values: []string{}}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpGt, Values: []string{"1", "2"}}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpGt, Values: []string{"a"}}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpGt, Values: []string{"-1"}}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpLt, Values: []string{}}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpLt, Values: []string{"1", "2"}}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpLt, Values: []string{"a"}}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpLt, Values: []string{"-1"}}, + } { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{requirement} + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + } + }) + }) + Context("priceAdjustment", func() { + It("should allow percentage value for priceAdjustment field", func() { + nodeOverlay.Spec.PriceAdjustment = "1%" + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + It("should allow percentage greater then 100%", func() { + nodeOverlay.Spec.PriceAdjustment = "123%" + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + It("should allow for integer value for ", func() { + nodeOverlay.Spec.PriceAdjustment = "43" + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + It("should only allow whole number percentage value ", func() { + nodeOverlay.Spec.PriceAdjustment = "343.5%" + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + }) + It("should limit percentage value to be no less then 1%", func() { + nodeOverlay.Spec.PriceAdjustment = "0.5%" + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + }) + It("should allow percentage greater then 100%", func() { + nodeOverlay.Spec.PriceAdjustment = "123%" + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + It("should not allow for float value", func() { + nodeOverlay.Spec.PriceAdjustment = "34.43" + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + }) + }) + Context("Capacity", func() { + It("should allow custom resources", func() { + nodeOverlay.Spec.Capacity = corev1.ResourceList{ + corev1.ResourceName("smarter-devices/fuse"): resource.MustParse("1"), + } + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + It("should not allow cpu resources override", func() { + nodeOverlay.Spec.Capacity = corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + } + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + }) + It("should not allow memory resources override", func() { + nodeOverlay.Spec.Capacity = corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("34Gi"), + } + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + }) + It("should not allow ephemeral-storage resources override", func() { + nodeOverlay.Spec.Capacity = corev1.ResourceList{ + corev1.ResourceEphemeralStorage: resource.MustParse("34Gi"), + } + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + }) + It("should not allow pod resources override", func() { + nodeOverlay.Spec.Capacity = corev1.ResourceList{ + corev1.ResourcePods: resource.MustParse("324"), + } + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + }) + }) +}) diff --git a/pkg/apis/v1alpha1/overlay.go b/pkg/apis/v1alpha1/overlay.go index 17f96e58a0..78bd629bff 100644 --- a/pkg/apis/v1alpha1/overlay.go +++ b/pkg/apis/v1alpha1/overlay.go @@ -28,12 +28,12 @@ type NodeOverlaySpec struct { // Requirements are layered with GetLabels and applied to every node. // +kubebuilder:validation:XValidation:message="requirements with operator 'In' must have a value defined",rule="self.all(x, x.operator == 'In' ? x.values.size() != 0 : true)" // +kubebuilder:validation:XValidation: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)" - // +kubebuilder:validation:XValidation: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)" // +kubebuilder:validation:MaxItems:=100 // +required Requirements []v1.NodeSelectorRequirement `json:"requirements,omitempty"` // UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). // Update Validation + // +kubebuilder:default:="100%" // +optional PriceAdjustment string `json:"priceAdjustment,omitempty"` // Capacity adds extended resource to instances types based on the selector provided @@ -59,7 +59,8 @@ type NodeOverlay struct { // +kubebuilder:validation:XValidation:message="need to define a capacity or priceAdjustment field ",rule="has(self.capacity) || has(self.priceAdjustment)" // +required - Spec NodeOverlaySpec `json:"spec"` + Spec NodeOverlaySpec `json:"spec"` + Status NodeOverlayStatus `json:"status,omitempty"` } // +kubebuilder:object:root=true diff --git a/pkg/apis/v1alpha1/suite_test.go b/pkg/apis/v1alpha1/suite_test.go new file mode 100644 index 0000000000..69df30566f --- /dev/null +++ b/pkg/apis/v1alpha1/suite_test.go @@ -0,0 +1,51 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1_test + +import ( + "context" + "testing" + + . "github.com/awslabs/operatorpkg/test/expectations" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "sigs.k8s.io/karpenter/pkg/apis" + "sigs.k8s.io/karpenter/pkg/test" + . "sigs.k8s.io/karpenter/pkg/utils/testing" +) + +var ctx context.Context +var env *test.Environment + +func TestAPIs(t *testing.T) { + ctx = TestContextWithLogger(t) + RegisterFailHandler(Fail) + RunSpecs(t, "v1") +} + +var _ = BeforeSuite(func() { + env = test.NewEnvironment(test.WithCRDs(apis.CRDs...)) +}) + +var _ = AfterEach(func() { + ExpectCleanedUp(ctx, env.Client) +}) + +var _ = AfterSuite(func() { + Expect(env.Stop()).To(Succeed(), "Failed to stop environment") +}) diff --git a/pkg/apis/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/v1alpha1/zz_generated.deepcopy.go index 096ab0536a..597b9976f9 100644 --- a/pkg/apis/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/v1alpha1/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ limitations under the License. package v1alpha1 import ( + "github.com/awslabs/operatorpkg/status" "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -31,6 +32,7 @@ func (in *NodeOverlay) DeepCopyInto(out *NodeOverlay) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeOverlay. @@ -116,3 +118,25 @@ func (in *NodeOverlaySpec) DeepCopy() *NodeOverlaySpec { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NodeOverlayStatus) DeepCopyInto(out *NodeOverlayStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]status.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeOverlayStatus. +func (in *NodeOverlayStatus) DeepCopy() *NodeOverlayStatus { + if in == nil { + return nil + } + out := new(NodeOverlayStatus) + in.DeepCopyInto(out) + return out +} From 1dd646bb9a5cba53174b961eb4a8f91e791e4923 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Tue, 10 Jun 2025 14:24:37 -0700 Subject: [PATCH 03/11] Fix testing and validation rules --- .../crds/karpenter.sh_nodeoverlays.yaml | 9 ++-- pkg/apis/crds/karpenter.sh_nodeoverlays.yaml | 9 ++-- pkg/apis/v1/labels.go | 9 ++++ pkg/apis/v1alpha1/nodelay_status.go | 2 +- pkg/apis/v1alpha1/nodeoverlay_validation.go | 14 +++-- .../nodeoverlay_validation_cel_test.go | 51 ++++++++----------- pkg/apis/v1alpha1/overlay.go | 6 +-- 7 files changed, 52 insertions(+), 48 deletions(-) diff --git a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml index 832cf65d90..8aa958b942 100644 --- a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml +++ b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml @@ -48,11 +48,15 @@ spec: x-kubernetes-int-or-string: true description: Capacity adds extended resource to instances types based on the selector provided type: object + x-kubernetes-validations: + - message: invalid resource restricted + rule: self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods'])) priceAdjustment: default: 100% description: |- UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). Update Validation + pattern: ^(-?\d*\.?\d+|-?\d+\.?\d*$|\d*\.?\d+%|\d+\.?\d*%)$ type: string requirements: description: Requirements are layered with GetLabels and applied to every node. @@ -122,12 +126,7 @@ spec: maximum: 100 minimum: 1 type: integer - required: - - requirements type: object - x-kubernetes-validations: - - message: 'need to define a capacity or priceAdjustment field ' - rule: has(self.capacity) || has(self.priceAdjustment) status: description: NodeOverlayStatus defines the observed state of NodeOverlay properties: diff --git a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml index 832cf65d90..8aa958b942 100644 --- a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml @@ -48,11 +48,15 @@ spec: x-kubernetes-int-or-string: true description: Capacity adds extended resource to instances types based on the selector provided type: object + x-kubernetes-validations: + - message: invalid resource restricted + rule: self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods'])) priceAdjustment: default: 100% description: |- UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). Update Validation + pattern: ^(-?\d*\.?\d+|-?\d+\.?\d*$|\d*\.?\d+%|\d+\.?\d*%)$ type: string requirements: description: Requirements are layered with GetLabels and applied to every node. @@ -122,12 +126,7 @@ spec: maximum: 100 minimum: 1 type: integer - required: - - requirements type: object - x-kubernetes-validations: - - message: 'need to define a capacity or priceAdjustment field ' - rule: has(self.capacity) || has(self.priceAdjustment) status: description: NodeOverlayStatus defines the observed state of NodeOverlay properties: diff --git a/pkg/apis/v1/labels.go b/pkg/apis/v1/labels.go index 38f7a0ef7b..69e1200de4 100644 --- a/pkg/apis/v1/labels.go +++ b/pkg/apis/v1/labels.go @@ -90,6 +90,15 @@ var ( v1.LabelWindowsBuild, ) + // WellKnownResources are resources that are known expected from the instance types + // provided by the cloud providers. + WellKnownResources = sets.New[v1.ResourceName]( + v1.ResourceCPU, + v1.ResourceMemory, + v1.ResourceEphemeralStorage, + v1.ResourcePods, + ) + // WellKnownValuesForRequirements are for requirements where a known set of values // is expected to be used for that requirement. For example, in the AWS provider, // only on-demand, spot, and reserved make sense as values for the capacity type requirement diff --git a/pkg/apis/v1alpha1/nodelay_status.go b/pkg/apis/v1alpha1/nodelay_status.go index 502764f1f8..7d5df21c92 100644 --- a/pkg/apis/v1alpha1/nodelay_status.go +++ b/pkg/apis/v1alpha1/nodelay_status.go @@ -24,7 +24,7 @@ import ( const ( // ConditionTypeValidationSucceeded = "ValidationSucceeded" condition indicates that the - // runtime-based configuration is valid for this NodeOverlay + // runtime-based configuration is valid and conflict for this NodeOverlay ConditionTypeValidationSucceeded = "ValidationSucceeded" ) diff --git a/pkg/apis/v1alpha1/nodeoverlay_validation.go b/pkg/apis/v1alpha1/nodeoverlay_validation.go index 38bfe022fa..47e730e57f 100644 --- a/pkg/apis/v1alpha1/nodeoverlay_validation.go +++ b/pkg/apis/v1alpha1/nodeoverlay_validation.go @@ -31,9 +31,8 @@ import ( ) // RuntimeValidate will be used to validate any part of the CRD that can not be validated at CRD creation -func (in *NodeOverlay) RuntimeValidate(ctx context.Context) (errs error) { - errs = multierr.Combine(in.Spec.validateRequirements(ctx)) - return errs +func (in *NodeOverlay) RuntimeValidate(ctx context.Context) error { + return multierr.Combine(in.Spec.validateRequirements(ctx), in.Spec.validateCapacity()) } // This function is used by the NodeClaim validation webhook to verify the nodepool requirements. @@ -48,6 +47,15 @@ func (in *NodeOverlaySpec) validateRequirements(ctx context.Context) (errs error return errs } +func (in *NodeOverlaySpec) validateCapacity() (errs error) { + for n := range in.Capacity { + if v1.WellKnownResources.Has(n) { + errs = multierr.Append(errs, fmt.Errorf("invalid capacity: %s in resource, restricted", n)) + } + } + return errs +} + func ValidateRequirement(ctx context.Context, requirement corev1.NodeSelectorRequirement) error { //nolint:gocyclo var errs error if normalized, ok := v1.NormalizedLabels[requirement.Key]; ok { diff --git a/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go b/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go index a766442ecc..882c8f4650 100644 --- a/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go +++ b/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go @@ -51,22 +51,6 @@ var _ = Describe("CEL/Validation", func() { }, } }) - - It("should define either a priceAdjustment or capacity", func() { - nodeOverlay.Spec.PriceAdjustment = "" - nodeOverlay.Spec.Capacity = nil - Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) - - nodeOverlay.Spec.PriceAdjustment = "1%" - nodeOverlay.Spec.Capacity = nil - Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) - - nodeOverlay.Spec.PriceAdjustment = "" - nodeOverlay.Spec.Capacity = corev1.ResourceList{ - corev1.ResourceName("smarter-devices/fuse"): resource.MustParse("1"), - } - Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) - }) Context("Requirements", func() { It("should succeed for valid requirement keys", func() { nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ @@ -99,8 +83,8 @@ var _ = Describe("CEL/Validation", func() { nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ {Key: v1.NodePoolLabelKey, Operator: corev1.NodeSelectorOpIn, Values: []string{randomdata.SillyName()}}, } - Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) - Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) }) It("should fail at runtime for requirement keys that are too long", func() { oldnodeOverlay := nodeOverlay.DeepCopy() @@ -218,25 +202,25 @@ var _ = Describe("CEL/Validation", func() { nodeOverlay.Spec.PriceAdjustment = "123%" Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) }) - It("should allow for integer value for ", func() { - nodeOverlay.Spec.PriceAdjustment = "43" - Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) - }) - It("should only allow whole number percentage value ", func() { - nodeOverlay.Spec.PriceAdjustment = "343.5%" + It("should not allow percentage less then 0%", func() { + nodeOverlay.Spec.PriceAdjustment = "-1%" Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) }) - It("should limit percentage value to be no less then 1%", func() { - nodeOverlay.Spec.PriceAdjustment = "0.5%" - Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + It("should allow integer value", func() { + nodeOverlay.Spec.PriceAdjustment = "43" + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) }) - It("should allow percentage greater then 100%", func() { - nodeOverlay.Spec.PriceAdjustment = "123%" + It("should allow negative integer value", func() { + nodeOverlay.Spec.PriceAdjustment = "-43" Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) }) - It("should not allow for float value", func() { + It("should allow float value", func() { nodeOverlay.Spec.PriceAdjustment = "34.43" - Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + It("should allow negative float value", func() { + nodeOverlay.Spec.PriceAdjustment = "-34.43" + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) }) }) Context("Capacity", func() { @@ -245,30 +229,35 @@ var _ = Describe("CEL/Validation", func() { corev1.ResourceName("smarter-devices/fuse"): resource.MustParse("1"), } Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) }) It("should not allow cpu resources override", func() { nodeOverlay.Spec.Capacity = corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("1"), } Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) }) It("should not allow memory resources override", func() { nodeOverlay.Spec.Capacity = corev1.ResourceList{ corev1.ResourceMemory: resource.MustParse("34Gi"), } Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) }) It("should not allow ephemeral-storage resources override", func() { nodeOverlay.Spec.Capacity = corev1.ResourceList{ corev1.ResourceEphemeralStorage: resource.MustParse("34Gi"), } Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) }) It("should not allow pod resources override", func() { nodeOverlay.Spec.Capacity = corev1.ResourceList{ corev1.ResourcePods: resource.MustParse("324"), } Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) }) }) }) diff --git a/pkg/apis/v1alpha1/overlay.go b/pkg/apis/v1alpha1/overlay.go index 78bd629bff..28d8831664 100644 --- a/pkg/apis/v1alpha1/overlay.go +++ b/pkg/apis/v1alpha1/overlay.go @@ -29,14 +29,16 @@ type NodeOverlaySpec struct { // +kubebuilder:validation:XValidation:message="requirements with operator 'In' must have a value defined",rule="self.all(x, x.operator == 'In' ? x.values.size() != 0 : true)" // +kubebuilder:validation:XValidation: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)" // +kubebuilder:validation:MaxItems:=100 - // +required + // +optional Requirements []v1.NodeSelectorRequirement `json:"requirements,omitempty"` // UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). // Update Validation + // +kubebuilder:validation:Pattern=`^(-?\d*\.?\d+|-?\d+\.?\d*$|\d*\.?\d+%|\d+\.?\d*%)$` // +kubebuilder:default:="100%" // +optional PriceAdjustment string `json:"priceAdjustment,omitempty"` // Capacity adds extended resource to instances types based on the selector provided + // +kubebuilder:validation:XValidation:message="invalid resource restricted",rule="self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods']))" // +optional Capacity v1.ResourceList `json:"capacity,omitempty"` // Weight is the priority given to the nodeoverlay while overriding node attributes. A higher @@ -57,8 +59,6 @@ type NodeOverlay struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - // +kubebuilder:validation:XValidation:message="need to define a capacity or priceAdjustment field ",rule="has(self.capacity) || has(self.priceAdjustment)" - // +required Spec NodeOverlaySpec `json:"spec"` Status NodeOverlayStatus `json:"status,omitempty"` } From fca0769151dc88d398e622ead0c23aa9a44e941e Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Tue, 10 Jun 2025 14:49:34 -0700 Subject: [PATCH 04/11] Update comments for Node Overlay --- .../crds/karpenter.sh_nodeoverlays.yaml | 33 +++++++++++++++---- pkg/apis/crds/karpenter.sh_nodeoverlays.yaml | 33 +++++++++++++++---- .../v1alpha1/{overlay.go => nodeoverlay.go} | 20 ++++++++--- test/pkg/environment/common/setup.go | 2 ++ 4 files changed, 71 insertions(+), 17 deletions(-) rename pkg/apis/v1alpha1/{overlay.go => nodeoverlay.go} (72%) diff --git a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml index 8aa958b942..34c7c6475d 100644 --- a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml +++ b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml @@ -16,7 +16,18 @@ spec: singular: nodeoverlay scope: Cluster versions: - - name: v1alpha1 + - additionalPrinterColumns: + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + - jsonPath: .spec.weight + name: Weight + priority: 1 + type: integer + name: v1alpha1 schema: openAPIV3Schema: properties: @@ -46,7 +57,10 @@ spec: - type: string pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true - description: Capacity adds extended resource to instances types based on the selector provided + description: |- + Capacity that will add extended resources only, and not replace any existing resources on the nodes. + These extended resources will be appended to the node's existing resource list. + Note: This field does not modify or override standard resources like CPU, memory, ephemeral-storage, or pods. type: object x-kubernetes-validations: - message: invalid resource restricted @@ -54,12 +68,18 @@ spec: priceAdjustment: default: 100% description: |- - UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). - Update Validation - pattern: ^(-?\d*\.?\d+|-?\d+\.?\d*$|\d*\.?\d+%|\d+\.?\d*%)$ + PriceAdjustment specifies a price changes for matching instance types. Accepts either: + - A fixed price modifier (e.g., -0.5, 1.2) + - A percentage modifier (e.g., 90% to decrease by 10%, 115% to increase by 15%) + Note: Percentage values must be positive. The adjustment is applied to whatever price unit is in use. + pattern: ^(-?\d*\.?\d+$|\d*\.?\d+%)$ type: string requirements: - description: Requirements are layered with GetLabels and applied to every node. + description: |- + Requirements constrain when this NodeOverlay is applied during scheduling simulation. + These requirements can match: + - Well-known labels (e.g., node.kubernetes.io/instance-type, karpenter.sh/nodepool) + - Custom labels from NodePool's spec.template.labels items: description: |- A node selector requirement is a selector that contains values, a key, and an operator @@ -192,3 +212,4 @@ spec: type: object served: true storage: true + subresources: {} diff --git a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml index 8aa958b942..34c7c6475d 100644 --- a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml @@ -16,7 +16,18 @@ spec: singular: nodeoverlay scope: Cluster versions: - - name: v1alpha1 + - additionalPrinterColumns: + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + - jsonPath: .spec.weight + name: Weight + priority: 1 + type: integer + name: v1alpha1 schema: openAPIV3Schema: properties: @@ -46,7 +57,10 @@ spec: - type: string pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true - description: Capacity adds extended resource to instances types based on the selector provided + description: |- + Capacity that will add extended resources only, and not replace any existing resources on the nodes. + These extended resources will be appended to the node's existing resource list. + Note: This field does not modify or override standard resources like CPU, memory, ephemeral-storage, or pods. type: object x-kubernetes-validations: - message: invalid resource restricted @@ -54,12 +68,18 @@ spec: priceAdjustment: default: 100% description: |- - UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). - Update Validation - pattern: ^(-?\d*\.?\d+|-?\d+\.?\d*$|\d*\.?\d+%|\d+\.?\d*%)$ + PriceAdjustment specifies a price changes for matching instance types. Accepts either: + - A fixed price modifier (e.g., -0.5, 1.2) + - A percentage modifier (e.g., 90% to decrease by 10%, 115% to increase by 15%) + Note: Percentage values must be positive. The adjustment is applied to whatever price unit is in use. + pattern: ^(-?\d*\.?\d+$|\d*\.?\d+%)$ type: string requirements: - description: Requirements are layered with GetLabels and applied to every node. + description: |- + Requirements constrain when this NodeOverlay is applied during scheduling simulation. + These requirements can match: + - Well-known labels (e.g., node.kubernetes.io/instance-type, karpenter.sh/nodepool) + - Custom labels from NodePool's spec.template.labels items: description: |- A node selector requirement is a selector that contains values, a key, and an operator @@ -192,3 +212,4 @@ spec: type: object served: true storage: true + subresources: {} diff --git a/pkg/apis/v1alpha1/overlay.go b/pkg/apis/v1alpha1/nodeoverlay.go similarity index 72% rename from pkg/apis/v1alpha1/overlay.go rename to pkg/apis/v1alpha1/nodeoverlay.go index 28d8831664..8153bce2f0 100644 --- a/pkg/apis/v1alpha1/overlay.go +++ b/pkg/apis/v1alpha1/nodeoverlay.go @@ -25,19 +25,26 @@ import ( ) type NodeOverlaySpec struct { - // Requirements are layered with GetLabels and applied to every node. + // Requirements constrain when this NodeOverlay is applied during scheduling simulation. + // These requirements can match: + // - Well-known labels (e.g., node.kubernetes.io/instance-type, karpenter.sh/nodepool) + // - Custom labels from NodePool's spec.template.labels // +kubebuilder:validation:XValidation:message="requirements with operator 'In' must have a value defined",rule="self.all(x, x.operator == 'In' ? x.values.size() != 0 : true)" // +kubebuilder:validation:XValidation: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)" // +kubebuilder:validation:MaxItems:=100 // +optional Requirements []v1.NodeSelectorRequirement `json:"requirements,omitempty"` - // UPDATE WORDING: PricePercent modifies the price of the simulated node (PriceAdjustment + (Price * PricePercent / 100)). - // Update Validation - // +kubebuilder:validation:Pattern=`^(-?\d*\.?\d+|-?\d+\.?\d*$|\d*\.?\d+%|\d+\.?\d*%)$` + // PriceAdjustment specifies a price changes for matching instance types. Accepts either: + // - A fixed price modifier (e.g., -0.5, 1.2) + // - A percentage modifier (e.g., 90% to decrease by 10%, 115% to increase by 15%) + // Note: Percentage values must be positive. The adjustment is applied to whatever price unit is in use. + // +kubebuilder:validation:Pattern=`^(-?\d*\.?\d+$|\d*\.?\d+%)$` // +kubebuilder:default:="100%" // +optional PriceAdjustment string `json:"priceAdjustment,omitempty"` - // Capacity adds extended resource to instances types based on the selector provided + // Capacity that will add extended resources only, and not replace any existing resources on the nodes. + // These extended resources will be appended to the node's existing resource list. + // Note: This field does not modify or override standard resources like CPU, memory, ephemeral-storage, or pods. // +kubebuilder:validation:XValidation:message="invalid resource restricted",rule="self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods']))" // +optional Capacity v1.ResourceList `json:"capacity,omitempty"` @@ -55,6 +62,9 @@ type NodeOverlaySpec struct { // +kubebuilder:object:root=true // +kubebuilder:storageversion // +kubebuilder:resource:path=nodeoverlays,scope=Cluster,categories=karpenter +// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="" +// +kubebuilder:printcolumn:name="Weight",type="integer",JSONPath=".spec.weight",priority=1,description="" type NodeOverlay struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/test/pkg/environment/common/setup.go b/test/pkg/environment/common/setup.go index a7982a646b..2f42bbb3ea 100644 --- a/test/pkg/environment/common/setup.go +++ b/test/pkg/environment/common/setup.go @@ -38,6 +38,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/apiutil" v1 "sigs.k8s.io/karpenter/pkg/apis/v1" + "sigs.k8s.io/karpenter/pkg/apis/v1alpha1" "sigs.k8s.io/karpenter/pkg/test" "sigs.k8s.io/karpenter/pkg/utils/pod" "sigs.k8s.io/karpenter/pkg/utils/pretty" @@ -66,6 +67,7 @@ var ( &schedulingv1.PriorityClass{}, &corev1.Node{}, &v1.NodeClaim{}, + &v1alpha1.NodeOverlay{}, &admissionregistrationv1.ValidatingAdmissionPolicy{}, &admissionregistrationv1.ValidatingAdmissionPolicyBinding{}, } From 1f7b40940c7ac58f413cc052dc9ef650219bcbc4 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Thu, 12 Jun 2025 11:35:13 -0700 Subject: [PATCH 05/11] Minor fixes --- kwok/charts/crds/karpenter.sh_nodeoverlays.yaml | 3 ++- pkg/apis/crds/karpenter.sh_nodeoverlays.yaml | 3 ++- pkg/apis/v1alpha1/nodeoverlay.go | 1 + pkg/apis/v1alpha1/{nodelay_status.go => nodeoverlay_status.go} | 0 4 files changed, 5 insertions(+), 2 deletions(-) rename pkg/apis/v1alpha1/{nodelay_status.go => nodeoverlay_status.go} (100%) diff --git a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml index 34c7c6475d..34d9ea169e 100644 --- a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml +++ b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml @@ -212,4 +212,5 @@ spec: type: object served: true storage: true - subresources: {} + subresources: + status: {} diff --git a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml index 34c7c6475d..34d9ea169e 100644 --- a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml @@ -212,4 +212,5 @@ spec: type: object served: true storage: true - subresources: {} + subresources: + status: {} diff --git a/pkg/apis/v1alpha1/nodeoverlay.go b/pkg/apis/v1alpha1/nodeoverlay.go index 8153bce2f0..c91407f28c 100644 --- a/pkg/apis/v1alpha1/nodeoverlay.go +++ b/pkg/apis/v1alpha1/nodeoverlay.go @@ -65,6 +65,7 @@ type NodeOverlaySpec struct { // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="" // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="" // +kubebuilder:printcolumn:name="Weight",type="integer",JSONPath=".spec.weight",priority=1,description="" +// +kubebuilder:subresource:status type NodeOverlay struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/pkg/apis/v1alpha1/nodelay_status.go b/pkg/apis/v1alpha1/nodeoverlay_status.go similarity index 100% rename from pkg/apis/v1alpha1/nodelay_status.go rename to pkg/apis/v1alpha1/nodeoverlay_status.go From d6048415b7c61dac55cd6667c3d32cf0dd92bfe2 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Thu, 12 Jun 2025 17:18:22 -0700 Subject: [PATCH 06/11] Add testing resources for Node Overlay --- pkg/test/expectations/expectations.go | 6 ++-- pkg/test/nodeoverlay.go | 49 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 pkg/test/nodeoverlay.go diff --git a/pkg/test/expectations/expectations.go b/pkg/test/expectations/expectations.go index e590a3f0f2..694eb1e3a1 100644 --- a/pkg/test/expectations/expectations.go +++ b/pkg/test/expectations/expectations.go @@ -55,6 +55,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" v1 "sigs.k8s.io/karpenter/pkg/apis/v1" + "sigs.k8s.io/karpenter/pkg/apis/v1alpha1" "sigs.k8s.io/karpenter/pkg/cloudprovider" "sigs.k8s.io/karpenter/pkg/controllers/nodeclaim/lifecycle" "sigs.k8s.io/karpenter/pkg/controllers/provisioning" @@ -64,7 +65,7 @@ import ( "sigs.k8s.io/karpenter/pkg/metrics" pscheduling "sigs.k8s.io/karpenter/pkg/scheduling" "sigs.k8s.io/karpenter/pkg/test" - "sigs.k8s.io/karpenter/pkg/test/v1alpha1" + testv1alpha1 "sigs.k8s.io/karpenter/pkg/test/v1alpha1" ) const ( @@ -258,8 +259,9 @@ func ExpectCleanedUp(ctx context.Context, c client.Client) { &corev1.PersistentVolume{}, &storagev1.StorageClass{}, &v1.NodePool{}, - &v1alpha1.TestNodeClass{}, + &testv1alpha1.TestNodeClass{}, &v1.NodeClaim{}, + &v1alpha1.NodeOverlay{}, } { for _, namespace := range namespaces.Items { wg.Add(1) diff --git a/pkg/test/nodeoverlay.go b/pkg/test/nodeoverlay.go new file mode 100644 index 0000000000..c972823376 --- /dev/null +++ b/pkg/test/nodeoverlay.go @@ -0,0 +1,49 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "fmt" + + "github.com/imdario/mergo" + corev1 "k8s.io/api/core/v1" + + "sigs.k8s.io/karpenter/pkg/apis/v1alpha1" +) + +// NodePool creates a test NodePool with defaults that can be overridden by overrides. +// Overrides are applied in order, with a last write wins semantic. +func NodeOverlay(overrides ...v1alpha1.NodeOverlay) *v1alpha1.NodeOverlay { + override := v1alpha1.NodeOverlay{} + for _, opts := range overrides { + if err := mergo.Merge(&override, opts, mergo.WithOverride); err != nil { + panic(fmt.Sprintf("failed to merge: %v", err)) + } + } + if override.Name == "" { + override.Name = RandomName() + } + if override.Spec.Requirements == nil { + override.Spec.Requirements = []corev1.NodeSelectorRequirement{} + } + no := &v1alpha1.NodeOverlay{ + ObjectMeta: ObjectMeta(override.ObjectMeta), + Spec: override.Spec, + Status: override.Status, + } + return no +} From a270d4abb04fd75ad18a4fdef8f8cf34c9f0c5c5 Mon Sep 17 00:00:00 2001 From: aengeda Date: Tue, 15 Jul 2025 21:25:40 +0000 Subject: [PATCH 07/11] Add pricing adjustment function --- .../crds/karpenter.sh_nodeoverlays.yaml | 2 +- pkg/apis/crds/karpenter.sh_nodeoverlays.yaml | 2 +- pkg/apis/v1alpha1/nodeoverlay.go | 23 ++++- pkg/apis/v1alpha1/suite_test.go | 88 ++++++++++++++++++- pkg/apis/v1alpha1/zz_generated.deepcopy.go | 2 +- 5 files changed, 111 insertions(+), 6 deletions(-) diff --git a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml index 34d9ea169e..82a09f5f0c 100644 --- a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml +++ b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml @@ -142,7 +142,7 @@ spec: ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, we will merge them in alphabetical order. - format: int64 + format: int32 maximum: 100 minimum: 1 type: integer diff --git a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml index 34d9ea169e..82a09f5f0c 100644 --- a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml @@ -142,7 +142,7 @@ spec: ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, we will merge them in alphabetical order. - format: int64 + format: int32 maximum: 100 minimum: 1 type: integer diff --git a/pkg/apis/v1alpha1/nodeoverlay.go b/pkg/apis/v1alpha1/nodeoverlay.go index c91407f28c..bcee23b8ac 100644 --- a/pkg/apis/v1alpha1/nodeoverlay.go +++ b/pkg/apis/v1alpha1/nodeoverlay.go @@ -18,6 +18,8 @@ package v1alpha1 import ( "sort" + "strconv" + "strings" "github.com/samber/lo" v1 "k8s.io/api/core/v1" @@ -56,7 +58,7 @@ type NodeOverlaySpec struct { // +kubebuilder:validation:Minimum:=1 // +kubebuilder:validation:Maximum:=100 // +optional - Weight *int64 `json:"weight,omitempty"` + Weight *int32 `json:"weight,omitempty"` } // +kubebuilder:object:root=true @@ -96,3 +98,22 @@ func (nol *NodeOverlayList) OrderByWeight() { return weightA > weightB }) } + +func (in *NodeOverlay) AdjustedPrice(instanceTypePrice float64) float64 { + // Check if adjustment is a percentage + isPercentage := strings.HasSuffix(in.Spec.PriceAdjustment, "%") + adjustment := in.Spec.PriceAdjustment + + // Remove the percentage sign if present + if isPercentage { + adjustment = strings.TrimSuffix(in.Spec.PriceAdjustment, "%") + } + + // Parse the adjustment value + // Due to the CEL validation we can assume that + // there will always be a valid float provided into the spec + adjValue := lo.Must(strconv.ParseFloat(adjustment, 64)) + + // Apply the adjustment + return lo.Ternary(isPercentage, instanceTypePrice*(adjValue/100), instanceTypePrice+adjValue) +} diff --git a/pkg/apis/v1alpha1/suite_test.go b/pkg/apis/v1alpha1/suite_test.go index 69df30566f..d0f9980c24 100644 --- a/pkg/apis/v1alpha1/suite_test.go +++ b/pkg/apis/v1alpha1/suite_test.go @@ -18,14 +18,19 @@ package v1alpha1_test import ( "context" + "math/rand/v2" "testing" . "github.com/awslabs/operatorpkg/test/expectations" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/samber/lo" "sigs.k8s.io/karpenter/pkg/apis" + "sigs.k8s.io/karpenter/pkg/apis/v1alpha1" "sigs.k8s.io/karpenter/pkg/test" + testexpectations "sigs.k8s.io/karpenter/pkg/test/expectations" + testv1alpha1 "sigs.k8s.io/karpenter/pkg/test/v1alpha1" . "sigs.k8s.io/karpenter/pkg/utils/testing" ) @@ -39,13 +44,92 @@ func TestAPIs(t *testing.T) { } var _ = BeforeSuite(func() { - env = test.NewEnvironment(test.WithCRDs(apis.CRDs...)) + env = test.NewEnvironment(test.WithCRDs(apis.CRDs...), test.WithCRDs(testv1alpha1.CRDs...)) }) var _ = AfterEach(func() { - ExpectCleanedUp(ctx, env.Client) + testexpectations.ExpectCleanedUp(ctx, env.Client) }) var _ = AfterSuite(func() { Expect(env.Stop()).To(Succeed(), "Failed to stop environment") }) + +var _ = Describe("NodeOverlay", func() { + Context("OrderByWeight", func() { + It("should order the NodeOverlay by weight", func() { + // Generate 10 NodeOverlay that have random weights, some might have the same weights + nos := lo.Times(10, func(_ int) *v1alpha1.NodeOverlay { + return test.NodeOverlay(v1alpha1.NodeOverlay{ + Spec: v1alpha1.NodeOverlaySpec{ + Weight: lo.ToPtr[int32](int32(rand.IntN(100) + 1)), //nolint:gosec + }, + }) + }) + lo.ForEach(nos, func(overlay *v1alpha1.NodeOverlay, _ int) { + ExpectApplied(ctx, env.Client, overlay) + }) + overlayList := &v1alpha1.NodeOverlayList{} + Expect(env.Client.List(ctx, overlayList)).To(BeNil()) + overlayList.OrderByWeight() + + lastWeight := 101 // This is above the allowed weight values + for _, overlay := range overlayList.Items { + Expect(lo.FromPtr(overlay.Spec.Weight)).To(BeNumerically("<=", lastWeight)) + lastWeight = int(lo.FromPtr(overlay.Spec.Weight)) + } + }) + It("should order the NodeOverlay by name when the weights are the same", func() { + // Generate 10 NodePools with the same weight + nos := lo.Times(10, func(_ int) *v1alpha1.NodeOverlay { + return test.NodeOverlay(v1alpha1.NodeOverlay{ + Spec: v1alpha1.NodeOverlaySpec{ + Weight: lo.ToPtr[int32](10), + }, + }) + }) + lo.ForEach(nos, func(overlay *v1alpha1.NodeOverlay, _ int) { + ExpectApplied(ctx, env.Client, overlay) + }) + overlayList := &v1alpha1.NodeOverlayList{} + Expect(env.Client.List(ctx, overlayList)).To(BeNil()) + overlayList.OrderByWeight() + + lastName := "zzzzzzzzzzzzzzzzzzzzzzzz" // large string value + for _, overlay := range overlayList.Items { + Expect(overlay.Name < lastName).To(BeTrue()) + lastName = overlay.Name + } + }) + }) + Context("AdjustedPrice", func() { + DescribeTable("should adjust price based overlay values", + func(priceAdjustment string, basePrice float64, expectedPrice float64) { + overlay := &v1alpha1.NodeOverlay{ + Spec: v1alpha1.NodeOverlaySpec{ + PriceAdjustment: priceAdjustment, + }, + } + adjustedPrice := overlay.AdjustedPrice(basePrice) + Expect(adjustedPrice).To(BeNumerically("==", expectedPrice)) + }, + // Percentage adjustment + Entry("No change", "100%", 10.0, 10.0), + Entry("10% decrease", "90%", 10.0, 9.0), + Entry("10% increase", "110%", 10.0, 11.0), + Entry("50% decrease", "50%", 10.0, 5.0), + Entry("100% increase", "200%", 10.0, 20.0), + Entry("Zero price", "0%", 10.0, 0.0), + Entry("Fractional price", "75%", 1.5, 1.125), + // Raw adjustment + Entry("No change", "0", 10.0, 10.0), + Entry("Add 5", "5", 10.0, 15.0), + Entry("Subtract 2.5", "-2.5", 10.0, 7.5), + Entry("Subtract to zero", "-10", 10.0, 0.0), + Entry("Negative Result", "-15", 10.0, -5.0), + Entry("Fractional price", "0.75", 1.25, 2.0), + Entry("Large price adjustment", "100", 0.001, 100.001), + Entry("Small price adjustment", "0.0001", 0.0001, 0.0002), + ) + }) +}) diff --git a/pkg/apis/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/v1alpha1/zz_generated.deepcopy.go index 597b9976f9..f4b113921f 100644 --- a/pkg/apis/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/v1alpha1/zz_generated.deepcopy.go @@ -104,7 +104,7 @@ func (in *NodeOverlaySpec) DeepCopyInto(out *NodeOverlaySpec) { } if in.Weight != nil { in, out := &in.Weight, &out.Weight - *out = new(int64) + *out = new(int32) **out = **in } } From 40e220cf54dc55aac1179e96bdd22166cb3174fb Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Wed, 16 Jul 2025 20:50:38 -0700 Subject: [PATCH 08/11] Update to add price field --- .../crds/karpenter.sh_nodeoverlays.yaml | 13 ++- pkg/apis/crds/karpenter.sh_nodeoverlays.yaml | 13 ++- pkg/apis/v1alpha1/nodeoverlay.go | 19 ++-- pkg/apis/v1alpha1/nodeoverlay_default_test.go | 16 +--- .../nodeoverlay_validation_cel_test.go | 94 ++++++++++++++++--- pkg/apis/v1alpha1/suite_test.go | 4 +- pkg/apis/v1alpha1/zz_generated.deepcopy.go | 10 ++ 7 files changed, 125 insertions(+), 44 deletions(-) diff --git a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml index 82a09f5f0c..b4108b5ab1 100644 --- a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml +++ b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml @@ -65,14 +65,16 @@ spec: x-kubernetes-validations: - message: invalid resource restricted rule: self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods'])) + price: + description: Price specifies amount for an instance types that match the specified labels. Users can override prices using a signed float representing the price override + pattern: ^\d+(\.\d+)?$ + type: string priceAdjustment: - default: 100% description: |- PriceAdjustment specifies a price changes for matching instance types. Accepts either: - A fixed price modifier (e.g., -0.5, 1.2) - - A percentage modifier (e.g., 90% to decrease by 10%, 115% to increase by 15%) - Note: Percentage values must be positive. The adjustment is applied to whatever price unit is in use. - pattern: ^(-?\d*\.?\d+$|\d*\.?\d+%)$ + - A percentage modifier (e.g., +10% for increase, -15% for decrees) + pattern: ^[+-](?:\d+(?:\.\d*)?|\.\d+)(?:%)?$ type: string requirements: description: |- @@ -147,6 +149,9 @@ spec: minimum: 1 type: integer type: object + x-kubernetes-validations: + - message: cannot set both 'price' and 'priceAdjustment' + rule: '!has(self.price) || !has(self.priceAdjustment)' status: description: NodeOverlayStatus defines the observed state of NodeOverlay properties: diff --git a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml index 82a09f5f0c..b4108b5ab1 100644 --- a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml @@ -65,14 +65,16 @@ spec: x-kubernetes-validations: - message: invalid resource restricted rule: self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods'])) + price: + description: Price specifies amount for an instance types that match the specified labels. Users can override prices using a signed float representing the price override + pattern: ^\d+(\.\d+)?$ + type: string priceAdjustment: - default: 100% description: |- PriceAdjustment specifies a price changes for matching instance types. Accepts either: - A fixed price modifier (e.g., -0.5, 1.2) - - A percentage modifier (e.g., 90% to decrease by 10%, 115% to increase by 15%) - Note: Percentage values must be positive. The adjustment is applied to whatever price unit is in use. - pattern: ^(-?\d*\.?\d+$|\d*\.?\d+%)$ + - A percentage modifier (e.g., +10% for increase, -15% for decrees) + pattern: ^[+-](?:\d+(?:\.\d*)?|\.\d+)(?:%)?$ type: string requirements: description: |- @@ -147,6 +149,9 @@ spec: minimum: 1 type: integer type: object + x-kubernetes-validations: + - message: cannot set both 'price' and 'priceAdjustment' + rule: '!has(self.price) || !has(self.priceAdjustment)' status: description: NodeOverlayStatus defines the observed state of NodeOverlay properties: diff --git a/pkg/apis/v1alpha1/nodeoverlay.go b/pkg/apis/v1alpha1/nodeoverlay.go index bcee23b8ac..198cfef74f 100644 --- a/pkg/apis/v1alpha1/nodeoverlay.go +++ b/pkg/apis/v1alpha1/nodeoverlay.go @@ -38,12 +38,14 @@ type NodeOverlaySpec struct { Requirements []v1.NodeSelectorRequirement `json:"requirements,omitempty"` // PriceAdjustment specifies a price changes for matching instance types. Accepts either: // - A fixed price modifier (e.g., -0.5, 1.2) - // - A percentage modifier (e.g., 90% to decrease by 10%, 115% to increase by 15%) - // Note: Percentage values must be positive. The adjustment is applied to whatever price unit is in use. - // +kubebuilder:validation:Pattern=`^(-?\d*\.?\d+$|\d*\.?\d+%)$` - // +kubebuilder:default:="100%" + // - A percentage modifier (e.g., +10% for increase, -15% for decrees) + // +kubebuilder:validation:Pattern=`^[+-](?:\d+(?:\.\d*)?|\.\d+)(?:%)?$` // +optional - PriceAdjustment string `json:"priceAdjustment,omitempty"` + PriceAdjustment *string `json:"priceAdjustment,omitempty"` + // Price specifies amount for an instance types that match the specified labels. Users can override prices using a signed float representing the price override + // +kubebuilder:validation:Pattern=`^\d+(\.\d+)?$` + // +optional + Price *string `json:"price,omitempty"` // Capacity that will add extended resources only, and not replace any existing resources on the nodes. // These extended resources will be appended to the node's existing resource list. // Note: This field does not modify or override standard resources like CPU, memory, ephemeral-storage, or pods. @@ -72,6 +74,7 @@ type NodeOverlay struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` + // +kubebuilder:validation:XValidation:message="cannot set both 'price' and 'priceAdjustment'",rule="!has(self.price) || !has(self.priceAdjustment)" Spec NodeOverlaySpec `json:"spec"` Status NodeOverlayStatus `json:"status,omitempty"` } @@ -101,12 +104,12 @@ func (nol *NodeOverlayList) OrderByWeight() { func (in *NodeOverlay) AdjustedPrice(instanceTypePrice float64) float64 { // Check if adjustment is a percentage - isPercentage := strings.HasSuffix(in.Spec.PriceAdjustment, "%") - adjustment := in.Spec.PriceAdjustment + isPercentage := strings.HasSuffix(lo.FromPtr(in.Spec.PriceAdjustment), "%") + adjustment := lo.FromPtr(in.Spec.PriceAdjustment) // Remove the percentage sign if present if isPercentage { - adjustment = strings.TrimSuffix(in.Spec.PriceAdjustment, "%") + adjustment = strings.TrimSuffix(lo.FromPtr(in.Spec.PriceAdjustment), "%") } // Parse the adjustment value diff --git a/pkg/apis/v1alpha1/nodeoverlay_default_test.go b/pkg/apis/v1alpha1/nodeoverlay_default_test.go index c816470f33..180ac741c3 100644 --- a/pkg/apis/v1alpha1/nodeoverlay_default_test.go +++ b/pkg/apis/v1alpha1/nodeoverlay_default_test.go @@ -22,12 +22,10 @@ import ( "github.com/Pallinder/go-randomdata" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" - v1 "sigs.k8s.io/karpenter/pkg/apis/v1" . "sigs.k8s.io/karpenter/pkg/apis/v1alpha1" ) @@ -37,22 +35,16 @@ var _ = Describe("CEL/Default", func() { BeforeEach(func() { nodeOverlay = &NodeOverlay{ ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}, - Spec: NodeOverlaySpec{ - Requirements: []corev1.NodeSelectorRequirement{ - { - Key: v1.CapacityTypeLabelKey, - Operator: corev1.NodeSelectorOpExists, - }, - }, - }, + Spec: NodeOverlaySpec{}, } }) Context("Defaults/TopLevel", func() { It("should default the priceAdjustment field when undefined", func() { Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(nodeOverlay), nodeOverlay)).To(Succeed()) - Expect(nodeOverlay.Spec.PriceAdjustment).ToNot(BeNil()) - Expect(nodeOverlay.Spec.PriceAdjustment).To(Equal("100%")) + Expect(nodeOverlay.Spec.Requirements).To(BeNil()) + Expect(nodeOverlay.Spec.PriceAdjustment).To(BeNil()) + Expect(nodeOverlay.Spec.Price).To(BeNil()) Expect(nodeOverlay.Spec.Capacity).To(BeNil()) Expect(nodeOverlay.Spec.Weight).To(BeNil()) }) diff --git a/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go b/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go index 882c8f4650..89e93fda27 100644 --- a/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go +++ b/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go @@ -23,6 +23,7 @@ import ( "github.com/Pallinder/go-randomdata" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/samber/lo" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -193,33 +194,98 @@ var _ = Describe("CEL/Validation", func() { } }) }) + It("shout not be able to set both price and priceAdjustment", func() { + nodeOverlay.Spec.Price = lo.ToPtr("0.432") + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("+10%") + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + }) Context("priceAdjustment", func() { - It("should allow percentage value for priceAdjustment field", func() { - nodeOverlay.Spec.PriceAdjustment = "1%" - Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + DescribeTable("Invalid Input", + func(input string) { + nodeOverlay.Spec.Price = lo.ToPtr(input) + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + }, + Entry("No explicit plus sign allowed", "+42"), + Entry("Must have leading digit", ".5"), + Entry("Must have trailing digits after decimal", "42."), + Entry("No percentage sign allowed", "42%"), + Entry("No commas allowed", "3,14"), + Entry("No scientific notation", "1e10"), + Entry("No hex notation", "0x42"), + Entry("No text", "forty-two"), + Entry("No letters", "42a"), + Entry("No spaces", "42 "), + Entry("No spaces", " 42"), + Entry("Multiple decimal points", "42.0.0"), + Entry("Just a sign", "-"), + Entry("Just a decimal point", "."), + Entry("No leading digit after sign", "-.42"), + ) + It("should not allow an unsigned priceAdjustment percentage", func() { + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("1%") + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) }) - It("should allow percentage greater then 100%", func() { - nodeOverlay.Spec.PriceAdjustment = "123%" - Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + It("should not allow an unsigned priceAdjustment integer", func() { + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("1") + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) }) - It("should not allow percentage less then 0%", func() { - nodeOverlay.Spec.PriceAdjustment = "-1%" + It("should not allow an unsigned priceAdjustment float", func() { + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("1.3") Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) }) - It("should allow integer value", func() { - nodeOverlay.Spec.PriceAdjustment = "43" + It("should allow positive percentage value for priceAdjustment field", func() { + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("+1%") + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + It("should allow negative percentage less then 0%", func() { + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("-1%") + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + It("should allow positive integer value", func() { + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("+43") Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) }) It("should allow negative integer value", func() { - nodeOverlay.Spec.PriceAdjustment = "-43" + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("-43") Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) }) - It("should allow float value", func() { - nodeOverlay.Spec.PriceAdjustment = "34.43" + It("should allow positive float value", func() { + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("+34.43") Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) }) It("should allow negative float value", func() { - nodeOverlay.Spec.PriceAdjustment = "-34.43" + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("-34.43") + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + }) + Context("price", func() { + DescribeTable("Invalid Input", + func(input string) { + nodeOverlay.Spec.Price = lo.ToPtr(input) + Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) + }, + Entry("No explicit plus sign allowed", "+42"), + Entry("Must have leading digit", ".5"), + Entry("Must have trailing digits after decimal", "42."), + Entry("No percentage sign allowed", "42%"), + Entry("No commas allowed", "3,14"), + Entry("No scientific notation", "1e10"), + Entry("No hex notation", "0x42"), + Entry("No text", "forty-two"), + Entry("No letters", "42a"), + Entry("No spaces", "42 "), + Entry("No spaces", " 42"), + Entry("Multiple decimal points", "42.0.0"), + Entry("Just a sign", "-"), + Entry("Just a decimal point", "."), + Entry("No leading digit after sign", "-.42"), + ) + It("should allow integer value", func() { + nodeOverlay.Spec.Price = lo.ToPtr("43") + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + It("should allow float value", func() { + nodeOverlay.Spec.Price = lo.ToPtr("34.43") Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) }) }) diff --git a/pkg/apis/v1alpha1/suite_test.go b/pkg/apis/v1alpha1/suite_test.go index d0f9980c24..23b51feaa0 100644 --- a/pkg/apis/v1alpha1/suite_test.go +++ b/pkg/apis/v1alpha1/suite_test.go @@ -40,7 +40,7 @@ var env *test.Environment func TestAPIs(t *testing.T) { ctx = TestContextWithLogger(t) RegisterFailHandler(Fail) - RunSpecs(t, "v1") + RunSpecs(t, "v1alpha1") } var _ = BeforeSuite(func() { @@ -107,7 +107,7 @@ var _ = Describe("NodeOverlay", func() { func(priceAdjustment string, basePrice float64, expectedPrice float64) { overlay := &v1alpha1.NodeOverlay{ Spec: v1alpha1.NodeOverlaySpec{ - PriceAdjustment: priceAdjustment, + PriceAdjustment: lo.ToPtr(priceAdjustment), }, } adjustedPrice := overlay.AdjustedPrice(basePrice) diff --git a/pkg/apis/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/v1alpha1/zz_generated.deepcopy.go index f4b113921f..cbe4a5b401 100644 --- a/pkg/apis/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/v1alpha1/zz_generated.deepcopy.go @@ -95,6 +95,16 @@ func (in *NodeOverlaySpec) DeepCopyInto(out *NodeOverlaySpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.PriceAdjustment != nil { + in, out := &in.PriceAdjustment, &out.PriceAdjustment + *out = new(string) + **out = **in + } + if in.Price != nil { + in, out := &in.Price, &out.Price + *out = new(string) + **out = **in + } if in.Capacity != nil { in, out := &in.Capacity, &out.Capacity *out = make(v1.ResourceList, len(*in)) From 6ac06832968e18a71829d835a065d953eb20bc48 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Thu, 17 Jul 2025 14:38:40 -0700 Subject: [PATCH 09/11] Update price adjustment to include price override --- .../crds/karpenter.sh_nodeoverlays.yaml | 2 +- pkg/apis/crds/karpenter.sh_nodeoverlays.yaml | 2 +- pkg/apis/v1/labels.go | 1 - pkg/apis/v1alpha1/nodeoverlay.go | 14 +++++- .../nodeoverlay_validation_cel_test.go | 7 --- pkg/apis/v1alpha1/suite_test.go | 43 +++++++++++++------ 6 files changed, 44 insertions(+), 25 deletions(-) diff --git a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml index b4108b5ab1..cfb4d5b8af 100644 --- a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml +++ b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml @@ -64,7 +64,7 @@ spec: type: object x-kubernetes-validations: - message: invalid resource restricted - rule: self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods'])) + rule: self.all(x, !(x in ['cpu', 'memory', 'pods'])) price: description: Price specifies amount for an instance types that match the specified labels. Users can override prices using a signed float representing the price override pattern: ^\d+(\.\d+)?$ diff --git a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml index b4108b5ab1..cfb4d5b8af 100644 --- a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml @@ -64,7 +64,7 @@ spec: type: object x-kubernetes-validations: - message: invalid resource restricted - rule: self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods'])) + rule: self.all(x, !(x in ['cpu', 'memory', 'pods'])) price: description: Price specifies amount for an instance types that match the specified labels. Users can override prices using a signed float representing the price override pattern: ^\d+(\.\d+)?$ diff --git a/pkg/apis/v1/labels.go b/pkg/apis/v1/labels.go index 415eec5ea9..f2f7686c44 100644 --- a/pkg/apis/v1/labels.go +++ b/pkg/apis/v1/labels.go @@ -96,7 +96,6 @@ var ( WellKnownResources = sets.New[v1.ResourceName]( v1.ResourceCPU, v1.ResourceMemory, - v1.ResourceEphemeralStorage, v1.ResourcePods, ) diff --git a/pkg/apis/v1alpha1/nodeoverlay.go b/pkg/apis/v1alpha1/nodeoverlay.go index 198cfef74f..d60c8ef0b1 100644 --- a/pkg/apis/v1alpha1/nodeoverlay.go +++ b/pkg/apis/v1alpha1/nodeoverlay.go @@ -49,7 +49,7 @@ type NodeOverlaySpec struct { // Capacity that will add extended resources only, and not replace any existing resources on the nodes. // These extended resources will be appended to the node's existing resource list. // Note: This field does not modify or override standard resources like CPU, memory, ephemeral-storage, or pods. - // +kubebuilder:validation:XValidation:message="invalid resource restricted",rule="self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods']))" + // +kubebuilder:validation:XValidation:message="invalid resource restricted",rule="self.all(x, !(x in ['cpu', 'memory', 'pods']))" // +optional Capacity v1.ResourceList `json:"capacity,omitempty"` // Weight is the priority given to the nodeoverlay while overriding node attributes. A higher @@ -103,6 +103,15 @@ func (nol *NodeOverlayList) OrderByWeight() { } func (in *NodeOverlay) AdjustedPrice(instanceTypePrice float64) float64 { + // if price or price adjustment is not defined, then we will return the same price + if in.Spec.Price == nil && in.Spec.PriceAdjustment == nil { + return instanceTypePrice + } + // if price is defined, then we will return the value given in the overlay + if in.Spec.Price != nil { + return lo.Must(strconv.ParseFloat(lo.FromPtr(in.Spec.Price), 64)) + } + // Check if adjustment is a percentage isPercentage := strings.HasSuffix(lo.FromPtr(in.Spec.PriceAdjustment), "%") adjustment := lo.FromPtr(in.Spec.PriceAdjustment) @@ -116,7 +125,8 @@ func (in *NodeOverlay) AdjustedPrice(instanceTypePrice float64) float64 { // Due to the CEL validation we can assume that // there will always be a valid float provided into the spec adjValue := lo.Must(strconv.ParseFloat(adjustment, 64)) + adjustedPrice := lo.Ternary(isPercentage, instanceTypePrice*(1+(adjValue/100)), instanceTypePrice+adjValue) // Apply the adjustment - return lo.Ternary(isPercentage, instanceTypePrice*(adjValue/100), instanceTypePrice+adjValue) + return lo.Ternary(adjustedPrice >= 0, adjustedPrice, 0) } diff --git a/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go b/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go index 89e93fda27..12a3568b1d 100644 --- a/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go +++ b/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go @@ -311,13 +311,6 @@ var _ = Describe("CEL/Validation", func() { Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) }) - It("should not allow ephemeral-storage resources override", func() { - nodeOverlay.Spec.Capacity = corev1.ResourceList{ - corev1.ResourceEphemeralStorage: resource.MustParse("34Gi"), - } - Expect(env.Client.Create(ctx, nodeOverlay)).ToNot(Succeed()) - Expect(nodeOverlay.RuntimeValidate(ctx)).ToNot(Succeed()) - }) It("should not allow pod resources override", func() { nodeOverlay.Spec.Capacity = corev1.ResourceList{ corev1.ResourcePods: resource.MustParse("324"), diff --git a/pkg/apis/v1alpha1/suite_test.go b/pkg/apis/v1alpha1/suite_test.go index 23b51feaa0..5068747013 100644 --- a/pkg/apis/v1alpha1/suite_test.go +++ b/pkg/apis/v1alpha1/suite_test.go @@ -114,22 +114,39 @@ var _ = Describe("NodeOverlay", func() { Expect(adjustedPrice).To(BeNumerically("==", expectedPrice)) }, // Percentage adjustment - Entry("No change", "100%", 10.0, 10.0), - Entry("10% decrease", "90%", 10.0, 9.0), - Entry("10% increase", "110%", 10.0, 11.0), - Entry("50% decrease", "50%", 10.0, 5.0), - Entry("100% increase", "200%", 10.0, 20.0), - Entry("Zero price", "0%", 10.0, 0.0), - Entry("Fractional price", "75%", 1.5, 1.125), + Entry("No change", "0%", 10.0, 10.0), + Entry("10% decrease", "-10%", 10.0, 9.0), + Entry("10% increase", "+10%", 10.0, 11.0), + Entry("50% decrease", "-50%", 10.0, 5.0), + Entry("100% increase", "+100%", 10.0, 20.0), + Entry("Zero price", "-100%", 10.0, 0.0), + Entry("Zero price", "-200%", 10.0, 0.0), + Entry("Fractional price", "-25%", 1.5, 1.125), // Raw adjustment - Entry("No change", "0", 10.0, 10.0), - Entry("Add 5", "5", 10.0, 15.0), + Entry("No change", "+0", 10.0, 10.0), + Entry("Add 5", "+5", 10.0, 15.0), Entry("Subtract 2.5", "-2.5", 10.0, 7.5), Entry("Subtract to zero", "-10", 10.0, 0.0), - Entry("Negative Result", "-15", 10.0, -5.0), - Entry("Fractional price", "0.75", 1.25, 2.0), - Entry("Large price adjustment", "100", 0.001, 100.001), - Entry("Small price adjustment", "0.0001", 0.0001, 0.0002), + Entry("Negative Result", "-15", 10.0, 0.0), + Entry("Fractional price", "+0.75", 1.25, 2.0), + Entry("Large price adjustment", "+100", 0.001, 100.001), + Entry("Small price adjustment", "+0.0001", 0.0001, 0.0002), ) + It("should override price", func() { + overlay := &v1alpha1.NodeOverlay{ + Spec: v1alpha1.NodeOverlaySpec{ + Price: lo.ToPtr("80.0"), + }, + } + adjustedPrice := overlay.AdjustedPrice(82781.0) + Expect(adjustedPrice).To(BeNumerically("==", 80)) + }) + It("should provide the same if price or priceAdjustment is not provided", func() { + overlay := &v1alpha1.NodeOverlay{ + Spec: v1alpha1.NodeOverlaySpec{}, + } + adjustedPrice := overlay.AdjustedPrice(82781.0) + Expect(adjustedPrice).To(BeNumerically("==", 82781)) + }) }) }) From 8b9a6dde5961b8efd2b95cdcd56a7a34cd9c2eb2 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Mon, 21 Jul 2025 11:04:03 -0700 Subject: [PATCH 10/11] Feedback changes --- kwok/charts/crds/karpenter.sh_nodeoverlays.yaml | 17 +++++++++-------- pkg/apis/crds/karpenter.sh_nodeoverlays.yaml | 17 +++++++++-------- pkg/apis/v1/labels.go | 1 + pkg/apis/v1alpha1/nodeoverlay.go | 17 ++++++++--------- pkg/apis/v1alpha1/nodeoverlay_default_test.go | 12 ++++++++++-- pkg/apis/v1alpha1/nodeoverlay_status.go | 2 -- .../v1alpha1/nodeoverlay_validation_cel_test.go | 7 ++++++- pkg/apis/v1alpha1/suite_test.go | 13 +++++++++++++ 8 files changed, 56 insertions(+), 30 deletions(-) diff --git a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml index b5dfd9a435..7f4a2bcaea 100644 --- a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml +++ b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml @@ -58,13 +58,13 @@ spec: pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true description: |- - Capacity that will add extended resources only, and not replace any existing resources on the nodes. - These extended resources will be appended to the node's existing resource list. + Capacity adds extended resources only, and not replace any existing resources. + These extended resources are appended to the node's existing resource list. Note: This field does not modify or override standard resources like CPU, memory, ephemeral-storage, or pods. type: object x-kubernetes-validations: - message: invalid resource restricted - rule: self.all(x, !(x in ['cpu', 'memory', 'pods'])) + rule: self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods'])) price: description: Price specifies amount for an instance types that match the specified labels. Users can override prices using a signed float representing the price override pattern: ^\d+(\.\d+)?$ @@ -139,15 +139,16 @@ spec: rule: 'self.all(x, (x.operator == ''Gt'' || x.operator == ''Lt'') ? (x.values.size() == 1 && int(x.values[0]) >= 0) : true)' weight: description: |- - Weight is the priority given to the nodeoverlay while overriding node attributes. A higher - numerical weight indicates that this nodeoverlay will be ordered - ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight - will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, - we will merge them in alphabetical order. + Weight defines the priority of this NodeOverlay when overriding node attributes. + NodeOverlays with higher numerical weights take precedence over those with lower weights. + If no weight is specified, the NodeOverlay is treated as having a weight of 0. + When multiple NodeOverlays have identical weights, they are merged in alphabetical order. format: int32 maximum: 100 minimum: 1 type: integer + required: + - requirements type: object x-kubernetes-validations: - message: cannot set both 'price' and 'priceAdjustment' diff --git a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml index b5dfd9a435..7f4a2bcaea 100644 --- a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml @@ -58,13 +58,13 @@ spec: pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true description: |- - Capacity that will add extended resources only, and not replace any existing resources on the nodes. - These extended resources will be appended to the node's existing resource list. + Capacity adds extended resources only, and not replace any existing resources. + These extended resources are appended to the node's existing resource list. Note: This field does not modify or override standard resources like CPU, memory, ephemeral-storage, or pods. type: object x-kubernetes-validations: - message: invalid resource restricted - rule: self.all(x, !(x in ['cpu', 'memory', 'pods'])) + rule: self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods'])) price: description: Price specifies amount for an instance types that match the specified labels. Users can override prices using a signed float representing the price override pattern: ^\d+(\.\d+)?$ @@ -139,15 +139,16 @@ spec: rule: 'self.all(x, (x.operator == ''Gt'' || x.operator == ''Lt'') ? (x.values.size() == 1 && int(x.values[0]) >= 0) : true)' weight: description: |- - Weight is the priority given to the nodeoverlay while overriding node attributes. A higher - numerical weight indicates that this nodeoverlay will be ordered - ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight - will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, - we will merge them in alphabetical order. + Weight defines the priority of this NodeOverlay when overriding node attributes. + NodeOverlays with higher numerical weights take precedence over those with lower weights. + If no weight is specified, the NodeOverlay is treated as having a weight of 0. + When multiple NodeOverlays have identical weights, they are merged in alphabetical order. format: int32 maximum: 100 minimum: 1 type: integer + required: + - requirements type: object x-kubernetes-validations: - message: cannot set both 'price' and 'priceAdjustment' diff --git a/pkg/apis/v1/labels.go b/pkg/apis/v1/labels.go index f2f7686c44..415eec5ea9 100644 --- a/pkg/apis/v1/labels.go +++ b/pkg/apis/v1/labels.go @@ -96,6 +96,7 @@ var ( WellKnownResources = sets.New[v1.ResourceName]( v1.ResourceCPU, v1.ResourceMemory, + v1.ResourceEphemeralStorage, v1.ResourcePods, ) diff --git a/pkg/apis/v1alpha1/nodeoverlay.go b/pkg/apis/v1alpha1/nodeoverlay.go index b57cc98b61..2ad29a74fd 100644 --- a/pkg/apis/v1alpha1/nodeoverlay.go +++ b/pkg/apis/v1alpha1/nodeoverlay.go @@ -34,7 +34,7 @@ type NodeOverlaySpec struct { // +kubebuilder:validation:XValidation:message="requirements with operator 'In' must have a value defined",rule="self.all(x, x.operator == 'In' ? x.values.size() != 0 : true)" // +kubebuilder:validation:XValidation: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)" // +kubebuilder:validation:MaxItems:=100 - // +optional + // +required Requirements []v1.NodeSelectorRequirement `json:"requirements,omitempty"` // PriceAdjustment specifies the price change for matching instance types. Accepts either: // - A fixed price modifier (e.g., -0.5, 1.2) @@ -46,17 +46,16 @@ type NodeOverlaySpec struct { // +kubebuilder:validation:Pattern=`^\d+(\.\d+)?$` // +optional Price *string `json:"price,omitempty"` - // Capacity that will add extended resources only, and not replace any existing resources on the nodes. - // These extended resources will be appended to the node's existing resource list. + // Capacity adds extended resources only, and not replace any existing resources. + // These extended resources are appended to the node's existing resource list. // Note: This field does not modify or override standard resources like CPU, memory, ephemeral-storage, or pods. - // +kubebuilder:validation:XValidation:message="invalid resource restricted",rule="self.all(x, !(x in ['cpu', 'memory', 'pods']))" + // +kubebuilder:validation:XValidation:message="invalid resource restricted",rule="self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods']))" // +optional Capacity v1.ResourceList `json:"capacity,omitempty"` - // Weight is the priority given to the nodeoverlay while overriding node attributes. A higher - // numerical weight indicates that this nodeoverlay will be ordered - // ahead of other nodeoverlay with lower weights. A nodeoverlay with no weight - // will be treated as if it is a nodeoverlay with a weight of 0. Two nodeoverlays that have the same weight, - // we will merge them in alphabetical order. + // Weight defines the priority of this NodeOverlay when overriding node attributes. + // NodeOverlays with higher numerical weights take precedence over those with lower weights. + // If no weight is specified, the NodeOverlay is treated as having a weight of 0. + // When multiple NodeOverlays have identical weights, they are merged in alphabetical order. // +kubebuilder:validation:Minimum:=1 // +kubebuilder:validation:Maximum:=100 // +optional diff --git a/pkg/apis/v1alpha1/nodeoverlay_default_test.go b/pkg/apis/v1alpha1/nodeoverlay_default_test.go index 180ac741c3..77de873ea2 100644 --- a/pkg/apis/v1alpha1/nodeoverlay_default_test.go +++ b/pkg/apis/v1alpha1/nodeoverlay_default_test.go @@ -22,6 +22,7 @@ import ( "github.com/Pallinder/go-randomdata" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -35,14 +36,21 @@ var _ = Describe("CEL/Default", func() { BeforeEach(func() { nodeOverlay = &NodeOverlay{ ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}, - Spec: NodeOverlaySpec{}, + Spec: NodeOverlaySpec{ + Requirements: []corev1.NodeSelectorRequirement{ + { + Key: "test", + Operator: corev1.NodeSelectorOpExists, + }, + }, + }, } }) Context("Defaults/TopLevel", func() { It("should default the priceAdjustment field when undefined", func() { Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(nodeOverlay), nodeOverlay)).To(Succeed()) - Expect(nodeOverlay.Spec.Requirements).To(BeNil()) + Expect(nodeOverlay.Spec.Requirements).ToNot(BeNil()) Expect(nodeOverlay.Spec.PriceAdjustment).To(BeNil()) Expect(nodeOverlay.Spec.Price).To(BeNil()) Expect(nodeOverlay.Spec.Capacity).To(BeNil()) diff --git a/pkg/apis/v1alpha1/nodeoverlay_status.go b/pkg/apis/v1alpha1/nodeoverlay_status.go index 7d5df21c92..6c7bce70d0 100644 --- a/pkg/apis/v1alpha1/nodeoverlay_status.go +++ b/pkg/apis/v1alpha1/nodeoverlay_status.go @@ -20,8 +20,6 @@ import ( "github.com/awslabs/operatorpkg/status" ) -// Note(Remove after decision point): Only set this up for runtime validation I will need to assess if this is necessary, but I get the feeling it will be - const ( // ConditionTypeValidationSucceeded = "ValidationSucceeded" condition indicates that the // runtime-based configuration is valid and conflict for this NodeOverlay diff --git a/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go b/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go index 12a3568b1d..e2701b66fb 100644 --- a/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go +++ b/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go @@ -173,7 +173,12 @@ var _ = Describe("CEL/Validation", func() { Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) }) It("should allow empty requirements", func() { - nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{} + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + { + Key: "test", + Operator: corev1.NodeSelectorOpExists, + }, + } Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) }) diff --git a/pkg/apis/v1alpha1/suite_test.go b/pkg/apis/v1alpha1/suite_test.go index 5068747013..3e4b537fd1 100644 --- a/pkg/apis/v1alpha1/suite_test.go +++ b/pkg/apis/v1alpha1/suite_test.go @@ -25,6 +25,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/samber/lo" + corev1 "k8s.io/api/core/v1" "sigs.k8s.io/karpenter/pkg/apis" "sigs.k8s.io/karpenter/pkg/apis/v1alpha1" @@ -63,6 +64,12 @@ var _ = Describe("NodeOverlay", func() { return test.NodeOverlay(v1alpha1.NodeOverlay{ Spec: v1alpha1.NodeOverlaySpec{ Weight: lo.ToPtr[int32](int32(rand.IntN(100) + 1)), //nolint:gosec + Requirements: []corev1.NodeSelectorRequirement{ + { + Key: "test", + Operator: corev1.NodeSelectorOpExists, + }, + }, }, }) }) @@ -85,6 +92,12 @@ var _ = Describe("NodeOverlay", func() { return test.NodeOverlay(v1alpha1.NodeOverlay{ Spec: v1alpha1.NodeOverlaySpec{ Weight: lo.ToPtr[int32](10), + Requirements: []corev1.NodeSelectorRequirement{ + { + Key: "test", + Operator: corev1.NodeSelectorOpExists, + }, + }, }, }) }) From 01465d81d33d2381b838df66d08b20a7905f4ee2 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Mon, 21 Jul 2025 17:24:08 -0700 Subject: [PATCH 11/11] Add price overlay applied label --- .../crds/karpenter.sh_nodeoverlays.yaml | 16 ++-- pkg/apis/crds/karpenter.sh_nodeoverlays.yaml | 16 ++-- pkg/apis/v1/labels.go | 4 +- pkg/apis/v1/nodeclaim_defaults.go | 22 +++++ pkg/apis/v1/nodepool_defaults.go | 24 ++++++ pkg/apis/v1alpha1/labels.go | 24 ++++++ pkg/apis/v1alpha1/nodeoverlay.go | 25 +++--- pkg/apis/v1alpha1/nodeoverlay_default_test.go | 60 ------------- pkg/apis/v1alpha1/nodeoverlay_validation.go | 85 ++----------------- ...test.go => nodeoverlay_validation_test.go} | 42 +++++++-- 10 files changed, 149 insertions(+), 169 deletions(-) create mode 100644 pkg/apis/v1/nodeclaim_defaults.go create mode 100644 pkg/apis/v1/nodepool_defaults.go create mode 100644 pkg/apis/v1alpha1/labels.go delete mode 100644 pkg/apis/v1alpha1/nodeoverlay_default_test.go rename pkg/apis/v1alpha1/{nodeoverlay_validation_cel_test.go => nodeoverlay_validation_test.go} (89%) diff --git a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml index 7f4a2bcaea..47b46372c1 100644 --- a/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml +++ b/kwok/charts/crds/karpenter.sh_nodeoverlays.yaml @@ -13,6 +13,8 @@ spec: kind: NodeOverlay listKind: NodeOverlayList plural: nodeoverlays + shortNames: + - overlays singular: nodeoverlay scope: Cluster versions: @@ -58,9 +60,9 @@ spec: pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true description: |- - Capacity adds extended resources only, and not replace any existing resources. + Capacity adds extended resources only, and does not replace any existing resources. These extended resources are appended to the node's existing resource list. - Note: This field does not modify or override standard resources like CPU, memory, ephemeral-storage, or pods. + Note: This field does not modify or override standard resources like cpu, memory, ephemeral-storage, or pods. type: object x-kubernetes-validations: - message: invalid resource restricted @@ -74,11 +76,11 @@ spec: PriceAdjustment specifies the price change for matching instance types. Accepts either: - A fixed price modifier (e.g., -0.5, 1.2) - A percentage modifier (e.g., +10% for increase, -15% for decrees) - pattern: ^[+-](?:\d+(?:\.\d*)?|\.\d+)(?:%)?$ + pattern: ^(([+-]{1}(\d*\.?\d+))|(\+{1}\d*\.?\d+%)|(^(-\d{1,2}(\.\d+)?%)$)|(-100%))$ type: string requirements: description: |- - Requirements constrain when this NodeOverlay is applied during scheduling simulation. + Requirements constrain when this NodeOverlay is applied during scheduling simulations. These requirements can match: - Well-known labels (e.g., node.kubernetes.io/instance-type, karpenter.sh/nodepool) - Custom labels from NodePool's spec.template.labels @@ -133,6 +135,8 @@ spec: maxItems: 100 type: array x-kubernetes-validations: + - message: requirements with operator 'NotIn' must have a value defined + rule: 'self.all(x, x.operator == ''NotIn'' ? x.values.size() != 0 : true)' - message: requirements with operator 'In' must have a value defined 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 @@ -141,10 +145,10 @@ spec: description: |- Weight defines the priority of this NodeOverlay when overriding node attributes. NodeOverlays with higher numerical weights take precedence over those with lower weights. - If no weight is specified, the NodeOverlay is treated as having a weight of 0. + If no weight is specified, the NodeOverlay is treated as having a weight of 0. When multiple NodeOverlays have identical weights, they are merged in alphabetical order. format: int32 - maximum: 100 + maximum: 10000 minimum: 1 type: integer required: diff --git a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml index 7f4a2bcaea..47b46372c1 100644 --- a/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeoverlays.yaml @@ -13,6 +13,8 @@ spec: kind: NodeOverlay listKind: NodeOverlayList plural: nodeoverlays + shortNames: + - overlays singular: nodeoverlay scope: Cluster versions: @@ -58,9 +60,9 @@ spec: pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true description: |- - Capacity adds extended resources only, and not replace any existing resources. + Capacity adds extended resources only, and does not replace any existing resources. These extended resources are appended to the node's existing resource list. - Note: This field does not modify or override standard resources like CPU, memory, ephemeral-storage, or pods. + Note: This field does not modify or override standard resources like cpu, memory, ephemeral-storage, or pods. type: object x-kubernetes-validations: - message: invalid resource restricted @@ -74,11 +76,11 @@ spec: PriceAdjustment specifies the price change for matching instance types. Accepts either: - A fixed price modifier (e.g., -0.5, 1.2) - A percentage modifier (e.g., +10% for increase, -15% for decrees) - pattern: ^[+-](?:\d+(?:\.\d*)?|\.\d+)(?:%)?$ + pattern: ^(([+-]{1}(\d*\.?\d+))|(\+{1}\d*\.?\d+%)|(^(-\d{1,2}(\.\d+)?%)$)|(-100%))$ type: string requirements: description: |- - Requirements constrain when this NodeOverlay is applied during scheduling simulation. + Requirements constrain when this NodeOverlay is applied during scheduling simulations. These requirements can match: - Well-known labels (e.g., node.kubernetes.io/instance-type, karpenter.sh/nodepool) - Custom labels from NodePool's spec.template.labels @@ -133,6 +135,8 @@ spec: maxItems: 100 type: array x-kubernetes-validations: + - message: requirements with operator 'NotIn' must have a value defined + rule: 'self.all(x, x.operator == ''NotIn'' ? x.values.size() != 0 : true)' - message: requirements with operator 'In' must have a value defined 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 @@ -141,10 +145,10 @@ spec: description: |- Weight defines the priority of this NodeOverlay when overriding node attributes. NodeOverlays with higher numerical weights take precedence over those with lower weights. - If no weight is specified, the NodeOverlay is treated as having a weight of 0. + If no weight is specified, the NodeOverlay is treated as having a weight of 0. When multiple NodeOverlays have identical weights, they are merged in alphabetical order. format: int32 - maximum: 100 + maximum: 10000 minimum: 1 type: integer required: diff --git a/pkg/apis/v1/labels.go b/pkg/apis/v1/labels.go index 415eec5ea9..ae41722314 100644 --- a/pkg/apis/v1/labels.go +++ b/pkg/apis/v1/labels.go @@ -91,8 +91,8 @@ var ( v1.LabelWindowsBuild, ) - // WellKnownResources are resources that are known expected from the instance types - // provided by the cloud providers. + // WellKnownResources are resources that are expected from the instance types + // provided by cloud providers. WellKnownResources = sets.New[v1.ResourceName]( v1.ResourceCPU, v1.ResourceMemory, diff --git a/pkg/apis/v1/nodeclaim_defaults.go b/pkg/apis/v1/nodeclaim_defaults.go new file mode 100644 index 0000000000..a7f9938907 --- /dev/null +++ b/pkg/apis/v1/nodeclaim_defaults.go @@ -0,0 +1,22 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1 + +import "context" + +// SetDefaults for the NodeClaim +func (in *NodeClaim) SetDefaults(_ context.Context) {} diff --git a/pkg/apis/v1/nodepool_defaults.go b/pkg/apis/v1/nodepool_defaults.go new file mode 100644 index 0000000000..98d0990aee --- /dev/null +++ b/pkg/apis/v1/nodepool_defaults.go @@ -0,0 +1,24 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1 + +import ( + "context" +) + +// SetDefaults for the NodePool +func (in *NodePool) SetDefaults(_ context.Context) {} diff --git a/pkg/apis/v1alpha1/labels.go b/pkg/apis/v1alpha1/labels.go new file mode 100644 index 0000000000..4aef81985f --- /dev/null +++ b/pkg/apis/v1alpha1/labels.go @@ -0,0 +1,24 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import "sigs.k8s.io/karpenter/pkg/apis" + +const ( + PriceOverlayAppliedAnnotationKey = apis.Group + "/price-overlay-applied" + CapacityOverlayAppliedAnnotationKey = apis.Group + "/capacity-overlay-applied" +) diff --git a/pkg/apis/v1alpha1/nodeoverlay.go b/pkg/apis/v1alpha1/nodeoverlay.go index 2ad29a74fd..2a8c2ce7c7 100644 --- a/pkg/apis/v1alpha1/nodeoverlay.go +++ b/pkg/apis/v1alpha1/nodeoverlay.go @@ -27,10 +27,11 @@ import ( ) type NodeOverlaySpec struct { - // Requirements constrain when this NodeOverlay is applied during scheduling simulation. + // Requirements constrain when this NodeOverlay is applied during scheduling simulations. // These requirements can match: // - Well-known labels (e.g., node.kubernetes.io/instance-type, karpenter.sh/nodepool) // - Custom labels from NodePool's spec.template.labels + // +kubebuilder:validation:XValidation:message="requirements with operator 'NotIn' must have a value defined",rule="self.all(x, x.operator == 'NotIn' ? x.values.size() != 0 : true)" // +kubebuilder:validation:XValidation:message="requirements with operator 'In' must have a value defined",rule="self.all(x, x.operator == 'In' ? x.values.size() != 0 : true)" // +kubebuilder:validation:XValidation: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)" // +kubebuilder:validation:MaxItems:=100 @@ -39,32 +40,32 @@ type NodeOverlaySpec struct { // PriceAdjustment specifies the price change for matching instance types. Accepts either: // - A fixed price modifier (e.g., -0.5, 1.2) // - A percentage modifier (e.g., +10% for increase, -15% for decrees) - // +kubebuilder:validation:Pattern=`^[+-](?:\d+(?:\.\d*)?|\.\d+)(?:%)?$` + // +kubebuilder:validation:Pattern=`^(([+-]{1}(\d*\.?\d+))|(\+{1}\d*\.?\d+%)|(^(-\d{1,2}(\.\d+)?%)$)|(-100%))$` // +optional PriceAdjustment *string `json:"priceAdjustment,omitempty"` // Price specifies amount for an instance types that match the specified labels. Users can override prices using a signed float representing the price override // +kubebuilder:validation:Pattern=`^\d+(\.\d+)?$` // +optional Price *string `json:"price,omitempty"` - // Capacity adds extended resources only, and not replace any existing resources. + // Capacity adds extended resources only, and does not replace any existing resources. // These extended resources are appended to the node's existing resource list. - // Note: This field does not modify or override standard resources like CPU, memory, ephemeral-storage, or pods. + // Note: This field does not modify or override standard resources like cpu, memory, ephemeral-storage, or pods. // +kubebuilder:validation:XValidation:message="invalid resource restricted",rule="self.all(x, !(x in ['cpu', 'memory', 'ephemeral-storage', 'pods']))" // +optional Capacity v1.ResourceList `json:"capacity,omitempty"` // Weight defines the priority of this NodeOverlay when overriding node attributes. // NodeOverlays with higher numerical weights take precedence over those with lower weights. - // If no weight is specified, the NodeOverlay is treated as having a weight of 0. + // If no weight is specified, the NodeOverlay is treated as having a weight of 0. // When multiple NodeOverlays have identical weights, they are merged in alphabetical order. // +kubebuilder:validation:Minimum:=1 - // +kubebuilder:validation:Maximum:=100 + // +kubebuilder:validation:Maximum:=10000 // +optional Weight *int32 `json:"weight,omitempty"` } // +kubebuilder:object:root=true // +kubebuilder:storageversion -// +kubebuilder:resource:path=nodeoverlays,scope=Cluster,categories=karpenter +// +kubebuilder:resource:path=nodeoverlays,scope=Cluster,categories=karpenter,shortName=overlays // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="" // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="" // +kubebuilder:printcolumn:name="Weight",type="integer",JSONPath=".spec.weight",priority=1,description="" @@ -115,16 +116,20 @@ func (in *NodeOverlay) AdjustedPrice(instanceTypePrice float64) float64 { isPercentage := strings.HasSuffix(lo.FromPtr(in.Spec.PriceAdjustment), "%") adjustment := lo.FromPtr(in.Spec.PriceAdjustment) - // Remove the percentage sign if present + var adjustedPrice float64 if isPercentage { adjustment = strings.TrimSuffix(lo.FromPtr(in.Spec.PriceAdjustment), "%") + // Parse the adjustment value + // Due to the CEL validation we can assume that + // there will always be a valid float provided into the spec + adjustedPrice = instanceTypePrice * (1 + (lo.Must(strconv.ParseFloat(adjustment, 64)) / 100)) + } else { + adjustedPrice = instanceTypePrice + lo.Must(strconv.ParseFloat(adjustment, 64)) } // Parse the adjustment value // Due to the CEL validation we can assume that // there will always be a valid float provided into the spec - adjValue := lo.Must(strconv.ParseFloat(adjustment, 64)) - adjustedPrice := lo.Ternary(isPercentage, instanceTypePrice*(1+(adjValue/100)), instanceTypePrice+adjValue) // Apply the adjustment return lo.Ternary(adjustedPrice >= 0, adjustedPrice, 0) diff --git a/pkg/apis/v1alpha1/nodeoverlay_default_test.go b/pkg/apis/v1alpha1/nodeoverlay_default_test.go deleted file mode 100644 index 77de873ea2..0000000000 --- a/pkg/apis/v1alpha1/nodeoverlay_default_test.go +++ /dev/null @@ -1,60 +0,0 @@ -/* -Copyright The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1alpha1_test - -import ( - "strings" - - "github.com/Pallinder/go-randomdata" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "sigs.k8s.io/controller-runtime/pkg/client" - - . "sigs.k8s.io/karpenter/pkg/apis/v1alpha1" -) - -var _ = Describe("CEL/Default", func() { - var nodeOverlay *NodeOverlay - - BeforeEach(func() { - nodeOverlay = &NodeOverlay{ - ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}, - Spec: NodeOverlaySpec{ - Requirements: []corev1.NodeSelectorRequirement{ - { - Key: "test", - Operator: corev1.NodeSelectorOpExists, - }, - }, - }, - } - }) - Context("Defaults/TopLevel", func() { - It("should default the priceAdjustment field when undefined", func() { - Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) - Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(nodeOverlay), nodeOverlay)).To(Succeed()) - Expect(nodeOverlay.Spec.Requirements).ToNot(BeNil()) - Expect(nodeOverlay.Spec.PriceAdjustment).To(BeNil()) - Expect(nodeOverlay.Spec.Price).To(BeNil()) - Expect(nodeOverlay.Spec.Capacity).To(BeNil()) - Expect(nodeOverlay.Spec.Weight).To(BeNil()) - }) - }) -}) diff --git a/pkg/apis/v1alpha1/nodeoverlay_validation.go b/pkg/apis/v1alpha1/nodeoverlay_validation.go index 8d4a002fee..c30bff9505 100644 --- a/pkg/apis/v1alpha1/nodeoverlay_validation.go +++ b/pkg/apis/v1alpha1/nodeoverlay_validation.go @@ -19,12 +19,10 @@ package v1alpha1 import ( "context" "fmt" - "strconv" - "github.com/samber/lo" "go.uber.org/multierr" + corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/validation" v1 "sigs.k8s.io/karpenter/pkg/apis/v1" ) @@ -39,9 +37,12 @@ func (in *NodeOverlay) RuntimeValidate(ctx context.Context) error { // NodeOverlay requirements only support well known labels. func (in *NodeOverlaySpec) validateRequirements(ctx context.Context) (errs error) { for _, requirement := range in.Requirements { - if err := ValidateRequirement(ctx, requirement); err != nil { + if err := v1.ValidateRequirement(ctx, v1.NodeSelectorRequirementWithMinValues{NodeSelectorRequirement: requirement}); err != nil { errs = multierr.Append(errs, fmt.Errorf("invalid value: %w in requirements, restricted", err)) } + if requirement.Operator == corev1.NodeSelectorOpNotIn && len(requirement.Values) == 0 { + errs = multierr.Append(errs, fmt.Errorf("key %s with operator %s must have a value defined", requirement.Key, requirement.Operator)) + } } return errs } @@ -54,79 +55,3 @@ func (in *NodeOverlaySpec) validateCapacity() (errs error) { } return errs } - -//nolint:gocyclo -func ValidateRequirement(ctx context.Context, requirement corev1.NodeSelectorRequirement) error { - var errs error - if normalized, ok := v1.NormalizedLabels[requirement.Key]; ok { - requirement.Key = normalized - } - if !v1.SupportedNodeSelectorOps.Has(string(requirement.Operator)) { - errs = multierr.Append(errs, fmt.Errorf("key %s has an unsupported operator %s not in %s", requirement.Key, requirement.Operator, v1.SupportedNodeSelectorOps.UnsortedList())) - } - if e := v1.IsRestrictedLabel(requirement.Key); e != nil { - errs = multierr.Append(errs, e) - } - // Validate that at least one value is valid for well-known labels with known values - if err := validateWellKnownValues(requirement); err != nil { - errs = multierr.Append(errs, err) - } - for _, err := range validation.IsQualifiedName(requirement.Key) { - errs = multierr.Append(errs, fmt.Errorf("key %s is not a qualified name, %s", requirement.Key, err)) - } - for _, value := range requirement.Values { - for _, err := range validation.IsValidLabelValue(value) { - errs = multierr.Append(errs, fmt.Errorf("invalid value %s for key %s, %s", value, requirement.Key, err)) - } - } - if requirement.Operator == corev1.NodeSelectorOpIn && len(requirement.Values) == 0 { - errs = multierr.Append(errs, fmt.Errorf("key %s with operator %s must have a value defined", requirement.Key, requirement.Operator)) - } - - if requirement.Operator == corev1.NodeSelectorOpGt || requirement.Operator == corev1.NodeSelectorOpLt { - if len(requirement.Values) != 1 { - errs = multierr.Append(errs, fmt.Errorf("key %s with operator %s must have a single positive integer value", requirement.Key, requirement.Operator)) - } else { - value, err := strconv.Atoi(requirement.Values[0]) - if err != nil || value < 0 { - errs = multierr.Append(errs, fmt.Errorf("key %s with operator %s must have a single positive integer value", requirement.Key, requirement.Operator)) - } - } - } - return errs -} - -// ValidateWellKnownValues checks if the requirement has well known values. -// An error will cause a NodePool's Readiness to transition to False. -// It returns an error if all values are invalid. -// It returns an error if there are not enough valid values to satisfy min values for a requirement with known values. -// It logs if invalid values are found but valid values can be used. -func validateWellKnownValues(requirement corev1.NodeSelectorRequirement) error { - // If the key doesn't have well-known values or the operator is not In, nothing to validate - if !v1.WellKnownLabels.Has(requirement.Key) || requirement.Operator != corev1.NodeSelectorOpIn { - return nil - } - - // If the key doesn't have well-known values defined, nothing to validate - knownValues, exists := v1.WellKnownValuesForRequirements[requirement.Key] - if !exists { - return nil - } - - values, invalidValues := lo.FilterReject(requirement.Values, func(val string, _ int) bool { - return knownValues.Has(val) - }) - - // If there are only invalid values, set an error to transition the nodepool's readiness to false - if len(values) == 0 { - return fmt.Errorf("no valid values found in %v for %s, expected one of: %v, got: %v", - requirement.Values, requirement.Key, knownValues, invalidValues) - } - - // If there are valid and invalid values, log the invalid values and proceed with valid values - if len(invalidValues) > 0 { - return fmt.Errorf("invalid values found for key in %s please correct found invalid values in %s, proceeding with valid values %s", requirement.Key, invalidValues, values) - } - - return nil -} diff --git a/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go b/pkg/apis/v1alpha1/nodeoverlay_validation_test.go similarity index 89% rename from pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go rename to pkg/apis/v1alpha1/nodeoverlay_validation_test.go index e2701b66fb..10d4a748e3 100644 --- a/pkg/apis/v1alpha1/nodeoverlay_validation_cel_test.go +++ b/pkg/apis/v1alpha1/nodeoverlay_validation_test.go @@ -37,9 +37,6 @@ var _ = Describe("CEL/Validation", func() { var nodeOverlay *NodeOverlay BeforeEach(func() { - if env.Version.Minor() < 25 { - Skip("CEL Validation is for 1.25>") - } nodeOverlay = &NodeOverlay{ ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}, Spec: NodeOverlaySpec{ @@ -53,6 +50,27 @@ var _ = Describe("CEL/Validation", func() { } }) Context("Requirements", func() { + It("should fail for no values for In operator", func() { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: "Test", Operator: corev1.NodeSelectorOpIn}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).NotTo(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).NotTo(Succeed()) + }) + It("should fail for no values for NotIn operator", func() { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: "Test", Operator: corev1.NodeSelectorOpNotIn}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).NotTo(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).NotTo(Succeed()) + }) + It("should succeed for valid requirement keys", func() { + nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ + {Key: "Test", Operator: corev1.NodeSelectorOpExists}, + } + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + Expect(nodeOverlay.RuntimeValidate(ctx)).To(Succeed()) + }) It("should succeed for valid requirement keys", func() { nodeOverlay.Spec.Requirements = []corev1.NodeSelectorRequirement{ {Key: "Test", Operator: corev1.NodeSelectorOpExists}, @@ -103,7 +121,7 @@ var _ = Describe("CEL/Validation", func() { {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpIn, Values: []string{"test"}}, {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpGt, Values: []string{"1"}}, {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpLt, Values: []string{"1"}}, - {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpNotIn}, + {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpNotIn, Values: []string{"1"}}, {Key: corev1.LabelTopologyZone, Operator: corev1.NodeSelectorOpExists}, } Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) @@ -224,7 +242,9 @@ var _ = Describe("CEL/Validation", func() { Entry("Multiple decimal points", "42.0.0"), Entry("Just a sign", "-"), Entry("Just a decimal point", "."), - Entry("No leading digit after sign", "-.42"), + Entry("No leading digit after sign", "-100.0%"), + Entry("less -100% float ", "-101.1%"), + Entry("less -100% integer ", "-129"), ) It("should not allow an unsigned priceAdjustment percentage", func() { nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("1%") @@ -246,6 +266,18 @@ var _ = Describe("CEL/Validation", func() { nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("-1%") Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) }) + It("should allow negative percentage -100%", func() { + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("-100%") + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + It("should allow positive percentage greater then 100%", func() { + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("+100.102%") + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) + It("should allow positive percentage greater then 100% with an integer", func() { + nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("+298%") + Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed()) + }) It("should allow positive integer value", func() { nodeOverlay.Spec.PriceAdjustment = lo.ToPtr("+43") Expect(env.Client.Create(ctx, nodeOverlay)).To(Succeed())