Skip to content

Commit

Permalink
Add node registration label
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathan-innis committed Jul 14, 2023
1 parent c8eec72 commit a51b3c3
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 12 deletions.
1 change: 1 addition & 0 deletions pkg/apis/v1alpha5/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const (
const (
ProvisionerNameLabelKey = Group + "/provisioner-name"
LabelNodeInitialized = Group + "/initialized"
LabelNodeRegistered = Group + "/registered"
LabelCapacityType = Group + "/capacity-type"
)

Expand Down
26 changes: 25 additions & 1 deletion pkg/controllers/machine/lifecycle/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ 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
// We can assume that all nodes will have this label and no back-propagation will be required once we hit v1
return reconcile.Result{}, r.backPropagateRegistrationLabel(ctx, machine)
}
if !machine.StatusConditions().GetCondition(v1alpha5.MachineLaunched).IsTrue() {
machine.StatusConditions().MarkFalse(v1alpha5.MachineRegistered, "MachineNotLaunched", "Machine is not launched")
Expand Down Expand Up @@ -105,10 +107,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
}
37 changes: 37 additions & 0 deletions pkg/controllers/machine/lifecycle/registration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
3 changes: 3 additions & 0 deletions pkg/controllers/provisioning/scheduling/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
31 changes: 20 additions & 11 deletions pkg/controllers/state/statenode.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ 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
// We can assume that all nodes will have this label and no back-propagation will be required once we hit v1
if !in.Registered() && !in.Initialized() {
return in.Machine.Name
}
return in.Node.Name
Expand Down Expand Up @@ -144,22 +146,26 @@ 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
// We can assume that all nodes will have this label and no back-propagation will be required once we hit v1
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
// We can assume that all nodes will have this label and no back-propagation will be required once we hit v1
if !in.Registered() && !in.Initialized() {
return in.Machine.Labels
}
return in.Node.Labels
Expand All @@ -179,7 +185,9 @@ 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
// We can assume that all nodes will have this label and no back-propagation will be required once we hit v1
if !in.Registered() && !in.Initialized() && in.Machine != nil {
taints = in.Machine.Spec.Taints
} else {
taints = in.Node.Spec.Taints
Expand All @@ -192,13 +200,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"
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/controllers/state/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions pkg/test/expectations/expectations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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])
}
Expand Down

0 comments on commit a51b3c3

Please sign in to comment.