Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[release-1.4] 馃悰 fix node label propagation #8444

Merged
merged 1 commit into from
Apr 1, 2023
Merged

[release-1.4] 馃悰 fix node label propagation #8444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/v1beta1/common_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ const (
// OwnerKindAnnotation is the annotation set on nodes identifying the owner kind.
OwnerKindAnnotation = "cluster.x-k8s.io/owner-kind"

// LabelsFromMachineAnnotation is the annotation set on nodes to track the labels originated from machines.
LabelsFromMachineAnnotation = "cluster.x-k8s.io/labels-from-machine"

// OwnerNameAnnotation is the annotation set on nodes identifying the owner name.
OwnerNameAnnotation = "cluster.x-k8s.io/owner-name"

Expand Down
1 change: 1 addition & 0 deletions docs/book/src/reference/labels_and_annotations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
| unsafe.topology.cluster.x-k8s.io/disable-update-class-name-check | It can be used to disable the webhook check on update that disallows a pre-existing Cluster to be populated with Topology information and Class. |
| cluster.x-k8s.io/cluster-name | It is set on nodes identifying the name of the cluster the node belongs to. |
| cluster.x-k8s.io/cluster-namespace | It is set on nodes identifying the namespace of the cluster the node belongs to. |
| cluster.x-k8s.io/labels-from-machine | It is set on nodes to track the labels originated from machines. |
| cluster.x-k8s.io/machine | It is set on nodes identifying the machine the node belongs to. |
| cluster.x-k8s.io/owner-kind | It is set on nodes identifying the owner kind. |
| cluster.x-k8s.io/owner-name | It is set on nodes identifying the owner name. |
Expand Down
4 changes: 0 additions & 4 deletions internal/controllers/machine/machine_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ import (
const (
// controllerName defines the controller used when creating clients.
controllerName = "machine-controller"

// machineManagerName is the manager name used for Server-Side-Apply (SSA) operations
// in the Machine controller.
machineManagerName = "capi-machine"
)

var (
Expand Down
101 changes: 52 additions & 49 deletions internal/controllers/machine/machine_controller_noderef.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,17 @@ import (
"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"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/api/v1beta1/index"
"sigs.k8s.io/cluster-api/controllers/noderefutil"
"sigs.k8s.io/cluster-api/internal/util/ssa"
"sigs.k8s.io/cluster-api/internal/util/taints"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
)

var (
Expand Down Expand Up @@ -105,40 +101,26 @@ func (r *Reconciler) reconcileNode(ctx context.Context, cluster *clusterv1.Clust
// Set the NodeSystemInfo.
machine.Status.NodeInfo = &node.Status.NodeInfo

// Reconcile node annotations.
patchHelper, err := patch.NewHelper(node, remoteClient)
if err != nil {
return ctrl.Result{}, err
}
desired := map[string]string{
// Compute all the annotations that CAPI is setting on nodes;
// CAPI only enforces some annotations and never changes or removes them.
nodeAnnotations := map[string]string{
clusterv1.ClusterNameAnnotation: machine.Spec.ClusterName,
clusterv1.ClusterNamespaceAnnotation: machine.GetNamespace(),
clusterv1.MachineAnnotation: machine.Name,
}
if owner := metav1.GetControllerOfNoCopy(machine); owner != nil {
desired[clusterv1.OwnerKindAnnotation] = owner.Kind
desired[clusterv1.OwnerNameAnnotation] = owner.Name
}
if annotations.AddAnnotations(node, desired) {
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
}
nodeAnnotations[clusterv1.OwnerKindAnnotation] = owner.Kind
nodeAnnotations[clusterv1.OwnerNameAnnotation] = owner.Name
}

updatedNode := unstructuredNode(node.Name, node.UID, getManagedLabels(machine.Labels))
err = ssa.Patch(ctx, remoteClient, machineManagerName, updatedNode, ssa.WithCachingProxy{Cache: r.ssaCache, Original: node})
if err != nil {
return ctrl.Result{}, errors.Wrap(err, "failed to apply labels to Node")
}
// Update `node` with the new version of the object.
if err := r.Client.Scheme().Convert(updatedNode, node, nil); err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to convert node to structured object %s", klog.KObj(node))
}
// Compute labels to be propagated from Machines to nodes.
// NOTE: CAPI should manage only a subset of node labels, everything else should be preserved.
// NOTE: Once we reconcile node labels for the first time, the NodeUninitializedTaint is removed from the node.
nodeLabels := getManagedLabels(machine.Labels)

// Reconcile node taints
if err := r.reconcileNodeTaints(ctx, remoteClient, node); err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile taints on Node %s", klog.KObj(node))
if err := r.patchNode(ctx, remoteClient, node, nodeLabels, nodeAnnotations); err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile Node %s", klog.KObj(node))
}

// Do the remaining node health checks, then set the node health to true if all checks pass.
Expand All @@ -156,17 +138,6 @@ 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 {
Expand Down Expand Up @@ -269,16 +240,48 @@ func (r *Reconciler) getNode(ctx context.Context, c client.Reader, providerID *n
return &nodeList.Items[0], nil
}

func (r *Reconciler) reconcileNodeTaints(ctx context.Context, remoteClient client.Client, node *corev1.Node) error {
patchHelper, err := patch.NewHelper(node, remoteClient)
if err != nil {
return errors.Wrapf(err, "failed to create patch helper for Node %s", klog.KObj(node))
// PatchNode is required to workaround an issue on Node.Status.Address which is incorrectly annotated as patchStrategy=merge
// and this causes SSA patch to fail in case there are two addresses with the same key https://github.com/kubernetes-sigs/cluster-api/issues/8417
func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client, node *corev1.Node, newLabels, newAnnotations map[string]string) error {
newNode := node.DeepCopy()

// Adds the annotations CAPI sets on the node.
hasAnnotationChanges := annotations.AddAnnotations(newNode, newAnnotations)

// Adds the labels from the Machine.
// NOTE: in order to handle deletion we are tracking the labels set from the Machine in an annotation.
// At the next reconcile we are going to use this for deleting labels previously set by the Machine, but
// not present anymore. Labels not set from machines should be always preserved.
if newNode.Labels == nil {
newNode.Labels = make(map[string]string)
}
hasLabelChanges := false
labelsFromPreviousReconcile := strings.Split(newNode.Annotations[clusterv1.LabelsFromMachineAnnotation], ",")
if len(labelsFromPreviousReconcile) == 1 && labelsFromPreviousReconcile[0] == "" {
labelsFromPreviousReconcile = []string{}
}
labelsFromCurrentReconcile := []string{}
for k, v := range newLabels {
if cur, ok := newNode.Labels[k]; !ok || cur != v {
newNode.Labels[k] = v
hasLabelChanges = true
}
labelsFromCurrentReconcile = append(labelsFromCurrentReconcile, k)
}
// Drop the NodeUninitializedTaint taint on the node.
if taints.RemoveNodeTaint(node, clusterv1.NodeUninitializedTaint) {
if err := patchHelper.Patch(ctx, node); err != nil {
return errors.Wrapf(err, "failed to patch Node %s to modify taints", klog.KObj(node))
for _, k := range labelsFromPreviousReconcile {
if _, ok := newLabels[k]; !ok {
delete(newNode.Labels, k)
hasLabelChanges = true
}
}
return nil
annotations.AddAnnotations(newNode, map[string]string{clusterv1.LabelsFromMachineAnnotation: strings.Join(labelsFromCurrentReconcile, ",")})

// Drop the NodeUninitializedTaint taint on the node given that we are reconciling labels.
hasTaintChanges := taints.RemoveNodeTaint(newNode, clusterv1.NodeUninitializedTaint)

if !hasAnnotationChanges && !hasLabelChanges && !hasTaintChanges {
return nil
}

return remoteClient.Patch(ctx, newNode, client.StrategicMergeFrom(node))
}