Repository navigation
feat(packaging): ship openshell-driver-vm in an RPM subpackage - #5152
vineethsai7 wants to merge 2 commits into
Conversation
Signed-off-by: Vineeth Sai Narajala <vnarajal@cisco.com>
|
I have read the DCO document and I hereby sign the DCO. |
Signed-off-by: Vineeth Sai Narajala <vnarajal@cisco.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f623a0cf69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Summary: OpenShell MicroVM compute driver | ||
| # The embedded libkrunfw library and guest Linux kernel have separate licenses. | ||
| License: Apache-2.0 AND BSD-3-Clause AND CC0-1.0 AND CDLA-Permissive-2.0 AND GPL-2.0-only AND ISC AND LGPL-2.1-only AND MIT AND Unicode-3.0 AND Zlib | ||
| Requires: %{name}-gateway = %{version}-%{release} |
There was a problem hiding this comment.
For Fedora/RHEL users who install this optional RPM from the published GitHub release assets, rerunning the supported install.sh flow for a later release becomes unsatisfiable: the installed old driver requires the old gateway exactly, while install_linux_rpm still downloads and passes only openshell, openshell-gateway, and openshell-prover (install.sh:1231-1266). With no repository supplying the matching new driver, the package transaction cannot replace the gateway, so include the matching driver RPM when it is already installed (or otherwise provide an atomic upgrade path).
Useful? React with 👍 / 👎.
| # The embedded libkrunfw library and guest Linux kernel have separate licenses. | ||
| License: Apache-2.0 AND BSD-3-Clause AND CC0-1.0 AND CDLA-Permissive-2.0 AND GPL-2.0-only AND ISC AND LGPL-2.1-only AND MIT AND Unicode-3.0 AND Zlib | ||
| Requires: %{name}-gateway = %{version}-%{release} | ||
| Requires: e2fsprogs >= 1.43 |
There was a problem hiding this comment.
On RHEL 8/glibc 2.28 hosts that the installation guide still says support Linux packages (docs/about/installation.mdx:102-105), this subpackage has no dependency for the current embedded libkrun runtime's GLIBC_2.34 floor. libkrun.so is compressed into the outer executable (crates/openshell-driver-vm/src/embedded_runtime.rs:25-30) and only dlopened when a VM boots (crates/openshell-driver-vm/src/ffi.rs:113-120), so RPM dependency generation and the new --version/startup smoke checks do not see that requirement; installation succeeds but the first sandbox fails to load libkrun. Build the embedded runtime to the documented 2.28 floor or declare the higher requirement and narrow the supported-host documentation.
AGENTS.md reference: AGENTS.md:L230-L230
Useful? React with 👍 / 👎.
Summary
Ship the VM compute driver as an optional RPM for x86_64 and aarch64. Fedora and RHEL users can install it with
dnfat the path the gateway already searches, without managing a separate release tarball.Related Issue
Closes #5146
The issue is still
state:triage-needed; design feedback is welcome.Design
The gateway already looks for
openshell-driver-vmunder/usr/libexec/openshell, the path used by the Debian package. The new RPM owns that directory and binary. It requiresopenshell-gateway = %{version}-%{release}so upgrades keep the two builds together, ande2fsprogs >= 1.43for VM disk operations. The existing RPM configuration continues to select Podman until the operator selectsvm.Bundling the driver in
openshell-gatewaywould put the VM binary and its host-tool dependency on every Podman installation. The current tarball workaround and a separate driver repository leave another artifact to pin and upgrade. Middleware, interceptors, providers, and OCSF events do not apply to package delivery; the gateway's existing subprocess lookup is the extension point used here.Changes
openshell-driver-vmat/usr/libexec/openshell/openshell-driver-vm, with an exact gateway requirement and ane2fsprogsrequirement.compute_driver = "vm"preflight.The workflow diff is limited to the driver artifact download and installed-package check. Changes under
.github/workflows/require upstream code-owner review.Usage
In the existing
~/.config/openshell/gateway.toml, change the gateway selector and keep the other settings:openshell-gateway config preflight --path ~/.config/openshell/gateway.toml systemctl --user restart openshell-gateway rpm -ql openshell-driver-vmThe RPM lists
/usr/libexec/openshell/openshell-driver-vm. No configuration field or event shape changes.Compatibility
The driver package is opt-in, and the existing Podman RPM default is unchanged. The gateway searches the installed driver path without a
driver_diroverride.dnf upgrademust select matching gateway and driver releases because of the exact RPM requirement.Release Notes
Fedora and RHEL users can install the VM driver from an RPM alongside the matching gateway. Install
openshell-driver-vmand selectcompute_driver = "vm"on a KVM host.Testing
cargo fmt --check; default-feature clippy with-D warningsforopenshell-gatewayandopenshell-driver-vm; 29 gateway VM unit tests, 7 gateway preflight tests, and 268 VM driver unit tests; workflow YAML parsing andbash -n; docs navigation and Fern checks.On the build box, a native
rpmbuildsmoke build using the v0.1.6 release binaries and temporary shims for unavailable Fedora macros produced the new RPM. The package queries returned:The shared build box has no Fedora packaging macros or container runtime, so a full Packit/Fedora build and the installed-package workflow step await CI. The preflight test binary used a temporary Z3 library path because the build box lacks the system library and the fixture clears
LD_LIBRARY_PATH. Fern reports zero errors and three warnings also present with the base documentation page. A VM-only clippy build with-D warningsreports three warnings in unchanged gateway code; clippy passes with the crate's configured default features.Follow-ups
Checklist
Review follow-up
Local strict-maintainer review of PR #5152 against fetched upstream
mainis inREVIEW-5152.md. All valid findings were fixed inf623a0cf69d72b4c5de800cc51946d53b3d95555:f623a0cf69; all three callers now wait forbuild-vm-driver.f623a0cf69; checks and mounts the binary.f623a0cf69; driver-specific license expression and texts added.f623a0cf69; startup smoke verifies the live child executable path.f623a0cf69; package ownership and preflight scope documented.Each P1/P2 has a regression case in
tasks/scripts/rpm_vm_packaging_test.py; F1 covers three callers. These six cases failed against the original branch inputs and passed after the fixes. The startup script was also run against locally built gateway and VM driver binaries.Assessed concerns marked not applicable:
/usr/bin/openshell-gatewaysearches/usr/libexec/openshellbefore~/.local/libexec/openshell. An explicitdriver_diris an intentional override.Requires: podman; that predates this PR and the issue accepted a driver package that requires the gateway.dnf installassumes a configured RPM source, as the existing RPM quick start does. This PR does not add repository management.libkrun.soassets require at mostGLIBC_2.34inreadelf --version-info; no newer glibc floor was found.Checks run on the build box through
dcb up-os-5146 'nice -n 15 <cmd>':cargo fmt --all -- --check: passed.cargo clippy -p openshell-gateway -p openshell-driver-vm --all-targets -- -D warnings: passed with configured default features.cargo test -p openshell-gateway --lib vm:: -- --test-threads=4: 29 passed.cargo test -p openshell-gateway --test config_preflight -- --test-threads=4: 8 passed.cargo test -p openshell-driver-vm --lib -- --test-threads=4: 268 passed.uv run --no-project --with pytest --with pytest-asyncio --with pyyaml pytest -q tasks/scripts/rpm_vm_packaging_test.py: 6 passed.bash tasks/scripts/test-packaging-assets.sh,bash -n tasks/scripts/test-rpm-vm-install.sh, Ruff lint and format, anduv run python scripts/update_license_headers.py --check: passed.uv run --no-project --with pyyaml python tasks/scripts/check_docs_nav.py: passed.npx --yes fern-api@5.112.0 checkinfern/: zero errors, three baseline warnings.The full Fedora package install lane and a KVM sandbox boot remain for upstream CI and a KVM qualification environment. Neither was run locally.
Manual verification
Result: PASS — The optional VM RPM owns
/usr/libexec/openshell/openshell-driver-vm, declares the exact gateway ande2fsprogsrequirements, and lets the PR-head gateway start the packaged driver from that default path. With the RPM absent, the upstream-main gateway fails startup with “vm compute driver binary not found.”Environment
mainatcc0642fa8955and PR headf623a0cf69d7. The source worktree was restored to the PR head afterward; no commit or GitHub write was made.e2fsprogs1.46.5, existingopenshell-gatewayRPM 0.1.5-1.fc44. Tests used one disposable user, a private HOME, private loopback ports, andcompute_driver = "vm"withoutdriver_dir. The existing gateway service and DefenseClaw were untouched.dcb up-os-5146withnice -n 15 cargo build --release --locked(onlyopenshell-gatewayandopenshell-driver-vm, withopenshell-gateway/vendored-z3). Both gateway binaries ran on RHEL with an Ubuntu 24.04 glibc loader and libraries confined to the disposable HOME; the host runtime libraries were unchanged.rpmbuild -bb. Its reduced spec carries the PR spec'sdriver-vmsubpackage name, dependency and license expressions, binary path, and GPL/LGPL notices. The payload is the official v0.1.6 GNU VM driver release binary, the type of artifact downloaded by the PR RPM workflow. The locally compiled PR driver required glibc 2.39 and could not run directly on this glibc 2.34 host; the release binary required at most glibc 2.28.Steps and before: upstream
mainupstream/main:openshell.spechas no%package driver-vmor%files driver-vm; confirm the host had neither the VM RPM nor its libexec binary.After: PR head
rpm -i --nodepsfor this isolated lookup check, preserving the existing gateway package and service./proc/<pid>/exetarget; it resolves to the RPM-owned default path. Preflight and startup both succeeded:vm compute driver binary not foundwith the same VM selector. Readback found the RPM, libexec directory, disposable user and HOME, staging files, proof processes, and private listeners absent. The pre-existing gateway RPM remained 0.1.5-1.fc44 and its service retained PID 1643.Not covered
dnf installwith a matching gateway RPM. The stack host's installed gateway was older; the ordinary RPM install test failed on the exact requirement as expected, and the manual lookup check used--nodepsto avoid replacing it./dev/kvm; aarch64 packaging and execution were not run.