PWX-38103 Ignore unhealthy nodes if a healthy one is found

for the same k8s node. If one healthy node is found then there is a PX node
which is healthy on that k8s node. The unhealthy ones are present because they
were previously running on that k8s node before. Such nodes will either find a
new k8s node or will get auto-decommissioned in case of a storageless nodes.

Signed-off-by: Piyush Nimbalkar <[email protected]>

pkg/controller/storagecluster/controller_test.go

@@ -4449,7 +4449,12 @@ func TestUpdateStorageClusterBasedOnStorageNodeStatuses(t *testing.T) {
  var storageNodes []*storageapi.StorageNode
  storageNodes = append(storageNodes, createStorageNode("k8s-node-0", true))
  storageNodes = append(storageNodes, createStorageNode("k8s-node-1", true))
+ // Create duplicate storage nodes with unhealthy statuses and one with healthy status.
+ // We need to verify that the one with healthy status is considered when calculating
+ // what nodes to upgrade next.
+ storageNodes = append(storageNodes, createStorageNode("k8s-node-2", false))
  storageNodes = append(storageNodes, createStorageNode("k8s-node-2", true))
+ storageNodes = append(storageNodes, createStorageNode("k8s-node-2", false))
  storageNodes = append(storageNodes, createStorageNode("not-k8s-node", false))
 
  driver.EXPECT().Validate(gomock.Any()).Return(nil).AnyTimes()
@@ -4508,7 +4513,7 @@ func TestUpdateStorageClusterBasedOnStorageNodeStatuses(t *testing.T) {
  require.Empty(t, podControl.DeletePodName)
 
  // TestCase: Mark the unhealthy storage node to healthy, the update should begin.
- storageNodes[3].Status = storageapi.Status_STATUS_OK
+ storageNodes[5].Status = storageapi.Status_STATUS_OK
  result, err = controller.Reconcile(context.TODO(), request)
  require.NoError(t, err)
  require.Empty(t, result)

pkg/controller/storagecluster/update.go

@@ -661,7 +661,19 @@ func (c *Controller) getPodAvailability(
  if len(storageNode.SchedulerNodeName) == 0 {
  nonK8sStorageNodes = append(nonK8sStorageNodes, storageNode)
  } else {
- storageNodeMap[storageNode.SchedulerNodeName] = storageNode
+ if existing, ok := storageNodeMap[storageNode.SchedulerNodeName]; ok {
+ // During a k8s upgrade, it may happen that a new node is started on the same k8s host using
+ // a scheduler node name that was previously used by a different node. In this case, if we have
+ // a storage node that is healthy, then we can ignore the old node which is not healthy as it will
+ // either find a new k8s node or it will be a storageless node that will get auto-decommissioned.
+ // It is mostly not possible that a storageless node will replace a storage node on the same node.
+ // But if it does, then refer - https://purestorage.atlassian.net/browse/PWX-38505 for more details.
+ if existing.Status != storageapi.Status_STATUS_OK && storageNode.Status == storageapi.Status_STATUS_OK {
+ storageNodeMap[storageNode.SchedulerNodeName] = storageNode
+ }
+ } else {
+ storageNodeMap[storageNode.SchedulerNodeName] = storageNode
+ }
  }
  }
  }