AIMVT-239: Add automated SSH key distribution test suite by ahskabir · Pull Request #393 · ROCm/cvs

ahskabir · 2026-09-09T05:45:29Z

Motivation

Multi-node RCCL and training test suites require password-less SSH between all cluster nodes as a prerequisite, but CVS has no automated way to set this up — users currently do it by hand per cluster. This adds a cluster_key_distribution test suite that distributes a shared cluster keypair, authorizes it (plus optionally the controlling station's own key) in authorized_keys, writes a managed ~/.ssh/config Host block, and verifies password-less connectivity — all driven by a single JSON config.

Technical Details

cvs/lib/ssh_keys_lib.py: config validation/normalization, SFTP key upload, authorized_keys injection (append-only, idempotent — never truncates the file), ~/.ssh/config managed-block rendering, and ring/full-mesh connectivity verification.
cvs/tests/cluster_key_distribution/ssh_keys_distribution.py: pytest suite, 6 ordered tests (dir setup → key distribution → cluster pubkey authorization → controlling-station pubkey authorization → ssh config install → passwordless SSH verification).
cvs/input/config_file/cluster_key_distribution/ssh_key_distribute_config.json: sample config.
ssh_dir_nfs_shared config flag: when the remote ~/.ssh dir is the same NFS-mounted directory on every node, all mutating steps (key upload, authorized_keys writes, ~/.ssh/config write) route through the head node only via orch.exec_on_head/orch.head, instead of running redundantly — and racily — on every node against the same underlying file. Without this, concurrent sed -i (temp-file + rename) from multiple nodes against one NFS-shared file reproduced as base64: write error: Stale file handle on a live cluster.
Incidental fix: pytest.ini had ignore:.Broken pipe.:BrokenPipeError in filterwarnings, which is invalid under pytest 8+ (BrokenPipeError isn't a Warning subclass) and aborted every test run with UsageError — removed, since filterwarnings never matched raised
exceptions anyway, only warnings.warn() calls.

Test Plan

58 mocked unit tests in cvs/lib/unittests/test_ssh_keys_lib.py (config validation, key upload incl. NFS-shared path, authorized_keys injection, ssh config rendering, connectivity verification) — no live cluster dependency.
make test (full suite: 1794 unit tests + CLI smoke tests).
Live run against a real 4-node cluster via the cluster_key_distribution suite, both with ssh_dir_nfs_shared true and false.

Test Result

make test: all 1794 unit tests pass, all CLI smoke tests pass.
Live 4-node cluster run: 6/6 suite tests pass, including install_ssh_config on the node that previously hit the stale-NFS-handle race — confirmed fixed with no errors in the run log.

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

Implements DCCS-6410 -- automated passwordless SSH setup across cluster nodes as a prerequisite for multi-node RCCL and training tests. - cvs/lib/ssh_keys_lib.py: config validation, hostname collection, wildcard derivation, ~/.ssh/config block rendering, remote command builders, and orchestration drivers (key upload, authorized_keys injection, connectivity verification). Supports NFS-shared ssh dirs via ssh_dir_nfs_shared -- uploads via the head node only when the remote dir is a shared mount, skipping redundant per-node transfers. - cvs/lib/unittests/test_ssh_keys_lib.py: 56 mocked unit tests, no live cluster dependency - cvs/tests/cluster_key_distribution/ssh_keys_distribution.py: pytest suite with 6 ordered tests covering dir setup, key distribution, authorized_keys injection (cluster + optional controlling station), ssh config installation, and end-to-end passwordless SSH verification - cvs/input/config_file/cluster_key_distribution/ssh_key_distribute_config.json: sample config with <changeme> placeholders Live-tested against a 4-node cluster. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

pytest 8+ requires filterwarnings categories to be Warning subclasses; BrokenPipeError is an OSError, so parsing this entry raises UsageError and aborts every test run under pytest>=8 (reproduced with pytest 9.1.1). filterwarnings only matches warnings.warn() calls anyway, so this entry never suppressed a raised BrokenPipeError. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ode on NFS-shared dirs When ssh_dir_nfs_shared is true, remote_ssh_dir is the same physical directory on every node. authorize_cluster_pubkey, authorize_controlling_station, and install_ssh_config still ran their mutating commands on all nodes concurrently, racing multiple hosts against the same underlying file. install_ssh_config's sed -i (temp-file + rename) hit this in practice -- reproduced live as "base64: write error: Stale file handle" on one node of a 4-node NFS-shared cluster. All three now dispatch through orch.exec_on_head instead of orch.exec when ssh_dir_nfs_shared is set, matching the head-only pattern already used for the SFTP key uploads. Live-tested against the 4-node cluster: 6/6 tests pass, no stale handle errors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

speriaswamy-amd

Looks good overall, requires couple of changes though:

speriaswamy-amd · 2026-09-09T16:43:07Z

+        for (src, dst), output in zip(
+            pairs, [out.get(node, "") for node in nodes if node != nodes[0] or len(nodes) < 2]
+        ):
+            results[(src, dst)] = "error" not in str(output).lower() and output is not None


Blocking: we use "error" not in str(raw).lower() for judging failures, we should be using exit nodes. probe with detailed=True (or ssh ... true && echo CVS_SSH_OK) and require exit code 0

speriaswamy-amd · 2026-09-09T16:51:38Z

+            raw = node_outputs[i] if i < len(node_outputs) else ""
+            results[(src, dst)] = raw is not None and "error" not in str(raw).lower()
+
+    else:


Blocking: This block generated n - 1 commands and executes on 1 node. But exec_cmd_list requires len(cmd_list) to match the host count, this would raise errors multinode job. Better option would be to use orch.exec(cmd, hosts=[src], detailed=True) as Pssh handle is deprecated

speriaswamy-amd · 2026-09-09T20:46:11Z

+                f"ssh -F {ssh_config_path} -o BatchMode=yes -o ConnectTimeout={timeout} {dst} true" for dst in peers
+            ]
+            # Run all probes from src via a temporary single-host handle
+            from cvs.lib.parallel_ssh_lib import Pssh


This is deprecated, should be using the new from cvs.lib.parallel.phandle import ParallelHandle

speriaswamy-amd · 2026-09-09T20:50:48Z

+    return result
+
+
+def _upload_local_file(orch, local_path, remote_path):


This method doesn't seem to be used anywhere?

speriaswamy-amd · 2026-09-10T18:21:46Z

+    exec_target = orch.exec_on_head if nfs_shared else orch.exec
+    out = exec_target(cmd, timeout=30, detailed=True)
+
+    results = {node: True for node in orch.all.reachable_hosts}


We turn one result into per node status dict, i.e even if First node fails other nodes stay True, here's a walk through from claude on 3+ nodes: ```
alk through a 3-node cluster, head = n1, NFS write fails on head (exit_code=1):

Start: {n1: True, n2: True, n3: True} — every node assumed success.
out is only {n1: {exit_code: 1}}.
The loop updates only n1 to False.
Return: {n1: False, n2: True, n3: True}.

We should follow `upload_cluster_keys` pattern and mark every node as failed

ahskabir and others added 3 commits September 8, 2026 19:02

ahskabir requested review from speriaswamy-amd September 9, 2026 05:54

speriaswamy-amd requested changes Sep 10, 2026

View reviewed changes

Navigation Menu

Uh oh!

AIMVT-239: Add automated SSH key distribution test suite - #393

AIMVT-239: Add automated SSH key distribution test suite#393
ahskabir wants to merge 3 commits into
mainfrom
feature-key-distribution

ahskabir commented Sep 9, 2026

Uh oh!

speriaswamy-amd left a comment

Uh oh!

speriaswamy-amd Sep 9, 2026

Uh oh!

speriaswamy-amd Sep 9, 2026

Uh oh!

speriaswamy-amd Sep 9, 2026

Uh oh!

speriaswamy-amd Sep 9, 2026

Uh oh!

speriaswamy-amd Sep 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		return result


		def _upload_local_file(orch, local_path, remote_path):

Uh oh!

Conversation

ahskabir commented Sep 9, 2026

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

Uh oh!

speriaswamy-amd left a comment

Choose a reason for hiding this comment

Uh oh!

speriaswamy-amd Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

speriaswamy-amd Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

speriaswamy-amd Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

speriaswamy-amd Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

speriaswamy-amd Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants