[clustermgtd] Fix an issue where the Slurm node Comment and Extra fields could alter the node information read by ParallelCluster.

[gmarciani]

Description of changes

Fix an issue where the Slurm node Comment and Extra fields could alter the node information read by ParallelCluster.

FAQ

1. This fix does not cover the case where Comment contains multiline strings. The only way to cover it is moving to scontrol show --json, which is not an option due to memory and latency issues at scale.

Reproducer

Node is healthy idle

[root@ip-27-6-44-30 ~]# scontrol show nodes q1-st-cr1-1
NodeName=q1-st-cr1-1 Arch=x86_64 CoresPerSocket=1 
   CPUAlloc=0 CPUEfctv=2 CPUTot=2 CPULoad=0.08
   AvailableFeatures=static,t3.medium,cr1
   ActiveFeatures=static,t3.medium,cr1
   Gres=(null)
   NodeAddr=27.6.85.112 NodeHostName=q1-st-cr1-1 Version=25.11.8
   OS=Linux 6.12.110-135.202.amzn2023.x86_64 #1 SMP PREEMPT_DYNAMIC Fri Sep 25 01:45:58 UTC 2026 
   RealMemory=3891 AllocMem=0 FreeMem=2452 Sockets=2 Boards=1
   State=IDLE+CLOUD ThreadsPerCore=1 TmpDisk=0 Weight=1 Owner=N/A MCS_label=N/A
   Partitions=q1 
   BootTime=2026-10-08T19:57:35 SlurmdStartTime=2026-10-08T20:00:08
   LastBusyTime=2026-10-08T20:00:11 ResumeAfterTime=None
   CfgTRES=cpu=2,mem=3891M,billing=2
   AllocTRES=
   CurrentWatts=0 AveWatts=0
   Reason=Node start up [root@2026-10-08T20:00:11]
   Comment=MGIACOMO custom reason
   InstanceId=i-003d87dc15788298b InstanceType=t3.medium

Then I set a comment with offending substring State=DOWN+DRAIN

[root@ip-27-6-44-30 ~]# scontrol update nodename=$NODE comment="MGIACOMO my comment State=DOWN+DRAIN"
[root@ip-27-6-44-30 ~]# scontrol show nodes q1-st-cr1-1
NodeName=q1-st-cr1-1 Arch=x86_64 CoresPerSocket=1 
   CPUAlloc=0 CPUEfctv=2 CPUTot=2 CPULoad=0.05
   AvailableFeatures=static,t3.medium,cr1
   ActiveFeatures=static,t3.medium,cr1
   Gres=(null)
   NodeAddr=27.6.85.112 NodeHostName=q1-st-cr1-1 Version=25.11.8
   OS=Linux 6.12.110-135.202.amzn2023.x86_64 #1 SMP PREEMPT_DYNAMIC Fri Sep 25 01:45:58 UTC 2026 
   RealMemory=3891 AllocMem=0 FreeMem=2451 Sockets=2 Boards=1
   State=IDLE+CLOUD ThreadsPerCore=1 TmpDisk=0 Weight=1 Owner=N/A MCS_label=N/A
   Partitions=q1 
   BootTime=2026-10-08T19:57:35 SlurmdStartTime=2026-10-08T20:00:08
   LastBusyTime=2026-10-08T20:00:11 ResumeAfterTime=None
   CfgTRES=cpu=2,mem=3891M,billing=2
   AllocTRES=
   CurrentWatts=0 AveWatts=0
   Reason=Node start up [root@2026-10-08T20:00:11]
   Comment=MGIACOMO my comment State=DOWN+DRAIN
   InstanceId=i-003d87dc15788298b InstanceType=t3.medium

On the next clustermgtd cycle, clustermgtd reads the state from the Comment field (State=DOWN+DRAIN) rather than the expected field State=IDLE+CLOUD and trigger an unintentional replacement.

[root@ip-27-6-44-30 ~]#  cat  /var/log/parallelcluster/clustermgtd
...
2026-10-08 20:30:18,627 - [slurm_plugin.slurm_resources:is_state_healthy] - WARNING - Node state check: node q1-st-cr1-1(27.6.85.112) in DRAINED and not within the replacement protection window, it is considered unhealthy and will be replaced, node state: DOWN+DRAIN
2026-10-08 20:30:18,628 - [slurm_plugin.clustermgtd:_maintain_nodes] - INFO - Found the following unhealthy static nodes: (x1) ['q1-st-cr1-1(27.6.85.112) in state DOWN+DRAIN']

Tests

• PR checks: added test coverage and verified that the new unit tests would fail (execution) without the fix proving they are able to capture rtegressions.
• Deployed a cluster and verified that the corruption shown above does not occur anymore.

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

[hanwen-cluster]

Great job @gmarciani ! Does this PR cover injection in multi-line comments?
I approved this PR because it is anyway an improvement with/without multi-line coverage

[hanwen-cluster]

Minor: what's the issue if we do not discard them? It would be cleaner if we don't add another global variable.
This is a minor style comment

[gmarciani]

If we do not discard them we would have two problems:

1. we would have data around that we do not need, bad practice in general
2. we need further code changes to handle them

Since the issue with having a new constant is code style, I suggest to keep the change as is.

[gmarciani]

Does this PR cover injection in multi-line comments?

Nope (added to description). the only way to fix the multine comment "attack" is moving from free text parsing to JSON, which is not an option due to memory and latency issues at scale.