Skip to content

Commit

Permalink
chore: fix warning logs (#1501)
Browse files Browse the repository at this point in the history
  • Loading branch information
njtran authored Aug 2, 2024
1 parent 3aecda2 commit c460767
Show file tree
Hide file tree
Showing 20 changed files with 168 additions and 49 deletions.
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ licenses: download ## Verifies dependency licenses
verify: ## Verify code. Includes codegen, docgen, dependencies, linting, formatting, etc
go mod tidy
go generate ./...
hack/mutation/nodepool.sh
hack/validation/kubelet.sh
hack/validation/taint.sh
hack/validation/requirements.sh
Expand Down
3 changes: 0 additions & 3 deletions hack/mutation/nodepool.sh

This file was deleted.

5 changes: 4 additions & 1 deletion kwok/charts/crds/karpenter.sh_nodepools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,10 @@ spec:
rule: self.all(x, x != "kubernetes.io/hostname")
type: object
spec:
description: NodeClaimSpec describes the desired state of the NodeClaim
description: |-
NodeClaimTemplateSpec describes the desired state of the NodeClaim in the Nodepool
NodeClaimTemplateSpec is used in the NodePool's NodeClaimTemplate, with the resource requests omitted since
users are not able to set resource requests in the NodePool.
properties:
expireAfter:
default: 720h
Expand Down
5 changes: 4 additions & 1 deletion pkg/apis/crds/karpenter.sh_nodepools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,10 @@ spec:
rule: self.all(x, x != "kubernetes.io/hostname")
type: object
spec:
description: NodeClaimSpec describes the desired state of the NodeClaim
description: |-
NodeClaimTemplateSpec describes the desired state of the NodeClaim in the Nodepool
NodeClaimTemplateSpec is used in the NodePool's NodeClaimTemplate, with the resource requests omitted since
users are not able to set resource requests in the NodePool.
properties:
expireAfter:
default: 720h
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/v1/nodeclaim_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type taintKeyEffect struct {
Effect v1.TaintEffect
}

func (in *NodeClaimSpec) validateTaints() (errs *apis.FieldError) {
func (in *NodeClaimTemplateSpec) validateTaints() (errs *apis.FieldError) {
existing := map[taintKeyEffect]struct{}{}
errs = errs.Also(validateTaintsField(in.Taints, existing, "taints"))
errs = errs.Also(validateTaintsField(in.StartupTaints, existing, "startupTaints"))
Expand Down Expand Up @@ -104,7 +104,7 @@ func validateTaintsField(taints []v1.Taint, existing map[taintKeyEffect]struct{}
// 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 *NodeClaimSpec) validateRequirements() (errs *apis.FieldError) {
func (in *NodeClaimTemplateSpec) validateRequirements() (errs *apis.FieldError) {
for i, requirement := range in.Requirements {
if err := ValidateRequirement(requirement); err != nil {
errs = errs.Also(apis.ErrInvalidArrayValue(err, "requirements", i))
Expand Down
72 changes: 71 additions & 1 deletion pkg/apis/v1/nodepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,77 @@ func (l Limits) ExceededBy(resources v1.ResourceList) error {
type NodeClaimTemplate struct {
ObjectMeta `json:"metadata,omitempty"`
// +required
Spec NodeClaimSpec `json:"spec"`
Spec NodeClaimTemplateSpec `json:"spec"`
}

// NodeClaimTemplateSpec describes the desired state of the NodeClaim in the Nodepool
// NodeClaimTemplateSpec is used in the NodePool's NodeClaimTemplate, with the resource requests omitted since
// users are not able to set resource requests in the NodePool.
type NodeClaimTemplateSpec struct {
// Taints will be applied to the NodeClaim's node.
// +optional
Taints []v1.Taint `json:"taints,omitempty"`
// StartupTaints are taints that are applied to nodes upon startup which are expected to be removed automatically
// within a short period of time, typically by a DaemonSet that tolerates the taint. These are commonly used by
// daemonsets to allow initialization and enforce startup ordering. StartupTaints are ignored for provisioning
// purposes in that pods are not required to tolerate a StartupTaint in order to have nodes provisioned for them.
// +optional
StartupTaints []v1.Taint `json:"startupTaints,omitempty"`
// 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 []NodeSelectorRequirementWithMinValues `json:"requirements" hash:"ignore"`
// NodeClassRef is a reference to an object that defines provider specific configuration
// +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.
//
// Warning: this feature takes precedence over a Pod's terminationGracePeriodSeconds value, and bypasses any blocked PDBs or the karpenter.sh/do-not-disrupt annotation.
//
// This field is intended to be used by cluster administrators to enforce that nodes can be cycled within a given time period.
// When set, drifted nodes will begin draining even if there are pods blocking eviction. Draining will respect PDBs and the do-not-disrupt annotation until the TGP is reached.
//
// Karpenter will preemptively delete pods so their terminationGracePeriodSeconds align with the node's terminationGracePeriod.
// If a pod would be terminated without being granted its full terminationGracePeriodSeconds prior to the node timeout,
// that pod will be deleted at T = node timeout - pod terminationGracePeriodSeconds.
//
// The feature can also be used to allow maximum time limits for long-running jobs which can delay node termination with preStop hooks.
// If left undefined, the controller will wait indefinitely for pods to be drained.
// +kubebuilder:validation:Pattern=`^([0-9]+(s|m|h))+$`
// +kubebuilder:validation:Type="string"
// +optional
TerminationGracePeriod *metav1.Duration `json:"terminationGracePeriod,omitempty"`
// ExpireAfter is the duration the controller will wait
// before terminating a node, measured from when the node is created. This
// is useful to implement features like eventually consistent node upgrade,
// memory leak protection, and disruption testing.
// +kubebuilder:default:="720h"
// +kubebuilder:validation:Pattern=`^(([0-9]+(s|m|h))+)|(Never)$`
// +kubebuilder:validation:Type="string"
// +kubebuilder:validation:Schemaless
// +optional
ExpireAfter NillableDuration `json:"expireAfter,omitempty"`
}

// This is used to convert between the NodeClaim's NodeClaimSpec to the Nodepool NodeClaimTemplate's NodeClaimSpec.
func (in *NodeClaimTemplate) ToNodeClaim() *NodeClaim {
return &NodeClaim{
ObjectMeta: metav1.ObjectMeta{
Labels: in.ObjectMeta.Labels,
Annotations: in.ObjectMeta.Annotations,
},
Spec: NodeClaimSpec{
Taints: in.Spec.Taints,
StartupTaints: in.Spec.StartupTaints,
Requirements: in.Spec.Requirements,
NodeClassRef: in.Spec.NodeClassRef,
TerminationGracePeriod: in.Spec.TerminationGracePeriod,
ExpireAfter: in.Spec.ExpireAfter,
},
}
}

type ObjectMeta struct {
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/v1/nodepool_conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ var _ = Describe("Convert V1 to V1beta1 NodePool API", func() {
v1nodepool = &NodePool{
Spec: NodePoolSpec{
Template: NodeClaimTemplate{
Spec: NodeClaimSpec{
Spec: NodeClaimTemplateSpec{
NodeClassRef: &NodeClassReference{
Name: "test",
Kind: "test",
Expand Down Expand Up @@ -293,7 +293,7 @@ var _ = Describe("Convert V1beta1 to V1 NodePool API", func() {
v1nodepool = &NodePool{
Spec: NodePoolSpec{
Template: NodeClaimTemplate{
Spec: NodeClaimSpec{
Spec: NodeClaimTemplateSpec{
NodeClassRef: &NodeClassReference{
Name: "test",
Kind: "test",
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/v1/nodepool_validation_cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ var _ = Describe("CEL/Validation", func() {
ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())},
Spec: NodePoolSpec{
Template: NodeClaimTemplate{
Spec: NodeClaimSpec{
Spec: NodeClaimTemplateSpec{
NodeClassRef: &NodeClassReference{
Kind: "NodeClaim",
Name: "default",
Expand Down
47 changes: 47 additions & 0 deletions pkg/apis/v1/zz_generated.deepcopy.go

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

2 changes: 1 addition & 1 deletion pkg/controllers/disruption/consolidation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2973,7 +2973,7 @@ var _ = Describe("Consolidation", func() {
},
Spec: v1.NodePoolSpec{
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
Requirements: []v1.NodeSelectorRequirementWithMinValues{},
NodeClassRef: &v1.NodeClassReference{
Name: "non-existent",
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/disruption/drift_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ var _ = Describe("Drift", func() {
}},
},
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
ExpireAfter: v1.NillableDuration{Duration: nil},
},
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/metrics/nodepool/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ var _ = Describe("Metrics", func() {
nodePool = test.NodePool(v1.NodePool{
Spec: v1.NodePoolSpec{
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
NodeClassRef: &v1.NodeClassReference{
Name: "default",
},
Expand Down
16 changes: 8 additions & 8 deletions pkg/controllers/nodeclaim/disruption/drift_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ var _ = Describe("Drift", func() {
"keyLabel2": "valueLabel2",
},
},
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
Requirements: nodePool.Spec.Template.Spec.Requirements,
NodeClassRef: &v1.NodeClassReference{
Kind: "fakeKind",
Expand Down Expand Up @@ -411,13 +411,13 @@ var _ = Describe("Drift", func() {
},
Entry("Annoations", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{"keyAnnotationTest": "valueAnnotationTest"}}}}}),
Entry("Labels", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{ObjectMeta: v1.ObjectMeta{Labels: map[string]string{"keyLabelTest": "valueLabelTest"}}}}}),
Entry("Taints", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimSpec{Taints: []corev1.Taint{{Key: "keytest2taint", Effect: corev1.TaintEffectNoExecute}}}}}}),
Entry("StartupTaints", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimSpec{StartupTaints: []corev1.Taint{{Key: "keytest2taint", Effect: corev1.TaintEffectNoExecute}}}}}}),
Entry("NodeClassRef APIVersion", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimSpec{NodeClassRef: &v1.NodeClassReference{Group: "testVersion"}}}}}),
Entry("NodeClassRef Name", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimSpec{NodeClassRef: &v1.NodeClassReference{Name: "testName"}}}}}),
Entry("NodeClassRef Kind", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimSpec{NodeClassRef: &v1.NodeClassReference{Kind: "testKind"}}}}}),
Entry("ExpireAfter", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimSpec{ExpireAfter: v1.NillableDuration{Duration: lo.ToPtr(100 * time.Minute)}}}}}),
Entry("TerminationGracePeriod", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimSpec{TerminationGracePeriod: &metav1.Duration{Duration: 100 * time.Minute}}}}}),
Entry("Taints", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimTemplateSpec{Taints: []corev1.Taint{{Key: "keytest2taint", Effect: corev1.TaintEffectNoExecute}}}}}}),
Entry("StartupTaints", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimTemplateSpec{StartupTaints: []corev1.Taint{{Key: "keytest2taint", Effect: corev1.TaintEffectNoExecute}}}}}}),
Entry("NodeClassRef APIVersion", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimTemplateSpec{NodeClassRef: &v1.NodeClassReference{Group: "testVersion"}}}}}),
Entry("NodeClassRef Name", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimTemplateSpec{NodeClassRef: &v1.NodeClassReference{Name: "testName"}}}}}),
Entry("NodeClassRef Kind", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimTemplateSpec{NodeClassRef: &v1.NodeClassReference{Kind: "testKind"}}}}}),
Entry("ExpireAfter", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimTemplateSpec{ExpireAfter: v1.NillableDuration{Duration: lo.ToPtr(100 * time.Minute)}}}}}),
Entry("TerminationGracePeriod", v1.NodePool{Spec: v1.NodePoolSpec{Template: v1.NodeClaimTemplate{Spec: v1.NodeClaimTemplateSpec{TerminationGracePeriod: &metav1.Duration{Duration: 100 * time.Minute}}}}}),
)
It("should not return drifted if karpenter.sh/nodepool-hash annotation is not present on the NodePool", func() {
nodePool.ObjectMeta.Annotations = map[string]string{}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/nodepool/hash/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ var _ = Describe("Static Drift Hash", func() {
"keyLabel": "valueLabel",
},
},
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
Taints: []corev1.Taint{
{
Key: "key",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ var _ = Describe("Instance Type Selection", func() {
nodePool = test.NodePool(v1.NodePool{
Spec: v1.NodePoolSpec{
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
Requirements: []v1.NodeSelectorRequirementWithMinValues{
{
NodeSelectorRequirement: corev1.NodeSelectorRequirement{
Expand Down
8 changes: 4 additions & 4 deletions pkg/controllers/provisioning/scheduling/nodeclaimtemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var MaxInstanceTypes = 60
// the fields in NodePool. These structs are maintained separately in order
// for fields like Requirements to be able to be stored more efficiently.
type NodeClaimTemplate struct {
v1.NodeClaimTemplate
v1.NodeClaim

NodePoolName string
InstanceTypeOptions cloudprovider.InstanceTypes
Expand All @@ -46,9 +46,9 @@ type NodeClaimTemplate struct {

func NewNodeClaimTemplate(nodePool *v1.NodePool) *NodeClaimTemplate {
nct := &NodeClaimTemplate{
NodeClaimTemplate: nodePool.Spec.Template,
NodePoolName: nodePool.Name,
Requirements: scheduling.NewRequirements(),
NodeClaim: *nodePool.Spec.Template.ToNodeClaim(),
NodePoolName: nodePool.Name,
Requirements: scheduling.NewRequirements(),
}
nct.Labels = lo.Assign(nct.Labels, map[string]string{v1.NodePoolLabelKey: nodePool.Name})
nct.Requirements.Add(scheduling.NewNodeSelectorRequirementsWithMinValues(nct.Spec.Requirements...).Values()...)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func benchmarkScheduler(b *testing.B, instanceCount, podCount int) {
nodePoolWithMinValues := test.NodePool(v1.NodePool{
Spec: v1.NodePoolSpec{
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
Requirements: []v1.NodeSelectorRequirementWithMinValues{
{
NodeSelectorRequirement: corev1.NodeSelectorRequirement{
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/provisioning/scheduling/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ var _ = Context("Scheduling", func() {
nodePool = test.NodePool(v1.NodePool{
Spec: v1.NodePoolSpec{
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
Requirements: []v1.NodeSelectorRequirementWithMinValues{
{
NodeSelectorRequirement: corev1.NodeSelectorRequirement{
Expand Down
10 changes: 5 additions & 5 deletions pkg/controllers/provisioning/scheduling/topology_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ var _ = Describe("Topology", func() {
nodePool = test.NodePool(v1.NodePool{
Spec: v1.NodePoolSpec{
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
Requirements: []v1.NodeSelectorRequirementWithMinValues{
{
NodeSelectorRequirement: corev1.NodeSelectorRequirement{
Expand Down Expand Up @@ -969,7 +969,7 @@ var _ = Describe("Topology", func() {
spotNodePool := test.NodePool(v1.NodePool{
Spec: v1.NodePoolSpec{
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
Requirements: []v1.NodeSelectorRequirementWithMinValues{
{
NodeSelectorRequirement: corev1.NodeSelectorRequirement{
Expand All @@ -993,7 +993,7 @@ var _ = Describe("Topology", func() {
onDemandNodePool := test.NodePool(v1.NodePool{
Spec: v1.NodePoolSpec{
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
Requirements: []v1.NodeSelectorRequirementWithMinValues{
{
NodeSelectorRequirement: corev1.NodeSelectorRequirement{
Expand Down Expand Up @@ -1060,7 +1060,7 @@ var _ = Describe("Topology", func() {
nodePoolB := test.NodePool(v1.NodePool{
Spec: v1.NodePoolSpec{
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
Requirements: []v1.NodeSelectorRequirementWithMinValues{
{
NodeSelectorRequirement: corev1.NodeSelectorRequirement{
Expand Down Expand Up @@ -2382,7 +2382,7 @@ var _ = Describe("Taints", func() {
nodePool = test.NodePool(v1.NodePool{
Spec: v1.NodePoolSpec{
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimSpec{
Spec: v1.NodeClaimTemplateSpec{
Requirements: []v1.NodeSelectorRequirementWithMinValues{
{
NodeSelectorRequirement: corev1.NodeSelectorRequirement{
Expand Down
Loading

0 comments on commit c460767

Please sign in to comment.