Skip to content

Port install_agfhc to the orch fixture; drop nfs_install [AIMVT-170]#181

Merged
atnair-amd merged 2 commits into
mainfrom
atnair/port-install-agfhc
May 20, 2026
Merged

Port install_agfhc to the orch fixture; drop nfs_install [AIMVT-170]#181
atnair-amd merged 2 commits into
mainfrom
atnair/port-install-agfhc

Conversation

@atnair-amd

Copy link
Copy Markdown
Collaborator

Tracking: AIMVT-170.

Changes

cvs/tests/health/install/install_agfhc.py:

  • Delete from cvs.lib.parallel_ssh_lib import * and the local phdl/shdl
    fixtures. The test signature becomes test_install_agfhc(orch, config_dict).
  • Remove the nfs_install head-vs-fleet conditional. Install runs
    unconditionally on every node via orch.exec (matches the merged
    install_rvs.py behavior). The package_tar_ball must exist at the same
    local path on every node.
  • Split multi-statement shell strings (cd X; cp ... && tar -xvf ...) into
    separate single-binary orch.exec calls. The single ./install invocation
    that genuinely needs cwd = install_dir (AGFHC's installer reads sibling
    files via relative paths) is wrapped in an explicit
    sudo bash -c 'cd X && ./install' so the cwd dependency is visible at the
    call site instead of relying on an implicit shell. Each call now invokes one
    binary, which is what ContainerOrchestrator's docker-exec transport
    requires.
  • Pass --rocm-tar to AGFHC's installer so it bypasses apt-based dependency
    resolution in favour of dpkg-deb extraction. Required on TheRock-rocm
    hosts (libs not tracked by dpkg); no-op on apt-rocm hosts.

cvs/input/config_file/health/mi300_health_config.json:

  • Remove "nfs_install": "True" from the agfhc block.

docs/reference/configuration-files/health.rst:

  • Remove the matching "nfs_install" entry from the inline JSON dropdown and
    the nfs_install row from the AGFHC parameter table.

Mechanical translation of phdl/shdl -> orch per orch fixture
pattern (parent: AIMVT-2). Deletes the local phdl/shdl fixtures
(orch lives in cvs/tests/conftest.py per AIMVT-172).

Also removes the nfs_install knob from this suite. Precedent:
install_rvs.py (merged via PR #174) already silently ignores
the field. The head-only optimization saved only a cp+tar
extraction step -- sudo ./install was already always fleet-wide
(legacy L172). After this PR, install runs unconditionally on
every node via orch.exec.

Behavior change: package_tar_ball must now exist at the same
local path on every node. Operators who previously relied on
nfs_install=True with the tarball only on a shared mount must
pre-stage the tarball per-node.

Out of scope: the rvs.nfs_install and transferbench.nfs_install
keys in mi300_health_config.json remain (covered by separate PRs).
Splits `cd X; cp/tar/install` shell-string chains into separate
orch.exec calls, each invoking a single binary with absolute paths.
This makes the test's command flow legible at the call site and works
correctly under ContainerOrchestrator's docker-exec transport, which
does not spawn a shell to interpret `cd`, `;`, `&&` and friends. The
single AGFHC `./install` call still requires cwd = install_dir
(installer reads sibling files via relative paths), so that one call
is wrapped in an explicit `sudo bash -c 'cd X && ./install'` to make
the cwd dependency visible at the call site rather than smuggling it
in via a shell chain.

Also passes `--rocm-tar` to AGFHC's installer: this bypasses apt-based
dependency resolution (which fails on TheRock-rocm systems because the
libs aren't tracked by dpkg) in favour of dpkg-deb direct extraction.
Safe on apt-rocm hosts too: dpkg-deb extracts the bundled debs into /,
which is correct either way.

Validated on a06u43 (single MI300X node, ROCm 7.13.0 in cvs-harness/
rocm-dev:latest container, externally-managed via launch:false):
1 passed in 26.13s. Container /opt/amd/agfhc/agfhc now exists post-run.

@urtiwari urtiwari left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@atnair-amd atnair-amd merged commit 6d37a7b into main May 20, 2026
2 checks passed
amd-droy pushed a commit that referenced this pull request Jun 25, 2026
…181)

* [AIMVT-170] Port install_agfhc.py to orch fixture and drop nfs_install

Mechanical translation of phdl/shdl -> orch per orch fixture
pattern (parent: AIMVT-2). Deletes the local phdl/shdl fixtures
(orch lives in cvs/tests/conftest.py per AIMVT-172).

Also removes the nfs_install knob from this suite. Precedent:
install_rvs.py (merged via PR #174) already silently ignores
the field. The head-only optimization saved only a cp+tar
extraction step -- sudo ./install was already always fleet-wide
(legacy L172). After this PR, install runs unconditionally on
every node via orch.exec.

Behavior change: package_tar_ball must now exist at the same
local path on every node. Operators who previously relied on
nfs_install=True with the tarball only on a shared mount must
pre-stage the tarball per-node.

Out of scope: the rvs.nfs_install and transferbench.nfs_install
keys in mi300_health_config.json remain (covered by separate PRs).

* [AIMVT-170] install_agfhc: one logical command per orch.exec; --rocm-tar

Splits `cd X; cp/tar/install` shell-string chains into separate
orch.exec calls, each invoking a single binary with absolute paths.
This makes the test's command flow legible at the call site and works
correctly under ContainerOrchestrator's docker-exec transport, which
does not spawn a shell to interpret `cd`, `;`, `&&` and friends. The
single AGFHC `./install` call still requires cwd = install_dir
(installer reads sibling files via relative paths), so that one call
is wrapped in an explicit `sudo bash -c 'cd X && ./install'` to make
the cwd dependency visible at the call site rather than smuggling it
in via a shell chain.

Also passes `--rocm-tar` to AGFHC's installer: this bypasses apt-based
dependency resolution (which fails on TheRock-rocm systems because the
libs aren't tracked by dpkg) in favour of dpkg-deb direct extraction.
Safe on apt-rocm hosts too: dpkg-deb extracts the bundled debs into /,
which is correct either way.

Validated on a06u43 (single MI300X node, ROCm 7.13.0 in cvs-harness/
rocm-dev:latest container, externally-managed via launch:false):
1 passed in 26.13s. Container /opt/amd/agfhc/agfhc now exists post-run.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants