Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/workflows/e2e-selfhosted.yml
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ jobs:
# instantly (observed on run 29320025393). Installing in place keeps the
# baked paths valid, and each scenario's data/runtimes symlink resolves to
# this same real tree.
prewarm="$RUNNER_WORKSPACE/e2e-prewarm"
prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2"
export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes"

# Build the rocm and rocmd binaries ONCE and reuse them for both the
Expand Down Expand Up @@ -454,7 +454,7 @@ jobs:
# is missing)` and fails (diagnosed on the box 2026-07-15). Installing in
# place, directly into the persistent shared dir, keeps the baked paths
# valid for all scenarios and for the end-of-run version probe.
prewarm="$RUNNER_WORKSPACE/e2e-prewarm"
prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2"
export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes"

# Build the rocm and rocmd binaries once; reuse them for pre-warm + suite.
Expand Down Expand Up @@ -605,7 +605,7 @@ jobs:
# every later serve sees status=unusable and fails (diagnosed on the Linux
# box 2026-07-15; the Windows scenario-8 cold-download failure is the same
# class). Install in place so the baked paths stay valid for all scenarios.
$prewarm = "$env:RUNNER_WORKSPACE\e2e-prewarm"
$prewarm = "$env:RUNNER_WORKSPACE\e2e-prewarm-multi-arch-v2"
$env:E2E_SHARED_RUNTIMES_DIR = "$prewarm\data\runtimes"

# Build the rocm and rocmd binaries once; reuse them for pre-warm + suite.
Expand Down Expand Up @@ -794,7 +794,7 @@ jobs:
# correctness, not speed: `install sdk` bakes ABSOLUTE paths into the
# runtime manifest, so installing into a per-scenario temp dir leaves
# every later serve pointing at a deleted install root.
prewarm="$RUNNER_WORKSPACE/e2e-prewarm"
prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2"
export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes"

# Build the rocm and rocmd binaries once; reuse them for pre-warm + suite.
Expand Down Expand Up @@ -924,7 +924,7 @@ jobs:
# optimization — `install sdk` bakes absolute paths into the runtime
# manifest, so installing anywhere temporary breaks every later serve.
# See e2e-gpu for the full rationale.
prewarm="$RUNNER_WORKSPACE/e2e-prewarm"
prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2"
export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes"

# See the e2e-gpu lane for why the e2e-test-hooks feature must match
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/nightly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ jobs:
# The shared dir IS the pre-warm's own data/runtimes; NEVER move it — a
# post-install mv invalidates the absolute paths install sdk bakes into
# the runtime manifest and every serve fails instantly (run 29320025393).
prewarm="$RUNNER_WORKSPACE/e2e-prewarm"
prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2"
export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes"

# Build both binaries once; reuse them for pre-warm + suite.
Expand Down Expand Up @@ -524,7 +524,7 @@ jobs:
# Separate PVC mounted at exactly this path by the runner's cluster
# overlay; if you change this path, change the overlay that mounts it too.
export E2E_SHARED_UV_CACHE_DIR="/var/tmp/rocm-e2e-uv-cache"
prewarm="$RUNNER_WORKSPACE/e2e-prewarm"
prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2"
export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes"

# See the e2e-gpu lane in e2e-selfhosted.yml for why the
Expand Down Expand Up @@ -620,7 +620,7 @@ jobs:
run: |
export CARGO_TARGET_DIR="$RUNNER_WORKSPACE/e2e-target"
export E2E_SHARED_CACHE_DIR="$RUNNER_WORKSPACE/e2e-shared"
prewarm="$RUNNER_WORKSPACE/e2e-prewarm"
prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2"
export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes"

# See the e2e-gpu lane in e2e-selfhosted.yml for why the
Expand Down Expand Up @@ -716,7 +716,7 @@ jobs:
# Install the shared runtime in place because its manifest contains
# absolute paths. The persistent runner workspace keeps those paths valid
# across scenarios and subsequent runs.
$prewarm = "$env:RUNNER_WORKSPACE\e2e-prewarm"
$prewarm = "$env:RUNNER_WORKSPACE\e2e-prewarm-multi-arch-v2"
$env:E2E_SHARED_RUNTIMES_DIR = "$prewarm\data\runtimes"

# See the e2e-gpu lane in e2e-selfhosted.yml for why the
Expand Down Expand Up @@ -870,7 +870,7 @@ jobs:
# correctness, not speed: `install sdk` bakes ABSOLUTE paths into the
# runtime manifest, so installing into a per-scenario temp dir leaves
# every later serve pointing at a deleted install root.
prewarm="$RUNNER_WORKSPACE/e2e-prewarm"
prewarm="$RUNNER_WORKSPACE/e2e-prewarm-multi-arch-v2"
export E2E_SHARED_RUNTIMES_DIR="$prewarm/data/runtimes"

# See the e2e-gpu lane in e2e-selfhosted.yml for why the
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions MANIFEST.md
Original file line number Diff line number Diff line change
Expand Up @@ -646,8 +646,8 @@ ROCm distribution. Two install formats are supported:
- **Wheel format** — Python wheel packages (`rocm`, `torch`, `torchvision`,
`torchaudio`) are resolved from AMD-hosted PyPI-compatible indexes and
installed via `uv` into a managed virtual environment. Release channel wheels
are served from `https://repo.amd.com/rocm/whl/<gpu-family>/`. Nightly
channel wheels are served from `https://rocm.nightlies.amd.com/v2/<gpu-family>/`.
are served from `https://repo.amd.com/rocm/whl-multi-arch`. Nightly channel
wheels are served from `https://rocm.nightlies.amd.com/whl-multi-arch`.
- **Tarball format** — Prebuilt SDK tarballs are downloaded from AMD-hosted
artifact storage. Release channel tarballs are served from
`https://repo.amd.com/rocm/tarball/`. Nightly tarballs are served from
Expand Down
1 change: 1 addition & 0 deletions apps/rocm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ rocm-engine-vllm = { path = "../../engines/vllm" }
rpassword.workspace = true
serde.workspace = true
serde_json.workspace = true
sha2.workspace = true
tar = "0.4"
tracing = "0.1"
tracing-appender = "0.2"
Expand Down
3 changes: 3 additions & 0 deletions apps/rocm/src/comfyui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2112,6 +2112,7 @@ mod tests {
..therock::RocmSdkPythonProbe::default()
}),
sdk_torch: None,
wheel_composition: None,
read_only: false,
imported_from: None,
installed_at_unix_ms: 100,
Expand Down Expand Up @@ -2178,6 +2179,7 @@ mod tests {
..Default::default()
}),
sdk_torch: None,
wheel_composition: None,
read_only: false,
imported_from: None,
installed_at_unix_ms: 100,
Expand Down Expand Up @@ -2275,6 +2277,7 @@ mod tests {
..therock::RocmSdkPythonProbe::default()
}),
sdk_torch: None,
wheel_composition: None,
read_only: false,
imported_from: None,
installed_at_unix_ms: 100,
Expand Down
81 changes: 48 additions & 33 deletions apps/rocm/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9194,6 +9194,7 @@ fn adopt_runtime_from_probe(
// Adoption does not install torch, so the build is derived from the SDK
// version instead.
sdk_torch: None,
wheel_composition: None,
read_only: true,
imported_from: Some(install_root),
installed_at_unix_ms: rocm_core::unix_time_millis(),
Expand Down Expand Up @@ -15821,7 +15822,7 @@ fn apply_runtime_update(
) -> Result<String> {
let manifests = therock::load_runtime_manifests(paths)?;
let source = select_runtime_update_source(&manifests, config, runtime_selector)?;
let plan = therock::runtime_update_plan(paths, source)?;
let plan = therock::runtime_update_plan(paths, source, &manifests)?;
let mut output = String::new();
let _ = writeln!(output, "runtime update");
let _ = writeln!(output, " source_runtime_key: {}", source.runtime_key);
Expand All @@ -15840,6 +15841,7 @@ fn apply_runtime_update(
therock::runtime_version_display(&plan.latest_version)
);
let _ = writeln!(output, " status: {}", plan.status);
let _ = writeln!(output, " target_runtime_key: {}", plan.target_runtime_key);
let _ = writeln!(output, " activate_after_install: {activate}");
if !plan.update_available {
let _ = writeln!(output, " result: no newer runtime found");
Expand All @@ -15848,13 +15850,12 @@ fn apply_runtime_update(

if dry_run {
let _ = writeln!(output, " mode: dry-run");
let install_plan = therock::install_sdk(
let install_plan = therock::install_sdk_for_update(
paths,
&source.channel,
&source.format,
None,
None,
None,
&source.family,
plan.device_target.as_deref(),
true,
)?;
let _ = writeln!(output, " install_plan:");
Expand All @@ -15864,18 +15865,26 @@ fn apply_runtime_update(
return Ok(output);
}

let install_output = therock::install_sdk(
let install_output = therock::install_sdk_for_update(
paths,
&source.channel,
&source.format,
None,
None,
None,
&source.family,
plan.device_target.as_deref(),
false,
)?;
let manifests_after = therock::load_runtime_manifests(paths)?;

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.

apply_runtime_update predicts a runtime key with one family, then installs with another.

plan.target_runtime_key comes from runtime_update_plan → resolve_latest_for_manifest (therock.rs:734), which passes Some(manifest.family.as_str()) as the family override. But the apply path just above calls install_sdk(paths, &source.channel, &source.format, None, None, None, false) — family_override = None. So the install re-derives the family from resolve_family's detection chain, and the family feeds the device-<target> extra, which feeds the composition, which feeds the key.

What makes that fatal now rather than merely surprising is the lookup narrowing on the next line. main matched on channel/format/family/version:

manifest.channel == source.channel && manifest.format == source.format
    && manifest.family == source.family && manifest.version == latest_version

This PR replaces it with an exact manifest.runtime_key == target_runtime_key. Any divergence between predicted and actual now aborts with "runtime install completed but the target runtime manifest was not found" — after a multi-gigabyte install has already succeeded and been written to disk.

The dry-run branch a few lines up has the same None, so the preview describes a different install than the apply performs.

Passing Some(&source.family) on both branches would keep the planned and applied keys in agreement. Worth doing regardless of whether a divergence is reachable today — xtask e2e-prewarm's new Decision::Repair drives exactly this path, and the failure mode is expensive.

let installed = select_installed_update_runtime(&manifests_after, source, &plan.latest_version)
.context("updated runtime install completed but the new runtime manifest was not found")?;
// By exact key, never by version: a same-version repair installs a sibling
// that shares the source's channel, format, family AND version, so a
// version match would just as happily return the stale runtime this update
// was meant to replace, and then activate it.
let installed = select_installed_update_runtime(&manifests_after, &plan.target_runtime_key)
.with_context(|| {
format!(
"runtime install completed but no manifest was written for the planned runtime key `{}`",
plan.target_runtime_key
)
})?;
let _ = writeln!(output, " installed_runtime_key: {}", installed.runtime_key);
let _ = writeln!(
output,
Expand Down Expand Up @@ -15940,15 +15949,11 @@ fn select_runtime_update_source<'a>(

fn select_installed_update_runtime<'a>(
manifests: &'a [therock::InstalledRuntimeManifest],
source: &therock::InstalledRuntimeManifest,
latest_version: &str,
target_runtime_key: &str,
) -> Option<&'a therock::InstalledRuntimeManifest> {
manifests.iter().find(|manifest| {
manifest.channel == source.channel
&& manifest.format == source.format
&& manifest.family == source.family
&& manifest.version == latest_version
})
manifests
.iter()
.find(|manifest| manifest.runtime_key == target_runtime_key)
}

pub(crate) fn render_automations_text(paths: &AppPaths, config: &RocmCliConfig) -> Result<String> {
Expand Down Expand Up @@ -30018,32 +30023,40 @@ ID_LIKE="suse opensuse"
}

#[test]
fn installed_update_runtime_matches_latest_version_and_family() {
let mut source = test_runtime_manifest_for_update(
"old-gfx120",
fn installed_update_runtime_is_selected_by_exact_target_key() {
// Everything a version match would have keyed on is identical here:
// same channel, format, family and version. Only the composition-keyed
// runtime key tells the freshly installed repair apart from the stale
// runtime it was installed to replace.
let stale = test_runtime_manifest_for_update(
"release-wheel-multi-arch-7-14-0",
"therock-release:gfx120X-all",
"gfx120X-all",
"7.13.0a20260416",
"7.14.0",
);
source.channel = "release".to_owned();
let wrong_family = test_runtime_manifest_for_update(
"new-gfx110",
"release-wheel-multi-arch-7-14-0-ffffffffffffffff",
"therock-release:gfx110X-all",
"gfx110X-all",
"7.14.0a20260531",
"7.14.0",
);
let target = test_runtime_manifest_for_update(
"new-gfx120",
let repaired = test_runtime_manifest_for_update(
"release-wheel-multi-arch-7-14-0-0123456789abcdef",
"therock-release:gfx120X-all",
"gfx120X-all",
"7.14.0a20260531",
"7.14.0",
);
let manifests = vec![wrong_family, target.clone()];
let manifests = vec![stale, wrong_family, repaired.clone()];

let selected = select_installed_update_runtime(&manifests, &source, "7.14.0a20260531")
.expect("matching updated runtime should be selected");
let selected = select_installed_update_runtime(&manifests, &repaired.runtime_key)
.expect("the side-by-side repair must be selected by its exact key");
assert_eq!(selected.runtime_key, repaired.runtime_key);

assert_eq!(selected.runtime_key, target.runtime_key);
assert!(
select_installed_update_runtime(&manifests, "release-wheel-multi-arch-7-15-0")
.is_none(),
"an install that wrote no manifest for the planned key must not resolve to a sibling"
);
}

fn write_test_pip_runtime(
Expand Down Expand Up @@ -30117,6 +30130,7 @@ ID_LIKE="suse opensuse"
..therock::RocmSdkPythonProbe::default()
}),
sdk_torch: None,
wheel_composition: None,
read_only: false,
imported_from: None,
installed_at_unix_ms,
Expand Down Expand Up @@ -30156,6 +30170,7 @@ ID_LIKE="suse opensuse"
pip_cache_dir: None,
rocm_sdk: None,
sdk_torch: None,
wheel_composition: None,
read_only: false,
imported_from: None,
installed_at_unix_ms: 1,
Expand Down
1 change: 1 addition & 0 deletions apps/rocm/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,7 @@ mod tests {
pip_cache_dir: None,
rocm_sdk: None,
sdk_torch: None,
wheel_composition: None,
read_only: false,
imported_from: None,
installed_at_unix_ms,
Expand Down
Loading
Loading