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 <kmuldoon@vmware.com>
  • Loading branch information
killianmuldoon committed Dec 12, 2022
1 parent 4ee2872 commit 31ad451
Show file tree
Hide file tree
Showing 8 changed files with 214 additions and 50 deletions.
2 changes: 2 additions & 0 deletions api/v1beta1/machine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,14 @@ const (
ExcludeWaitForNodeVolumeDetachAnnotation = "machine.cluster.x-k8s.io/exclude-wait-for-node-volume-detach"

// MachineSetNameLabel is the label set on machines if they're controlled by MachineSet.
// Note: The value of this label may be a hash if the control plane name is longer than 63 characters.
MachineSetNameLabel = "cluster.x-k8s.io/set-name"

// MachineDeploymentNameLabel is the label set on machines if they're controlled by MachineDeployment.
MachineDeploymentNameLabel = "cluster.x-k8s.io/deployment-name"

// MachineControlPlaneNameLabel is the label set on machines if they're controlled by a ControlPlane.
// Note: The value of this label may be a hash if the control plane name is longer than 63 characters.
MachineControlPlaneNameLabel = "cluster.x-k8s.io/control-plane-name"

// PreDrainDeleteHookAnnotationPrefix annotation specifies the prefix we
Expand Down
6 changes: 4 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"

capilabels "sigs.k8s.io/cluster-api/internal/labels"
"sigs.k8s.io/cluster-api/util/version"
)

Expand Down Expand Up @@ -64,8 +65,9 @@ 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
// Note: MustFormatValue is used here as the value of this label will be a hash if the MachineSet name is longer than 63 characters.
m.Spec.Selector.MatchLabels[MachineSetNameLabel] = capilabels.MustFormatValue(m.Name)
m.Spec.Template.Labels[MachineSetNameLabel] = capilabels.MustFormatValue(m.Name)
}

if m.Spec.Template.Spec.Version != nil && !strings.HasPrefix(*m.Spec.Template.Spec.Version, "v") {
Expand Down
4 changes: 3 additions & 1 deletion controlplane/kubeadm/internal/cluster_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package internal
import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
capilabels "sigs.k8s.io/cluster-api/internal/labels"
)

// ControlPlaneMachineLabelsForCluster returns a set of labels to add to a control plane machine for this specific cluster.
Expand All @@ -34,6 +35,7 @@ func ControlPlaneMachineLabelsForCluster(kcp *controlplanev1.KubeadmControlPlane
// Always force these labels over the ones coming from the spec.
labels[clusterv1.ClusterNameLabel] = clusterName
labels[clusterv1.MachineControlPlaneLabel] = ""
labels[clusterv1.MachineControlPlaneNameLabel] = kcp.Name
// Note: MustFormatValue is used here as the label value can be a hash if the control plane name is longer than 63 characters.
labels[clusterv1.MachineControlPlaneNameLabel] = capilabels.MustFormatValue(kcp.Name)
return labels
}
6 changes: 4 additions & 2 deletions controlplane/kubeadm/internal/controllers/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal"
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
"sigs.k8s.io/cluster-api/feature"
"sigs.k8s.io/cluster-api/internal/labels"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/collections"
Expand Down Expand Up @@ -340,8 +341,9 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster *
// NOTE: cluster.x-k8s.io/control-plane is already set at this stage (it is used when reading controlPlane.Machines).
for i := range controlPlane.Machines {
machine := controlPlane.Machines[i]
if value, ok := machine.Labels[clusterv1.MachineControlPlaneNameLabel]; !ok || value != kcp.Name {
machine.Labels[clusterv1.MachineControlPlaneNameLabel] = kcp.Name
// Note: MustEqualValue is used here as the label value can be a hash if the control plane name is longer than 63 characters.
if value, ok := machine.Labels[clusterv1.MachineControlPlaneNameLabel]; !ok || !labels.MustEqualValue(value, kcp.Name) {
machine.Labels[clusterv1.MachineControlPlaneNameLabel] = labels.MustFormatValue(kcp.Name)
}
}

Expand Down
85 changes: 43 additions & 42 deletions docs/book/src/reference/labels_and_annotations.md

Large diffs are not rendered by default.

8 changes: 5 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"
capilabels "sigs.k8s.io/cluster-api/internal/labels"
"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,8 @@ 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 &&
// Note: MustEqualValue is used here as the value of this label will be a hash if the MachineSet name is longer than 63 characters.
if msNameLabelValue, ok := machine.Labels[clusterv1.MachineSetNameLabel]; ok && capilabels.MustEqualValue(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 +312,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] = capilabels.MustFormatValue(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 +560,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] = capilabels.MustFormatValue(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
63 changes: 63 additions & 0 deletions internal/labels/helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
Copyright 2021 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 labels contains functions to validate and compare values used in Kubernetes labels.
package labels

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

"k8s.io/apimachinery/pkg/util/validation"
)

// MustFormatValue returns the passed inputLabelValue if it meets the standards for a Kubernetes label value.
// If the name is not a valid label value this function returns a hash which meets the requirements.
func MustFormatValue(str string) string {
// a valid Kubernetes label value must:
// - be less than 64 characters long.
// - be an empty string OR consist of alphanumeric characters, '-', '_' or '.'.
// - start and end with an alphanumeric character
if len(validation.IsValidLabelValue(str)) == 0 {
return str
}
hasher := fnv.New32a()
_, err := hasher.Write([]byte(str))
if err != nil {
// At time of writing the implementation of fnv's Write function can never return an error.
// If this changes in a future go version this function will panic.
panic(err)
}
return base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(hasher.Sum(nil))
}

// MustEqualValue returns true if the actualLabelValue equals either the inputLabelValue or the hashed
// value of the inputLabelValue.
func MustEqualValue(str, labelValue string) bool {
expectedLabelValue := MustFormatValue(str)
if labelValue == expectedLabelValue {
return true
}
hasher := fnv.New32a()
// The Write function should never return an error.
_, err := hasher.Write([]byte(labelValue))
if err != nil {
// At time of writing the implementation of fnv's Write function can never return an error.
// If this changes in a future go version this function will panic.
panic(err)
}
return expectedLabelValue == base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(hasher.Sum(nil))
}
90 changes: 90 additions & 0 deletions internal/labels/helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
Copyright 2021 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 labels

import (
"testing"

"github.com/onsi/gomega"
)

func TestNameLabelValue(t *testing.T) {
g := gomega.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 := MustFormatValue(tt.machineSetName)
g.Expect(got).To(gomega.Equal(tt.want))
})
}
}

func TestMustMatchLabelValueForName(t *testing.T) {
g := gomega.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 := MustEqualValue(tt.machineSetName, tt.labelValue)
g.Expect(got).To(gomega.Equal(tt.want))
})
}
}

0 comments on commit 31ad451

Please sign in to comment.