chore: Require spec.template.spec.requirements to be set and change…

… `spec.template.spec.kubelet` field name in memory (aws#521)
gfcroft · Sep 19, 2023 · 847fa5b · 847fa5b
1 parent 69a6492
commit 847fa5b
Show file tree

Hide file tree

Showing 21 changed files with 186 additions and 154 deletions.
diff --git a/pkg/apis/crds/karpenter.sh_nodeclaims.yaml b/pkg/apis/crds/karpenter.sh_nodeclaims.yaml
@@ -68,8 +68,10 @@ spec:
             description: NodeClaimSpec describes the desired state of the NodeClaim
             properties:
               kubelet:
-                description: KubeletConfiguration are options passed to the kubelet
-                  when provisioning nodes
+                description: Kubelet defines args to be used when configuring kubelet
+                  on provisioned nodes. They are a subset of the upstream types, recognizing
+                  not all options may be supported. Wherever possible, the types and
+                  names should reflect the upstream kubelet types.
                 properties:
                   clusterDNS:
                     description: clusterDNS is a list of IP addresses for the cluster
@@ -288,6 +290,7 @@ spec:
                 type: array
             required:
             - nodeClass
+            - requirements
             type: object
           status:
             description: NodeClaimStatus defines the observed state of NodeClaim

diff --git a/pkg/apis/crds/karpenter.sh_nodepools.yaml b/pkg/apis/crds/karpenter.sh_nodepools.yaml
@@ -121,8 +121,11 @@ spec:
                       NodeClaim
                     properties:
                       kubelet:
-                        description: KubeletConfiguration are options passed to the
-                          kubelet when provisioning nodes
+                        description: Kubelet defines args to be used when configuring
+                          kubelet on provisioned nodes. They are a subset of the upstream
+                          types, recognizing not all options may be supported. Wherever
+                          possible, the types and names should reflect the upstream
+                          kubelet types.
                         properties:
                           clusterDNS:
                             description: clusterDNS is a list of IP addresses for
@@ -356,6 +359,7 @@ spec:
                         type: array
                     required:
                     - nodeClass
+                    - requirements
                     type: object
                 type: object
               weight:

diff --git a/pkg/apis/v1beta1/nodeclaim.go b/pkg/apis/v1beta1/nodeclaim.go
@@ -32,14 +32,15 @@ type NodeClaimSpec struct {
 	// +optional
 	StartupTaints []v1.Taint `json:"startupTaints,omitempty"`
 	// Requirements are layered with GetLabels and applied to every node.
-	// +optional
-	Requirements []v1.NodeSelectorRequirement `json:"requirements,omitempty" hash:"ignore"`
+	// +required
+	Requirements []v1.NodeSelectorRequirement `json:"requirements" hash:"ignore"`
 	// Resources models the resource requirements for the NodeClaim to launch
 	// +optional
 	Resources ResourceRequirements `json:"resources,omitempty" hash:"ignore"`
-	// KubeletConfiguration are options passed to the kubelet when provisioning nodes
-	// +optional
-	KubeletConfiguration *Kubelet `json:"kubelet,omitempty"`
+	// Kubelet defines args to be used when configuring kubelet on provisioned nodes.
+	// They are a subset of the upstream types, recognizing not all options may be supported.
+	// Wherever possible, the types and names should reflect the upstream kubelet types.
+	Kubelet *KubeletConfiguration `json:"kubelet,omitempty"`
 	// NodeClass is a reference to an object that defines provider specific configuration
 	// +required
 	NodeClass *NodeClassReference `json:"nodeClass"`
@@ -56,12 +57,12 @@ type ResourceRequirements struct {
 	Requests v1.ResourceList `json:"requests,omitempty"`
 }
 
-// Kubelet defines args to be used when configuring kubelet on provisioned nodes.
+// KubeletConfiguration defines args to be used when configuring kubelet on provisioned nodes.
 // They are a subset of the upstream types, recognizing not all options may be supported.
 // Wherever possible, the types and names should reflect the upstream kubelet types.
 // https://pkg.go.dev/k8s.io/kubelet/config/v1beta1#KubeletConfiguration
 // https://github.com/kubernetes/kubernetes/blob/9f82d81e55cafdedab619ea25cabf5d42736dacf/cmd/kubelet/app/options/options.go#L53
-type Kubelet struct {
+type KubeletConfiguration struct {
 	// clusterDNS is a list of IP addresses for the cluster DNS server.
 	// Note that not all providers may use all addresses.
 	//+optional

diff --git a/pkg/apis/v1beta1/nodeclaim_validation.go b/pkg/apis/v1beta1/nodeclaim_validation.go
@@ -69,7 +69,7 @@ func (in *NodeClaimSpec) validate() (errs *apis.FieldError) {
 	return errs.Also(
 		in.validateTaints(),
 		in.validateRequirements(),
-		in.KubeletConfiguration.validate().ViaField("kubeletConfiguration"),
+		in.Kubelet.validate().ViaField("kubeletConfiguration"),
 	)
 }
 
@@ -169,7 +169,7 @@ func (in *NodeClaimSpec) validateRequirement(requirement v1.NodeSelectorRequirem
 	return errs
 }
 
-func (in *Kubelet) validate() (errs *apis.FieldError) {
+func (in *KubeletConfiguration) validate() (errs *apis.FieldError) {
 	if in == nil {
 		return
 	}
@@ -185,7 +185,7 @@ func (in *Kubelet) validate() (errs *apis.FieldError) {
 	)
 }
 
-func (in *Kubelet) validateEvictionSoftGracePeriod() (errs *apis.FieldError) {
+func (in *KubeletConfiguration) validateEvictionSoftGracePeriod() (errs *apis.FieldError) {
 	for k := range in.EvictionSoftGracePeriod {
 		if !SupportedEvictionSignals.Has(k) {
 			errs = errs.Also(apis.ErrInvalidKeyName(k, "evictionSoftGracePeriod"))
@@ -194,7 +194,7 @@ func (in *Kubelet) validateEvictionSoftGracePeriod() (errs *apis.FieldError) {
 	return errs
 }
 
-func (in *Kubelet) validateEvictionSoftPairs() (errs *apis.FieldError) {
+func (in *KubeletConfiguration) validateEvictionSoftPairs() (errs *apis.FieldError) {
 	evictionSoftKeys := sets.New(lo.Keys(in.EvictionSoft)...)
 	evictionSoftGracePeriodKeys := sets.New(lo.Keys(in.EvictionSoftGracePeriod)...)
 
@@ -251,7 +251,7 @@ func validateEvictionThresholds(m map[string]string, fieldName string) (errs *ap
 }
 
 // Validate validateImageGCHighThresholdPercent
-func (in *Kubelet) validateImageGCHighThresholdPercent() (errs *apis.FieldError) {
+func (in *KubeletConfiguration) validateImageGCHighThresholdPercent() (errs *apis.FieldError) {
 	if in.ImageGCHighThresholdPercent != nil && ptr.Int32Value(in.ImageGCHighThresholdPercent) < ptr.Int32Value(in.ImageGCLowThresholdPercent) {
 		return errs.Also(apis.ErrInvalidValue("must be greater than imageGCLowThresholdPercent", "imageGCHighThresholdPercent"))
 	}
@@ -260,7 +260,7 @@ func (in *Kubelet) validateImageGCHighThresholdPercent() (errs *apis.FieldError)
 }
 
 // Validate imageGCLowThresholdPercent
-func (in *Kubelet) validateImageGCLowThresholdPercent() (errs *apis.FieldError) {
+func (in *KubeletConfiguration) validateImageGCLowThresholdPercent() (errs *apis.FieldError) {
 	if in.ImageGCHighThresholdPercent != nil && ptr.Int32Value(in.ImageGCLowThresholdPercent) > ptr.Int32Value(in.ImageGCHighThresholdPercent) {
 		return errs.Also(apis.ErrInvalidValue("must be less than imageGCHighThresholdPercent", "imageGCLowThresholdPercent"))
 	}

diff --git a/pkg/apis/v1beta1/nodeclaim_validation_test.go b/pkg/apis/v1beta1/nodeclaim_validation_test.go
@@ -165,15 +165,15 @@ var _ = Describe("Validation", func() {
 	})
 	Context("Kubelet", func() {
 		It("should fail on kubeReserved with invalid keys", func() {
-			nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+			nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 				KubeReserved: v1.ResourceList{
 					v1.ResourcePods: resource.MustParse("2"),
 				},
 			}
 			Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
 		})
 		It("should fail on systemReserved with invalid keys", func() {
-			nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+			nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 				SystemReserved: v1.ResourceList{
 					v1.ResourcePods: resource.MustParse("2"),
 				},
@@ -183,7 +183,7 @@ var _ = Describe("Validation", func() {
 		Context("Eviction Signals", func() {
 			Context("Eviction Hard", func() {
 				It("should succeed on evictionHard with valid keys", func() {
-					nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 						EvictionHard: map[string]string{
 							"memory.available":   "5%",
 							"nodefs.available":   "10%",
@@ -196,31 +196,31 @@ var _ = Describe("Validation", func() {
 					Expect(nodeClaim.Validate(ctx)).To(Succeed())
 				})
 				It("should fail on evictionHard with invalid keys", func() {
-					nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 						EvictionHard: map[string]string{
 							"memory": "5%",
 						},
 					}
 					Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
 				})
 				It("should fail on invalid formatted percentage value in evictionHard", func() {
-					nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 						EvictionHard: map[string]string{
 							"memory.available": "5%3",
 						},
 					}
 					Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
 				})
 				It("should fail on invalid percentage value (too large) in evictionHard", func() {
-					nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 						EvictionHard: map[string]string{
 							"memory.available": "110%",
 						},
 					}
 					Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
 				})
 				It("should fail on invalid quantity value in evictionHard", func() {
-					nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 						EvictionHard: map[string]string{
 							"memory.available": "110GB",
 						},
@@ -231,7 +231,7 @@ var _ = Describe("Validation", func() {
 		})
 		Context("Eviction Soft", func() {
 			It("should succeed on evictionSoft with valid keys", func() {
-				nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 					EvictionSoft: map[string]string{
 						"memory.available":   "5%",
 						"nodefs.available":   "10%",
@@ -252,7 +252,7 @@ var _ = Describe("Validation", func() {
 				Expect(nodeClaim.Validate(ctx)).To(Succeed())
 			})
 			It("should fail on evictionSoft with invalid keys", func() {
-				nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 					EvictionSoft: map[string]string{
 						"memory": "5%",
 					},
@@ -263,7 +263,7 @@ var _ = Describe("Validation", func() {
 				Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
 			})
 			It("should fail on invalid formatted percentage value in evictionSoft", func() {
-				nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 					EvictionSoft: map[string]string{
 						"memory.available": "5%3",
 					},
@@ -274,7 +274,7 @@ var _ = Describe("Validation", func() {
 				Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
 			})
 			It("should fail on invalid percentage value (too large) in evictionSoft", func() {
-				nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 					EvictionSoft: map[string]string{
 						"memory.available": "110%",
 					},
@@ -285,7 +285,7 @@ var _ = Describe("Validation", func() {
 				Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
 			})
 			It("should fail on invalid quantity value in evictionSoft", func() {
-				nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 					EvictionSoft: map[string]string{
 						"memory.available": "110GB",
 					},
@@ -296,7 +296,7 @@ var _ = Describe("Validation", func() {
 				Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
 			})
 			It("should fail when eviction soft doesn't have matching grace period", func() {
-				nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 					EvictionSoft: map[string]string{
 						"memory.available": "200Mi",
 					},
@@ -307,13 +307,13 @@ var _ = Describe("Validation", func() {
 		Context("GCThresholdPercent", func() {
 			Context("ImageGCHighThresholdPercent", func() {
 				It("should succeed on a imageGCHighThresholdPercent", func() {
-					nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 						ImageGCHighThresholdPercent: ptr.Int32(10),
 					}
 					Expect(nodeClaim.Validate(ctx)).To(Succeed())
 				})
 				It("should fail when imageGCHighThresholdPercent is less than imageGCLowThresholdPercent", func() {
-					nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 						ImageGCHighThresholdPercent: ptr.Int32(50),
 						ImageGCLowThresholdPercent:  ptr.Int32(60),
 					}
@@ -322,13 +322,13 @@ var _ = Describe("Validation", func() {
 			})
 			Context("ImageGCLowThresholdPercent", func() {
 				It("should succeed on a imageGCLowThresholdPercent", func() {
-					nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 						ImageGCLowThresholdPercent: ptr.Int32(10),
 					}
 					Expect(nodeClaim.Validate(ctx)).To(Succeed())
 				})
 				It("should fail when imageGCLowThresholdPercent is greather than imageGCHighThresheldPercent", func() {
-					nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 						ImageGCHighThresholdPercent: ptr.Int32(50),
 						ImageGCLowThresholdPercent:  ptr.Int32(60),
 					}
@@ -338,7 +338,7 @@ var _ = Describe("Validation", func() {
 		})
 		Context("Eviction Soft Grace Period", func() {
 			It("should succeed on evictionSoftGracePeriod with valid keys", func() {
-				nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 					EvictionSoft: map[string]string{
 						"memory.available":   "5%",
 						"nodefs.available":   "10%",
@@ -359,15 +359,15 @@ var _ = Describe("Validation", func() {
 				Expect(nodeClaim.Validate(ctx)).To(Succeed())
 			})
 			It("should fail on evictionSoftGracePeriod with invalid keys", func() {
-				nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 					EvictionSoftGracePeriod: map[string]metav1.Duration{
 						"memory": {Duration: time.Minute},
 					},
 				}
 				Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
 			})
 			It("should fail when eviction soft grace period doesn't have matching threshold", func() {
-				nodeClaim.Spec.KubeletConfiguration = &Kubelet{
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
 					EvictionSoftGracePeriod: map[string]metav1.Duration{
 						"memory.available": {Duration: time.Minute},
 					},