fix: allow non-churn empty nodes to be disrupted (#2206)

.golangci.yaml

@@ -1,83 +1,106 @@
-# See https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml
+version: "2"
 run:
-  tests: true
-  timeout: 5m
   build-tags:
     - test_performance
+  tests: true
+  timeout: 5m
 linters:
   enable:
     - asciicheck
     - bidichk
-    - errorlint
     - copyloopvar
+    - errorlint
+    - gocyclo
+    - goheader
     - gosec
+    - misspell
+    - nilerr
     - revive
-    - stylecheck
+    - staticcheck
     - tparallel
     - unconvert
     - unparam
-    - gocyclo
-    - govet
-    - goimports
-    - goheader
-    - misspell
-    - nilerr
   disable:
     - prealloc
-linters-settings:
-  gocyclo:
-    min-complexity: 11
-  govet:
-    enable-all: true
-    disable:
-      - fieldalignment
-  revive:
-    rules:
-      - name: dot-imports
-        disabled: true
-  stylecheck:
-    dot-import-whitelist:
-      - "github.com/onsi/ginkgo/v2"
-      - "github.com/onsi/gomega"
-  misspell:
-    locale: US
-    ignore-words: []
-  goimports:
-    local-prefixes: sigs.k8s.io/karpenter
-  goheader:
-    template: |-
-      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
+  settings:
+    gocyclo:
+      min-complexity: 11
+    goheader:
+      template: |-
+        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
+            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.
-  gci:
-    sections:
-      - standard # Standard section: captures all standard packages.
-      - default # Default section: contains all imports that could not be matched to another section type.
-      - localmodule # Local module section: contains all local packages. This section is not present unless explicitly enabled.
-    skip-generated: true
+        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.
+    govet:
+      disable:
+        - fieldalignment
+      enable-all: true
+    misspell:
+      locale: US
+    revive:
+      rules:
+        - name: dot-imports
+          disabled: true
+    staticcheck:
+      dot-import-whitelist:
+        - github.com/onsi/ginkgo/v2
+        - github.com/onsi/gomega
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    rules:
+      - linters:
+          - goheader
+        path: zz_(.+)\.go
+      - linters:
+          - goheader
+        path: scheduling_benchmark_test.go
+      - path: (.+)\.go$
+        text: declaration of "(err|ctx)" shadows declaration at
+    paths:
+      - tools
+      - website
+      - hack
+      - charts
+      - designs
+      - third_party$
+      - builtin$
+      - examples$
 issues:
   fix: true
-  exclude: ['declaration of "(err|ctx)" shadows declaration at']
-  exclude-dirs:
-    - tools
-    - website
-    - hack
-    - charts
-    - designs
-  exclude-rules:
-  - linters:
-    - goheader
-    path: 'zz_(.+)\.go'
-  - linters:
-    - goheader
-    path: 'scheduling_benchmark_test.go'
+formatters:
+  enable:
+    - goimports
+  settings:
+    gci:
+      sections:
+        - standard
+        - default
+        - localmodule
+    goimports:
+      local-prefixes:
+        - sigs.k8s.io/karpenter
+  exclusions:
+    generated: lax
+    paths:
+      - tools
+      - website
+      - hack
+      - charts
+      - designs
+      - third_party$
+      - builtin$
+      - examples$

go.mod

@@ -1,6 +1,6 @@
 module sigs.k8s.io/karpenter
 
-go 1.24.2
+go 1.24.6
 
 require (
 	github.com/Pallinder/go-randomdata v1.2.0

hack/toolchain.sh

@@ -11,7 +11,10 @@ main() {
 
 tools() {
     go install github.com/google/go-licenses@latest
-    go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
+    # asciicheck is a dependency of golangci-lint that got removed so golangci changed their go.mod to use the forked version
+    # fix - https://github.com/golangci/golangci-lint/issues/6017
+    # change to latest once golangci releases new version with the fix
+    go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@main
     go install github.com/mikefarah/yq/v4@latest
     go install github.com/google/ko@latest
     go install github.com/norwoodj/helm-docs/cmd/helm-docs@latest

kwok/apis/crds/karpenter.kwok.sh_kwoknodeclasses.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.18.0
+    controller-gen.kubebuilder.io/version: v0.19.0
   name: kwoknodeclasses.karpenter.kwok.sh
 spec:
   group: karpenter.kwok.sh

kwok/charts/crds/karpenter.sh_nodeclaims.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.18.0
+    controller-gen.kubebuilder.io/version: v0.19.0
   name: nodeclaims.karpenter.sh
 spec:
   group: karpenter.sh

kwok/charts/crds/karpenter.sh_nodepools.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.18.0
+    controller-gen.kubebuilder.io/version: v0.19.0
   name: nodepools.karpenter.sh
 spec:
   group: karpenter.sh

kwok/cloudprovider/cloudprovider.go

@@ -74,7 +74,7 @@ func (c CloudProvider) Delete(ctx context.Context, nodeClaim *v1.NodeClaim) erro
 }
 
 func (c CloudProvider) Get(ctx context.Context, providerID string) (*v1.NodeClaim, error) {
-	nodeName := strings.Replace(providerID, kwokProviderPrefix, "", -1)
+	nodeName := strings.ReplaceAll(providerID, kwokProviderPrefix, "")
 	node := &corev1.Node{}
 	if err := c.kubeClient.Get(ctx, types.NamespacedName{Name: nodeName}, node); err != nil {
 		if errors.IsNotFound(err) {
@@ -141,7 +141,7 @@ func (c CloudProvider) getInstanceType(instanceTypeName string) (*cloudprovider.
 }
 
 func (c CloudProvider) toNode(nodeClaim *v1.NodeClaim) (*corev1.Node, error) {
-	newName := strings.Replace(namesgenerator.GetRandomName(0), "_", "-", -1)
+	newName := strings.ReplaceAll(namesgenerator.GetRandomName(0), "_", "-")
 	//nolint
 	newName = fmt.Sprintf("%s-%d", newName, rand.Uint32())
 

kwok/cloudprovider/helpers.go

@@ -177,8 +177,8 @@ func newInstanceType(options InstanceTypeOptions) *cloudprovider.InstanceType {
 				Requirements: scheduling.NewRequirements(lo.Map(off.Requirements, func(req corev1.NodeSelectorRequirement, _ int) *scheduling.Requirement {
 					return scheduling.NewRequirement(req.Key, req.Operator, req.Values...)
 				})...),
-				Price:     off.Offering.Price,
-				Available: off.Offering.Available,
+				Price:     off.Price,
+				Available: off.Available,
 			}
 		}),
 		Capacity: options.Resources,

pkg/apis/crds/karpenter.sh_nodeclaims.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.18.0
+    controller-gen.kubebuilder.io/version: v0.19.0
   name: nodeclaims.karpenter.sh
 spec:
   group: karpenter.sh

pkg/apis/crds/karpenter.sh_nodepools.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.18.0
+    controller-gen.kubebuilder.io/version: v0.19.0
   name: nodepools.karpenter.sh
 spec:
   group: karpenter.sh

pkg/apis/v1/duration.go

@@ -70,7 +70,7 @@ func (d NillableDuration) MarshalJSON() ([]byte, error) {
 		return d.Raw, nil
 	}
 	if d.Duration != nil {
-		return json.Marshal(d.Duration.String())
+		return json.Marshal(d.String())
 	}
 	return json.Marshal(Never)
 }
@@ -81,7 +81,7 @@ func (d NillableDuration) ToUnstructured() interface{} {
 		return d.Raw
 	}
 	if d.Duration != nil {
-		return d.Duration.String()
+		return d.String()
 	}
 	return Never
 }

pkg/apis/v1/nodepool.go

@@ -217,8 +217,8 @@ type NodeClaimTemplateSpec struct {
 func (in *NodeClaimTemplate) ToNodeClaim() *NodeClaim {
 	return &NodeClaim{
 		ObjectMeta: metav1.ObjectMeta{
-			Labels:      in.ObjectMeta.Labels,
-			Annotations: in.ObjectMeta.Annotations,
+			Labels:      in.Labels,
+			Annotations: in.Annotations,
 		},
 		Spec: NodeClaimSpec{
 			Taints:                 in.Spec.Taints,

pkg/controllers/disruption/consolidation.go

@@ -180,7 +180,7 @@ func (c *consolidation) computeConsolidation(ctx context.Context, candidates ...
 
 	// sort the instanceTypes by price before we take any actions like truncation for spot-to-spot consolidation or finding the nodeclaim
 	// that meets the minimum requirement after filteringByPrice
-	results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions = results.NewNodeClaims[0].InstanceTypeOptions.OrderByPrice(results.NewNodeClaims[0].Requirements)
+	results.NewNodeClaims[0].InstanceTypeOptions = results.NewNodeClaims[0].InstanceTypeOptions.OrderByPrice(results.NewNodeClaims[0].Requirements)
 
 	if allExistingAreSpot &&
 		results.NewNodeClaims[0].Requirements.Get(v1.CapacityTypeLabelKey).Has(v1.CapacityTypeSpot) {
@@ -199,7 +199,7 @@ func (c *consolidation) computeConsolidation(ctx context.Context, candidates ...
 		}
 		return Command{}, pscheduling.Results{}, nil
 	}
-	if len(results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions) == 0 {
+	if len(results.NewNodeClaims[0].InstanceTypeOptions) == 0 {
 		if len(candidates) == 1 {
 			c.recorder.Publish(disruptionevents.Unconsolidatable(candidates[0].Node, candidates[0].NodeClaim, "Can't replace with a cheaper node")...)
 		}
@@ -240,7 +240,7 @@ func (c *consolidation) computeSpotToSpotConsolidation(ctx context.Context, cand
 	// Since we are sure that the replacement nodeclaim considered for the spot candidates are spot, we will enforce it through the requirements.
 	results.NewNodeClaims[0].Requirements.Add(scheduling.NewRequirement(v1.CapacityTypeLabelKey, corev1.NodeSelectorOpIn, v1.CapacityTypeSpot))
 	// All possible replacements for the current candidate compatible with spot offerings
-	results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions = results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions.Compatible(results.NewNodeClaims[0].Requirements)
+	results.NewNodeClaims[0].InstanceTypeOptions = results.NewNodeClaims[0].InstanceTypeOptions.Compatible(results.NewNodeClaims[0].Requirements)
 
 	// filterByPrice returns the instanceTypes that are lower priced than the current candidate and any error that indicates the input couldn't be filtered.
 	var err error
@@ -251,7 +251,7 @@ func (c *consolidation) computeSpotToSpotConsolidation(ctx context.Context, cand
 		}
 		return Command{}, pscheduling.Results{}, nil
 	}
-	if len(results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions) == 0 {
+	if len(results.NewNodeClaims[0].InstanceTypeOptions) == 0 {
 		if len(candidates) == 1 {
 			c.recorder.Publish(disruptionevents.Unconsolidatable(candidates[0].Node, candidates[0].NodeClaim, "Can't replace with a cheaper node")...)
 		}
@@ -272,9 +272,9 @@ func (c *consolidation) computeSpotToSpotConsolidation(ctx context.Context, cand
 	// We check whether we have 15 cheaper instances than the current candidate instance. If this is the case, we know the following things:
 	//   1) The current candidate is not in the set of the 15 cheapest instance types and
 	//   2) There were at least 15 options cheaper than the current candidate.
-	if len(results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions) < MinInstanceTypesForSpotToSpotConsolidation {
+	if len(results.NewNodeClaims[0].InstanceTypeOptions) < MinInstanceTypesForSpotToSpotConsolidation {
 		c.recorder.Publish(disruptionevents.Unconsolidatable(candidates[0].Node, candidates[0].NodeClaim, fmt.Sprintf("SpotToSpotConsolidation requires %d cheaper instance type options than the current candidate to consolidate, got %d",
-			MinInstanceTypesForSpotToSpotConsolidation, len(results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions)))...)
+			MinInstanceTypesForSpotToSpotConsolidation, len(results.NewNodeClaims[0].InstanceTypeOptions)))...)
 		return Command{}, pscheduling.Results{}, nil
 	}
 
@@ -289,10 +289,10 @@ func (c *consolidation) computeSpotToSpotConsolidation(ctx context.Context, cand
 	// Taking this to 15 types, we need to only send the 15 cheapest types in the CreateInstanceFromTypes call so that the resulting instance is always in that set of 15 and we won’t immediately consolidate.
 	if results.NewNodeClaims[0].Requirements.HasMinValues() {
 		// Here we are trying to get the max of the minimum instances required to satisfy the minimum requirement and the default 15 to cap the instances for spot-to-spot consolidation.
-		minInstanceTypes, _ := results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions.SatisfiesMinValues(results.NewNodeClaims[0].Requirements)
-		results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions = lo.Slice(results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions, 0, lo.Max([]int{MinInstanceTypesForSpotToSpotConsolidation, minInstanceTypes}))
+		minInstanceTypes, _ := results.NewNodeClaims[0].InstanceTypeOptions.SatisfiesMinValues(results.NewNodeClaims[0].Requirements)
+		results.NewNodeClaims[0].InstanceTypeOptions = lo.Slice(results.NewNodeClaims[0].InstanceTypeOptions, 0, lo.Max([]int{MinInstanceTypesForSpotToSpotConsolidation, minInstanceTypes}))
 	} else {
-		results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions = lo.Slice(results.NewNodeClaims[0].NodeClaimTemplate.InstanceTypeOptions, 0, MinInstanceTypesForSpotToSpotConsolidation)
+		results.NewNodeClaims[0].InstanceTypeOptions = lo.Slice(results.NewNodeClaims[0].InstanceTypeOptions, 0, MinInstanceTypesForSpotToSpotConsolidation)
 	}
 
 	return Command{
@@ -305,7 +305,7 @@ func (c *consolidation) computeSpotToSpotConsolidation(ctx context.Context, cand
 func getCandidatePrices(candidates []*Candidate) (float64, error) {
 	var price float64
 	for _, c := range candidates {
-		compatibleOfferings := c.instanceType.Offerings.Compatible(scheduling.NewLabelRequirements(c.StateNode.Labels()))
+		compatibleOfferings := c.instanceType.Offerings.Compatible(scheduling.NewLabelRequirements(c.Labels()))
 		if len(compatibleOfferings) == 0 {
 			return 0.0, fmt.Errorf("unable to determine offering for %s/%s/%s", c.instanceType.Name, c.capacityType, c.zone)
 		}