Cluster health monitor consumes the CVS cluster file

cvs/cli_plugins/exec_plugin.py

@@ -15,7 +15,8 @@ def get_parser(self, subparsers):
         parser = subparsers.add_parser("exec", help="Execute a command on all nodes in the cluster")
         parser.add_argument("--cmd", required=True, help="Command to execute on all nodes")
         parser.add_argument(
-            "--cluster_file", help="Path to cluster configuration JSON file (overrides CLUSTER_FILE env var)"
+            "--cluster_file",
+            help="Path to cluster configuration JSON file (takes precedence over CLUSTER_FILE env var)",
         )
         parser.set_defaults(_plugin=self)
         return parser
@@ -24,11 +25,12 @@ def get_epilog(self):
         return """
 Exec Commands:
   cvs exec --cmd "hostname" --cluster_file cluster.json    Execute hostname on all nodes
-  CLUSTER_FILE=cluster.json cvs exec --cmd "hostname"      Use env var for cluster file"""
+  CLUSTER_FILE=cluster.json cvs exec --cmd "hostname"      Use env var for cluster file (fallback)"""
 
     def run(self, args):
-        # Determine cluster file: env var takes precedence, then --cluster_file
-        cluster_file = os.environ.get('CLUSTER_FILE') or args.cluster_file
+        # CLI flag wins; env var is the fallback. Matches cvs scp and
+        # cvs monitor check_cluster_health for consistency.
+        cluster_file = args.cluster_file or os.environ.get('CLUSTER_FILE')
         if not cluster_file:
             print("Error: No cluster file specified. Set CLUSTER_FILE environment variable or use --cluster_file.")
             sys.exit(1)

cvs/cli_plugins/scp_plugin.py

@@ -18,7 +18,8 @@ def get_parser(self, subparsers):
         parser.add_argument("--dest", help="Remote destination path (defaults to same as source)")
         parser.add_argument("--recurse", action="store_true", help="Copy directories recursively")
         parser.add_argument(
-            "--cluster_file", help="Path to cluster configuration JSON file (overrides CLUSTER_FILE env var)"
+            "--cluster_file",
+            help="Path to cluster configuration JSON file (takes precedence over CLUSTER_FILE env var)",
         )
         parser.add_argument("--parallel", type=int, default=20, help="Number of parallel SCP operations (default: 20)")
         parser.set_defaults(_plugin=self)
@@ -31,7 +32,7 @@ def get_epilog(self):
   cvs scp --file /local/file.txt --dest /remote/file.txt                Copy file to specific remote path
   cvs scp --file /local/dir --dest /remote/dir --recurse                Copy directory recursively
   cvs scp --file /path/to/file.txt --parallel 10                        Use 10 parallel SCP operations
-  CLUSTER_FILE=cluster.json cvs scp --file /path/to/file.txt            Use env var for cluster file"""
+  CLUSTER_FILE=cluster.json cvs scp --file /path/to/file.txt            Use env var for cluster file (fallback)"""
 
     def copy_file_to_host(self, log, host, username, pkey, local_file, remote_path, recurse=False):
         """Copy file to a single host via SCP."""
@@ -43,8 +44,9 @@ def copy_file_to_host(self, log, host, username, pkey, local_file, remote_path,
             return f"{host}: FAILED - {str(e)}"
 
     def run(self, args):
-        # Determine cluster file: env var takes precedence, then --cluster_file
-        cluster_file = os.environ.get('CLUSTER_FILE') or args.cluster_file
+        # CLI flag wins; env var is the fallback. Matches cvs exec and
+        # cvs monitor check_cluster_health for consistency.
+        cluster_file = args.cluster_file or os.environ.get('CLUSTER_FILE')
         if not cluster_file:
             print("Error: No cluster file specified. Set CLUSTER_FILE environment variable or use --cluster_file.")
             sys.exit(1)

cvs/monitors/README.md

@@ -22,32 +22,82 @@ napshot of all counters (GPU/NIC) while your training/inference workloads are in
 
 ### Usage for Cluster Health Checker
 
+The Cluster Health Checker consumes the same `--cluster_file` JSON used by
+`cvs run`, `cvs exec`, and `cvs scp` (see [`cvs/input/cluster_file/README.md`](../input/cluster_file/README.md)
+for the schema). The node list, SSH username, and private key are all read
+from the cluster file - no separate hosts file or credentials need to be
+passed on the CLI.
+
+Just like `cvs exec` and `cvs scp`, the cluster file can be supplied either
+via `--cluster_file <path>` or by exporting `CLUSTER_FILE=<path>` once per
+shell. An explicit `--cluster_file` flag always wins over the env var, so
+you can `export CLUSTER_FILE=...` once for the common case and still
+override it on a single invocation by passing `--cluster_file` on the CLI.
 
 ```
-(myenv) [ubuntu-host]~/cvs:(main)$
-(myenv) [ubuntu-host]~/cvs:(main)$python3 ./cvs/monitors/check_cluster_health.py -h
-usage: check_cluster_health.py [-h] --hosts_file HOSTS_FILE --username USERNAME (--password PASSWORD | --key_file KEY_FILE)
-                               [--iterations ITERATIONS] [--time_between_iters TIME_BETWEEN_ITERS] [--report_file REPORT_FILE]
+(myenv) [ubuntu-host]~/cvs:(main)$ cvs monitor check_cluster_health --help
+usage: cvs monitor check_cluster_health [-h] (--cluster_file CLUSTER_FILE | --hosts_file HOSTS_FILE)
+                                        [--username USERNAME] [--password PASSWORD] [--key_file KEY_FILE]
+                                        [--iterations ITERATIONS] [--time_between_iters TIME_BETWEEN_ITERS]
+                                        [--report_file REPORT_FILE]
 
 Check Cluster Health
 
 options:
   -h, --help            show this help message and exit
+  --cluster_file CLUSTER_FILE
+                        Path to a CVS cluster JSON file (see cvs/input/cluster_file/cluster.json).
+                        Provides node list, username, and SSH key. Recommended.
+                        Takes precedence over the CLUSTER_FILE environment variable.
   --hosts_file HOSTS_FILE
-                        File name with list of IP address one per line
-  --username USERNAME   Username to ssh to the hosts
-  --password PASSWORD   Password for username
-  --key_file KEY_FILE   Private Keyfile for username
+                        [DEPRECATED] File with one host IP/hostname per line. Use --cluster_file instead.
+  --username USERNAME   SSH username (required with --hosts_file)
+  --password PASSWORD   SSH password (only valid with --hosts_file)
+  --key_file KEY_FILE   SSH private key file (only valid with --hosts_file)
   --iterations ITERATIONS
                         Number of iterations to run the checks
   --time_between_iters TIME_BETWEEN_ITERS
-                        Time duration to sleep between iterations ..
+                        Time duration to sleep between iterations
   --report_file REPORT_FILE
-(myenv) [ubuntu-host]~/cvs:(main)$
-(myenv) [ubuntu-host]~/cvs/cvs:(main)$
-(myenv) [ubuntu-host]~/cvs/cvs/monitors:(main)$
-(myenv) [ubuntu-host]~/cvs/cvs/monitors:(main)$python3 ./check_cluster_health.py --hosts_file /home/user1/hosts_file.txt --username user1  --key_file /home/user1/.ssh/id_rsa --iterations 2
+                        Output HTML report file path
+```
+
+#### Recommended: run with a cluster file
+
+```
+cvs monitor check_cluster_health \
+    --cluster_file cvs/input/cluster_file/cluster.json \
+    --iterations 2
+```
+
+Or set `CLUSTER_FILE` once and reuse it across CVS commands:
+
+```
+export CLUSTER_FILE=cvs/input/cluster_file/cluster.json
+cvs monitor check_cluster_health --iterations 2
+cvs exec --cmd "hostname"
+cvs scp --file /path/to/file.txt
+```
+
+Or directly:
+
+```
+python3 ./cvs/monitors/check_cluster_health.py \
+    --cluster_file cvs/input/cluster_file/cluster.json \
+    --iterations 2
+```
+
+#### Deprecated: legacy hosts file
 
+`--hosts_file` is still accepted for backward compatibility but emits a
+deprecation warning. Lines starting with `#` and blank lines are ignored.
+
+```
+python3 ./cvs/monitors/check_cluster_health.py \
+    --hosts_file /home/user1/hosts_file.txt \
+    --username user1 \
+    --key_file /home/user1/.ssh/id_rsa \
+    --iterations 2
 ```
 
 ### Debugging using RDMA Statistics Table

cvs/monitors/check_cluster_health.py

@@ -5,7 +5,9 @@
 All code contained here is Property of Advanced Micro Devices, Inc.
 '''
 
+import os
 import sys
+import json
 import logging
 import argparse
 import time
@@ -16,10 +18,54 @@
 from cvs.lib import linux_utils
 from cvs.lib import rocm_plib
 from cvs.lib import html_lib
+from cvs.lib import utils_lib
 
 log = logging.getLogger()
 
 
+def load_cluster_file(cluster_file_path):
+    """
+    Load and validate a CVS cluster file for the cluster health monitor.
+
+    Parses the JSON cluster file used by ``cvs run`` / ``cvs exec`` and
+    extracts the SSH credentials and node list. Path placeholders such as
+    ``{user-id}`` are resolved via :func:`utils_lib.resolve_cluster_config_placeholders`.
+
+    Args:
+        cluster_file_path (str): Path to the cluster JSON file.
+
+    Returns:
+        tuple: ``(node_list, username, priv_key_file)`` where ``node_list``
+        is the list of host IPs / hostnames (the keys of ``node_dict``).
+
+    Raises:
+        FileNotFoundError: If the cluster file does not exist.
+        ValueError: If the cluster file is missing required keys, has no
+            nodes, or contains invalid JSON.
+    """
+    try:
+        with open(cluster_file_path, 'r') as f:
+            cluster = json.load(f)
+    except json.JSONDecodeError as e:
+        raise ValueError(f"Invalid JSON in cluster file '{cluster_file_path}': {e}") from e
+
+    cluster = utils_lib.resolve_cluster_config_placeholders(cluster)
+
+    username = cluster.get('username')
+    if not username:
+        raise ValueError(f"'username' missing from cluster file '{cluster_file_path}'")
+
+    priv_key_file = cluster.get('priv_key_file')
+    if not priv_key_file:
+        raise ValueError(f"'priv_key_file' missing from cluster file '{cluster_file_path}'")
+
+    node_list = list((cluster.get('node_dict') or {}).keys())
+    if not node_list:
+        raise ValueError(f"'node_dict' is empty or missing in cluster file '{cluster_file_path}'")
+
+    return node_list, username, priv_key_file
+
+
 def general_health_checks(
     phdl,
 ):
@@ -331,35 +377,105 @@ def get_description(self):
         return "Generate a health report of your cluster by collecting various logs and metrics"
 
     def get_parser(self):
-        parser = argparse.ArgumentParser(description="Check Cluster Health")
-        parser.add_argument("--hosts_file", required=True, help="File name with list of IP address one per line")
-        parser.add_argument("--username", required=True, help="Username to ssh to the hosts")
-        group = parser.add_mutually_exclusive_group(required=True)
-        group.add_argument("--password", help="Password for username")
-        group.add_argument("--key_file", help="Private Keyfile for username")
+        parser = argparse.ArgumentParser(
+            description="Check Cluster Health",
+            epilog=(
+                "Examples:\n"
+                "  cvs monitor check_cluster_health --cluster_file cluster.json --iterations 2\n"
+                "  CLUSTER_FILE=cluster.json cvs monitor check_cluster_health --iterations 2"
+            ),
+            formatter_class=argparse.RawDescriptionHelpFormatter,
+        )
+        # Source of hosts: either a CVS cluster file (preferred, can also be
+        # supplied via the CLUSTER_FILE env var) or a legacy one-IP-per-line
+        # hosts file. The "exactly one source" rule is enforced at runtime
+        # in _resolve_connection so the env var can satisfy --cluster_file.
+        source = parser.add_mutually_exclusive_group(required=False)
+        source.add_argument(
+            "--cluster_file",
+            help=(
+                "Path to a CVS cluster JSON file "
+                "(see cvs/input/cluster_file/cluster.json). "
+                "Provides node list, username, and SSH key. Recommended. "
+                "Takes precedence over the CLUSTER_FILE environment variable."
+            ),
+        )
+        source.add_argument(
+            "--hosts_file",
+            help="[DEPRECATED] File with one host IP/hostname per line. Use --cluster_file instead.",
+        )
+        # Credentials - only required when --hosts_file is used.
+        parser.add_argument("--username", help="SSH username (required with --hosts_file)")
+        parser.add_argument("--password", help="SSH password (only valid with --hosts_file)")
+        parser.add_argument("--key_file", help="SSH private key file (only valid with --hosts_file)")
         parser.add_argument("--iterations", type=int, default=2, help="Number of iterations to run the checks")
         parser.add_argument(
             "--time_between_iters", type=int, default=60, help="Time duration to sleep between iterations"
         )
         parser.add_argument("--report_file", default="./cluster_report.html", help="Output HTML report file path")
         return parser
 
-    def monitor(self, args):
-        """Execute the cluster health check monitoring."""
-        # Read host IP addresses from input file.
+    def _resolve_connection(self, args):
+        """Return ``(node_list, username, pkey, password)`` based on parsed args.
+
+        Resolution order for the cluster file matches ``cvs exec`` / ``cvs scp``:
+        an explicit ``--cluster_file`` flag takes precedence, then the
+        ``CLUSTER_FILE`` environment variable is used as a fallback.
+        ``--hosts_file`` remains as a deprecated last-resort path.
+        """
+        # CLI flag wins; env var is the fallback. This matches standard Unix
+        # tooling and keeps cvs exec / cvs scp / cvs monitor consistent.
+        cluster_file = args.cluster_file or os.environ.get('CLUSTER_FILE')
+
+        if cluster_file and args.hosts_file:
+            print("ERROR: --hosts_file cannot be combined with --cluster_file or CLUSTER_FILE. Aborting.")
+            sys.exit(1)
+
+        if cluster_file:
+            if args.username or args.password or args.key_file:
+                print(
+                    "ERROR: --username/--password/--key_file are not allowed with --cluster_file; "
+                    "credentials come from the cluster file. Aborting."
+                )
+                sys.exit(1)
+            node_list, username, pkey = load_cluster_file(cluster_file)
+            return node_list, username, pkey, None
+
+        if not args.hosts_file:
+            print(
+                "ERROR: no cluster file specified. Set the CLUSTER_FILE environment variable, "
+                "pass --cluster_file, or (deprecated) pass --hosts_file. Aborting."
+            )
+            sys.exit(1)
+
+        # Legacy --hosts_file path.
+        print(
+            "WARNING: --hosts_file is deprecated. Switch to --cluster_file (see cvs/input/cluster_file/cluster.json)."
+        )
+        if not args.username:
+            print("ERROR: --username is required when using --hosts_file. Aborting.")
+            sys.exit(1)
+        if bool(args.password) == bool(args.key_file):
+            print("ERROR: exactly one of --password or --key_file is required with --hosts_file. Aborting.")
+            sys.exit(1)
         with open(args.hosts_file, "r") as f:
-            node_list = [line.strip() for line in f if line.strip()]
+            node_list = [line.strip() for line in f if line.strip() and not line.lstrip().startswith("#")]
         if not node_list:
-            print("ERROR !! No hosts in the file, this is mandatory, aborting !!")
+            print(f"ERROR: no hosts found in '{args.hosts_file}'. Aborting.")
             sys.exit(1)
+        return node_list, args.username, args.key_file, args.password
+
+    def monitor(self, args):
+        """Execute the cluster health check monitoring."""
+        node_list, username, pkey, password = self._resolve_connection(args)
 
         html_report_file = args.report_file
 
-        # Connect to all hosts in the cluster ..
-        if args.key_file:
-            phdl = parallel_ssh_lib.Pssh(log, node_list, user=args.username, pkey=args.key_file)
-        elif args.password:
-            phdl = parallel_ssh_lib.Pssh(log, node_list, user=args.username, password=args.password)
+        # Connect to all hosts in the cluster.
+        if pkey:
+            phdl = parallel_ssh_lib.Pssh(log, node_list, user=username, pkey=pkey)
+        else:
+            phdl = parallel_ssh_lib.Pssh(log, node_list, user=username, password=password)
 
         start_time = phdl.exec('date')