From 1819921111a6e36ac42c5b89b151bf2a2c802460 Mon Sep 17 00:00:00 2001 From: Kornilios Kourtis Date: Thu, 27 Aug 2020 16:06:53 +0200 Subject: [PATCH] daemon: properly maintain node lists on updates [ upstream commit 5550c0f3f2206d05f3ef3af569ab756cbba94fae ] 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 Signed-off-by: Alexandre Perrin --- daemon/status.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/daemon/status.go b/daemon/status.go index 171bac21a486..27e093db80e7 100644 --- a/daemon/status.go +++ b/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 }