From 081a0a92917d5dab590dd895c76803ef541c5acb Mon Sep 17 00:00:00 2001 From: enxebre Date: Tue, 20 Sep 2022 10:46:07 +0200 Subject: [PATCH] Add support to reconcile labels from Machines to Nodes --- api/v1beta1/machine_types.go | 17 ++- .../machine/machine_controller_noderef.go | 47 ++++++- .../machine_controller_noderef_test.go | 129 ++++++++++++++++++ 3 files changed, 191 insertions(+), 2 deletions(-) diff --git a/api/v1beta1/machine_types.go b/api/v1beta1/machine_types.go index 1c7ab204c348..0b485cd39c73 100644 --- a/api/v1beta1/machine_types.go +++ b/api/v1beta1/machine_types.go @@ -19,7 +19,6 @@ package v1beta1 import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - capierrors "sigs.k8s.io/cluster-api/errors" ) @@ -56,6 +55,22 @@ const ( // This annotation can be set on BootstrapConfig or Machine objects. The value set on the Machine object takes precedence. // This annotation can only be used on Control Plane Machines. MachineCertificatesExpiryDateAnnotation = "machine.cluster.x-k8s.io/certificates-expiry" + + // NodeRoleLabelPrefix is one of the CAPI managed prefix Node labels. + NodeRoleLabelPrefix = "node-role.kubernetes.io/" + // NodeRestrictionLabelPrefix is one of the CAPI managed prefix Node labels. + NodeRestrictionLabelPrefix = "node-restriction.kubernetes.io/" + // NodeCAPILabelPrefix is one of the CAPI managed prefix Node labels. + NodeCAPILabelPrefix = "node.cluster.x-k8s.io/" +) + +var ( + // ManagedNodeLabelPrefixes is a slice with all the CAPI managed prefix Node labels. + ManagedNodeLabelPrefixes = []string{ + NodeRoleLabelPrefix, + NodeRestrictionLabelPrefix, + NodeCAPILabelPrefix, + } ) // ANCHOR: MachineSpec diff --git a/internal/controllers/machine/machine_controller_noderef.go b/internal/controllers/machine/machine_controller_noderef.go index 29b5a397679f..d95b770914ba 100644 --- a/internal/controllers/machine/machine_controller_noderef.go +++ b/internal/controllers/machine/machine_controller_noderef.go @@ -19,6 +19,7 @@ package machine import ( "context" "fmt" + "strings" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -109,7 +110,11 @@ func (r *Reconciler) reconcileNode(ctx context.Context, cluster *clusterv1.Clust desired[clusterv1.OwnerKindAnnotation] = owner.Kind desired[clusterv1.OwnerNameAnnotation] = owner.Name } - if annotations.AddAnnotations(node, desired) { + annotationsHaveChanged := annotations.AddAnnotations(node, desired) + + managedLabelsHaveChanged := reconcileManagedLabels(machine, node) + + if annotationsHaveChanged || managedLabelsHaveChanged { if err := patchHelper.Patch(ctx, node); err != nil { log.V(2).Info("Failed patch node to set annotations", "err", err, "node name", node.Name) return ctrl.Result{}, err @@ -131,6 +136,46 @@ func (r *Reconciler) reconcileNode(ctx context.Context, cluster *clusterv1.Clust return ctrl.Result{}, nil } +func reconcileManagedLabels(machine *clusterv1.Machine, node *corev1.Node) bool { + // Reconcile managed Labels. + var labelsHaveChanged bool + desiredLabels := getManagedLabels(machine.Labels) + currentNodeLabels := getManagedLabels(node.Labels) + + // Reconcile desired labels. + for desiredKey, desiredValue := range desiredLabels { + if currentValue, ok := currentNodeLabels[desiredKey]; !ok || desiredValue != currentValue { + node.Labels[desiredKey] = desiredValue + labelsHaveChanged = true + } + } + + // Delete current labels which are not desired anymore. + for currentKey, _ := range currentNodeLabels { + if _, ok := desiredLabels[currentKey]; !ok { + labelsHaveChanged = true + delete(node.Labels, currentKey) + } + } + + return labelsHaveChanged +} + +// getManagedLabels gets a map[string]string and returns another map[string]string +// filtering out labels not managed by CAPI. +func getManagedLabels(labels map[string]string) map[string]string { + managedLabels := make(map[string]string) + for key, value := range labels { + for _, managedPrefix := range clusterv1.ManagedNodeLabelPrefixes { + if strings.HasPrefix(key, managedPrefix) { + managedLabels[key] = value + } + } + } + + return managedLabels +} + // summarizeNodeConditions summarizes a Node's conditions and returns the summary of condition statuses and concatenate failed condition messages: // if there is at least 1 semantically-negative condition, summarized status = False; // if there is at least 1 semantically-positive condition when there is 0 semantically negative condition, summarized status = True; diff --git a/internal/controllers/machine/machine_controller_noderef_test.go b/internal/controllers/machine/machine_controller_noderef_test.go index 2cf369dac81f..f5a42480f125 100644 --- a/internal/controllers/machine/machine_controller_noderef_test.go +++ b/internal/controllers/machine/machine_controller_noderef_test.go @@ -227,3 +227,132 @@ func TestSummarizeNodeConditions(t *testing.T) { }) } } + +func TestGetManagedLabels(t *testing.T) { + // Create managedLabels map from known managed prefixes. + managedLabels := map[string]string{} + for _, prefix := range clusterv1.ManagedNodeLabelPrefixes { + managedLabels[prefix] = "" + } + + // Append arbitrary labels. + allLabels := map[string]string{ + "foo": "", + "bar": "", + } + for k, v := range managedLabels { + allLabels[k] = v + } + + g := NewWithT(t) + got := getManagedLabels(allLabels) + g.Expect(got).To(BeEquivalentTo(managedLabels)) +} + +func TestReconcileManagedLabels(t *testing.T) { + allLabels := map[string]string{ + "foo": "", + "bar": "", + } + for _, prefix := range clusterv1.ManagedNodeLabelPrefixes { + allLabels[prefix] = "" + } + + testCases := []struct { + name string + node *corev1.Node + machine *clusterv1.Machine + expectedLabels map[string]string + expected bool + }{ + { + name: "when Node is missing have wanted managed labels they should be added", + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "foo": "A", + clusterv1.NodeCAPILabelPrefix: "anything", + }, + }, + }, + machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + clusterv1.NodeRoleLabelPrefix: "", + clusterv1.NodeRestrictionLabelPrefix: "", + clusterv1.NodeCAPILabelPrefix: "stamp", + "foo": "B", + }, + }, + }, + expectedLabels: map[string]string{ + clusterv1.NodeRoleLabelPrefix: "", + clusterv1.NodeRestrictionLabelPrefix: "", + clusterv1.NodeCAPILabelPrefix: "stamp", + "foo": "A", + }, + expected: true, + }, + { + name: "when Node is have unwanted managed labels they should be removed", + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + clusterv1.NodeRestrictionLabelPrefix: "anything", + clusterv1.NodeCAPILabelPrefix: "anything", + "foo": "A", + }, + }, + }, + machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + clusterv1.NodeRoleLabelPrefix: "anything", + "foo": "B", + }, + }, + }, + expectedLabels: map[string]string{ + clusterv1.NodeRoleLabelPrefix: "anything", + "foo": "A", + }, + expected: true, + }, + { + name: "when all managed labels match nothing is needed", + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + clusterv1.NodeRestrictionLabelPrefix: "anything", + clusterv1.NodeCAPILabelPrefix: "anything", + "foo": "A", + }, + }, + }, + machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + clusterv1.NodeRestrictionLabelPrefix: "anything", + clusterv1.NodeCAPILabelPrefix: "anything", + "foo": "B", + }, + }, + }, + expectedLabels: map[string]string{ + clusterv1.NodeRestrictionLabelPrefix: "anything", + clusterv1.NodeCAPILabelPrefix: "anything", + "foo": "A", + }, + expected: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + + g := NewWithT(t) + needsUpdated := reconcileManagedLabels(tc.machine, tc.node) + g.Expect(tc.expected).To(Equal(needsUpdated)) + g.Expect(tc.expectedLabels).To(BeEquivalentTo(tc.node.Labels)) + }) + } +}