Skip to content

Commit

Permalink
md: drop templatesuffix from machineset names
Browse files Browse the repository at this point in the history
  • Loading branch information
chrischdi committed Aug 24, 2023
1 parent 3b55934 commit 72fbba9
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 11 deletions.
17 changes: 9 additions & 8 deletions internal/controllers/machinedeployment/machinedeployment_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -359,18 +359,19 @@ func cloneStringMap(in map[string]string) map[string]string {
const (
maxNameLength = 63
randomLength = 5
maxGeneratedNameLength = maxNameLength - randomLength
MaxGeneratedNameLength = maxNameLength - randomLength

Check failure on line 362 in internal/controllers/machinedeployment/machinedeployment_sync.go

View workflow job for this annotation

GitHub Actions / lint

exported: exported const MaxGeneratedNameLength should have comment (or a comment on this block) or be unexported (revive)

Check warning on line 362 in internal/controllers/machinedeployment/machinedeployment_sync.go

View workflow job for this annotation

GitHub Actions / lint

exported: exported const MaxGeneratedNameLength should have comment (or a comment on this block) or be unexported (revive)

Check failure on line 362 in internal/controllers/machinedeployment/machinedeployment_sync.go

View workflow job for this annotation

GitHub Actions / lint

exported: exported const MaxGeneratedNameLength should have comment (or a comment on this block) or be unexported (revive)

Check warning on line 362 in internal/controllers/machinedeployment/machinedeployment_sync.go

View workflow job for this annotation

GitHub Actions / lint

exported: exported const MaxGeneratedNameLength should have comment (or a comment on this block) or be unexported (revive)
)

// computeNewMachineSetName generates a new name for the MachineSet just like
// 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 {
if len(base) > maxGeneratedNameLength {
base = base[:maxGeneratedNameLength]
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package machinedeployment
import (
"context"
"fmt"
"strings"
"testing"
"time"

Expand All @@ -31,13 +32,12 @@ import (
"k8s.io/client-go/tools/record"
"k8s.io/klog/v2/klogr"
"k8s.io/utils/pointer"

Check failure on line 34 in internal/controllers/machinedeployment/machinedeployment_sync_test.go

View workflow job for this annotation

GitHub Actions / lint

File is not `gci`-ed with --skip-generated -s standard -s default (gci)

Check failure on line 34 in internal/controllers/machinedeployment/machinedeployment_sync_test.go

View workflow job for this annotation

GitHub Actions / lint

File is not `gci`-ed with --skip-generated -s standard -s default (gci)
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
capierrors "sigs.k8s.io/cluster-api/errors"
"sigs.k8s.io/cluster-api/internal/controllers/machinedeployment/mdutil"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/controller-runtime/pkg/client"

Check failure on line 39 in internal/controllers/machinedeployment/machinedeployment_sync_test.go

View workflow job for this annotation

GitHub Actions / lint

File is not `gci`-ed with --skip-generated -s standard -s default (gci)

Check failure on line 39 in internal/controllers/machinedeployment/machinedeployment_sync_test.go

View workflow job for this annotation

GitHub Actions / lint

File is not `gci`-ed with --skip-generated -s standard -s default (gci)
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

func TestCalculateStatus(t *testing.T) {
Expand Down Expand Up @@ -799,3 +799,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)
}
})
}
}

0 comments on commit 72fbba9

Please sign in to comment.