Repository navigation
feat(cli): support Podman in openshell doctor check #3694
Description
Activity
I'd like to take this accepted issue.
I would avoid adding a second ad-hoc driver detector inside
doctor check. The check should use the same resolved compute-driver choice that normal gateway startup would use, otherwisedoctorcan disagree with the runtime it is supposed to diagnose.Proposed first slice:
- resolve the configured compute driver using the existing config semantics;
- Docker -> preserve the current
docker infocheck; - Podman -> run the equivalent Podman connectivity/version check;
- report the selected driver in the output;
- make Podman failure guidance name the relevant socket/config rather than mentioning
DOCKER_HOST; - preserve the existing exit-code contract.
Tests:
- Docker success/failure unchanged;
- Podman success;
- Podman executable/socket unreachable;
- selected Podman with Docker absent must not fail because Docker is absent.
I would leave VM/Kubernetes-specific doctor checks for follow-ups.
If that scope matches the accepted issue, I can implement it.
@kvnloo — since you've claimed this, wanted to share an alternative to consider before you start, rather than duplicate your effort. Not trying to take the issue.
Your plan's principle (avoid a second driver detector, don't let
doctordisagree with the runtime it's diagnosing) is right, but resolving "the configured driver" the same way gateway startup does isn't actually cheap: that logic lives inopenshell-server(ComputeDriverRegistry,config_file.rs's schema-versionedgateway.tomlparsing), whichopenshell-clidoesn't depend on in production — only as a[dev-dependencies]test-support crate. Makingdoctor checkshare it means either pulling the whole gateway/driver-registry stack into the client binary, or re-parsinggateway.tomla second time outside the canonical loader (which risks exactly the kind of drift you're trying to avoid). It also doesn't have an answer for the common case wheredoctor checkruns before anygateway.tomlexists yet — it's a hidden pre-install dev diagnostic (hide = trueinmain.rs), so there's often nothing to resolve.There's also prior art worth knowing about: #3748 (ericcurtin, closed but reopenable — he said "re-open if you'd like") took a "Docker if installed, Podman otherwise" approach. That has a gap: on a host with both installed, Podman never gets checked even if that's what you actually use.
Alternative: check every installed runtime independently, don't try to resolve "the" configured one at all.
- For each of Docker/Podman, only report a section if that binary is actually on
PATH— skip entirely if not, so a Docker-only host's output is unchanged and a Podman-only host doesn't get a spurious "Docker: not found" line. - No config-file reading, no
openshell-serverdependency, no resolution step to keep in sync with anything — nothing to drift. - Default (no flag): pass if at least one installed runtime is healthy — matches the command's own docstring ("validate system prerequisites for running a gateway," not "validate my specific driver"). Correctly handles the case your approach and feat(cli): support Podman in doctor check #3748 both miss: a host with a broken, unused Docker install sitting around and a perfectly healthy Podman it actually uses — reports success instead of a false failure.
- Added
--driver <docker|podman>for the one case default behavior can't resolve on its own: both installed, and the one you actually use is broken while the other happens to work. Passing--drivermakes that one runtime's result the only thing that matters, with zero ambiguity.
Diff below compiles clean (
cargo check -p openshell-cli, zero warnings). No test changes included yet — the existingdocker_preflight.rstests rely on ambientPATHincluding whatever's actually installed on the runner, so they'd need PATH-isolation fixes to stay deterministic under this change (happy to detail that separately if useful).diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index f656fd883..b51d7d142 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1402,13 +1402,35 @@ enum GatewayCommands { enum DoctorCommands { /// Validate system prerequisites for running a gateway. /// - /// Checks that a Docker-compatible runtime is installed, running, and - /// reachable. Reports version info and socket path. + /// Checks every installed local container runtime (Docker, Podman) and + /// reports each independently. Reports version info and socket path. + /// Pass --driver to check only one runtime, which is required for a + /// meaningful pass/fail result on a host with more than one installed. /// /// Examples: /// openshell doctor check + /// openshell doctor check --driver podman #[command(help_template = LEAF_HELP_TEMPLATE)] - Check, + Check { + /// Only check this runtime, ignoring any others installed. + #[arg(long)] + driver: Option<DoctorRuntime>, + }, +} + +#[derive(Clone, Debug, ValueEnum)] +enum DoctorRuntime { + Docker, + Podman, +} + +impl DoctorRuntime { + fn as_str(&self) -> &'static str { + match self { + Self::Docker => "docker", + Self::Podman => "podman", + } + } } #[derive(Subcommand, Debug)] @@ -2683,8 +2705,8 @@ async fn run_async() -> Result<()> { Some(Commands::Doctor { command: Some(command), }) => match command { - DoctorCommands::Check => { - run::doctor_check()?; + DoctorCommands::Check { driver } => { + run::doctor_check(driver.as_ref().map(DoctorRuntime::as_str))?; } }, Some(Commands::Doctor { command: None }) => { diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index ce63bc9f7..fb8fcea70 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -209,46 +209,193 @@ fn current_user_to_json(view: &CurrentUserView) -> serde_json::Value { }) } +/// Outcome of probing a single container runtime binary. +enum RuntimeCheck { + /// The binary is not on `PATH` at all. + NotInstalled, + /// The binary ran successfully; carries its reported version string. + Ok(String), + /// The binary is on `PATH` but the command failed; carries its stderr. + Failed(String), +} + +/// Run `binary args...` and classify the result. +/// +/// Distinguishes "not installed" (spawn fails with `NotFound`) from +/// "installed but unreachable" (the process runs and exits non-zero) so +/// callers can skip runtimes that aren't present instead of reporting a +/// spurious failure for them. +fn check_runtime(binary: &str, args: &[&str]) -> Result<RuntimeCheck> { + match Command::new(binary).args(args).output() { + Ok(output) if output.status.success() => { + let version = String::from_utf8_lossy(&output.stdout).trim().to_string(); + Ok(RuntimeCheck::Ok(version)) + } + Ok(output) => { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + Ok(RuntimeCheck::Failed(stderr)) + } + Err(err) if err.kind() == ErrorKind::NotFound => Ok(RuntimeCheck::NotInstalled), + Err(err) => Err(err) + .into_diagnostic() + .wrap_err(format!("failed to execute {binary} {}", args.join(" "))), + } +} + +/// Print the Docker section and return whether it's healthy. +fn report_docker(stdout: &mut impl Write, check: &RuntimeCheck) -> Result<bool> { + write!(stdout, " Docker ............. ").into_diagnostic()?; + match check { + RuntimeCheck::Ok(version) => { + writeln!(stdout, "ok (version {version})").into_diagnostic()?; + write!(stdout, " DOCKER_HOST ........ ").into_diagnostic()?; + match std::env::var("DOCKER_HOST") { + Ok(val) => writeln!(stdout, "{val}").into_diagnostic()?, + Err(_) => writeln!(stdout, "(not set, using default socket)").into_diagnostic()?, + } + Ok(true) + } + RuntimeCheck::Failed(stderr) => { + writeln!(stdout, "FAILED").into_diagnostic()?; + writeln!(stdout, " {stderr}").into_diagnostic()?; + writeln!( + stdout, + " Check DOCKER_HOST and run 'docker info' directly for details." + ) + .into_diagnostic()?; + Ok(false) + } + RuntimeCheck::NotInstalled => unreachable!("caller filters out NotInstalled"), + } +} + +/// Print the Podman section and return whether it's healthy. +fn report_podman(stdout: &mut impl Write, check: &RuntimeCheck) -> Result<bool> { + write!(stdout, " Podman ............. ").into_diagnostic()?; + match check { + RuntimeCheck::Ok(version) => { + writeln!(stdout, "ok (version {version})").into_diagnostic()?; + write!(stdout, " CONTAINER_HOST ...... ").into_diagnostic()?; + match std::env::var("CONTAINER_HOST") { + Ok(val) => writeln!(stdout, "{val}").into_diagnostic()?, + Err(_) => writeln!(stdout, "(not set, using default socket)").into_diagnostic()?, + } + Ok(true) + } + RuntimeCheck::Failed(stderr) => { + writeln!(stdout, "FAILED").into_diagnostic()?; + writeln!(stdout, " {stderr}").into_diagnostic()?; + writeln!( + stdout, + " Check CONTAINER_HOST and run 'podman info' directly for details." + ) + .into_diagnostic()?; + Ok(false) + } + RuntimeCheck::NotInstalled => unreachable!("caller filters out NotInstalled"), + } +} + /// Validate system prerequisites for running a gateway. /// -/// Checks Docker connectivity and reports the result. Returns exit code 0 -/// if all checks pass, 1 otherwise. -pub fn doctor_check() -> Result<()> { - use std::io::Write; +/// With `driver` unset, checks every locally installed container runtime +/// (Docker, Podman) and reports each independently, in the style the +/// original Docker-only check used. A runtime that isn't installed is +/// omitted entirely rather than reported as failed, so a Podman-only host's +/// output doesn't list an absent Docker. Passing overall requires at least +/// one installed runtime to be reachable. +/// +/// With `driver` set to `"docker"` or `"podman"`, checks only that runtime +/// and bases the result solely on it — the other runtime's presence or +/// health has no effect. This is the only way to get a meaningful pass/fail +/// signal on a host where more than one runtime is installed: without it, a +/// broken driver you actually use can pass silently because an unrelated, +/// unused runtime happens to work. +pub fn doctor_check(driver: Option<&str>) -> Result<()> { let mut stdout = std::io::stdout().lock(); writeln!(stdout, "Checking system prerequisites...\n").into_diagnostic()?; - - // --- Docker connectivity --- - write!(stdout, " Docker ............. ").into_diagnostic()?; stdout.flush().into_diagnostic()?; - let output = Command::new("docker") - .args(["info", "--format", "{{.ServerVersion}}"]) - .output() - .into_diagnostic() - .wrap_err("failed to execute docker info")?; - - if output.status.success() { - let version = String::from_utf8_lossy(&output.stdout); - let version_str = version.trim(); - writeln!(stdout, "ok (version {version_str})").into_diagnostic()?; + match driver { + Some(driver) => doctor_check_single(&mut stdout, driver), + None => doctor_check_auto(&mut stdout), + } +} - // --- DOCKER_HOST --- - write!(stdout, " DOCKER_HOST ........ ").into_diagnostic()?; - match std::env::var("DOCKER_HOST") { - Ok(val) => writeln!(stdout, "{val}").into_diagnostic()?, - Err(_) => writeln!(stdout, "(not set, using default socket)").into_diagnostic()?, +/// Check exactly one named runtime; its result alone decides pass/fail. +fn doctor_check_single(stdout: &mut impl Write, driver: &str) -> Result<()> { + let healthy = match driver { + "docker" => { + match check_runtime("docker", &["info", "--format", "{{.ServerVersion}}"])? { + RuntimeCheck::NotInstalled => { + writeln!(stdout).into_diagnostic()?; + return Err(miette::miette!("docker is not installed")); + } + check => report_docker(stdout, &check)?, + } } + "podman" => { + match check_runtime("podman", &["info", "--format", "{{.Version.Version}}"])? { + RuntimeCheck::NotInstalled => { + writeln!(stdout).into_diagnostic()?; + return Err(miette::miette!("podman is not installed")); + } + check => report_podman(stdout, &check)?, + } + } + other => { + return Err(miette::miette!( + "unsupported driver '{other}': expected docker or podman" + )); + } + }; - writeln!(stdout, "\nAll checks passed.").into_diagnostic()?; - return Ok(()); + if !healthy { + writeln!(stdout).into_diagnostic()?; + return Err(miette::miette!( + "{driver} is not reachable; see the FAILED check above" + )); } - writeln!(stdout, "FAILED").into_diagnostic()?; - writeln!(stdout).into_diagnostic()?; - let stderr = String::from_utf8_lossy(&output.stderr); - Err(miette::miette!("docker info failed: {}", stderr.trim())) + writeln!(stdout, "\nAll checks passed.").into_diagnostic()?; + Ok(()) +} + +/// Check every installed runtime; passes if at least one is healthy. +fn doctor_check_auto(stdout: &mut impl Write) -> Result<()> { + let docker = check_runtime("docker", &["info", "--format", "{{.ServerVersion}}"])?; + let podman = check_runtime("podman", &["info", "--format", "{{.Version.Version}}"])?; + + let mut installed = 0usize; + let mut healthy = 0usize; + + if !matches!(docker, RuntimeCheck::NotInstalled) { + installed += 1; + healthy += usize::from(report_docker(stdout, &docker)?); + } + + if !matches!(podman, RuntimeCheck::NotInstalled) { + installed += 1; + healthy += usize::from(report_podman(stdout, &podman)?); + } + + if installed == 0 { + writeln!(stdout).into_diagnostic()?; + return Err(miette::miette!( + "no supported container runtime found on PATH: install Docker or Podman" + )); + } + + if healthy == 0 { + writeln!(stdout).into_diagnostic()?; + return Err(miette::miette!( + "no installed container runtime is reachable; see the FAILED checks above" + )); + } + + writeln!(stdout, "\nAll checks passed.").into_diagnostic()?; + Ok(()) } fn sandbox_should_persist(keep: bool, forward: Option<&ForwardSpec>, expose: Option<u16>) -> bool {
Happy to hand off the branch or just leave this here for you to adapt — whichever's useful. Also cc @ericcurtin since #3748 covered the same ground.
Reacted by Kevin RajanReacted by Kevin Rajan- For each of Docker/Podman, only report a section if that binary is actually on
@kvnloo — since you've claimed this, wanted to share an alternative to consider before you start, rather than duplicate your effort. Not trying to take the issue.
Your plan's principle (avoid a second driver detector, don't let
doctordisagree with the runtime it's diagnosing) is right, but resolving "the configured driver" the same way gateway startup does isn't actually cheap: that logic lives inopenshell-server(ComputeDriverRegistry,config_file.rs's schema-versionedgateway.tomlparsing), whichopenshell-clidoesn't depend on in production — only as a[dev-dependencies]test-support crate. Makingdoctor checkshare it means either pulling the whole gateway/driver-registry stack into the client binary, or re-parsinggateway.tomla second time outside the canonical loader (which risks exactly the kind of drift you're trying to avoid). It also doesn't have an answer for the common case wheredoctor checkruns before anygateway.tomlexists yet — it's a hidden pre-install dev diagnostic (hide = trueinmain.rs), so there's often nothing to resolve.There's also prior art worth knowing about: #3748 (ericcurtin, closed but reopenable — he said "re-open if you'd like") took a "Docker if installed, Podman otherwise" approach. That has a gap: on a host with both installed, Podman never gets checked even if that's what you actually use.
Alternative: check every installed runtime independently, don't try to resolve "the" configured one at all.
- For each of Docker/Podman, only report a section if that binary is actually on
PATH— skip entirely if not, so a Docker-only host's output is unchanged and a Podman-only host doesn't get a spurious "Docker: not found" line. - No config-file reading, no
openshell-serverdependency, no resolution step to keep in sync with anything — nothing to drift. - Default (no flag): pass if at least one installed runtime is healthy — matches the command's own docstring ("validate system prerequisites for running a gateway," not "validate my specific driver"). Correctly handles the case your approach and feat(cli): support Podman in doctor check #3748 both miss: a host with a broken, unused Docker install sitting around and a perfectly healthy Podman it actually uses — reports success instead of a false failure.
- Added
--driver <docker|podman>for the one case default behavior can't resolve on its own: both installed, and the one you actually use is broken while the other happens to work. Passing--drivermakes that one runtime's result the only thing that matters, with zero ambiguity.
Diff below compiles clean (
cargo check -p openshell-cli, zero warnings). No test changes included yet — the existingdocker_preflight.rstests rely on ambientPATHincluding whatever's actually installed on the runner, so they'd need PATH-isolation fixes to stay deterministic under this change (happy to detail that separately if useful).diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs
index f656fd883..b51d7d142 100644
--- a/crates/openshell-cli/src/main.rs
+++ b/crates/openshell-cli/src/main.rs
@@ -1402,13 +1402,35 @@ enum GatewayCommands {
enum DoctorCommands {
/// Validate system prerequisites for running a gateway.
///- /// Checks that a Docker-compatible runtime is installed, running, and
- /// reachable. Reports version info and socket path.
- /// Checks every installed local container runtime (Docker, Podman) and
- /// reports each independently. Reports version info and socket path.
- /// Pass --driver to check only one runtime, which is required for a
- /// meaningful pass/fail result on a host with more than one installed.
///
/// Examples:
/// openshell doctor check - /// openshell doctor check --driver podman
#[command(help_template = LEAF_HELP_TEMPLATE)]
- Check,
- Check {
-
/// Only check this runtime, ignoring any others installed. -
#[arg(long)] -
driver: Option<DoctorRuntime>, - },
+}
+#[derive(Clone, Debug, ValueEnum)]
+enum DoctorRuntime {- Docker,
- Podman,
+}
+impl DoctorRuntime {
- fn as_str(&self) -> &'static str {
-
match self { -
Self::Docker => "docker", -
Self::Podman => "podman", -
} - }
}
#[derive(Subcommand, Debug)]
@@ -2683,8 +2705,8 @@ async fn run_async() -> Result<()> {
Some(Commands::Doctor {
command: Some(command),
}) => match command {-
DoctorCommands::Check => { -
run::doctor_check()?;
-
DoctorCommands::Check { driver } => { -
run::doctor_check(driver.as_ref().map(DoctorRuntime::as_str))?; } }, Some(Commands::Doctor { command: None }) => {
diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs
index ce63bc9f7..fb8fcea70 100644
--- a/crates/openshell-cli/src/run.rs
+++ b/crates/openshell-cli/src/run.rs
@@ -209,46 +209,193 @@ fn current_user_to_json(view: &CurrentUserView) -> serde_json::Value {
})
}+/// Outcome of probing a single container runtime binary.
+enum RuntimeCheck {- /// The binary is not on
PATHat all. - NotInstalled,
- /// The binary ran successfully; carries its reported version string.
- Ok(String),
- /// The binary is on
PATHbut the command failed; carries its stderr. - Failed(String),
+}
+/// Run
binary args...and classify the result.
+///
+/// Distinguishes "not installed" (spawn fails withNotFound) from
+/// "installed but unreachable" (the process runs and exits non-zero) so
+/// callers can skip runtimes that aren't present instead of reporting a
+/// spurious failure for them.
+fn check_runtime(binary: &str, args: &[&str]) -> Result {- match Command::new(binary).args(args).output() {
-
Ok(output) if output.status.success() => { -
let version = String::from_utf8_lossy(&output.stdout).trim().to_string(); -
Ok(RuntimeCheck::Ok(version)) -
} -
Ok(output) => { -
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); -
Ok(RuntimeCheck::Failed(stderr)) -
} -
Err(err) if err.kind() == ErrorKind::NotFound => Ok(RuntimeCheck::NotInstalled), -
Err(err) => Err(err) -
.into_diagnostic() -
.wrap_err(format!("failed to execute {binary} {}", args.join(" "))), - }
+}
+/// Print the Docker section and return whether it's healthy.
+fn report_docker(stdout: &mut impl Write, check: &RuntimeCheck) -> Result {- write!(stdout, " Docker ............. ").into_diagnostic()?;
- match check {
-
RuntimeCheck::Ok(version) => { -
writeln!(stdout, "ok (version {version})").into_diagnostic()?; -
write!(stdout, " DOCKER_HOST ........ ").into_diagnostic()?; -
match std::env::var("DOCKER_HOST") { -
Ok(val) => writeln!(stdout, "{val}").into_diagnostic()?, -
Err(_) => writeln!(stdout, "(not set, using default socket)").into_diagnostic()?, -
} -
Ok(true) -
} -
RuntimeCheck::Failed(stderr) => { -
writeln!(stdout, "FAILED").into_diagnostic()?; -
writeln!(stdout, " {stderr}").into_diagnostic()?; -
writeln!( -
stdout, -
" Check DOCKER_HOST and run 'docker info' directly for details." -
) -
.into_diagnostic()?; -
Ok(false) -
} -
RuntimeCheck::NotInstalled => unreachable!("caller filters out NotInstalled"), - }
+}
+/// Print the Podman section and return whether it's healthy.
+fn report_podman(stdout: &mut impl Write, check: &RuntimeCheck) -> Result {- write!(stdout, " Podman ............. ").into_diagnostic()?;
- match check {
-
RuntimeCheck::Ok(version) => { -
writeln!(stdout, "ok (version {version})").into_diagnostic()?; -
write!(stdout, " CONTAINER_HOST ...... ").into_diagnostic()?; -
match std::env::var("CONTAINER_HOST") { -
Ok(val) => writeln!(stdout, "{val}").into_diagnostic()?, -
Err(_) => writeln!(stdout, "(not set, using default socket)").into_diagnostic()?, -
} -
Ok(true) -
} -
RuntimeCheck::Failed(stderr) => { -
writeln!(stdout, "FAILED").into_diagnostic()?; -
writeln!(stdout, " {stderr}").into_diagnostic()?; -
writeln!( -
stdout, -
" Check CONTAINER_HOST and run 'podman info' directly for details." -
) -
.into_diagnostic()?; -
Ok(false) -
} -
RuntimeCheck::NotInstalled => unreachable!("caller filters out NotInstalled"), - }
+}
/// Validate system prerequisites for running a gateway.
///
-/// Checks Docker connectivity and reports the result. Returns exit code 0
-/// if all checks pass, 1 otherwise.
-pub fn doctor_check() -> Result<()> {-
use std::io::Write;
+/// Withdriverunset, checks every locally installed container runtime
+/// (Docker, Podman) and reports each independently, in the style the
+/// original Docker-only check used. A runtime that isn't installed is
+/// omitted entirely rather than reported as failed, so a Podman-only host's
+/// output doesn't list an absent Docker. Passing overall requires at least
+/// one installed runtime to be reachable.
+///
+/// Withdriverset to"docker"or"podman", checks only that runtime
+/// and bases the result solely on it — the other runtime's presence or
+/// health has no effect. This is the only way to get a meaningful pass/fail
+/// signal on a host where more than one runtime is installed: without it, a
+/// broken driver you actually use can pass silently because an unrelated,
+/// unused runtime happens to work.
+pub fn doctor_check(driver: Option<&str>) -> Result<()> {
let mut stdout = std::io::stdout().lock();writeln!(stdout, "Checking system prerequisites...\n").into_diagnostic()?;
-
// --- Docker connectivity ---
-
write!(stdout, " Docker ............. ").into_diagnostic()?;
stdout.flush().into_diagnostic()?; -
let output = Command::new("docker")
-
.args(["info", "--format", "{{.ServerVersion}}"]) -
.output() -
.into_diagnostic() -
.wrap_err("failed to execute docker info")?; -
if output.status.success() {
-
let version = String::from_utf8_lossy(&output.stdout); -
let version_str = version.trim(); -
writeln!(stdout, "ok (version {version_str})").into_diagnostic()?;
- match driver {
-
Some(driver) => doctor_check_single(&mut stdout, driver), -
None => doctor_check_auto(&mut stdout), - }
+}
-
// --- DOCKER_HOST --- -
write!(stdout, " DOCKER_HOST ........ ").into_diagnostic()?; -
match std::env::var("DOCKER_HOST") { -
Ok(val) => writeln!(stdout, "{val}").into_diagnostic()?, -
Err(_) => writeln!(stdout, "(not set, using default socket)").into_diagnostic()?,
+/// Check exactly one named runtime; its result alone decides pass/fail.
+fn doctor_check_single(stdout: &mut impl Write, driver: &str) -> Result<()> {- let healthy = match driver {
-
"docker" => { -
match check_runtime("docker", &["info", "--format", "{{.ServerVersion}}"])? { -
RuntimeCheck::NotInstalled => { -
writeln!(stdout).into_diagnostic()?; -
return Err(miette::miette!("docker is not installed")); -
} -
check => report_docker(stdout, &check)?, -
} } -
"podman" => { -
match check_runtime("podman", &["info", "--format", "{{.Version.Version}}"])? { -
RuntimeCheck::NotInstalled => { -
writeln!(stdout).into_diagnostic()?; -
return Err(miette::miette!("podman is not installed")); -
} -
check => report_podman(stdout, &check)?, -
} -
} -
other => { -
return Err(miette::miette!( -
"unsupported driver '{other}': expected docker or podman" -
)); -
} - };
-
writeln!(stdout, "\nAll checks passed.").into_diagnostic()?; -
return Ok(());
- if !healthy {
-
writeln!(stdout).into_diagnostic()?; -
return Err(miette::miette!( -
"{driver} is not reachable; see the FAILED check above" -
}
));
- writeln!(stdout, "FAILED").into_diagnostic()?;
- writeln!(stdout).into_diagnostic()?;
- let stderr = String::from_utf8_lossy(&output.stderr);
- Err(miette::miette!("docker info failed: {}", stderr.trim()))
- writeln!(stdout, "\nAll checks passed.").into_diagnostic()?;
- Ok(())
+}
+/// Check every installed runtime; passes if at least one is healthy.
+fn doctor_check_auto(stdout: &mut impl Write) -> Result<()> {- let docker = check_runtime("docker", &["info", "--format", "{{.ServerVersion}}"])?;
- let podman = check_runtime("podman", &["info", "--format", "{{.Version.Version}}"])?;
- let mut installed = 0usize;
- let mut healthy = 0usize;
- if !matches!(docker, RuntimeCheck::NotInstalled) {
-
installed += 1; -
healthy += usize::from(report_docker(stdout, &docker)?); - }
- if !matches!(podman, RuntimeCheck::NotInstalled) {
-
installed += 1; -
healthy += usize::from(report_podman(stdout, &podman)?); - }
- if installed == 0 {
-
writeln!(stdout).into_diagnostic()?; -
return Err(miette::miette!( -
"no supported container runtime found on PATH: install Docker or Podman" -
)); - }
- if healthy == 0 {
-
writeln!(stdout).into_diagnostic()?; -
return Err(miette::miette!( -
"no installed container runtime is reachable; see the FAILED checks above" -
)); - }
- writeln!(stdout, "\nAll checks passed.").into_diagnostic()?;
- Ok(())
}
fn sandbox_should_persist(keep: bool, forward: Option<&ForwardSpec>, expose: Option) -> bool {
Happy to hand off the branch or just leave this here for you to adapt — whichever's useful. Also cc @ericcurtin since #3748 covered the same ground.Go for it I'd say @politerealism
Reacted by Polite_realism- For each of Docker/Podman, only report a section if that binary is actually on
Thanks — this is useful context, especially the CLI/server dependency boundary and the pre-config case.
I started a downstream implementation using the “probe installed runtimes independently + explicit "--driver" override” approach rather than duplicating gateway config parsing in the CLI.
One adjustment from the sketch: "OPENSHELL_PODMAN_SOCKET" is an OpenShell driver setting, so the Podman probe explicitly maps it to "podman --url unix://...". Otherwise "doctor" could validate a different Podman connection from the one the gateway actually uses.
The current slice keeps this narrow:
- Docker + Podman only
- default mode probes installed runtimes independently
- "--driver docker|podman" makes a specific runtime authoritative
- Docker behavior/guidance stays intact
- Podman errors name "OPENSHELL_PODMAN_SOCKET"
- tests isolate "PATH" so host Docker/Podman installs can't affect results
- mixed-runtime and socket-forwarding cases are covered
Draft implementation: kvnloo#1
I'm comparing the default pass/fail semantics against #3748, #3211, the gateway's configured/auto-detected driver behavior, and the existing Podman driver checks before promoting it upstream.
I say just throw it in a PR. Your addition is exactly what we needed here. Tag me and I'll review it when your done :)
Ok I re-opened, but if you need any further changes, please build on top :)
- added a commit that references this issue
on Sep 30, 2026
User Story
As an OpenShell user running the Podman compute driver, I want
openshell doctor checkto validate my Podman setup, so that I get the same actionable preflight diagnostics Docker users already get, instead of a check that always reports on Docker regardless of which driver I actually use.Problem Statement
openshell doctor check(crates/openshell-cli/src/run.rs:215-251) unconditionally shells out todocker infoto validate system prerequisites. It has no driver-detection logic and no Podman (or other compute driver) branch, so a user running the Podman driver gets a check that's irrelevant to their actual setup — it reports on Docker connectivity even when Docker isn't installed or in use, and it never validates the Podman socket, version, or rootless configuration the Podman driver actually depends on.Impact / Why This Matters
Today, Podman users have no equivalent of
doctor checkto diagnose a broken Podman setup (missing socket, wrong Podman version, rootless misconfiguration) before runningopenshell. They're left with raw driver/gateway error messages or manualpodman infoinspection, instead of the single actionable pass/fail summary Docker users get. This is inconsistent with OpenShell's multi-driver design, where Podman is a fully supported, first-class compute driver, not a secondary one.Proposed Design
openshell doctor checkshould detect which compute driver is configured (or accept an explicit override) and run the appropriate connectivity check: keep the existingdocker info-based check for Docker, and add an equivalent check for Podman — verifying the Podman socket is reachable, reporting the Podman version, and validating rootless/rootful configuration analogous to whatcrates/openshell-driver-podman/src/driver.rs's own startup check already does inside the driver. The user-facing output format (labeled check line, "ok"/"FAILED" status, actionable guidance text) should stay consistent with the existing Docker check's style.Acceptance Criteria
openshell doctor checkvalidates Podman connectivity when the configured/detected driver is Podman, with output in the same style as the existing Docker check.--format {{.ServerVersion}}.DOCKER_HOST-mentioning guidance for Docker.Alternatives Considered
Leave
doctor checkDocker-only and document it as such. Rejected because it leaves Podman users — a fully supported, first-class driver — without an equivalent preflight diagnostic tool, and the command's own framing ("system prerequisites," not "Docker prerequisites") implies broader driver coverage than it currently delivers.Agent Investigation
Found while implementing #3663 (Podman CI coverage gaps).
e2e/rust/tests/docker_preflight.rstests exactly this Docker-only behavior end to end.doctor_check()incrates/openshell-cli/src/run.rs:215-251unconditionally runsdocker info --format {{.ServerVersion}}with no driver detection at all — confirmed by reading the function directly, not inferred. No Podman-equivalent CLI diagnostic exists. (A newe2e/rust/tests/podman_preflight.rswas added in PR #3690, but it tests a different thing: the Podman driver's behavior when its socket is unreachable, not thedoctor checkCLI command — it does not address this gap.)Checklist