[Node] Log that an unhealthy static node is not protected from replacement

[hehe7318]

Description of changes

A static node that clustermgtd is replacing is shielded from being replaced again while it bootstraps (the replacement protection window). When that protection did not apply, the logs never said so: they showed the node being terminated and relaunched with no indication that it had lost, or never had, the grace period. This is what made the maintenance reservation issue hard to diagnose, and the same gap affects every other reason a node can end up unprotected.

• When a DOWN or DRAINED static node is found unhealthy, the existing warning now also reports that the node is not within the replacement protection window and that it will be replaced. This is the point where the decision is made, and it holds for every reason the window may not apply.
• The log listing the unhealthy static nodes now includes the node states, so the state that led to the replacement is visible without correlating against scontrol output.

The node state carries the reason, so the logging does not assume a specific cause and stays accurate if the conditions change.

This also fixes the format string of the debug log covering the opposite case (node still protected), which was missing a placeholder and raised TypeError: not all arguments converted during string formatting whenever it was emitted.

Example

A node being replaced fails its EC2 health check while bootstrapping, so it is removed from the replacement set and replaced again immediately:

Detected failed health check for static node in replacement. No longer considering nodes ['queue1-st-cr1-1'] as in replacement process. Will attempt to replace node again immediately.
Node state check: node queue1-st-cr1-1(192.168.104.165) in DOWN and not within the replacement protection window, it is considered unhealthy and will be replaced, node state: DOWN+CLOUD   <-- reason and consequence added
Found the following unhealthy static nodes: (x1) ['queue1-st-cr1-1(192.168.104.165) in state DOWN+CLOUD']   <-- node states added
Setting unhealthy static nodes to DOWN
Terminating instances backing unhealthy static nodes
Terminating instances (x1) ['i-021cb4b7fa1050c13']
Launching new instances for unhealthy static nodes

A DRAINED node reads the same way:

Node state check: node queue1-st-cr1-1(192.168.104.165) in DRAINED and not within the replacement protection window, it is considered unhealthy and will be replaced, node state: IDLE+CLOUD+DRAIN

While a node is inside the protection window it is reported as healthy and only a debug line is emitted, so the warning above only shows up for nodes that are not shielded.

Tests

• Unit tests passed, including new cases asserting the unhealthy-state warning reports the missing protection window for DOWN and DRAINED nodes, and that no warning is emitted while the node is still protected.
• Verified on a real cluster with this branch installed via DevSettings/NodePackage.

References

• Follow-up to the COE action item for clearer termination logging.

Checklist

• Make sure you are pointing to the right branch.
• If you're creating a patch for a branch other than develop add the branch name as prefix in the PR title (e.g. [release-3.6]).
• Check all commits' messages are clear, describing what and why vs how.
• Make sure to have added unit tests or integration tests to cover the new/modified code.
• Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[gmarciani]

This array can potentially contain thousands of entries. Are we sure the logger would be able to handle?

[hehe7318]

I checked whether the logger actually has a problem with the full list.

It does not: clustermgtd logs through a plain StreamHandler to stdout, and supervisord writes that to /var/log/parallelcluster/clustermgtd with stdout_logfile_maxbytes = 0, i.e. no size cap and no rotation on that stream. There is no line-length limit anywhere in that path.

More to the point, the surrounding logs already print full lists of the same size — Found the following unhealthy static nodes listed every node before this PR, and delete_instances logs every instance id. So this does not introduce a new order of magnitude.

I also moved the states onto the existing Found the following unhealthy static nodes line instead of adding a second duplicated line about the same set of nodes, so the node names and their states are reported together and Terminating instances backing unhealthy static nodes is left untouched:

Found the following unhealthy static nodes: (x1) ['queue1-st-cr1-1(192.168.104.165) in state DOWN+CLOUD']