From 68676c2af1e88acbf7a19669fcba30accba9ebed Mon Sep 17 00:00:00 2001
From: Eric Fried <efried@redhat.com>
Date: Fri, 14 Jun 2024 17:46:58 -0500
Subject: [PATCH] machinepool: sort ownedLabels, ownedTaints

With these unsorted, it was possible to thrash MachinePools.

HIVE-2541

(cherry picked from commit e243c4396ee43edeb994d9a0754605d8d9992992)
---
 .../machinepool/machinepool_controller.go     | 17 ++++++++---
 .../machinepool_controller_test.go            | 29 ++++++++++++++-----
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/pkg/controller/machinepool/machinepool_controller.go b/pkg/controller/machinepool/machinepool_controller.go
index 0a915edbdbd..225b90e7802 100644
--- a/pkg/controller/machinepool/machinepool_controller.go
+++ b/pkg/controller/machinepool/machinepool_controller.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os"
 	"reflect"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@@ -1049,19 +1050,27 @@ func (r *ReconcileMachinePool) updatePoolStatusForMachineSets(
 // updateOwnedLabelsAndTaints updates OwnedLabels and OwnedTaints in the MachinePool.Status, by fetching the relevant entries sans duplicates from MachinePool.Spec.
 func updateOwnedLabelsAndTaints(pool *hivev1.MachinePool) hivev1.MachinePoolStatus {
 	// Update our tracked labels...
-	pool.Status.OwnedLabels = make([]string, len(pool.Spec.Labels))
+	ownedLabels := make([]string, len(pool.Spec.Labels))
 	i := 0
 	for labelKey := range pool.Spec.Labels {
-		pool.Status.OwnedLabels[i] = labelKey
+		ownedLabels[i] = labelKey
 		i++
 	}
+	sort.Strings(ownedLabels)
+	pool.Status.OwnedLabels = ownedLabels
 
 	// ...and taints
 	uniqueTaints := *controllerutils.GetUniqueTaints(&pool.Spec.Taints)
-	pool.Status.OwnedTaints = make([]hivev1.TaintIdentifier, len(uniqueTaints))
+	ownedTaints := make([]hivev1.TaintIdentifier, len(uniqueTaints))
 	for i, taint := range uniqueTaints {
-		pool.Status.OwnedTaints[i] = controllerutils.IdentifierForTaint(&taint)
+		ownedTaints[i] = controllerutils.IdentifierForTaint(&taint)
 	}
+	sort.Slice(ownedTaints, func(i, j int) bool {
+		// It is not important that these be actually "sorted" -- just that they are
+		// ordered deterministically.
+		return fmt.Sprintf("%v", ownedTaints[i]) < fmt.Sprintf("%v", ownedTaints[j])
+	})
+	pool.Status.OwnedTaints = ownedTaints
 	return pool.Status
 }
 
diff --git a/pkg/controller/machinepool/machinepool_controller_test.go b/pkg/controller/machinepool/machinepool_controller_test.go
index ad316f11314..6dd72b1369f 100644
--- a/pkg/controller/machinepool/machinepool_controller_test.go
+++ b/pkg/controller/machinepool/machinepool_controller_test.go
@@ -1400,24 +1400,37 @@ func TestUpdateOwnedLabelsTaints(t *testing.T) {
 			name: "Carry over labels from spec",
 			machinePool: func() *hivev1.MachinePool {
 				mp := testMachinePoolWithoutLabelsTaints()
-				mp.Spec.Labels = make(map[string]string)
-				mp.Spec.Labels["test-label"] = "test-value"
+				mp.Spec.Labels = map[string]string{
+					"test-label":              "test-value",
+					"a-smaller-sorting-label": "z-bigger-value",
+				}
 				return mp
 			}(),
-			expectedOwnedLabels: []string{"test-label"},
+			expectedOwnedLabels: []string{"a-smaller-sorting-label", "test-label"},
 		},
 		{
 			name: "Carry over taints from spec",
 			machinePool: func() *hivev1.MachinePool {
 				mp := testMachinePoolWithoutLabelsTaints()
-				mp.Spec.Taints = append(mp.Spec.Taints, corev1.Taint{
-					Key:    "test-taint",
-					Value:  "test-value",
-					Effect: "NoSchedule",
-				})
+				mp.Spec.Taints = append(mp.Spec.Taints,
+					corev1.Taint{
+						Key:    "test-taint",
+						Value:  "test-value",
+						Effect: "NoSchedule",
+					},
+					corev1.Taint{
+						Key:    "another-taint",
+						Value:  "test-value",
+						Effect: "NoSchedule",
+					},
+				)
 				return mp
 			}(),
 			expectedOwnedTaints: []hivev1.TaintIdentifier{
+				{
+					Key:    "another-taint",
+					Effect: "NoSchedule",
+				},
 				{
 					Key:    "test-taint",
 					Effect: "NoSchedule",