Merge pull request #9298 from chrischdi/pr-shorter-machineset-names

🐛 MD controller: use regular random suffix for MachineSets, ensure max length 63

internal/controllers/machinedeployment/machinedeployment_sync.go

@@ -240,9 +240,9 @@ func (r *Reconciler) computeDesiredMachineSet(deployment *clusterv1.MachineDeplo
 		// Append a random string at the end of template hash. This is required to distinguish MachineSets that
 		// could be created with the same spec as a result of rolloutAfter. If not, computeDesiredMachineSet
 		// will end up updating the existing MachineSet instead of creating a new one.
-		uniqueIdentifierLabelValue = fmt.Sprintf("%d-%s", templateHash, apirand.String(5))
-
-		name = computeNewMachineSetName(deployment.Name+"-", apirand.SafeEncodeString(uniqueIdentifierLabelValue))
+		var randomSuffix string
+		name, randomSuffix = computeNewMachineSetName(deployment.Name + "-")
+		uniqueIdentifierLabelValue = fmt.Sprintf("%d-%s", templateHash, randomSuffix)
 
 		// Add foregroundDeletion finalizer to MachineSet if the MachineDeployment has it.
 		if sets.New[string](deployment.Finalizers...).Has(metav1.FinalizerDeleteDependents) {
@@ -366,11 +366,12 @@ const (
 // the upstream SimpleNameGenerator.
 // Note: We had to extract the logic as we want to use the MachineSet name suffix as
 // unique identifier for the MachineSet.
-func computeNewMachineSetName(base, suffix string) string {
+func computeNewMachineSetName(base string) (string, string) {
 	if len(base) > maxGeneratedNameLength {
 		base = base[:maxGeneratedNameLength]
 	}
-	return fmt.Sprintf("%s%s", base, suffix)
+	r := apirand.String(randomLength)
+	return fmt.Sprintf("%s%s", base, r), r
 }
 
 // scale scales proportionally in order to mitigate risk. Otherwise, scaling up can increase the size

internal/controllers/machinedeployment/machinedeployment_sync_test.go

@@ -19,6 +19,7 @@ package machinedeployment
 import (
 	"context"
 	"fmt"
+	"strings"
 	"testing"
 	"time"
 
@@ -799,3 +800,39 @@ func assertCondition(t *testing.T, from conditions.Getter, condition *clusterv1.
 		}
 	}
 }
+
+func Test_computeNewMachineSetName(t *testing.T) {
+	tests := []struct {
+		base       string
+		wantPrefix string
+	}{
+		{
+			"a",
+			"a",
+		},
+		{
+			fmt.Sprintf("%058d", 0),
+			fmt.Sprintf("%058d", 0),
+		},
+		{
+			fmt.Sprintf("%059d", 0),
+			fmt.Sprintf("%058d", 0),
+		},
+		{
+			fmt.Sprintf("%0100d", 0),
+			fmt.Sprintf("%058d", 0),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("base=%q, wantPrefix=%q", tt.base, tt.wantPrefix), func(t *testing.T) {
+			got, gotSuffix := computeNewMachineSetName(tt.base)
+			gotPrefix := strings.TrimSuffix(got, gotSuffix)
+			if gotPrefix != tt.wantPrefix {
+				t.Errorf("computeNewMachineSetName() = (%v, %v) wantPrefix %v", got, gotSuffix, tt.wantPrefix)
+			}
+			if len(got) > maxNameLength {
+				t.Errorf("expected %s to be of max length %d", got, maxNameLength)
+			}
+		})
+	}
+}