From af137e269b48610845e40ec65115d9d76e2b7c56 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Thu, 24 Aug 2023 14:39:54 +0200 Subject: [PATCH] md: drop templatesuffix from machineset names --- .../machinedeployment_sync.go | 11 +++--- .../machinedeployment_sync_test.go | 37 +++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/internal/controllers/machinedeployment/machinedeployment_sync.go b/internal/controllers/machinedeployment/machinedeployment_sync.go index 546ecd85f86f..b6400c544b2e 100644 --- a/internal/controllers/machinedeployment/machinedeployment_sync.go +++ b/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 diff --git a/internal/controllers/machinedeployment/machinedeployment_sync_test.go b/internal/controllers/machinedeployment/machinedeployment_sync_test.go index 0367d3d0cbe0..5aa03be75c94 100644 --- a/internal/controllers/machinedeployment/machinedeployment_sync_test.go +++ b/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) + } + }) + } +}