feat(server): conditional compilation of compute drivers by dimityrmirchev · Pull Request #2163 · NVIDIA/OpenShell

dimityrmirchev · 2026-07-07T07:23:38Z

Summary

Adds per-driver Cargo features (driver-kubernetes, driver-docker, driver-podman, driver-vm) on openshell-server, all default-on. Building --no-default-features and selecting a subset produces a gateway that carries only those drivers, trimming binary size and supply-chain surface. Mirrors the existing telemetry feature pattern; the default build is behaviorally unchanged.

Warning

A zero-driver build (cargo build -p openshell-server --no-default-features) is a supported extension-only gateway shape: no in-tree driver code is linked, and compute_drivers must name an out-of-tree driver reached via [openshell.drivers.<name>].socket_path. Configuring a built-in that this binary was not built with still fails at startup with a message naming the Cargo flag that re-enables it.

This is a breaking change as setting --no-default-features in previous versions produced a geteway that had only telemetry disabled, but still all 4 default drivers compiled in.

Related Issue

Fixes #1943

Related to #1999 and #2061

Changes

Cargo features. New driver-kubernetes, driver-docker, driver-podman, and driver-vm features on openshell-server, all in the new default set. The three container-driver crates flip to optional = true. The VM driver has no in-server crate dependency, so driver-vm gates the in-server launcher plumbing rather than an optional dep.
Configured-but-not-compiled driver fails at startup.
Compile-out markers on each driver. Each driver crate declares a #[used] static COMPILE_MARKER: &str = "OPENSHELL_DRIVER_MARKER:<name>". This is an explicit, documented contract — the compile-out guard grepping for driver-specific text no longer depends on implicit metadata.
New CI verification. tasks/scripts/verify-drivers-compiled-out.sh greps built binaries for the driver markers. A new rust:verify:drivers-off mise task walks the matrix: default (all four drivers present), each single-driver build, one mixed set (docker+podman), and a zero-driver extension-only build that must link cleanly and contain none of the driver markers. The zero-driver build is followed by cargo test -p openshell-server --lib --no-default-features so the #[cfg]-gated extension-only tests actually execute in CI.
Docs. README.md gains a "Choosing Which Compute Drivers Are Compiled In" section and updates the telemetry-off build example. Note: I am not sure if this is the best place for this piece of documentation and am open to suggestions.
architecture/compute-runtimes.md gains a "Compile-Time Driver Selection" section, docs/reference/gateway-config.mdx gains a matching reference section under Driver References with a per-feature table and worked examples including the extension-only build.

Note

This PR adds roughly 28 #[cfg(feature = "driver-*")] gates across openshell-server — I am not a fan of how much clutter that is, but on today's crate layout it is the cost of compile-time driver selection. Extracting driver-specific compute wiring into a dedicated crate (tracked in #1999) would delete about two-thirds of these gates: driver imports, the ComputeRuntime::new_* constructors, the Builtin match arms and the unreachable! exhaustiveness arm, the *_config_from_context and apply_*_runtime_defaults helpers, compute::vm, and the SupervisorReadiness impl would all move behind a driver-agnostic entry point in the new crate. The residual gates would cluster around auth::k8s_sa — a gateway-level authenticator that happens to talk to a K8s API server — whose home is a separate design question.

Testing

mise run pre-commit passes
Unit tests added/updated
E2E tests added/updated (if applicable)

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs updated (if applicable)

copy-pr-bot · 2026-07-07T07:23:42Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

github-actions · 2026-07-07T07:23:53Z

All contributors have signed the DCO ✍️ ✅
_{Posted by the DCO Assistant Lite bot.}

dimityrmirchev · 2026-07-07T07:25:25Z

I have read the DCO document and I hereby sign the DCO.

Introduce driver-kubernetes, driver-docker, driver-podman, and driver-vm cargo features on openshell-server, all enabled by default. Building with --no-default-features and selecting a subset produces a gateway that only carries the requested drivers. The three container-driver crates flip to optional dependencies. The VM driver runs out-of-process already, so driver-vm gates the in-server launcher and match arm rather than a Cargo dep. Feature gates cover: - driver crate imports and re-exports in compute/mod.rs - ComputeRuntime::new_docker / new_kubernetes / new_podman - Docker's ShutdownCleanup / StartupResume trait impls - the compute::vm module and VmComputeConfig re-export - driver-specific *_config_from_context and apply_*_runtime_defaults - the K8s ServiceAccount authenticator bootstrap in ServerState - the auth::k8s_sa module and state field - authenticator-chain construction in multiplex.rs - SupervisorReadiness impl for SupervisorSessionRegistry - driver-specific unit tests in cli.rs, lib.rs, driver_config.rs A zero-driver build is a supported extension-only gateway shape: it carries no in-tree driver code and speaks compute_driver.proto over a Unix socket to an out-of-tree driver named in compute_drivers with a matching [openshell.drivers.<name>].socket_path. The runtime gate in resolve_configured_compute_driver (see the follow-up commit) still rejects a built-in that was compiled out with the "rebuild with --features driver-X" message, so silent misconfiguration is impossible. Default builds are byte-equivalent to before. Each of the four single-driver builds compiles cleanly under -D warnings, as does the docker + podman combo and the zero-driver extension-only build. Signed-off-by: Dimitar Mirchev <28221091+dimityrmirchev@users.noreply.github.com>

Move the availability check out of the build_compute_runtime match arm into resolve_configured_compute_driver so it fires alongside the other config-time driver validation. Configuring a driver that this gateway was not built with now fails at startup with a message naming both the driver and the Cargo flag that re-enables it. Auto-detection (empty --drivers) filters through filtered_detect_driver: if openshell-core's environment probe picks a driver that was compiled out, we return None and let the caller produce the "no suitable driver" error rather than silently starting a different backend. In an extension-only build (zero built-ins compiled in) the message adapts to point operators at the [openshell.drivers.<name>].socket_path path instead of listing built-in names that don't exist in this binary; otherwise it lists only the built-ins actually linked and mentions that extension drivers are also valid. Adds unit tests that exercise the compiled-out rejection path and the extension-only empty-drivers message. The latter is #[cfg]-gated so it only compiles when no driver features are on. Signed-off-by: Dimitar Mirchev <28221091+dimityrmirchev@users.noreply.github.com>

Adds verify-drivers-compiled-out.sh, a mise task that exercises the per-driver Cargo features, and a matching step in the branch-checks workflow. The script mirrors verify-telemetry-compiled-out.sh: a table of stable driver-exclusive marker strings pulled from each driver crate, with present/absent/only modes. The 'only' mode asserts presence for a selected driver list AND absence for every other driver in one sweep, so a single script invocation covers both directions per build. The mise task exercises: default (all four drivers present), each single-driver build, one mixed feature set (docker+podman), and a zero-driver extension-only build that must link cleanly and must contain none of the four driver markers. After the zero-driver build the task also runs `cargo test -p openshell-server --lib --no-default-features` so the #[cfg]-gated extension-only unit tests actually execute in CI rather than being silently skipped by the default-features test job. The telemetry-off task uses a mixed --features driver-...,driver-... gateway build so the telemetry-marker absence check still exercises a representative in-tree-driver build path. Signed-off-by: Dimitar Mirchev <28221091+dimityrmirchev@users.noreply.github.com>

- README.md gains a "Choosing Which Compute Drivers Are Compiled In" section covering the four driver features, the extension-only build shape, and the startup-time compiled-out rejection. - architecture/compute-runtimes.md gains a "Compile-Time Driver Selection" section covering the four driver features, the extension-only zero-driver build, the startup-time missing-driver error, filtered auto-detection, and the always-available extension driver path. - docs/reference/gateway-config.mdx gains a matching reference section under "Choosing Which Drivers Are Compiled In" with a per-feature table, single-driver / mixed / extension-only build examples, and the startup-time rejection contract. Signed-off-by: Dimitar Mirchev <28221091+dimityrmirchev@users.noreply.github.com>

elezar · 2026-07-07T13:06:00Z

+    /// e.g. `--no-default-features` exercises all four rejection messages;
+    /// the default build has every driver and the loop body is skipped.
+    #[test]
+    fn configured_compute_driver_rejects_compiled_out_builtin() {


Question: What about building a map and checking missing vs present regardless of the feature set?

Should be addressed with 2d26c6e

elezar · 2026-07-07T13:08:26Z

+    #[cfg(feature = "driver-kubernetes")]
    if let Some(k8s) = state.k8s_sa_authenticator.clone() {
        authenticators.push(k8s);
    }


When I see something like this, I wonder why we need to toggle these features at such low-levels. If state.k8s_sa_authenticator is never set when the driver-kubernetes feature is not enabled, then if should just be skipped here. I would assume that this field itself doesn't add any drastic external dependnecies. (is a possible issue the fact that the k8s_sa_authenticator stores the real type instead of an auth::authenticator::Authenticator?

is a possible issue the fact that the k8s_sa_authenticator stores the real type instead of an auth::authenticator::Authenticator

Yes, I think this is the problem. Without this guard the gateway will fails to compile. An alternative would be to delete the concrete type as you suggested.

elezar · 2026-07-07T13:10:01Z

+#[cfg(feature = "driver-docker")]
 impl openshell_driver_docker::SupervisorReadiness for SupervisorSessionRegistry {
    fn is_supervisor_connected(&self, sandbox_id: &str) -> bool {
        Self::is_connected(self, sandbox_id)
    }
 }


This will be removed in #2153.

elezar · 2026-07-07T13:14:55Z

+  # gateway needs at least one compute driver to link, so build with a
+  # single-driver feature set that excludes telemetry.
+  "cargo build -p openshell-server --bin openshell-gateway --no-default-features --features driver-kubernetes,driver-docker,driver-podman,driver-vm",
  "tasks/scripts/verify-telemetry-compiled-out.sh absent target/debug/openshell-gateway",


Question: How similar is verify-telemetry-compiled-out.sh to verify-drivers-compiled-out.sh? Does it make sense to use the same mechanism there to test for being compiled out?

I think the telemetry feature can make use of the same marker mechanism as the drivers. Would such change be suitable for this PR or a subsequent one? As for the two scripts I think we can leave them separate for now. A potential verify-marker.sh script would reduce some duplication.

I'm ok to leave the refactor out of scope for this PR, but to do so in a follow-up.

elezar · 2026-07-07T13:16:49Z

  "tasks/scripts/verify-telemetry-compiled-out.sh absent target/debug/openshell-sandbox",
 ]
+
+["rust:verify:drivers-off"]


Does it make sense to generate a different named output for each of the features that w'ere testing here so that these verification tests are more self-contained?

The rest should be addressed with 61529cf

elezar · 2026-07-07T14:02:46Z

+        // Unreachable at runtime. This fallback is required for match exhaustiveness
+        // in non-default builds (including the extension-only build with zero driver features),
+        // where some or all of the arms above are cfg'd out.
+        #[cfg(not(all(
+            feature = "driver-kubernetes",
+            feature = "driver-docker",
+            feature = "driver-podman",
+            feature = "driver-vm",
+        )))]
+        ConfiguredComputeDriver::Builtin(kind) => {
+            unreachable!(
+                "compute driver '{}' passed startup validation but is not compiled into this gateway",
+                kind.as_str()
+            );
+        }


Would matching on _ => make sense -- at the expense of a possibly more generic error message? My conern is that each of these feature checks needs to be updated as dirvers are added or removed.

Addressed with 86a6880

elezar · 2026-07-07T14:04:50Z

+    )]
    pub gateway_port: u16,
+    #[cfg_attr(not(feature = "driver-vm"), allow(dead_code))]
    pub gateway_tls_enabled: bool,


Prefer #[cfg] directly on the fields over #[cfg_attr(..., allow(dead_code))]. The cfg_attr form uses a double-negated predicate (not(any(...))) that is harder to read and fragile — if a new driver reads one of these fields, the suppression predicate needs updating in sync or the warning silently returns.

With #[cfg] the struct accurately reflects what exists per build:

#[derive(Clone, Copy)] pub struct DriverStartupContext<'a> { pub file: Option<&'a config_file::ConfigFile>, #[cfg(any(feature = "driver-docker", feature = "driver-podman", feature = "driver-vm"))] pub guest_tls: Option<&'a GuestTlsPaths>, #[cfg(any(feature = "driver-podman", feature = "driver-vm"))] pub gateway_port: u16, #[cfg(feature = "driver-vm")] pub gateway_tls_enabled: bool, pub endpoint_overrides: &'a BTreeMap<String, PathBuf>, }

The three construction sites (run_server, test_context_with_endpoint_overrides, test_driver_startup) each need matching #[cfg] on the three field assignments — the same pattern used throughout the rest of this PR. Non-blocking; the current approach compiles and is tested.

60927a1 should make things more explicit. I also made apply_runtime_defaults to not return the tls paths but only apply the defaults. resolved_local_tls is called instead if any of the docker, podman or vm drivers are used.

elezar · 2026-07-07T14:21:08Z

Bug: filtered_detect_driver does not fall through to compiled-in alternatives

filtered_detect_driver wraps detect_driver() with a compiled-in check, but detect_driver short-circuits at the first detected driver. If that driver was compiled out, the filter returns None and the gateway fails to start — even if a compiled-in driver is available in the environment.

Concretely: a --no-default-features --features driver-docker binary deployed on a Kubernetes node will fail with "no compute driver configured", because KUBERNETES_SERVICE_HOST causes detect_driver to return Kubernetes before Docker is ever checked.

This regression is introduced by this PR — before per-driver features, detect_driver always returned a usable driver because all four were always compiled in.

The fix is to have detect_driver (or a new variant in openshell-core) return drivers in priority order so the server-side filter can pick the first compiled-in match. #2137 proposes exactly this as part of a broader change — it would be worth aligning with or depending on that work rather than implementing a one-off workaround here.

elezar · 2026-07-07T14:36:58Z

A couple of additional suggestions around configured_compute_driver:

Remove no_driver_configured_message and compiled_in_builtin_names — branching on compiled-in drivers to produce different error messages leaks build configuration into operator-facing output; a single static message works for all builds
Change the "not compiled into this gateway; rebuild with --features" error to "not supported by this gateway build" — operators using packaged binaries can't act on Cargo flags

Cover both directions of the compile-time driver gate in one test: a compiled-in driver must resolve to its Builtin variant; a compiled-out driver must fail with a message naming the Cargo feature that re-enables it. An exhaustive match on `ComputeDriverKind` inside the test makes a new variant a compile error until it's given a feature gate. Derive `Hash` on `ComputeDriverKind` so it can key the driver-features map. Signed-off-by: Dimitar Mirchev <28221091+dimityrmirchev@users.noreply.github.com>

Break the flat 12-step run list into seven hidden sibling tasks — one positive control, four single-driver builds, one mixed docker+podman build, and one zero-driver build (which also runs the extension-only unit tests). The top-level task becomes a `depends` aggregator over them. Each sub-task runs the exact same cargo build + verify-drivers-compiled- out.sh pair as before, so total work and CI wall clock are unchanged. The split lets a developer re-run a single leg in isolation (`mise run rust:verify:drivers-off:podman`) after a failure, and mise's per-task log prefix makes a failing leg trivially identifiable in CI output. The parent task keeps the same name so `.github/workflows/branch-checks.yml` needs no change. Signed-off-by: Dimitar Mirchev <28221091+dimityrmirchev@users.noreply.github.com>

Replace the `#[cfg(not(all(feature = "driver-*")))]`-gated `ConfiguredComputeDriver::Builtin(kind)` catch-all in the dispatch match with a plain `_ => unreachable!(...)` guarded by `#[allow(unreachable_patterns)]`. Adding or removing a driver feature no longer requires updating this arm, and there is no feature list to drift out of sync with the enum. The arm remains genuinely unreachable at runtime: config validation in `configured_compute_driver` rejects builtins whose feature was compiled out before dispatch runs. The trade-off is that the panic message no longer names the specific driver — acceptable because the actionable error operators see is the validation-layer message, which does name the driver and the Cargo feature that re-enables it. Signed-off-by: Dimitar Mirchev <28221091+dimityrmirchev@users.noreply.github.com>

dimityrmirchev · 2026-07-08T09:29:30Z

Bug: filtered_detect_driver does not fall through to compiled-in alternatives

filtered_detect_driver wraps detect_driver() with a compiled-in check, but detect_driver short-circuits at the first detected driver. If that driver was compiled out, the filter returns None and the gateway fails to start — even if a compiled-in driver is available in the environment.

Concretely: a --no-default-features --features driver-docker binary deployed on a Kubernetes node will fail with "no compute driver configured", because KUBERNETES_SERVICE_HOST causes detect_driver to return Kubernetes before Docker is ever checked.

This regression is introduced by this PR — before per-driver features, detect_driver always returned a usable driver because all four were always compiled in.

The fix is to have detect_driver (or a new variant in openshell-core) return drivers in priority order so the server-side filter can pick the first compiled-in match. #2137 proposes exactly this as part of a broader change — it would be worth aligning with or depending on that work rather than implementing a one-off workaround here.

Let's wait for #2137 to land so I can rebase on top of that and introduce a proper fix.

dimityrmirchev · 2026-07-08T09:37:13Z

Remove no_driver_configured_message and compiled_in_builtin_names — branching on compiled-in drivers to produce different error messages leaks build configuration into operator-facing output; a single static message works for all builds

Isn't it better for operators to know which drivers are supported in the current build? A generic message would not directly give the operator the information they need to address the error.

elezar · 2026-07-08T09:54:08Z

Remove no_driver_configured_message and compiled_in_builtin_names — branching on compiled-in drivers to produce different error messages leaks build configuration into operator-facing output; a single static message works for all builds

Isn't it better for operators to know which drivers are supported in the current build? A generic message would not directly give the operator the information they need to address the error.

While I don't disagree that the list of supported drivers is useful information, extracting this from the error message would not be my first suggestion of how to do this. Are the markers visible in the compiled binary to be inspected, or could we consider including this in some info / doctor output?

dimityrmirchev · 2026-07-08T11:09:11Z

Remove no_driver_configured_message and compiled_in_builtin_names — branching on compiled-in drivers to produce different error messages leaks build configuration into operator-facing output; a single static message works for all builds

Isn't it better for operators to know which drivers are supported in the current build? A generic message would not directly give the operator the information they need to address the error.

While I don't disagree that the list of supported drivers is useful information, extracting this from the error message would not be my first suggestion of how to do this. Are the markers visible in the compiled binary to be inspected, or could we consider including this in some info / doctor output?

As far as I can tell the markers survive a release build.

cargo build -p openshell-server --bin openshell-gateway --release
strings ./target/release/openshell-gateway | grep OPENSHELL_DRIVER_MARKER
... truncated output

I agree that an info subcommand for the gateway can serve better than the error message. We can follow up with an issue about that.

I will proceed with your initial suggestion about unifying the err message.

Edit: Discussion addressed with 85c4144 and 3a7a654

…llow(dead_code))] Gate driver-specific fields on `ServerStartupConfig` and `DriverStartupContext` (and the `GuestTlsPaths` type they reference) with `#[cfg]` directly instead of double-negated `#[cfg_attr(not(any(...)), allow(dead_code))]`. The direct form makes the struct definition accurately reflect what exists per build and removes the drift risk where a new driver reading one of these fields would silently re-suppress the warning if the exclusion predicate went stale. Every construction site (`run_server`, `prepare_server_config`, `test_context_with_endpoint_overrides`, `test_driver_startup`) is updated to gate the matching field assignments with the same predicate, so the gates cannot drift apart. Split `apply_runtime_defaults` into a pure-side-effect `Result<()>` plus a separate `resolved_local_tls` accessor so the call site no longer needs a two-branch `#[cfg]` split around `let _ = ...` just to keep the return value from being unused under `--no-default-features`. The two are now clean concerns: one mutates args, one reads TLS paths off disk. Callers invoke whichever they need. Signed-off-by: Dimitar Mirchev <28221091+dimityrmirchev@users.noreply.github.com>

…t-supported error Operators running packaged gateway binaries (Docker images, Helm charts, Homebrew) can't act on "rebuild openshell-server with --features driver-<name>" — that advice is aimed at contributors editing the source tree. Replace with a shorter operator-oriented phrase that says what's wrong without prescribing a fix outside the operator's reach. Update `configured_compute_driver_matches_compiled_features` to assert the new phrase and the driver name rather than the Cargo flag. Signed-off-by: Dimitar Mirchev <28221091+dimityrmirchev@users.noreply.github.com>

Drop `no_driver_configured_message` and `compiled_in_builtin_names` in favor of an inline static error. The dynamic list of supported drivers belonged in a discoverable channel (a future `openshell-gateway info` subcommand, tracked separately) rather than in an error message the operator can only reach by triggering a failure. Adjust `configured_compute_driver_empty_points_at_extensions_when_no_builtins` to assert only the socket_path escape hatch, which the single static message still surfaces for the extension-only build shape. Signed-off-by: Dimitar Mirchev <28221091+dimityrmirchev@users.noreply.github.com>

dimityrmirchev requested review from a team, derekwaynecarr, maxamillion and mrunalp as code owners July 7, 2026 07:23

elezar reviewed Jul 7, 2026

View reviewed changes

Comment thread crates/openshell-server/src/compute/mod.rs Outdated

dimityrmirchev added 4 commits July 7, 2026 14:26

dimityrmirchev force-pushed the 1943-conditional-driver-compilation branch from dde468f to 9a1e04c Compare July 7, 2026 12:08

elezar reviewed Jul 7, 2026

View reviewed changes

Comment thread crates/openshell-server/Cargo.toml

elezar reviewed Jul 7, 2026

View reviewed changes

dimityrmirchev added 3 commits July 8, 2026 09:19

dimityrmirchev added 3 commits July 8, 2026 14:22

Uh oh!

Conversation

dimityrmirchev commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Jul 7, 2026

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dimityrmirchev commented Jul 7, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elezar Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elezar commented Jul 7, 2026

Uh oh!

elezar commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dimityrmirchev commented Jul 8, 2026

Uh oh!

dimityrmirchev commented Jul 8, 2026

Uh oh!

elezar commented Jul 8, 2026

Uh oh!

dimityrmirchev commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dimityrmirchev commented Jul 7, 2026 •

edited

Loading

github-actions Bot commented Jul 7, 2026 •

edited

Loading

elezar Jul 7, 2026 •

edited

Loading

elezar commented Jul 7, 2026 •

edited

Loading

dimityrmirchev commented Jul 8, 2026 •

edited

Loading