Skip to content
Open
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
66 changes: 66 additions & 0 deletions apps/rocm/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6869,6 +6869,70 @@ fn ensure_torch_runtime_dep(approved: bool, dep: &TorchRuntimeDep) {
}
}

/// Refuse an `apt-get install` step that would remove ROCm/AMDGPU packages.
///
/// `apt-get install -y` assumes yes for *removals* as well as installs, so a
/// dependency solution that evicts the ROCm stack would otherwise be applied
/// unattended (this runs automatically under `rocm install sdk` whenever root or
/// passwordless sudo is available). Simulating first turns that silent breakage
/// into an actionable error naming every package at risk.
///
/// Only apt is gated: the dnf/zypper/pacman plans install additive runtime
/// packages and have no equivalent assume-yes removal path. A simulation that
/// cannot be run (apt missing, transient failure) is not treated as a refusal —
/// the real command reports the failure with better context.
fn ensure_apt_install_preserves_rocm(argv: &[String]) -> Result<()> {
let Some(simulate_argv) = rocm_core::openmpi::apt_simulate_argv(argv) else {
return Ok(());
};
let (program, args) = simulate_argv
.split_first()
.context("apt simulation has no program to run")?;
let Ok(output) = ProcessCommand::new(program)
.args(args)
.stdin(Stdio::null())
.output()
else {
return Ok(());
};
if !output.status.success() {
return Ok(());
}

let removals =
rocm_core::openmpi::parse_apt_simulate_removals(&String::from_utf8_lossy(&output.stdout));
let protected: Vec<&String> = removals
.iter()
.filter(|package| rocm_core::openmpi::is_protected_rocm_package(package))
.collect();
if protected.is_empty() {
return Ok(());
}

let rendered = |packages: &[&String]| {
packages
.iter()
.map(|package| package.as_str())
.collect::<Vec<_>>()
.join(", ")
};
let others: Vec<&String> = removals
.iter()
.filter(|package| !rocm_core::openmpi::is_protected_rocm_package(package))
.collect();
let other_detail = if others.is_empty() {
String::new()
} else {
format!("\n other packages it would remove: {}", rendered(&others))
};
bail!(
"refusing to run `{}`: it would remove ROCm packages and break this installation\n ROCm packages it would remove: {}{}\n action: install the dependency manually after resolving the conflict (for example with a package version that does not evict ROCm), then rerun",
argv.join(" "),
rendered(&protected),
other_detail
);
}

fn run_system_package_install_plan(
plan: &rocm_core::openmpi::SystemPackageInstallPlan,
) -> Result<()> {
Expand All @@ -6878,6 +6942,8 @@ fn run_system_package_install_plan(
// are not already root (where `sudo` may be absent); the argv runs
// directly without a shell.
let argv = command.resolved_argv(root);
// Never let an automatic dependency install evict the ROCm stack.
ensure_apt_install_preserves_rocm(&argv)?;
// Inherit stdin so an interactive `sudo` password prompt (the case the
// `--yes` approval exists for) can be answered. When already root or
// passwordless sudo is configured, sudo does not prompt and the inherited
Expand Down
192 changes: 192 additions & 0 deletions crates/rocm-core/src/openmpi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,95 @@ pub(crate) fn parse_os_release_field(text: &str, key: &str) -> Option<String> {
None
}

/// Build a non-mutating apt invocation for a planned `apt-get install` command.
///
/// Returns `None` for other commands. Any `sudo` prefix is dropped along with
/// assume-yes flags: `apt-get -s` changes nothing, so it needs neither root nor
/// approval, and keeping the resulting argv visibly non-destructive makes it
/// safe to log and test.
pub fn apt_simulate_argv(argv: &[String]) -> Option<Vec<String>> {
let argv = match argv.split_first() {
Some((program, rest)) if program == "sudo" => rest,
_ => argv,
};
if argv.first().map(String::as_str) != Some("apt-get")
|| !argv.iter().any(|arg| arg == "install")
{
return None;
}

let mut simulated = Vec::with_capacity(argv.len() + 1);
simulated.push("apt-get".to_owned());
simulated.push("-s".to_owned());
simulated.extend(
argv.iter()
.skip(1)
.filter(|arg| !matches!(arg.as_str(), "-y" | "--yes" | "--assume-yes"))
.cloned(),
);
Some(simulated)
}

/// Parse package removals from `apt-get -s` output.
///
/// Apt emits one `Remv <package> ...` operation per package after its readable
/// summary. Parsing those operation records avoids depending on summary wrapping
/// and strips apt's optional trailing `*` marker.
pub fn parse_apt_simulate_removals(output: &str) -> Vec<String> {
output
.lines()
.filter_map(|line| {
let package = line
.trim_start()
.strip_prefix("Remv ")?
.split_whitespace()
.next()?;
Some(package.trim_end_matches('*').to_owned())
})
.collect()
}

/// Whether an apt package belongs to the ROCm/AMDGPU system stack and must not
/// be removed as a side effect of automatic dependency setup.
///
/// Matching is by *family prefix* rather than an exhaustive package list: ROCm
/// ships hundreds of packages and adds more every release, so enumerating exact
/// names would silently stop protecting whatever was added last. The prefixes
/// below cover the component families ROCm publishes — note that many are
/// dashless (`hipblas`, `rocminfo`, `hsakmt-roct`, `llvm-amdgpu`), so a
/// dash-separated match would miss them.
///
/// This deliberately favors recall over precision. A false positive only makes
/// the CLI refuse an automatic install and ask the user to resolve it by hand,
/// whereas a false negative silently breaks their ROCm installation. The
/// prefixes are still specific enough not to match the packages these plans
/// legitimately install or replace (`openmpi-bin`, `libnuma1`, `mpich`, ...).
pub fn is_protected_rocm_package(package: &str) -> bool {
const PREFIXES: &[&str] = &[
"amd-smi",
"amdgpu",
"comgr",
// hip, hipcc, hipblas, hipfft, hipsolver, hip-runtime-amd, hipify-clang
"hip",
// hsa-rocr, hsakmt-roct, hsa-amd-aqlprofile
"hsa",
"llvm-amdgpu",
"migraphx",
"miopen",
"mivisionx",
"openmp-extras",
"rccl",
// rocm*, plus the dashless libraries: rocblas, rocfft, rocrand,
// rocsolver, rocsparse, rocthrust, rocprim, rocprofiler, roctracer,
// rocalution, rocwmma, rocdecode, rocjpeg, rocdbgapi, rocminfo, rocr-runtime
"roc",
"rpp",
];
// Strip any architecture qualifier (`amdgpu-core:amd64`) before matching.
let package = package.split(':').next().unwrap_or(package);
PREFIXES.iter().any(|prefix| package.starts_with(prefix))
}

fn resolve_package_manager(os_id: &str, id_like: &str) -> Option<PackageManager> {
const APT: &[&str] = &[
"ubuntu",
Expand Down Expand Up @@ -1299,6 +1388,109 @@ mod tests {
assert!(plan.reason.contains("manually"));
}

#[test]
fn apt_simulate_argv_drops_assume_yes_and_simulates() {
let install = InstallCommand::sudo(&["apt-get", "install", "-y", "openmpi-bin"]);
assert_eq!(
apt_simulate_argv(&install.argv).unwrap(),
vec!["apt-get", "-s", "install", "openmpi-bin"]
);
// The executor passes the resolved argv, which carries a `sudo` prefix
// when not already root; the simulation must still be recognized.
assert_eq!(
apt_simulate_argv(&install.resolved_argv(false)).unwrap(),
vec!["apt-get", "-s", "install", "openmpi-bin"]
);
// Non-install apt commands and other package managers are not simulated.
assert!(apt_simulate_argv(&InstallCommand::sudo(&["apt-get", "update"]).argv).is_none());
assert!(
apt_simulate_argv(&InstallCommand::sudo(&["dnf", "install", "-y", "openmpi"]).argv)
.is_none()
);
}

#[test]
fn parses_removals_from_apt_simulation() {
// Mirrors the EAI-7957 report: OpenMPI setup pulling older distro
// toolchain packages while marking the ROCm stack for removal.
let output = "\
Reading package lists...
The following packages will be REMOVED:
rocm rocm-hip rocm-hip-runtime-dev mivisionx-dev rpp-dev
The following NEW packages will be installed:
openmpi-bin libopenmpi-dev
Remv rocm [7.14.0]
Remv rocm-hip [7.14.0]
Remv rocm-hip-runtime-dev* [7.14.0]
Remv mivisionx-dev [7.14.0]
Remv rpp-dev [7.14.0]
Inst openmpi-bin (4.1.6 Ubuntu:24.04 [amd64])
";
assert_eq!(
parse_apt_simulate_removals(output),
vec![
"rocm",
"rocm-hip",
"rocm-hip-runtime-dev",
"mivisionx-dev",
"rpp-dev"
]
);
// A purely additive transaction removes nothing.
assert!(parse_apt_simulate_removals("Inst libnuma1 (2.0.18 [amd64])\n").is_empty());
}

#[test]
fn protects_rocm_stack_packages_only() {
for package in [
// The packages the EAI-7957 report saw apt mark for removal.
"rocm",
"rocm-hip",
"rocm-hip-runtime-dev",
"mivisionx-dev",
"rpp-dev",
// Core packages whose names do not follow a `<family>-` shape, so a
// dash-separated match would miss them and let apt remove the ROCm
// stack anyway.
"hsakmt-roct",
"llvm-amdgpu",
"rocminfo",
"hipblas",
"rocblas",
"hipcc",
"comgr",
"hsa-rocr",
"openmp-extras-runtime",
// Driver packages and architecture-qualified names.
"amdgpu-dkms",
"hip-runtime-amd",
"amdgpu-core:amd64",
] {
assert!(
is_protected_rocm_package(package),
"{package} must be protected"
);
}
// The packages these plans legitimately install, and the MPI
// implementation apt may swap out to satisfy them, must stay removable
// or the guard would block its own install.
for package in [
"openmpi-bin",
"libopenmpi-dev",
"openmpi4",
"mpich",
"libmpich-dev",
"libnuma1",
"numactl",
"libatomic1",
] {
assert!(
!is_protected_rocm_package(package),
"{package} must not be protected"
);
}
}

#[test]
fn install_command_prepends_sudo_only_when_not_root() {
let command = InstallCommand::sudo(&["dnf", "install", "-y", "openmpi"]);
Expand Down
27 changes: 27 additions & 0 deletions tests/e2e-cucumber/features/dependency_guard.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
Feature: System dependency installs preserve ROCm

# vLLM needs the OpenMPI runtime, which the CLI installs through the host
# package manager. `apt-get install -y` assumes yes for *removals* as well as
# installs, so when apt chose to satisfy OpenMPI by evicting the ROCm stack it
# did so unattended — reported in the field as `rocm install sdk` removing
# rocm, rocm-hip, rocm-hip-runtime-dev, mivisionx-dev and rpp-dev while pulling
# an older toolchain, breaking a working ROCm install.
#
# The package manager is planted rather than real, so the scenario owns the
# dependency solution apt reports and needs no GPU, no engine install and no
# network. Linux-only: the dependency-setup path does not run on Windows.
#
# It also assumes an apt host, which every Linux lane is — the guard is
# apt-specific by design, because only `apt-get install -y` assumes yes for
# removals. On a Linux host whose /etc/os-release selects dnf/zypper/pacman
# the CLI plans a different, additive command and there is no refusal to
# assert; move this behind a package-manager capability gate if such a lane
# is ever added.
@id:deps-guard-refuses-rocm-removal @requires-os:linux
Scenario: 1 - Installing a dependency never silently removes ROCm
Given a machine with a registered ROCm runtime
And installing OpenMPI would remove the ROCm packages
When the user installs the vLLM engine and approves system changes
Then the CLI refuses instead of removing them
And it does so before changing anything on the system
And it lists every ROCm package that would have been removed
1 change: 1 addition & 0 deletions tests/e2e-cucumber/tests/e2e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod e2e {
pub mod bench_steps;
pub mod chat_steps;
pub mod dash_steps;
pub mod dependency_guard_steps;
pub mod diagnose_steps;
pub mod engines_steps;
pub mod examine_steps;
Expand Down
Loading
Loading