diff --git a/pkg/controller/node/node_controller.go b/pkg/controller/node/node_controller.go index d8c5b65999..78023992b4 100644 --- a/pkg/controller/node/node_controller.go +++ b/pkg/controller/node/node_controller.go @@ -1667,20 +1667,52 @@ func (ctrl *Controller) applyCustomPoolLabels(node *corev1.Node, poolName string } // Extract labels from the pool's node selector - if pool.Spec.NodeSelector == nil || pool.Spec.NodeSelector.MatchLabels == nil { - klog.V(4).Infof("MachineConfigPool %s has no node selector labels", poolName) + if pool.Spec.NodeSelector == nil { + klog.Infof("MachineConfigPool %s has no node selector", poolName) return nil } - labelsToApply := pool.Spec.NodeSelector.MatchLabels + labelsToApply := map[string]string{} + + maps.Copy(labelsToApply, pool.Spec.NodeSelector.MatchLabels) + + // For matchExpressions, derive a label only when the value is unambiguous: + // - In with a single value, key=value + // - Exists, key="" + // On a conflict with an existing key, priortize the value from MatchLabels + for _, expr := range pool.Spec.NodeSelector.MatchExpressions { + switch expr.Operator { + case metav1.LabelSelectorOpIn: + if len(expr.Values) == 1 { + if _, alreadySet := labelsToApply[expr.Key]; !alreadySet { + labelsToApply[expr.Key] = expr.Values[0] + } + } else { + klog.Infof("MachineConfigPool %s matchExpression key %q has multiple values, skipping", poolName, expr.Key) + } + case metav1.LabelSelectorOpExists: + if _, alreadySet := labelsToApply[expr.Key]; !alreadySet { + labelsToApply[expr.Key] = "" + } + } + } + if len(labelsToApply) == 0 { + klog.Infof("MachineConfigPool %s has no applicable node selector labels", poolName) return nil } - klog.Infof("Node %s was booted into custom pool %s; applying node selector labels: %v", poolName, node.Name, labelsToApply) + klog.Infof("Node %s was booted into custom pool %s; applying node selector labels: %v", node.Name, poolName, labelsToApply) // Apply the labels to the node and add annotation indicating custom pool labels were applied _, err = internal.UpdateNodeRetry(ctrl.kubeClient.CoreV1().Nodes(), ctrl.nodeLister, node.Name, func(node *corev1.Node) { + // Highly unlikely, but check to be safe + if node.Labels == nil { + node.Labels = map[string]string{} + } + if node.Annotations == nil { + node.Annotations = map[string]string{} + } // Apply the custom pool labels maps.Copy(node.Labels, labelsToApply) diff --git a/pkg/controller/node/node_controller_test.go b/pkg/controller/node/node_controller_test.go index d07ffe88b6..b7a2a722f9 100644 --- a/pkg/controller/node/node_controller_test.go +++ b/pkg/controller/node/node_controller_test.go @@ -2238,6 +2238,199 @@ func TestFilterCustomPoolBootedNodes(t *testing.T) { } } +func TestApplyCustomPoolLabels(t *testing.T) { + t.Parallel() + + baseNode := func(name string) *corev1.Node { + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{"node-role.kubernetes.io/worker": ""}, + Annotations: map[string]string{}, + }, + } + } + + pool := func(name string, sel *metav1.LabelSelector) *mcfgv1.MachineConfigPool { + return helpers.NewMachineConfigPoolBuilder(name).WithNodeSelector(sel).MachineConfigPool() + } + + tests := []struct { + name string + pool *mcfgv1.MachineConfigPool + poolName string + expectErr bool + expectedLabels map[string]string + expectAnnot bool // whether CustomPoolLabelsAppliedAnnotationKey should be set + }{ + { + name: "matchLabels only", + poolName: "infra", + pool: pool("infra", &metav1.LabelSelector{ + MatchLabels: map[string]string{"node-role.kubernetes.io/infra": ""}, + }), + expectedLabels: map[string]string{ + "node-role.kubernetes.io/worker": "", + "node-role.kubernetes.io/infra": "", + }, + expectAnnot: true, + }, + { + name: "matchExpressions In single value", + poolName: "infra", + pool: pool("infra", &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + {Key: "node-role.kubernetes.io/infra", Operator: metav1.LabelSelectorOpIn, Values: []string{""}}, + }, + }), + expectedLabels: map[string]string{ + "node-role.kubernetes.io/worker": "", + "node-role.kubernetes.io/infra": "", + }, + expectAnnot: true, + }, + { + name: "matchExpressions In multiple values skipped", + poolName: "infra", + pool: pool("infra", &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + {Key: "zone", Operator: metav1.LabelSelectorOpIn, Values: []string{"us-east-1a", "us-east-1b"}}, + }, + }), + // no new labels — ambiguous In is skipped, nothing to apply + expectedLabels: map[string]string{ + "node-role.kubernetes.io/worker": "", + }, + expectAnnot: false, + }, + { + name: "matchExpressions Exists", + poolName: "infra", + pool: pool("infra", &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + {Key: "node-role.kubernetes.io/infra", Operator: metav1.LabelSelectorOpExists}, + }, + }), + expectedLabels: map[string]string{ + "node-role.kubernetes.io/worker": "", + "node-role.kubernetes.io/infra": "", + }, + expectAnnot: true, + }, + { + name: "matchExpressions NotIn and DoesNotExist are skipped", + poolName: "infra", + pool: pool("infra", &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + {Key: "bad-key", Operator: metav1.LabelSelectorOpNotIn, Values: []string{"val"}}, + {Key: "another-key", Operator: metav1.LabelSelectorOpDoesNotExist}, + }, + }), + expectedLabels: map[string]string{ + "node-role.kubernetes.io/worker": "", + }, + expectAnnot: false, + }, + { + name: "matchLabels and matchExpressions both applied", + poolName: "infra", + pool: pool("infra", &metav1.LabelSelector{ + MatchLabels: map[string]string{"node-role.kubernetes.io/infra": ""}, + MatchExpressions: []metav1.LabelSelectorRequirement{ + {Key: "topology.kubernetes.io/zone", Operator: metav1.LabelSelectorOpIn, Values: []string{"us-east-1a"}}, + }, + }), + expectedLabels: map[string]string{ + "node-role.kubernetes.io/worker": "", + "node-role.kubernetes.io/infra": "", + "topology.kubernetes.io/zone": "us-east-1a", + }, + expectAnnot: true, + }, + { + name: "matchLabels wins over Exists for same key", + poolName: "infra", + pool: pool("infra", &metav1.LabelSelector{ + MatchLabels: map[string]string{"node-role.kubernetes.io/infra": "specific-value"}, + MatchExpressions: []metav1.LabelSelectorRequirement{ + {Key: "node-role.kubernetes.io/infra", Operator: metav1.LabelSelectorOpExists}, + }, + }), + expectedLabels: map[string]string{ + "node-role.kubernetes.io/worker": "", + "node-role.kubernetes.io/infra": "specific-value", + }, + expectAnnot: true, + }, + { + name: "matchLabels wins over In for same key", + poolName: "infra", + pool: pool("infra", &metav1.LabelSelector{ + MatchLabels: map[string]string{"node-role.kubernetes.io/infra": "from-matchlabels"}, + MatchExpressions: []metav1.LabelSelectorRequirement{ + {Key: "node-role.kubernetes.io/infra", Operator: metav1.LabelSelectorOpIn, Values: []string{"from-expression"}}, + }, + }), + expectedLabels: map[string]string{ + "node-role.kubernetes.io/worker": "", + "node-role.kubernetes.io/infra": "from-matchlabels", + }, + expectAnnot: true, + }, + { + name: "nil node selector returns without error", + poolName: "infra", + pool: pool("infra", nil), + expectedLabels: map[string]string{ + "node-role.kubernetes.io/worker": "", + }, + expectAnnot: false, + }, + { + name: "pool not found returns error", + poolName: "nonexistent", + pool: nil, + expectErr: true, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + f := newFixture(t) + node := baseNode("test-node") + + f.nodeLister = append(f.nodeLister, node) + f.kubeobjects = append(f.kubeobjects, node) + + if test.pool != nil { + f.mcpLister = append(f.mcpLister, test.pool) + f.objects = append(f.objects, test.pool) + } + + c := f.newController() + + err := c.applyCustomPoolLabels(node, test.poolName) + + if test.expectErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + updated, getErr := f.kubeclient.CoreV1().Nodes().Get(context.TODO(), node.Name, metav1.GetOptions{}) + require.NoError(t, getErr) + + assert.Equal(t, test.expectedLabels, updated.Labels) + + _, hasAnnot := updated.Annotations[daemonconsts.CustomPoolLabelsAppliedAnnotationKey] + assert.Equal(t, test.expectAnnot, hasAnnot, "CustomPoolLabelsAppliedAnnotationKey presence mismatch") + }) + } +} + // adds annotation to the node func addNodeAnnotations(node *corev1.Node, annotations map[string]string) { if node.Annotations == nil {