daemon: properly maintain node lists on updates

[ upstream commit 5550c0f ]

NodeAdd and NodeUpdate update the node state for clients so that they
can return the changes when client requests so. If a node was added and
then updated, its old and new version would be on the added list and its
old on the removed list. Instead, we can just update the node on the
added list.

Note that the setNodes() function on pkg/health/server/prober.go first
deletes the removed nodes and then adds the new ones, which means that
the old version of the node would be added and remain as stale on the
health server.

This was found during investigation of issues with inconsistent health
reports when nodes are added/removed from the cluster (e.g., #11532),
and it seems to fix inconsistencies observed a small-scale test I did to
reproduce the issue.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

daemon/status.go

@@ -228,9 +228,19 @@ func (c *clusterNodesClient) NodeAdd(newNode node.Node) error {
 
 func (c *clusterNodesClient) NodeUpdate(oldNode, newNode node.Node) error {
 	c.Lock()
+	defer c.Unlock()
+
+	// If the node is on the added list, just update it
+	for i, added := range c.NodesAdded {
+		if added.Name == newNode.Fullname() {
+			c.NodesAdded[i] = newNode.GetModel()
+			return nil
+		}
+	}
+
+	// otherwise, add the new node and remove the old one
 	c.NodesAdded = append(c.NodesAdded, newNode.GetModel())
 	c.NodesRemoved = append(c.NodesRemoved, oldNode.GetModel())
-	c.Unlock()
 	return nil
 }