From 4bc897274c57fe08fbfcf658490f8113d3c38eaa 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 | 7 +++ .../controllers/machine/machine_controller.go | 1 + .../machine/machine_controller_noderef.go | 45 +++++++++++++++++++ .../machine_controller_noderef_test.go | 26 +++++++++++ .../machine/machine_controller_phases_test.go | 42 ++++++++++------- .../machine/machine_controller_test.go | 16 +++---- 6 files changed, 112 insertions(+), 25 deletions(-) diff --git a/api/v1beta1/machine_types.go b/api/v1beta1/machine_types.go index 73bb663a044c..db3455859af7 100644 --- a/api/v1beta1/machine_types.go +++ b/api/v1beta1/machine_types.go @@ -62,6 +62,13 @@ 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 Node label prefixes. + NodeRoleLabelPrefix = "node-role.kubernetes.io" + // NodeRestrictionLabelDomain is one of the CAPI managed Node label domains. + NodeRestrictionLabelDomain = "node-restriction.kubernetes.io" + // ManagedNodeLabelDomain is one of the CAPI managed Node label domains. + ManagedNodeLabelDomain = "node.cluster.x-k8s.io" ) // ANCHOR: MachineSpec diff --git a/internal/controllers/machine/machine_controller.go b/internal/controllers/machine/machine_controller.go index f32f1b7d4b1a..2ba71f8b7dc5 100644 --- a/internal/controllers/machine/machine_controller.go +++ b/internal/controllers/machine/machine_controller.go @@ -91,6 +91,7 @@ type Reconciler struct { // nodeDeletionRetryTimeout determines how long the controller will retry deleting a node // during a single reconciliation. nodeDeletionRetryTimeout time.Duration + disableNodeLabelSync bool } func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error { diff --git a/internal/controllers/machine/machine_controller_noderef.go b/internal/controllers/machine/machine_controller_noderef.go index 29b5a397679f..1b5d576770fb 100644 --- a/internal/controllers/machine/machine_controller_noderef.go +++ b/internal/controllers/machine/machine_controller_noderef.go @@ -19,10 +19,13 @@ package machine import ( "context" "fmt" + "strings" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -116,6 +119,17 @@ func (r *Reconciler) reconcileNode(ctx context.Context, cluster *clusterv1.Clust } } + if !r.disableNodeLabelSync { + options := []client.PatchOption{ + client.FieldOwner("capi-machine"), + client.ForceOwnership, + } + nodePatch := unstructuredNode(node.Name, node.UID, getManagedLabels(machine.Labels)) + if err := remoteClient.Patch(ctx, nodePatch, client.Apply, options...); err != nil { + return ctrl.Result{}, errors.Wrap(err, "failed to apply patch label to the node") + } + } + // Do the remaining node health checks, then set the node health to true if all checks pass. status, message := summarizeNodeConditions(node) if status == corev1.ConditionFalse { @@ -131,6 +145,37 @@ func (r *Reconciler) reconcileNode(ctx context.Context, cluster *clusterv1.Clust return ctrl.Result{}, nil } +// unstructuredNode returns a raw unstructured from Node input. +func unstructuredNode(name string, uid types.UID, labels map[string]string) *unstructured.Unstructured { + obj := &unstructured.Unstructured{} + obj.SetAPIVersion("v1") + obj.SetKind("Node") + obj.SetName(name) + obj.SetUID(uid) + obj.SetLabels(labels) + return obj +} + +// 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 { + dnsSubdomainOrName := strings.Split(key, "/")[0] + if dnsSubdomainOrName == clusterv1.NodeRoleLabelPrefix { + managedLabels[key] = value + } + if dnsSubdomainOrName == clusterv1.NodeRestrictionLabelDomain || strings.HasSuffix(dnsSubdomainOrName, "."+clusterv1.NodeRestrictionLabelDomain) { + managedLabels[key] = value + } + if dnsSubdomainOrName == clusterv1.ManagedNodeLabelDomain || strings.HasSuffix(dnsSubdomainOrName, "."+clusterv1.ManagedNodeLabelDomain) { + 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 6eedb805bf64..610475b5f586 100644 --- a/internal/controllers/machine/machine_controller_noderef_test.go +++ b/internal/controllers/machine/machine_controller_noderef_test.go @@ -227,3 +227,29 @@ func TestSummarizeNodeConditions(t *testing.T) { }) } } + +func TestGetManagedLabels(t *testing.T) { + // Create managedLabels map from known managed prefixes. + managedLabels := map[string]string{} + managedLabels[clusterv1.ManagedNodeLabelDomain] = "" + managedLabels["custom-prefix."+clusterv1.NodeRestrictionLabelDomain] = "" + managedLabels["custom-prefix."+clusterv1.NodeRestrictionLabelDomain+"/anything"] = "" + managedLabels[clusterv1.NodeRoleLabelPrefix+"/anything"] = "" + + // Append arbitrary labels. + allLabels := map[string]string{ + "foo": "", + "bar": "", + "company.xyz/node.cluster.x-k8s.io": "not-managed", + "gpu-node.cluster.x-k8s.io": "not-managed", + "company.xyz/node-restriction.kubernetes.io": "not-managed", + "gpu-node-restriction.kubernetes.io": "not-managed", + } + for k, v := range managedLabels { + allLabels[k] = v + } + + g := NewWithT(t) + got := getManagedLabels(allLabels) + g.Expect(got).To(BeEquivalentTo(managedLabels)) +} diff --git a/internal/controllers/machine/machine_controller_phases_test.go b/internal/controllers/machine/machine_controller_phases_test.go index 8294c731623a..e97e1d6455a4 100644 --- a/internal/controllers/machine/machine_controller_phases_test.go +++ b/internal/controllers/machine/machine_controller_phases_test.go @@ -117,6 +117,7 @@ func TestReconcileMachinePhases(t *testing.T) { infraConfig := defaultInfra.DeepCopy() r := &Reconciler{ + disableNodeLabelSync: true, Client: fake.NewClientBuilder(). WithScheme(scheme.Scheme). WithObjects(defaultCluster, @@ -155,6 +156,7 @@ func TestReconcileMachinePhases(t *testing.T) { infraConfig := defaultInfra.DeepCopy() r := &Reconciler{ + disableNodeLabelSync: true, Client: fake.NewClientBuilder(). WithScheme(scheme.Scheme). WithObjects(defaultCluster, @@ -198,6 +200,7 @@ func TestReconcileMachinePhases(t *testing.T) { machine.Status.LastUpdated = &lastUpdated r := &Reconciler{ + disableNodeLabelSync: true, Client: fake.NewClientBuilder(). WithScheme(scheme.Scheme). WithObjects(defaultCluster, @@ -268,8 +271,7 @@ func TestReconcileMachinePhases(t *testing.T) { node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "machine-test-node", - Namespace: metav1.NamespaceDefault, + Name: "machine-test-node", }, Spec: corev1.NodeSpec{ProviderID: "test://id-1"}, } @@ -285,8 +287,9 @@ func TestReconcileMachinePhases(t *testing.T) { defaultKubeconfigSecret, ).Build() r := &Reconciler{ - Client: cl, - Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), cl, scheme.Scheme, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}), + disableNodeLabelSync: true, + Client: cl, + Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), cl, scheme.Scheme, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}), } res, err := r.reconcile(ctx, defaultCluster, machine) @@ -334,8 +337,7 @@ func TestReconcileMachinePhases(t *testing.T) { node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "machine-test-node", - Namespace: metav1.NamespaceDefault, + Name: "machine-test-node", }, Spec: corev1.NodeSpec{ProviderID: "test://id-1"}, } @@ -351,8 +353,9 @@ func TestReconcileMachinePhases(t *testing.T) { defaultKubeconfigSecret, ).Build() r := &Reconciler{ - Client: cl, - Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), cl, scheme.Scheme, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}), + disableNodeLabelSync: true, + Client: cl, + Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), cl, scheme.Scheme, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}), } res, err := r.reconcile(ctx, defaultCluster, machine) @@ -410,8 +413,7 @@ func TestReconcileMachinePhases(t *testing.T) { machine.Status.LastUpdated = &lastUpdated node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "machine-test-node", - Namespace: metav1.NamespaceDefault, + Name: "machine-test-node", }, Spec: corev1.NodeSpec{ProviderID: "test://id-1"}, } @@ -427,8 +429,9 @@ func TestReconcileMachinePhases(t *testing.T) { defaultKubeconfigSecret, ).Build() r := &Reconciler{ - Client: cl, - Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), cl, scheme.Scheme, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}), + disableNodeLabelSync: true, + Client: cl, + Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), cl, scheme.Scheme, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}), } res, err := r.reconcile(ctx, defaultCluster, machine) @@ -487,8 +490,9 @@ func TestReconcileMachinePhases(t *testing.T) { ).Build() r := &Reconciler{ - Client: cl, - Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), cl, scheme.Scheme, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}), + disableNodeLabelSync: true, + Client: cl, + Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), cl, scheme.Scheme, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}), } res, err := r.reconcile(ctx, defaultCluster, machine) @@ -568,9 +572,10 @@ func TestReconcileMachinePhases(t *testing.T) { infraConfig, ).Build() r := &Reconciler{ - Client: cl, - Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), cl, scheme.Scheme, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}), - recorder: record.NewFakeRecorder(32), + disableNodeLabelSync: true, + Client: cl, + Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), cl, scheme.Scheme, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}), + recorder: record.NewFakeRecorder(32), } res, err := r.reconcileDelete(ctx, defaultCluster, machine) @@ -867,6 +872,7 @@ func TestReconcileBootstrap(t *testing.T) { bootstrapConfig := &unstructured.Unstructured{Object: tc.bootstrapConfig} r := &Reconciler{ + disableNodeLabelSync: true, Client: fake.NewClientBuilder(). WithObjects(tc.machine, builder.GenericBootstrapConfigCRD.DeepCopy(), @@ -1077,6 +1083,7 @@ func TestReconcileInfrastructure(t *testing.T) { infraConfig := &unstructured.Unstructured{Object: tc.infraConfig} r := &Reconciler{ + disableNodeLabelSync: true, Client: fake.NewClientBuilder(). WithObjects(tc.machine, builder.GenericBootstrapConfigCRD.DeepCopy(), @@ -1318,6 +1325,7 @@ func TestReconcileCertificateExpiry(t *testing.T) { g := NewWithT(t) r := &Reconciler{ + disableNodeLabelSync: true, Client: fake.NewClientBuilder(). WithObjects( tc.machine, diff --git a/internal/controllers/machine/machine_controller_test.go b/internal/controllers/machine/machine_controller_test.go index 6df3a24e1f6d..afd0c4ced089 100644 --- a/internal/controllers/machine/machine_controller_test.go +++ b/internal/controllers/machine/machine_controller_test.go @@ -615,8 +615,7 @@ func TestReconcileRequest(t *testing.T) { node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: metav1.NamespaceDefault, + Name: "test", }, Spec: corev1.NodeSpec{ProviderID: "test://id-1"}, } @@ -726,8 +725,9 @@ func TestReconcileRequest(t *testing.T) { ).Build() r := &Reconciler{ - Client: clientFake, - Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), clientFake, scheme.Scheme, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}), + disableNodeLabelSync: true, + Client: clientFake, + Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), clientFake, scheme.Scheme, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}), } result, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: util.ObjectKey(&tc.machine)}) @@ -830,8 +830,7 @@ func TestMachineConditions(t *testing.T) { node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: metav1.NamespaceDefault, + Name: "test", }, Spec: corev1.NodeSpec{ProviderID: "test://id-1"}, } @@ -971,8 +970,9 @@ func TestMachineConditions(t *testing.T) { ).Build() r := &Reconciler{ - Client: clientFake, - Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), clientFake, scheme.Scheme, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}), + disableNodeLabelSync: true, + Client: clientFake, + Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), clientFake, scheme.Scheme, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}), } _, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: util.ObjectKey(&machine)})