Merge branch 'main' into limit-by-nodepool

.github/workflows/kind-e2e.yaml

@@ -1,7 +1,5 @@
 name: kind-e2e
 on:
-  push:
-    branches: [main]
   workflow_dispatch:
 jobs:
   kind-e2e:

.gitignore

@@ -4,6 +4,8 @@ coverage.html
 *.test
 *.cpuprofile
 *.heapprofile
+go.work
+go.work.sum
 
 # Common in OSs and IDEs
 .idea

hack/kwok/requirements.sh

@@ -8,7 +8,4 @@ yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.req
 
 # NodePool Validation:
 yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.requirements.items.properties.key.x-kubernetes-validations  += [
-    {"message": "label domain \"karpenter.kwok.sh\" is restricted", "rule": "self in [\"karpenter.kwok.sh/instance-cpu\", \"karpenter.kwok.sh/instance-memory\", \"karpenter.kwok.sh/instance-family\", \"karpenter.kwok.sh/instance-size\"] || !self.find(\"^([^/]+)\").endsWith(\"karpenter.kwok.sh\")"}]' -i kwok/charts/crds/karpenter.sh_nodepools.yaml
-
-# Add ExampleReason in KwoK CloudProvider
-yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.disruption.properties.budgets.items.properties.reasons.items.enum += [ "ExampleReason" ]' -i kwok/charts/crds/karpenter.sh_nodepools.yaml
+    {"message": "label domain \"karpenter.kwok.sh\" is restricted", "rule": "self in [\"karpenter.kwok.sh/instance-cpu\", \"karpenter.kwok.sh/instance-memory\", \"karpenter.kwok.sh/instance-family\", \"karpenter.kwok.sh/instance-size\"] || !self.find(\"^([^/]+)\").endsWith(\"karpenter.kwok.sh\")"}]' -i kwok/charts/crds/karpenter.sh_nodepools.yaml

kwok/apis/v1alpha1/kwoknodeclass.go

@@ -18,8 +18,6 @@ package v1alpha1
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	v1 "sigs.k8s.io/karpenter/pkg/apis/v1"
 )
 
 // KWOKNodeClass is the Schema for the KWOKNodeClass API
@@ -40,5 +38,3 @@ type KWOKNodeClassList struct {
 	metav1.ListMeta `json:"metadata,omitempty"`
 	Items           []KWOKNodeClass `json:"items"`
 }
-
-const DisruptionReasonExampleReason v1.DisruptionReason = "ExampleReason"

kwok/charts/crds/karpenter.sh_nodeclaims.yaml

@@ -88,12 +88,21 @@ spec:
                       description: API version of the referent
                       pattern: ^[^/]*$
                       type: string
+                      x-kubernetes-validations:
+                        - message: group may not be empty
+                          rule: self != ''
                     kind:
                       description: 'Kind of the referent; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds"'
                       type: string
+                      x-kubernetes-validations:
+                        - message: kind may not be empty
+                          rule: self != ''
                     name:
                       description: 'Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names'
                       type: string
+                      x-kubernetes-validations:
+                        - message: name may not be empty
+                          rule: self != ''
                   required:
                     - group
                     - kind

kwok/charts/crds/karpenter.sh_nodepools.yaml

@@ -114,16 +114,13 @@ spec:
                             description: |-
                               Reasons is a list of disruption methods that this budget applies to. If Reasons is not set, this budget applies to all methods.
                               Otherwise, this will apply to each reason defined.
-                              allowed reasons are Underutilized, Empty, and Drifted and additional CloudProvider-specific reasons.
+                              allowed reasons are Underutilized, Empty, and Drifted.
                             items:
-                              description: |-
-                                DisruptionReason defines valid reasons for disruption budgets.
-                                CloudProviders will need to append to the list of enums when implementing cloud provider disruption reasons
+                              description: DisruptionReason defines valid reasons for disruption budgets.
                               enum:
                                 - Underutilized
                                 - Empty
                                 - Drifted
-                                - ExampleReason
                               type: string
                             type: array
                           schedule:
@@ -232,17 +229,31 @@ spec:
                               description: API version of the referent
                               pattern: ^[^/]*$
                               type: string
+                              x-kubernetes-validations:
+                                - message: group may not be empty
+                                  rule: self != ''
                             kind:
                               description: 'Kind of the referent; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds"'
                               type: string
+                              x-kubernetes-validations:
+                                - message: kind may not be empty
+                                  rule: self != ''
                             name:
                               description: 'Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names'
                               type: string
+                              x-kubernetes-validations:
+                                - message: name may not be empty
+                                  rule: self != ''
                           required:
                             - group
                             - kind
                             - name
                           type: object
+                          x-kubernetes-validations:
+                            - message: nodeClassRef.group is immutable
+                              rule: self.group == oldSelf.group
+                            - message: nodeClassRef.kind is immutable
+                              rule: self.kind == oldSelf.kind
                         requirements:
                           description: Requirements are layered with GetLabels and applied to every node.
                           items:

kwok/cloudprovider/cloudprovider.go

@@ -63,10 +63,6 @@ func (c CloudProvider) Create(ctx context.Context, nodeClaim *v1.NodeClaim) (*v1
 	return c.toNodeClaim(node)
 }
 
-func (c CloudProvider) DisruptionReasons() []v1.DisruptionReason {
-	return []v1.DisruptionReason{v1alpha1.DisruptionReasonExampleReason}
-}
-
 func (c CloudProvider) Delete(ctx context.Context, nodeClaim *v1.NodeClaim) error {
 	if err := c.kubeClient.Delete(ctx, nodeClaim); err != nil {
 		if errors.IsNotFound(err) {

pkg/apis/crds/karpenter.sh_nodeclaims.yaml

@@ -88,12 +88,21 @@ spec:
                       description: API version of the referent
                       pattern: ^[^/]*$
                       type: string
+                      x-kubernetes-validations:
+                        - message: group may not be empty
+                          rule: self != ''
                     kind:
                       description: 'Kind of the referent; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds"'
                       type: string
+                      x-kubernetes-validations:
+                        - message: kind may not be empty
+                          rule: self != ''
                     name:
                       description: 'Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names'
                       type: string
+                      x-kubernetes-validations:
+                        - message: name may not be empty
+                          rule: self != ''
                   required:
                     - group
                     - kind

pkg/apis/crds/karpenter.sh_nodepools.yaml

@@ -114,11 +114,9 @@ spec:
                             description: |-
                               Reasons is a list of disruption methods that this budget applies to. If Reasons is not set, this budget applies to all methods.
                               Otherwise, this will apply to each reason defined.
-                              allowed reasons are Underutilized, Empty, and Drifted and additional CloudProvider-specific reasons.
+                              allowed reasons are Underutilized, Empty, and Drifted.
                             items:
-                              description: |-
-                                DisruptionReason defines valid reasons for disruption budgets.
-                                CloudProviders will need to append to the list of enums when implementing cloud provider disruption reasons
+                              description: DisruptionReason defines valid reasons for disruption budgets.
                               enum:
                                 - Underutilized
                                 - Empty
@@ -231,17 +229,31 @@ spec:
                               description: API version of the referent
                               pattern: ^[^/]*$
                               type: string
+                              x-kubernetes-validations:
+                                - message: group may not be empty
+                                  rule: self != ''
                             kind:
                               description: 'Kind of the referent; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds"'
                               type: string
+                              x-kubernetes-validations:
+                                - message: kind may not be empty
+                                  rule: self != ''
                             name:
                               description: 'Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names'
                               type: string
+                              x-kubernetes-validations:
+                                - message: name may not be empty
+                                  rule: self != ''
                           required:
                             - group
                             - kind
                             - name
                           type: object
+                          x-kubernetes-validations:
+                            - message: nodeClassRef.group is immutable
+                              rule: self.group == oldSelf.group
+                            - message: nodeClassRef.kind is immutable
+                              rule: self.kind == oldSelf.kind
                         requirements:
                           description: Requirements are layered with GetLabels and applied to every node.
                           items:

pkg/apis/v1/labels.go

@@ -21,6 +21,7 @@ import (
 	"strings"
 
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 
 	"sigs.k8s.io/karpenter/pkg/apis"
@@ -141,3 +142,7 @@ func GetLabelDomain(key string) string {
 	}
 	return ""
 }
+
+func NodeClassLabelKey(gk schema.GroupKind) string {
+	return fmt.Sprintf("%s/%s", gk.Group, strings.ToLower(gk.Kind))
+}

pkg/apis/v1/nodeclaim.go

@@ -20,6 +20,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
 // NodeClaimSpec describes the desired state of the NodeClaim
@@ -97,17 +98,27 @@ type ResourceRequirements struct {
 
 type NodeClassReference struct {
 	// Kind of the referent; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds"
+	// +kubebuilder:validation:XValidation:rule="self != ''",message="kind may not be empty"
 	// +required
 	Kind string `json:"kind"`
 	// Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names
+	// +kubebuilder:validation:XValidation:rule="self != ''",message="name may not be empty"
 	// +required
 	Name string `json:"name"`
 	// API version of the referent
+	// +kubebuilder:validation:XValidation:rule="self != ''",message="group may not be empty"
 	// +kubebuilder:validation:Pattern=`^[^/]*$`
 	// +required
 	Group string `json:"group"`
 }
 
+func (ncr *NodeClassReference) GroupKind() schema.GroupKind {
+	return schema.GroupKind{
+		Group: ncr.Group,
+		Kind:  ncr.Kind,
+	}
+}
+
 // +kubebuilder:object:generate=false
 type Provider = runtime.RawExtension
 

pkg/apis/v1/nodeclaim_validation_cel_test.go

@@ -46,8 +46,9 @@ var _ = Describe("Validation", func() {
 			ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())},
 			Spec: NodeClaimSpec{
 				NodeClassRef: &NodeClassReference{
-					Kind: "NodeClaim",
-					Name: "default",
+					Group: "karpenter.test.sh",
+					Kind:  "TestNodeClaim",
+					Name:  "default",
 				},
 				Requirements: []NodeSelectorRequirementWithMinValues{
 					{

pkg/apis/v1/nodepool.go

@@ -19,7 +19,6 @@ package v1
 import (
 	"fmt"
 	"math"
-	"sort"
 	"strconv"
 
 	"github.com/mitchellh/hashstructure/v2"
@@ -89,7 +88,7 @@ type Disruption struct {
 type Budget struct {
 	// Reasons is a list of disruption methods that this budget applies to. If Reasons is not set, this budget applies to all methods.
 	// Otherwise, this will apply to each reason defined.
-	// allowed reasons are Underutilized, Empty, and Drifted and additional CloudProvider-specific reasons.
+	// allowed reasons are Underutilized, Empty, and Drifted.
 	// +optional
 	Reasons []DisruptionReason `json:"reasons,omitempty"`
 	// Nodes dictates the maximum number of NodeClaims owned by this NodePool
@@ -129,7 +128,6 @@ const (
 )
 
 // DisruptionReason defines valid reasons for disruption budgets.
-// CloudProviders will need to append to the list of enums when implementing cloud provider disruption reasons
 // +kubebuilder:validation:Enum={Underutilized,Empty,Drifted}
 type DisruptionReason string
 
@@ -182,6 +180,8 @@ type NodeClaimTemplateSpec struct {
 	// +required
 	Requirements []NodeSelectorRequirementWithMinValues `json:"requirements" hash:"ignore"`
 	// NodeClassRef is a reference to an object that defines provider specific configuration
+	// +kubebuilder:validation:XValidation:rule="self.group == oldSelf.group",message="nodeClassRef.group is immutable"
+	// +kubebuilder:validation:XValidation:rule="self.kind == oldSelf.kind",message="nodeClassRef.kind is immutable"
 	// +required
 	NodeClassRef *NodeClassReference `json:"nodeClassRef"`
 	// TerminationGracePeriod is the maximum duration the controller will wait before forcefully deleting the pods on a node, measured from when deletion is first initiated.
@@ -290,23 +290,6 @@ type NodePoolList struct {
 	Items           []NodePool `json:"items"`
 }
 
-// OrderByWeight orders the NodePools in the NodePoolList by their priority weight in-place.
-// This priority evaluates the following things in precedence order:
-//  1. NodePools that have a larger weight are ordered first
-//  2. If two NodePools have the same weight, then the NodePool with the name later in the alphabet will come first
-func (nl *NodePoolList) OrderByWeight() {
-	sort.Slice(nl.Items, func(a, b int) bool {
-		weightA := lo.FromPtr(nl.Items[a].Spec.Weight)
-		weightB := lo.FromPtr(nl.Items[b].Spec.Weight)
-
-		if weightA == weightB {
-			// Order NodePools by name for a consistent ordering when sorting equal weight
-			return nl.Items[a].Name > nl.Items[b].Name
-		}
-		return weightA > weightB
-	})
-}
-
 // MustGetAllowedDisruptions calls GetAllowedDisruptionsByReason if the error is not nil. This reduces the
 // amount of state that the disruption controller must reconcile, while allowing the GetAllowedDisruptionsByReason()
 // to bubble up any errors in validation.

pkg/apis/v1/nodepool_budgets_test.go

@@ -78,7 +78,6 @@ var _ = Describe("Budgets", func() {
 					DisruptionReasonUnderutilized,
 					DisruptionReasonDrifted,
 					DisruptionReasonEmpty,
-					"CloudProviderDisruptionReason",
 				},
 				Nodes:    "0",
 				Schedule: lo.ToPtr("@weekly"),
@@ -93,12 +92,11 @@ var _ = Describe("Budgets", func() {
 				},
 			},
 		}
-		allKnownDisruptionReasons = append([]DisruptionReason{
+		allKnownDisruptionReasons = []DisruptionReason{
 			DisruptionReasonEmpty,
 			DisruptionReasonUnderutilized,
-			DisruptionReasonDrifted},
-			DisruptionReason("CloudProviderDisruptionReason"),
-		)
+			DisruptionReasonDrifted,
+		}
 	})
 
 	Context("GetAllowedDisruptionsByReason", func() {

pkg/apis/v1/nodepool_default_test.go

@@ -42,8 +42,9 @@ var _ = Describe("CEL/Default", func() {
 				Template: NodeClaimTemplate{
 					Spec: NodeClaimTemplateSpec{
 						NodeClassRef: &NodeClassReference{
-							Kind: "NodeClaim",
-							Name: "default",
+							Group: "karpenter.test.sh",
+							Kind:  "TestNodeClaim",
+							Name:  "default",
 						},
 						Requirements: []NodeSelectorRequirementWithMinValues{
 							{

pkg/apis/v1/nodepool_status.go

@@ -27,8 +27,6 @@ const (
 	ConditionTypeValidationSucceeded = "ValidationSucceeded"
 	// ConditionTypeNodeClassReady = "NodeClassReady" condition indicates that underlying nodeClass was resolved and is reporting as Ready
 	ConditionTypeNodeClassReady = "NodeClassReady"
-	// ConditionTypeUnhealthy = "Unhealthy" condition indicates when the nodepool has more 20% of nodes in an unhealthy state
-	ConditionTypeUnhealthy = "Unhealthy"
 )
 
 // NodePoolStatus defines the observed state of NodePool