Skip to content

Commit

Permalink
Add name hashing for long MS names
Browse files Browse the repository at this point in the history
Signed-off-by: killianmuldoon <[email protected]>
  • Loading branch information
killianmuldoon committed Dec 8, 2022
1 parent 4ee2872 commit 520d3b6
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 5 deletions.
5 changes: 3 additions & 2 deletions api/v1beta1/machineset_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"

"sigs.k8s.io/cluster-api/internal/controllers/machineset/msutil"
"sigs.k8s.io/cluster-api/util/version"
)

Expand Down Expand Up @@ -64,8 +65,8 @@ func (m *MachineSet) Default() {
}

if len(m.Spec.Selector.MatchLabels) == 0 && len(m.Spec.Selector.MatchExpressions) == 0 {
m.Spec.Selector.MatchLabels[MachineSetNameLabel] = m.Name
m.Spec.Template.Labels[MachineSetNameLabel] = m.Name
m.Spec.Selector.MatchLabels[MachineSetNameLabel] = msutil.MachineSetNameLabelValue(m.Name)
m.Spec.Template.Labels[MachineSetNameLabel] = msutil.MachineSetNameLabelValue(m.Name)
}

if m.Spec.Template.Spec.Version != nil && !strings.HasPrefix(*m.Spec.Template.Spec.Version, "v") {
Expand Down
7 changes: 4 additions & 3 deletions internal/controllers/machineset/machineset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"sigs.k8s.io/cluster-api/controllers/noderefutil"
"sigs.k8s.io/cluster-api/controllers/remote"
"sigs.k8s.io/cluster-api/internal/controllers/machine"
"sigs.k8s.io/cluster-api/internal/controllers/machineset/msutil"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/collections"
Expand Down Expand Up @@ -298,7 +299,7 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster,
mdNameOnMachineSet, mdNameSetOnMachineSet := machineSet.Labels[clusterv1.MachineDeploymentNameLabel]
mdNameOnMachine := machine.Labels[clusterv1.MachineDeploymentNameLabel]

if msName, ok := machine.Labels[clusterv1.MachineSetNameLabel]; ok && msName == machineSet.Name &&
if msNameLabelValue, ok := machine.Labels[clusterv1.MachineSetNameLabel]; ok && msutil.LabelValueMatchesMachineSet(machineSet.Name, msNameLabelValue) &&
(!mdNameSetOnMachineSet || mdNameOnMachineSet == mdNameOnMachine) {
// Continue if the MachineSet name label is already set correctly and
// either the MachineDeployment name label is not set on the MachineSet or
Expand All @@ -310,7 +311,7 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster,
if err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to apply %s label to Machine %q", clusterv1.MachineSetNameLabel, machine.Name)
}
machine.Labels[clusterv1.MachineSetNameLabel] = machineSet.Name
machine.Labels[clusterv1.MachineSetNameLabel] = msutil.MachineSetNameLabelValue(machineSet.Name)
// Propagate the MachineDeploymentNameLabel from MachineSet to Machine if it is set on the MachineSet.
if mdNameSetOnMachineSet {
machine.Labels[clusterv1.MachineDeploymentNameLabel] = mdNameOnMachineSet
Expand Down Expand Up @@ -558,7 +559,7 @@ func (r *Reconciler) getNewMachine(machineSet *clusterv1.MachineSet) *clusterv1.

// Enforce that the MachineSetNameLabel label is set
// Note: the MachineSetNameLabel is added by the default webhook to MachineSet.spec.template.labels if a spec.selector is empty.
machine.Labels[clusterv1.MachineSetNameLabel] = machineSet.Name
machine.Labels[clusterv1.MachineSetNameLabel] = msutil.MachineSetNameLabelValue(machineSet.Name)
// Propagate the MachineDeploymentNameLabel from MachineSet to Machine if it exists.
if mdName, ok := machineSet.Labels[clusterv1.MachineDeploymentNameLabel]; ok {
machine.Labels[clusterv1.MachineDeploymentNameLabel] = mdName
Expand Down
46 changes: 46 additions & 0 deletions internal/controllers/machineset/msutil/names.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
Copyright 2022 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// Package msutil implements MachineDeployment utilities.
package msutil

import (
"encoding/base64"
"hash/fnv"
)

// MachineSetNameLabelValue returns the machineSetName if it meets the standards for a Kubernetes label value.
// If the name can not be used as a label value this function returns a hash.
func MachineSetNameLabelValue(machineSetName string) string {
if len(machineSetName) <= 63 {
return machineSetName
}
hasher := fnv.New32a()
var _, _ = hasher.Write([]byte(machineSetName)) // Can never return error.
return base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(hasher.Sum(nil))
}

// LabelValueMatchesMachineSet returns true if the passed labelValue equals either the machineSetName or the hashed
// value of the name if it's overly long.
func LabelValueMatchesMachineSet(machineSetName, labelValue string) bool {
expectedLabelValue := MachineSetNameLabelValue(machineSetName)
if labelValue == expectedLabelValue {
return true
}
hasher := fnv.New32a()
var _, _ = hasher.Write([]byte(labelValue)) // Can never return error.
return expectedLabelValue == base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(hasher.Sum(nil))
}
90 changes: 90 additions & 0 deletions internal/controllers/machineset/msutil/names_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
Copyright 2022 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package msutil

import (
"testing"

. "github.com/onsi/gomega"
)

func TestMachineSetNameOrHash(t *testing.T) {
g := NewWithT(t)
tests := []struct {
name string
machineSetName string
want string
}{
{
name: "return the name if it's less than 63 characters",
machineSetName: "machineSetName",
want: "machineSetName",
},
{
name: "return for a name with more than 63 characters",
machineSetName: "machineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetName",
want: "FR_ghQ",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := MachineSetNameLabelValue(tt.machineSetName)
g.Expect(got).To(Equal(tt.want))
})
}
}

func TestLabelValueMatchesMachineSet(t *testing.T) {
g := NewWithT(t)
tests := []struct {
name string
machineSetName string
labelValue string
want bool
}{
{
name: "match labels when MachineSet name is short",
machineSetName: "ms1",
labelValue: "ms1",
want: true,
},
{
name: "don't match different labels when MachineSet name is short",
machineSetName: "ms1",
labelValue: "notMS1",
want: false,
},
{
name: "don't match labels when MachineSet name is long",
machineSetName: "machineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetName",
labelValue: "Nx4RdE",
want: false,
},
{
name: "match labels when MachineSet name is long",
machineSetName: "machineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetName",
labelValue: "FR_ghQ",
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := LabelValueMatchesMachineSet(tt.machineSetName, tt.labelValue)
g.Expect(got).To(Equal(tt.want))
})
}
}

0 comments on commit 520d3b6

Please sign in to comment.