Fix recovery of unknow state for NodeMetadataUpToData condition

Signed-off-by: Danil Grigorev <danil.grigorev@suse.com>
rancher · Feb 16, 2024 · 10f58f4 · 10f58f4
1 parent f5879b1
commit 10f58f4
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 10 deletions.
diff --git a/pkg/rke2/workload_cluster.go b/pkg/rke2/workload_cluster.go
@@ -168,12 +168,6 @@ func (w *Workload) PatchNodes(ctx context.Context, cp *ControlPlane) error {
 				errList = append(errList, errors.Wrapf(err, "failed to patch node %s", node.Name))
 			}
 
-			if !conditions.Has(machine, controlplanev1.NodeMetadataUpToDate) {
-				conditions.MarkTrue(
-					machine,
-					controlplanev1.NodeMetadataUpToDate)
-			}
-
 			continue
 		}
 
@@ -482,6 +476,8 @@ func (w *Workload) UpdateNodeMetadata(ctx context.Context, controlPlane *Control
 			nodeName = machine.Status.NodeRef.Name
 		}
 
+		conditions.MarkTrue(machine, controlplanev1.NodeMetadataUpToDate)
+
 		node, nodeFound := w.Nodes[nodeName]
 		if !nodeFound {
 			conditions.MarkUnknown(

diff --git a/pkg/rke2/workload_cluster_test.go b/pkg/rke2/workload_cluster_test.go
@@ -120,8 +120,8 @@ var _ = Describe("Node metadata propagation", func() {
 
 		w := NewWorkload(testEnv.GetClient())
 		cp, err := NewControlPlane(ctx, testEnv.GetClient(), nil, nil, machines)
-		w.InitWorkload(ctx, cp)
 		Expect(err).ToNot(HaveOccurred())
+		Expect(w.InitWorkload(ctx, cp)).ToNot(HaveOccurred())
 		Expect(w.UpdateNodeMetadata(ctx, cp)).ToNot(HaveOccurred())
 		Expect(w.Nodes).To(HaveLen(0))
 		Expect(cp.rke2Configs).To(HaveLen(1))
@@ -144,8 +144,8 @@ var _ = Describe("Node metadata propagation", func() {
 
 		w := NewWorkload(testEnv.GetClient())
 		cp, err := NewControlPlane(ctx, testEnv.GetClient(), nil, nil, machines)
-		w.InitWorkload(ctx, cp)
 		Expect(err).ToNot(HaveOccurred())
+		Expect(w.InitWorkload(ctx, cp)).ToNot(HaveOccurred())
 		Expect(w.UpdateNodeMetadata(ctx, cp)).ToNot(HaveOccurred())
 		Expect(w.Nodes).To(HaveLen(1))
 		Expect(cp.rke2Configs).To(HaveLen(0))
@@ -158,6 +158,46 @@ var _ = Describe("Node metadata propagation", func() {
 		))
 	})
 
+	It("should recover from error conditions on successfull node patch", func() {
+		Expect(testEnv.Create(ctx, config)).To(Succeed())
+		Expect(testEnv.Create(ctx, machine)).To(Succeed())
+
+		machines := collections.FromMachineList(&clusterv1.MachineList{Items: []clusterv1.Machine{
+			*machine,
+		}})
+
+		w := NewWorkload(testEnv.GetClient())
+		cp, err := NewControlPlane(ctx, testEnv.GetClient(), nil, nil, machines)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(w.InitWorkload(ctx, cp)).ToNot(HaveOccurred())
+		Expect(w.UpdateNodeMetadata(ctx, cp)).ToNot(HaveOccurred())
+		Expect(w.Nodes).To(HaveLen(0))
+		Expect(cp.rke2Configs).To(HaveLen(1))
+		Expect(cp.Machines).To(HaveLen(1))
+		Expect(conditions.Get(cp.Machines[machine.Name], controlplanev1.NodeMetadataUpToDate)).To(HaveField(
+			"Status", Equal(corev1.ConditionUnknown),
+		))
+		Expect(conditions.Get(cp.Machines[machine.Name], controlplanev1.NodeMetadataUpToDate)).To(HaveField(
+			"Message", Equal("associated node not found"),
+		))
+
+		Expect(testEnv.Create(ctx, node)).To(Succeed())
+		Eventually(ctx, func() map[string]*corev1.Node {
+			Expect(w.InitWorkload(ctx, cp)).ToNot(HaveOccurred())
+			return w.Nodes
+		}).Should(HaveLen(1))
+		Expect(w.UpdateNodeMetadata(ctx, cp)).ToNot(HaveOccurred())
+		Expect(cp.rke2Configs).To(HaveLen(1))
+		Expect(cp.Machines).To(HaveLen(1))
+		Expect(conditions.Get(cp.Machines[machine.Name], controlplanev1.NodeMetadataUpToDate)).To(HaveField(
+			"Status", Equal(corev1.ConditionTrue),
+		))
+		Expect(w.Nodes[nodeName].GetAnnotations()).To(Equal(map[string]string{
+			"test":                      "true",
+			clusterv1.MachineAnnotation: nodeName,
+		}))
+	})
+
 	It("should set the node annotations", func() {
 		Expect(testEnv.Create(ctx, node)).To(Succeed())
 		Expect(testEnv.Create(ctx, config)).To(Succeed())
@@ -169,7 +209,7 @@ var _ = Describe("Node metadata propagation", func() {
 
 		w := NewWorkload(testEnv.GetClient())
 		cp, err := NewControlPlane(ctx, testEnv.GetClient(), nil, nil, machines)
-		w.InitWorkload(ctx, cp)
+		Expect(w.InitWorkload(ctx, cp)).ToNot(HaveOccurred())
 		Expect(err).ToNot(HaveOccurred())
 		Expect(w.UpdateNodeMetadata(ctx, cp)).ToNot(HaveOccurred())
 		Expect(w.Nodes).To(HaveLen(1))
@@ -207,12 +247,66 @@ var _ = Describe("Node metadata propagation", func() {
 
 		w := NewWorkload(testEnv.GetClient())
 		cp, err := NewControlPlane(ctx, testEnv.GetClient(), nil, nil, machines)
-		w.InitWorkload(ctx, cp)
 		Expect(err).ToNot(HaveOccurred())
+		Expect(w.InitWorkload(ctx, cp)).ToNot(HaveOccurred())
+		Expect(w.UpdateNodeMetadata(ctx, cp)).ToNot(HaveOccurred())
+		Expect(w.Nodes).To(HaveLen(1))
+		Expect(cp.rke2Configs).To(HaveLen(1))
+		Expect(cp.Machines).To(HaveLen(1))
+		Expect(conditions.Get(cp.Machines[machineDifferentNode.Name], controlplanev1.NodeMetadataUpToDate)).To(HaveField(
+			"Status", Equal(corev1.ConditionTrue),
+		))
+		Expect(w.Nodes[nodeName].GetAnnotations()).To(Equal(map[string]string{
+			"test":                      "true",
+			clusterv1.MachineAnnotation: machineDifferentNode.Name,
+		}))
+
+		result := &corev1.Node{}
+		Expect(testEnv.Get(ctx, client.ObjectKeyFromObject(node), result)).To(Succeed())
+		Expect(result.GetAnnotations()).To(Equal(map[string]string{
+			"test":                      "true",
+			clusterv1.MachineAnnotation: machineDifferentNode.Name,
+		}))
+	})
+
+	It("should recover from error condition on successfull node patch for arbitrary node name", func() {
+		node.SetAnnotations(map[string]string{
+			clusterv1.MachineAnnotation: machineDifferentNode.Name,
+		})
+		Expect(testEnv.Create(ctx, node)).To(Succeed())
+		Expect(testEnv.Create(ctx, config)).To(Succeed())
+		Expect(testEnv.Create(ctx, machineDifferentNode)).To(Succeed())
+
+		machines := collections.FromMachineList(&clusterv1.MachineList{Items: []clusterv1.Machine{
+			*machineDifferentNode,
+		}})
+
+		w := NewWorkload(testEnv.GetClient())
+		cp, err := NewControlPlane(ctx, testEnv.GetClient(), nil, nil, machines)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(w.InitWorkload(ctx, cp)).ToNot(HaveOccurred())
 		Expect(w.UpdateNodeMetadata(ctx, cp)).ToNot(HaveOccurred())
 		Expect(w.Nodes).To(HaveLen(1))
 		Expect(cp.rke2Configs).To(HaveLen(1))
 		Expect(cp.Machines).To(HaveLen(1))
+		Expect(conditions.Get(cp.Machines[machineDifferentNode.Name], controlplanev1.NodeMetadataUpToDate)).To(HaveField(
+			"Status", Equal(corev1.ConditionUnknown),
+		))
+		Expect(conditions.Get(cp.Machines[machineDifferentNode.Name], controlplanev1.NodeMetadataUpToDate)).To(HaveField(
+			"Message", Equal("associated node not found"),
+		))
+
+		machineDifferentNode.Status = machineNodeRefStatus
+		Expect(testEnv.Status().Update(ctx, machineDifferentNode)).To(Succeed())
+
+		machines = collections.FromMachineList(&clusterv1.MachineList{Items: []clusterv1.Machine{
+			*machineDifferentNode,
+		}})
+		cp, err = NewControlPlane(ctx, testEnv.GetClient(), nil, nil, machines)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(w.InitWorkload(ctx, cp)).ToNot(HaveOccurred())
+		Expect(w.UpdateNodeMetadata(ctx, cp)).ToNot(HaveOccurred())
+
 		Expect(conditions.Get(cp.Machines[machineDifferentNode.Name], controlplanev1.NodeMetadataUpToDate)).To(HaveField(
 			"Status", Equal(corev1.ConditionTrue),
 		))
@@ -228,6 +322,7 @@ var _ = Describe("Node metadata propagation", func() {
 			clusterv1.MachineAnnotation: machineDifferentNode.Name,
 		}))
 	})
+
 })
 
 var _ = Describe("Cloud-init fields validation", func() {