controller: update is node done check to handle image mode disabling case

Author: isabella-janssen

pkg/controller/common/layered_node_state.go

@@ -26,14 +26,16 @@ func NewLayeredNodeState(n *corev1.Node) *LayeredNodeState {
 	return &LayeredNodeState{node: n}
 }
 
-// Checks if the node is done "working" and that the node is targeting is MachineConfig
-// determined by the pool. If in layered mode, the image annotations are also checked
-// checked against the OCL objects.
+// Checks if the node is done "working." For a node in both layered and non-layered MCPs, the
+// node's state annotation must be "Done" and it must be targeting the correct MachineConfig. For
+// `layered` MCPs, the node's desired image annotation must match the image defined in the
+// MachineOsConfig and the desired MachineConfig must match the config version defined in the
+// MachineOSBuild. For non-layered MCPs, the node must not have a desired image annotation value.
 func (l *LayeredNodeState) IsDone(mcp *mcfgv1.MachineConfigPool, layered bool, mosc *mcfgv1.MachineOSConfig, mosb *mcfgv1.MachineOSBuild) bool {
 	if layered {
 		return l.IsNodeDone() && l.IsDesiredMachineConfigEqualToPool(mcp) && l.IsDesiredEqualToBuild(mosc, mosb)
 	}
-	return l.IsNodeDone() && l.IsDesiredMachineConfigEqualToPool(mcp)
+	return l.IsNodeDone() && l.IsDesiredMachineConfigEqualToPool(mcp) && !l.IsDesiredImageAnnotationPresentOnNode()
 }
 
 // The original behavior of getUnavailableMachines is: getUnavailableMachines

pkg/controller/node/status_test.go

@@ -287,6 +287,28 @@ func TestGetUpdatedMachines(t *testing.T) {
 			newNode("node-1", machineConfigV1, machineConfigV1),
 			newNodeWithReady("node-2", machineConfigV1, machineConfigV1, corev1.ConditionTrue),
 		},
+	}, {
+		name: "Pool with image mode disabling, one node updating, 2 nodes not not acted upon",
+		nodes: []*corev1.Node{
+			newLayeredNode("node-0", machineConfigV0, machineConfigV0, imageV1, ""),
+			newLayeredNode("node-1", machineConfigV0, machineConfigV0, imageV1, imageV1),
+			newLayeredNode("node-2", machineConfigV0, machineConfigV0, imageV1, imageV1),
+		},
+		currentConfig: machineConfigV0,
+		updated:       nil,
+		layered:       false,
+	}, {
+		name: "Pool with image mode disabling, one node updated, 1 node updating, 1 node not acted upon",
+		nodes: []*corev1.Node{
+			newNode("node-0", machineConfigV0, machineConfigV0),
+			newLayeredNode("node-1", machineConfigV0, machineConfigV0, imageV1, ""),
+			newLayeredNode("node-2", machineConfigV0, machineConfigV0, imageV1, imageV1),
+		},
+		currentConfig: machineConfigV0,
+		updated: []*corev1.Node{
+			newNode("node-0", machineConfigV0, machineConfigV0),
+		},
+		layered: false,
 	},
 	}