Add node registration label

pkg/apis/crds/karpenter.sh_machines.yaml

@@ -3,8 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.11.3
-  creationTimestamp: null
+    controller-gen.kubebuilder.io/version: v0.12.0
   name: machines.karpenter.sh
 spec:
   group: karpenter.sh

pkg/apis/crds/karpenter.sh_provisioners.yaml

@@ -3,8 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.11.3
-  creationTimestamp: null
+    controller-gen.kubebuilder.io/version: v0.12.0
   name: provisioners.karpenter.sh
 spec:
   group: karpenter.sh

pkg/apis/v1alpha5/labels.go

@@ -34,6 +34,7 @@ const (
 const (
 	ProvisionerNameLabelKey = Group + "/provisioner-name"
 	LabelNodeInitialized    = Group + "/initialized"
+	LabelNodeRegistered     = Group + "/registered"
 	LabelCapacityType       = Group + "/capacity-type"
 )
 

pkg/apis/v1alpha5/provisioner_validation.go

@@ -104,7 +104,7 @@ func (s *ProvisionerSpec) validateTTLSecondsAfterEmpty() (errs *apis.FieldError)
 }
 
 // Validate the constraints
-func (s *ProvisionerSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
+func (s *ProvisionerSpec) Validate(_ context.Context) (errs *apis.FieldError) {
 	return errs.Also(
 		s.validateProvider(),
 		s.validateLabels(),

pkg/controllers/consistency/nodeshape.go

@@ -35,7 +35,7 @@ func NewNodeShape(provider cloudprovider.CloudProvider) Check {
 	}
 }
 
-func (n *NodeShape) Check(ctx context.Context, node *v1.Node, machine *v1alpha5.Machine) ([]Issue, error) {
+func (n *NodeShape) Check(_ context.Context, node *v1.Node, machine *v1alpha5.Machine) ([]Issue, error) {
 	// ignore machines that are deleting
 	if !machine.DeletionTimestamp.IsZero() {
 		return nil, nil

pkg/controllers/deprovisioning/expiration.go

@@ -58,7 +58,7 @@ func NewExpiration(clk clock.Clock, kubeClient client.Client, cluster *state.Clu
 }
 
 // ShouldDeprovision is a predicate used to filter deprovisionable nodes
-func (e *Expiration) ShouldDeprovision(ctx context.Context, c *Candidate) bool {
+func (e *Expiration) ShouldDeprovision(_ context.Context, c *Candidate) bool {
 	// Filter out nodes without the TTL defined or expired.
 	if c.provisioner == nil || c.provisioner.Spec.TTLSecondsUntilExpired == nil {
 		return false

pkg/controllers/machine/lifecycle/registration.go

@@ -41,7 +41,8 @@ type Registration struct {
 
 func (r *Registration) Reconcile(ctx context.Context, machine *v1alpha5.Machine) (reconcile.Result, error) {
 	if machine.StatusConditions().GetCondition(v1alpha5.MachineRegistered).IsTrue() {
-		return reconcile.Result{}, nil
+		// TODO @joinnis: Remove the back-propagation of this label onto the Node once all Nodes are guaranteed to have this label
+		return reconcile.Result{}, r.backPropagateRegistrationLabel(ctx, machine)
 	}
 	if !machine.StatusConditions().GetCondition(v1alpha5.MachineLaunched).IsTrue() {
 		machine.StatusConditions().MarkFalse(v1alpha5.MachineRegistered, "MachineNotLaunched", "Machine is not launched")
@@ -105,10 +106,32 @@ func (r *Registration) syncNode(ctx context.Context, machine *v1alpha5.Machine,
 		node.Spec.Taints = scheduling.Taints(node.Spec.Taints).Merge(machine.Spec.Taints)
 		node.Spec.Taints = scheduling.Taints(node.Spec.Taints).Merge(machine.Spec.StartupTaints)
 	}
+	node.Labels = lo.Assign(node.Labels, map[string]string{
+		v1alpha5.LabelNodeRegistered: "true",
+	})
 	if !equality.Semantic.DeepEqual(stored, node) {
 		if err := r.kubeClient.Patch(ctx, node, client.MergeFrom(stored)); err != nil {
 			return fmt.Errorf("syncing node labels, %w", err)
 		}
 	}
 	return nil
 }
+
+// backPropagateRegistrationLabel ports the `karpenter.sh/registered` label onto nodes that are registered by the Machine
+// but don't have this label on the Node yet
+func (r *Registration) backPropagateRegistrationLabel(ctx context.Context, machine *v1alpha5.Machine) error {
+	node, err := machineutil.NodeForMachine(ctx, r.kubeClient, machine)
+	stored := node.DeepCopy()
+	if err != nil {
+		return machineutil.IgnoreDuplicateNodeError(machineutil.IgnoreNodeNotFoundError(err))
+	}
+	node.Labels = lo.Assign(node.Labels, map[string]string{
+		v1alpha5.LabelNodeRegistered: "true",
+	})
+	if !equality.Semantic.DeepEqual(stored, node) {
+		if err := r.kubeClient.Patch(ctx, node, client.MergeFrom(stored)); err != nil {
+			return fmt.Errorf("syncing node registration label, %w", err)
+		}
+	}
+	return nil
+}

pkg/controllers/machine/lifecycle/registration_test.go

@@ -72,6 +72,43 @@ var _ = Describe("Registration", func() {
 		node = ExpectExists(ctx, env.Client, node)
 		ExpectOwnerReferenceExists(node, machine)
 	})
+	It("should sync the karpenter.sh/registered label to the Node when the Node comes online", func() {
+		machine := test.Machine(v1alpha5.Machine{
+			ObjectMeta: metav1.ObjectMeta{
+				Labels: map[string]string{
+					v1alpha5.ProvisionerNameLabelKey: provisioner.Name,
+				},
+			},
+		})
+		ExpectApplied(ctx, env.Client, provisioner, machine)
+		ExpectReconcileSucceeded(ctx, machineController, client.ObjectKeyFromObject(machine))
+		machine = ExpectExists(ctx, env.Client, machine)
+
+		node := test.Node(test.NodeOptions{ProviderID: machine.Status.ProviderID})
+		ExpectApplied(ctx, env.Client, node)
+		ExpectReconcileSucceeded(ctx, machineController, client.ObjectKeyFromObject(machine))
+		node = ExpectExists(ctx, env.Client, node)
+		Expect(node.Labels).To(HaveKeyWithValue(v1alpha5.LabelNodeRegistered, "true"))
+	})
+	It("should sync the karpenter.sh/registered label to the Node if the Machine already registered", func() {
+		machine := test.Machine(v1alpha5.Machine{
+			ObjectMeta: metav1.ObjectMeta{
+				Labels: map[string]string{
+					v1alpha5.ProvisionerNameLabelKey: provisioner.Name,
+				},
+			},
+		})
+		machine.StatusConditions().MarkTrue(v1alpha5.MachineLaunched)
+		machine.StatusConditions().MarkTrue(v1alpha5.MachineRegistered)
+		machine.StatusConditions().MarkTrue(v1alpha5.MachineInitialized)
+
+		node := test.Node(test.NodeOptions{ProviderID: machine.Status.ProviderID})
+		ExpectApplied(ctx, env.Client, machine, node)
+
+		ExpectReconcileSucceeded(ctx, machineController, client.ObjectKeyFromObject(machine))
+		node = ExpectExists(ctx, env.Client, node)
+		Expect(node.Labels).To(HaveKeyWithValue(v1alpha5.LabelNodeRegistered, "true"))
+	})
 	It("should sync the labels to the Node when the Node comes online", func() {
 		machine := test.Machine(v1alpha5.Machine{
 			ObjectMeta: metav1.ObjectMeta{

pkg/controllers/node/drift.go

@@ -36,7 +36,7 @@ type Drift struct {
 	cloudProvider cloudprovider.CloudProvider
 }
 
-func (d *Drift) Reconcile(ctx context.Context, provisioner *v1alpha5.Provisioner, node *v1.Node) (reconcile.Result, error) {
+func (d *Drift) Reconcile(ctx context.Context, _ *v1alpha5.Provisioner, node *v1.Node) (reconcile.Result, error) {
 	// node is not ready yet, so we don't consider it to be drifted
 	if node.Labels[v1alpha5.LabelNodeInitialized] != "true" {
 		return reconcile.Result{}, nil

pkg/controllers/provisioning/scheduling/suite_test.go

@@ -1990,6 +1990,7 @@ var _ = Describe("In-Flight Nodes", func() {
 			machine1 := bindings.Get(initialPod).Machine
 			node1 := bindings.Get(initialPod).Node
 			machine1.StatusConditions().MarkTrue(v1alpha5.MachineInitialized)
+			node1.Labels = lo.Assign(node1.Labels, map[string]string{v1alpha5.LabelNodeInitialized: "true"})
 
 			// delete the pod so that the node is empty
 			ExpectDeleted(ctx, env.Client, initialPod)
@@ -2057,6 +2058,8 @@ var _ = Describe("In-Flight Nodes", func() {
 			machine1 := bindings.Get(initialPod).Machine
 			node1 := bindings.Get(initialPod).Node
 			machine1.StatusConditions().MarkTrue(v1alpha5.MachineInitialized)
+			node1.Labels = lo.Assign(node1.Labels, map[string]string{v1alpha5.LabelNodeInitialized: "true"})
+
 			node1.Spec.Taints = []v1.Taint{startupTaint}
 			node1.Status.Capacity = v1.ResourceList{v1.ResourcePods: resource.MustParse("10")}
 			ExpectApplied(ctx, env.Client, machine1, node1)

pkg/controllers/state/statenode.go

@@ -114,7 +114,8 @@ func (in *StateNode) Name() string {
 	if in.Machine == nil {
 		return in.Node.Name
 	}
-	if !in.Machine.StatusConditions().GetCondition(v1alpha5.MachineRegistered).IsTrue() {
+	// TODO @joinnis: The !in.Initialized() check can be removed when we can assume that all nodes have the v1alpha5.NodeRegisteredLabel on them
+	if !in.Registered() && !in.Initialized() {
 		return in.Machine.Name
 	}
 	return in.Node.Name
@@ -144,22 +145,24 @@ func (in *StateNode) Annotations() map[string]string {
 	if in.Machine == nil {
 		return in.Node.Annotations
 	}
-	if !in.Machine.StatusConditions().GetCondition(v1alpha5.MachineRegistered).IsTrue() {
+	// TODO @joinnis: The !in.Initialized() check can be removed when we can assume that all nodes have the v1alpha5.NodeRegisteredLabel on them
+	if !in.Registered() && !in.Initialized() {
 		return in.Machine.Annotations
 	}
 	return in.Node.Annotations
 }
 
 func (in *StateNode) Labels() map[string]string {
-	// If the machine exists and the state node isn't initialized
+	// If the machine exists and the state node isn't registered
 	// use the machine representation of the labels
 	if in.Node == nil {
 		return in.Machine.Labels
 	}
 	if in.Machine == nil {
 		return in.Node.Labels
 	}
-	if !in.Machine.StatusConditions().GetCondition(v1alpha5.MachineRegistered).IsTrue() {
+	// TODO @joinnis: The !in.Initialized() check can be removed when we can assume that all nodes have the v1alpha5.NodeRegisteredLabel on them
+	if !in.Registered() && !in.Initialized() {
 		return in.Machine.Labels
 	}
 	return in.Node.Labels
@@ -179,7 +182,8 @@ func (in *StateNode) Taints() []v1.Taint {
 	}
 
 	var taints []v1.Taint
-	if !in.Initialized() && in.Machine != nil {
+	// TODO @joinnis: The !in.Initialized() check can be removed when we can assume that all nodes have the v1alpha5.NodeRegisteredLabel on them
+	if !in.Registered() && !in.Initialized() && in.Machine != nil {
 		taints = in.Machine.Spec.Taints
 	} else {
 		taints = in.Node.Spec.Taints
@@ -192,13 +196,14 @@ func (in *StateNode) Taints() []v1.Taint {
 	})
 }
 
-func (in *StateNode) Initialized() bool {
-	if in.Machine != nil {
-		if in.Node != nil && in.Machine.StatusConditions().GetCondition(v1alpha5.MachineInitialized).IsTrue() {
-			return true
-		}
-		return false
+func (in *StateNode) Registered() bool {
+	if in.Node != nil {
+		return in.Node.Labels[v1alpha5.LabelNodeRegistered] == "true"
 	}
+	return false
+}
+
+func (in *StateNode) Initialized() bool {
 	if in.Node != nil {
 		return in.Node.Labels[v1alpha5.LabelNodeInitialized] == "true"
 	}

pkg/controllers/state/suite_test.go

@@ -534,7 +534,9 @@ var _ = Describe("Inflight Nodes", func() {
 		ExpectReconcileSucceeded(ctx, nodeController, client.ObjectKeyFromObject(node))
 
 		ExpectMakeMachinesInitialized(ctx, env.Client, machine)
+		ExpectMakeNodesInitialized(ctx, env.Client, node)
 		ExpectReconcileSucceeded(ctx, machineController, client.ObjectKeyFromObject(machine))
+		ExpectReconcileSucceeded(ctx, nodeController, client.ObjectKeyFromObject(node))
 
 		ExpectStateNodeCount("==", 1)
 		stateNode := ExpectStateNodeExists(node)

pkg/test/expectations/expectations.go

@@ -336,6 +336,7 @@ func ExpectMachineDeployedWithOffset(offset int, ctx context.Context, c client.C
 
 	// Mock the machine launch and node joining at the apiserver
 	node := test.MachineLinkedNode(m)
+	node.Labels = lo.Assign(node.Labels, map[string]string{v1alpha5.LabelNodeRegistered: "true"})
 	ExpectAppliedWithOffset(offset+1, ctx, c, m, node)
 	ExpectWithOffset(offset+1, cluster.UpdateNode(ctx, node)).To(Succeed())
 	cluster.UpdateMachine(m)
@@ -381,6 +382,7 @@ func ExpectMakeNodesInitializedWithOffset(offset int, ctx context.Context, c cli
 	ExpectMakeNodesReadyWithOffset(offset+1, ctx, c, nodes...)
 
 	for i := range nodes {
+		nodes[i].Labels[v1alpha5.LabelNodeRegistered] = "true"
 		nodes[i].Labels[v1alpha5.LabelNodeInitialized] = "true"
 		ExpectAppliedWithOffset(offset+1, ctx, c, nodes[i])
 	}

pkg/webhooks/webhooks.go

@@ -38,7 +38,7 @@ func NewWebhooks() []knativeinjection.ControllerConstructor {
 	}
 }
 
-func NewCRDValidationWebhook(ctx context.Context, w configmap.Watcher) *controller.Impl {
+func NewCRDValidationWebhook(ctx context.Context, _ configmap.Watcher) *controller.Impl {
 	return validation.NewAdmissionController(ctx,
 		"validation.webhook.karpenter.sh",
 		"/validate/karpenter.sh",
@@ -48,7 +48,7 @@ func NewCRDValidationWebhook(ctx context.Context, w configmap.Watcher) *controll
 	)
 }
 
-func NewConfigValidationWebhook(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
+func NewConfigValidationWebhook(ctx context.Context, _ configmap.Watcher) *controller.Impl {
 	return configmaps.NewAdmissionController(ctx,
 		"validation.webhook.config.karpenter.sh",
 		"/validate/config.karpenter.sh",