feat(driver-native): add native compute driver#2312
Conversation
7a55505 to
5fa410c
Compare
|
If we want to go full daemonless, containerd daemon dependency can eventually be removed, it just keeps the scope of this PR smaller for this iteration. |
Shouldn't it be named openshell-driver-containerd ?
Oh. That seems like it needs a really strong justification. This relates to #981 - I still would really like to move away from openshell driving lots of low level custom sandboxing itself towards one where it just reuses existing container framework tooling - especially where e.g. the proxy is a sidecar or other associated process etc. |
containerd-daemon dependency might eventually go away, it's undecided, it just keeps the implementation smaller for now, it only handles transport, runc or crun is orchestrated directly by OpenShell. This is sorta linked to that: oci-client crate in the Rust ecosystem I think, is not the right call. docker, podman, k8s remain separate drivers. |
|
But thanks @cgwalters I did rename: to the "containerd" part of the name does distract from the purpose here. It's OpenShell that kicks off the runc's or crun's. containerd has nothing to do with the isolation. |
Would you expand on this a bit? |
@krishicks see here, it's described in more detail: |
It's not really explained there either:
What's the problem that's being solved in #2313, and why is oci-client a mistake or not the right call? |
The OCI protocol is ossified... This new Rust crate is used in projects like oras, which is known to be unreliable compared to implementations based on containerd or containers/image |
|
It can be reliable on OCI registries like Docker Hub, but it's the internal ones where you get the bug reports, hand-rolled OCI clients reliably break on obscure auth schemes, manifest quirks, and layer edge cases |
|
I'm a bit skeptical of the oci-client crate being problematic as far as talking to registries, I think the larger picture thing is ecosystem stuff like finding and using pull secrets, which is outside the scope of that crate. And container storage. |
|
Thanks for putting this together. As discussed in our sync, the goal of an Using containerd for reliable registry access, content storage, unpacking, and That makes it especially important for the current implementation to make Conceptually, I would like to see something along these lines: Each provider may produce a backend-specific artifact—for example, OCI mounts I also think we should explicitly evaluate how much of the existing VM rootfs There are also parts of the VM driver that we should consider extracting instead
The VM-specific ext4, boot, libkrun, and VFIO code would remain in the VM While reviewing the implementation, I also found several issues that appear to
The unit tests cover the individual helpers well, but the real containerd/runtime tests are ignored and there is no native gateway end-to-end lane. I think an end-to-end test using the real supervisor is important before merging because it would catch the endpoint and lifecycle issues above. My preference would be to treat this PR as the start of an OCI sandbox Given the architectural questions and correctness issues above, I do not think |
"I do not, however, consider a required system containerd daemon to be a viable long-term design.". I agree here. I've been waiting for opinions and decisions on: before changing that here. I might not have communicated that great here. The three daemonless, out-of-the-box, single-node options, I can think of are:
I think 3. carries the most risk FWIW. And would likely end up in a multi-year effort.
The rest sounds ok, I think we can figure it out. |
Adds openshell-driver-native, an OCI/runc-based compute driver that builds sandboxes from Linux namespaces and cgroups v2. A system-provided containerd is used only for image pull, unpack, and snapshot management: this driver, not containerd, drives the configured low-level OCI runtime (runc/crun) directly, and containerd never creates a Container or Task for these sandboxes. Also extracts the VM driver's nftables ruleset generator into a shared openshell-nft-ruleset crate, and wires ComputeDriverKind::Native through openshell-core and every openshell-server compute-driver selection path. Hardening from review: sandbox names are validated against path traversal, the runtime's --root is scoped under the driver's own state directory instead of the shared global default, the sandbox endpoint no longer defaults to an unreachable loopback address, per-sandbox subnet allocation detects and avoids collisions instead of a stateless hash, cleanup after a failed create no longer leaves a mounted rootfs and bundle directory behind, and sandbox token files are written with owner-only permissions. Verified end to end against a real containerd 2.x + runc/crun install (see tests/containerd_integration.rs, #[ignore]d in CI since CI has no containerd). Known gaps (rootless mode, image-based supervisor injection, full CDI GPU support, polling-based watch) are documented in the driver README and docs/reference/sandbox-compute-drivers.mdx. Related: NVIDIA#2255 Signed-off-by: Eric Curtin <eric.curtin@docker.com>
5fa410c to
16ed6e6
Compare
|
@elezar pushed a round of fixes for the correctness issues you flagged (loopback endpoint, path traversal, runtime --root, subnet collisions, cleanup-on-error, token permissions), all with new unit tests. Left the RootfsProvider/SandboxProvisioner restructuring out of this push since it's a bigger design conversation. Want to spin that off as a follow-up issue/RFC before I touch the driver boundary? |
Is reusing / adapting the implementation the VM uses an option? While I understand that #2313 raises questions about the robustness of the implementation and the long-term maintenance costs, it may be useful to separate the discussion around what we use for rootfs construction (home-rolled vs containerd lib) from what we're using to provision the sandboxes (libkrun vs OCI). |
Yes I agree, that's an option and should be the end goal. Consolidate on whatever the transport is. |
Summary
Adds
openshell-driver-native, an OCI/runc-based compute driver that builds sandboxes from Linux namespaces and cgroups v2. A system-provided containerd is used only for image pull, unpack, and snapshot management: this driver, not containerd, drives the configured low-level OCI runtime (runc/crun) directly, and containerd never creates aContainerorTaskfor these sandboxes.This is a draft PR opened directly at the requester's instruction, ahead of the normal review gate this repo's agent workflow uses (posting a plan and waiting for
state:agent-ready). That plan is posted on the issue for reference.Related Issue
Related to #2255
Changes
crates/openshell-driver-native/(new crate): image pull/chain-ID resolution/snapshot management against containerd, protected from GC via a per-sandbox lease (image.rs); OCI runtime-spec generation (spec.rs); directcreate/start/state/kill/deleteinvocation of the configured runtime (runtime.rs); per-sandbox network namespace + veth + nftables (network.rs); GPU kernel device-node passthrough (gpu.rs); theComputeDrivergRPC service (driver.rs/grpc.rs).crates/openshell-nft-ruleset/(new crate): the VM driver's nftables ruleset generator extracted so both drivers share the same NAT + default-deny shape.crates/openshell-core,crates/openshell-server:ComputeDriverKind::Nativewired through config, telemetry, and every compute-driver selection path.docs/reference/sandbox-compute-drivers.mdx,[openshell.drivers.native]example indocs/reference/gateway-config.mdx,architecture/mentions,crates/openshell-driver-native/README.md.Hardening from @elezar's review: sandbox names are validated against path traversal; the runtime's
--rootis scoped under the driver's own state directory instead of the shared global default; the sandbox endpoint no longer defaults to an unreachable loopback address; per-sandbox subnet allocation detects and avoids collisions instead of a stateless hash; cleanup after a failed create no longer leaves a mounted rootfs and bundle directory behind; sandbox token files are written with owner-only permissions.Testing
cargo fmt --all -- --checkpassescargo clippy --workspace --all-targets -- -D warningspassescargo test --workspace --libpassesmarkdownlint-cli2and the mermaid linter pass on changed docsrunc/cruninstall: pull, chain-ID resolve, lease-protected snapshot prepare, bundle mount,create/start/state/deleteagainst both runtimes, stop, delete (tests/containerd_integration.rs,#[ignore]d since CI has no containerd); confirmed a snapshot without a lease is reaped by containerd's GC while one with a lease survives; confirmed the fullopenshell-gatewaybinary connects and selects this driver correctly.Known gaps (documented in the driver README and
docs/reference/sandbox-compute-drivers.mdx): rootless mode is implemented but not yet functional (containerd's prepared snapshot isn't remapped to match, so it defaults off); the supervisor binary is bind-mounted from a host path rather than sourced from an image like the other drivers; GPU support is kernel-device-node passthrough only, not full CDI injection;WatchSandboxespolls rather than subscribing to containerd's event stream; no dedicated E2E lane yet.Not addressed in this PR: @elezar's architectural proposal to split this into a
RootfsProvider/SandboxProvisionerstructure and share more of the VM driver's pipeline (subnet allocation, nftables lifecycle, supervisor env construction, transactional cleanup). That's a larger restructuring worth its own discussion; see the review thread.Checklist