chore: Fix consistent ordering of NodePools and Provisioners (#860) for v0.33.x (#882)

Author: jonathan-innis

pkg/apis/v1beta1/nodepool.go

@@ -155,10 +155,19 @@ type NodePoolList struct {
 	Items           []NodePool `json:"items"`
 }
 
-// OrderByWeight orders the nodepools in the NodePoolList
-// by their priority weight in-place
+// 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 (pl *NodePoolList) OrderByWeight() {
 	sort.Slice(pl.Items, func(a, b int) bool {
-		return ptr.Int32Value(pl.Items[a].Spec.Weight) > ptr.Int32Value(pl.Items[b].Spec.Weight)
+		weightA := ptr.Int32Value(pl.Items[a].Spec.Weight)
+		weightB := ptr.Int32Value(pl.Items[b].Spec.Weight)
+
+		if weightA == weightB {
+			// Order NodePools by name for a consistent ordering when sorting equal weight
+			return pl.Items[a].Name > pl.Items[b].Name
+		}
+		return weightA > weightB
 	})
 }

pkg/apis/v1beta1/suite_test.go

@@ -16,13 +16,16 @@ package v1beta1_test
 
 import (
 	"context"
+	"math/rand"
 	"testing"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/samber/lo"
 	. "knative.dev/pkg/logging/testing"
 
 	"sigs.k8s.io/karpenter/pkg/apis"
+	"sigs.k8s.io/karpenter/pkg/apis/v1beta1"
 	"sigs.k8s.io/karpenter/pkg/operator/scheme"
 	"sigs.k8s.io/karpenter/pkg/test"
 	. "sigs.k8s.io/karpenter/pkg/test/expectations"
@@ -48,3 +51,50 @@ var _ = AfterEach(func() {
 var _ = AfterSuite(func() {
 	Expect(env.Stop()).To(Succeed(), "Failed to stop environment")
 })
+
+var _ = Describe("OrderByWeight", func() {
+	It("should order the NodePools by weight", func() {
+		// Generate 10 NodePools that have random weights, some might have the same weights
+		var nodePools []v1beta1.NodePool
+		for i := 0; i < 10; i++ {
+			np := test.NodePool(v1beta1.NodePool{
+				Spec: v1beta1.NodePoolSpec{
+					Weight: lo.ToPtr[int32](int32(rand.Intn(100) + 1)), //nolint:gosec
+				},
+			})
+			nodePools = append(nodePools, *np)
+		}
+
+		nodePools = lo.Shuffle(nodePools)
+		nodePoolList := v1beta1.NodePoolList{Items: nodePools}
+		nodePoolList.OrderByWeight()
+
+		lastWeight := 101 // This is above the allowed weight values
+		for _, np := range nodePoolList.Items {
+			Expect(lo.FromPtr(np.Spec.Weight)).To(BeNumerically("<=", lastWeight))
+			lastWeight = int(lo.FromPtr(np.Spec.Weight))
+		}
+	})
+	It("should order the NodePools by name when the weights are the same", func() {
+		// Generate 10 NodePools with the same weight
+		var nodePools []v1beta1.NodePool
+		for i := 0; i < 10; i++ {
+			np := test.NodePool(v1beta1.NodePool{
+				Spec: v1beta1.NodePoolSpec{
+					Weight: lo.ToPtr[int32](10),
+				},
+			})
+			nodePools = append(nodePools, *np)
+		}
+
+		nodePools = lo.Shuffle(nodePools)
+		nodePoolList := v1beta1.NodePoolList{Items: nodePools}
+		nodePoolList.OrderByWeight()
+
+		lastName := "zzzzzzzzzzzzzzzzzzzzzzzz" // large string value
+		for _, np := range nodePoolList.Items {
+			Expect(np.Name < lastName).To(BeTrue())
+			lastName = np.Name
+		}
+	})
+})