Repository navigation
test(e2e): migrate the features and drivers suites to testkit - #5137
matthewgrossman wants to merge 9 commits into
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
🌿 Preview your docs: https://nvidia-preview-pr-5137.docs.buildwithfern.com/openshell |
|
/ok to test b07fbc4 |
|
Label |
|
/ok to test 78af387 |
78af387 to
7a8d936
Compare
|
/ok to test 7a8d936 |
|
/ok to test e9281a5 |
7a8d936 to
e9281a5
Compare
Add the container-engine and podman support crates for suites that build images into the gateway's store or compare against Podman's own containers, and add expect_compute_driver and base_image to testkit. Run the support crates' unit tests in the Rust test lists. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Rewrite tests/suites/features/oci-image as testkit tests (package oci-image) that build their images through OPENSHELL_TEST_CONTAINER_ENGINE, which the Docker and Podman environments now publish. Run it through e2e-suites.yml on the Docker rootful, Podman rootful and Podman rootless rows, and retire the tmachine oci-image testsuite, its playbook and its archive. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Add fedora-podman-{rootful,rootless}-keycloak environments and run keycloak-provider-refresh there with the rpm installer, replacing the old provider-refresh crate, its tmachine testsuite and its archive. Podman has no daemon to restart Keycloak at boot, so start the container with --restart always and enable podman-restart.service for the user that runs it.
feature-specific-integration now runs the features family through e2e-suites.yml in Branch E2E, Release Dev and Release Qualification, and absorbs Branch E2E's separate e2e-suites job.
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Rewrite tests/suites/drivers/podman as a testkit test (package podman-driver) that compares a sandbox's UID map with the one Podman gives a container in the same user-namespace mode. Each fedora-podman-{rootful,rootless}-userns-<mode> environment configures that mode for the binaries installer's gateway and publishes it; the packaged installers refuse such environments rather than test without the mode.
driver-specific-integration now runs the drivers family through e2e-suites.yml, and the e2e-podman tmachine row moves to its own podman-e2e-integration job. Retire the driver-podman testsuite, its playbooks, fixtures and archive.
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Every suite that used the CLI conformance library now runs on testkit, so remove the crate, its lockfile entry and its row in the AGENTS.md architecture table. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…andbox In auto mode Podman gives each running container its own UID range. The ported test ran the reference container while the sandbox still held the first range, so the reference got the next one and the auto rows failed. Run the reference first, as the old playbooks did, and let it pull the base image if it is missing. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
The old default-userns baseline asserted that gateway.toml had no userns, uidmap or gidmap settings. The driver fragment is now their only source, so remove it in default mode instead of only skipping the write. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
|
/ok to test d0d6e10 |
e9281a5 to
d0d6e10
Compare
Replace the per-package cargo fmt and cargo clippy lines for the conformance, drivers and features test workspaces with one line per workspace, in the Branch Checks Format and Lint steps and in the rust:lint, rust:format and rust:format:check tasks. New suite crates are covered without editing the lint lists, and the features workspace's kubernetes-updates crate is now linted; it raises no warnings. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
|
/ok to test 988d5c3 |
| # The package isn't named `podman` because the support crate it depends on is. | ||
| [package] | ||
| name = "openshell-test-suite-podman" | ||
| name = "podman-driver" |
There was a problem hiding this comment.
Suggestion, not a blocker: I'd like the driver suites to follow tests/suites/drivers/<driver>, with the package named after the driver (podman, and later docker, kubernetes, vm), and to rename any crate that clashes with that.
Today the only clash is the tests/support/podman support crate, which is why this package is podman-driver (see the comment above). With package equal to driver, --package podman in the runner, -p podman in cargo and the drivers/podman path all line up, and a per-driver filter on e2e-suites.yml can select a driver by package name without a separate driver field.
Doing it here is cheap: this suite is the only package under drivers. After merge the rename also touches the mise tasks, the CI rows, the runner's --package values and the docs.
Concretely:
- rename
tests/support/podmantopodman-support(the Rust crate becomespodman_support); - give
container-engineandkeycloakthe same-supportsuffix so the scheme is consistent, even though they don't clash yet; - rename this package to
podmanand drop the explanatory comment above; - update the lockfiles,
packagevalues ine2e-suites.ymland any-p podman-driverreferences.
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| # The package isn't named `podman` because the support crate it depends on is. | ||
| [package] |
There was a problem hiding this comment.
Question, related to the naming comment on podman/Cargo.toml: should drivers be a single workspace, or should each driver be its own workspace (tests/suites/drivers/podman/Cargo.toml with its own [workspace] and lockfile)?
I'd consider driver-specific tests to be decoupled from each other. Each driver would own its suite, and the shared tooling (openshell-test-runner, testkit, the support crates) would run it against a gateway configured with that one driver, as expect_compute_driver("podman") and the fedora-podman-*-userns-* environments already assume.
With per-driver workspaces:
- a change to one driver's tests or dependencies can't affect the others' builds or caches;
--familywould become a path (drivers/podman), so the runner would need to accept a slash and reject..and absolute paths (tests/runner/src/main.rs), andbundle.rsmay need to sanitise it in artifact names;- the cost is a lockfile and a list entry per driver (
ALIGNED_WORKSPACES, fmt and clippy) unless those lists become globs.
I don't think this has to happen in this PR. drivers has one package today, so moving it later is a git mv plus the lists, and --family drivers --package podman keeps working in the meantime. I'd like to know whether you'd prefer per-driver workspaces as the direction, so we don't build more on the single workspace first. If so, I'm happy to open a follow-up issue.
| # installer registers with the CLI: `openshell` for the packages, | ||
| # `tmachine` for the k3s installers. | ||
| # `tmachine` for the k3s and binaries installers. | ||
| rows='[ |
There was a problem hiding this comment.
As a follow-up, maybe we can consider maintaining this in some local-tracked file (although I suppose this would just be the "plan" or "matrix" that we were talking about in the initial discussions). Something like:
{
"defaults": { "runner": "ubuntu-24.04" },
"suites": {
"conformance": [
{ "environment": "ubuntu-docker-rootful", "installer": "deb" },
{ "environment": "fedora-podman-rootful", "installer": "rpm" },
{ "environment": "fedora-podman-rootless", "installer": "rpm" },
{ "environment": "ubuntu-k3s", "installer": "k3s" },
{ "environment": "ubuntu-k3s", "installer": "k3s-ha" }
],
"features/keycloak-provider-refresh": [
{ "environment": "ubuntu-docker-rootful-keycloak", "installer": "deb" },
{ "environment": "fedora-podman-rootful-keycloak", "installer": "rpm" },
{ "environment": "fedora-podman-rootless-keycloak", "installer": "rpm" }
],
"features/oci-image": [
{ "environment": "ubuntu-docker-rootful", "installer": "deb" },
{ "environment": "fedora-podman-rootful", "installer": "rpm" },
{ "environment": "fedora-podman-rootless", "installer": "rpm" }
],
"drivers/podman": [
{ "environment": "fedora-podman-rootful-userns-default", "installer": "binaries" },
{ "environment": "fedora-podman-rootful-userns-auto", "installer": "binaries" },
{ "environment": "fedora-podman-rootful-userns-keep-id", "installer": "binaries" },
{ "environment": "fedora-podman-rootful-userns-private", "installer": "binaries" },
{ "environment": "fedora-podman-rootless-userns-default", "installer": "binaries" },
{ "environment": "fedora-podman-rootless-userns-auto", "installer": "binaries" },
{ "environment": "fedora-podman-rootless-userns-keep-id", "installer": "binaries" },
{ "environment": "fedora-podman-rootless-userns-private", "installer": "binaries" }
]
}
}
although we could also consider a packages entry instead of defining these explicitly.
| # The packaged gateway's configuration comes from its environment file, so it | ||
| # can't apply an environment's Podman driver settings. Fail rather than test a | ||
| # gateway without them. | ||
| - name: Require no environment Podman driver settings |
There was a problem hiding this comment.
For context, not a request for this PR: #4289 (fix(tmachine): restore package-installed Podman qualification) is the follow-up that would let the Podman driver suite run from the packaged installers (rpm), so the userns rows could eventually move off binaries.
Keeping the assert here is the right behaviour for this PR. The packaged gateway can't apply these settings today, and failing is better than testing without the mode.
#4289 applies the settings by writing them into the packaged service's XDG gateway.toml and restarting the installed gateway. It still targets the old tests/ansible/... layout and conflicts with this PR (it edits default-userns-baseline.yaml, tests.yaml and userns-profile.yaml, which this PR removes). I'd expect it to be reworked on top of this PR after it merges, replacing this assert with that mechanism. The two approaches shouldn't both write [openshell.drivers.podman] on the same host, so the /etc/openshell-test/podman-driver.toml splice and the XDG file will need to be reconciled at that point.
|
Proposal for discussion, not a request to change this PR: let a testkit test register custom cleanup for resources testkit doesn't track. Problem. Testkit deletes tracked sandboxes after a pass, failure or panic, but the OCI suite ( Sketch. cx.on_cleanup("remove images", async move || engine.remove_images().await);
Effect here. Tests. Unit tests for ordering, failure and panic reporting. A deliberate-failure test that registers a hook which writes a marker, then panics, and asserts the test fails and the marker exists (the idea from #4464). Where. Since this PR is what introduces the leaking cleanup, it seems best to land the testkit change first as its own small PR and rebase this one onto it, rather than ship the inline cleanup and rewrite it right after. Happy to discuss, @matthewgrossman. |
Rename the Podman driver suite package from podman-driver to podman, so the tests/suites/drivers/<driver> path, cargo -p and the runner's --package value match. Its support crate took that name, so give the support crates a -support suffix: container-engine-support, keycloak-support and podman-support. Their directories under tests/support are unchanged. Update the dependents, the lockfiles, the e2e-suites matrix rows and TESTING.md, which now states both naming rules. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
|
/ok to test c41046c |
Summary
Port the remaining suites that still use
crates/openshell-conformance(features/oci-image,features/provider-refresh/keycloakand the Podman user-namespace suite indrivers/podman) to testkit, and run them throughe2e-suites.ymlwith explicit CI rows. This is a parity change: each suite keeps its current environments and installers. The last commit deletescrates/openshell-conformance.Related Issue
Part of #3954
Changes
tests/support, packages named<name>-support):container-engine-supportbuilds images into the gateway's image store throughOPENSHELL_TEST_CONTAINER_ENGINE.podman-supportreadsOPENSHELL_TEST_PODMAN_USERNSand computes Podman's own UID map for a mode. testkit gainsContext::expect_compute_driverandtestkit::base_image()(OPENSHELL_TEST_BASE_IMAGE, or the default sandbox image). The support unit tests run in the Rust test lists. The existing Keycloak fixture is renamed tokeycloak-supportto match; its directory doesn't change.tests/suites/features/oci-image, package renamed tooci-image): rewritten as#[testkit::test]tests. The Docker, rootful Podman and rootless Podman environments publishOPENSHELL_TEST_CONTAINER_ENGINE(docker,sudo -n podman,podman).mise run e2e:oci-image(:podman)and the ARM64 Docker lane use the new package name.keycloak-provider-refresh, from test(features): run feature tests against environment services, starting with Keycloak #4891) already matches the old test. This PR adds thefedora-podman-{rootful,rootless}-keycloakenvironments for it and deletes the oldprovider-refresh/keycloakcrate. On Podman, thekeycloakrole now starts the container with--restart alwaysand enablespodman-restart.service(the system unit when rootful, tmachine's user unit when rootless), so Keycloak comes back when a cached disk boots. Docker keepsunless-stopped.tests/suites/drivers/podman, packagepodman; driver suites are named after the driver): one test compares the sandbox's UID map with Podman's for the configured mode and checks that the non-root workload owns and can write/sandbox. As in the old playbooks, the reference container runs and is removed before the sandbox is created: inautomode Podman gives each running container its own range, so the sandbox gets the range the reference released.fedora-podman-{rootful,rootless}-userns-{default,auto,keep-id,private}, runplaybooks/drivers/podman/userns.yaml. It writes the mode's[openshell.drivers.podman]settings to/etc/openshell-test/podman-driver.tomland publishesOPENSHELL_TEST_PODMAN_USERNS. Indefaultmode it removes that file, sogateway.tomlhas nouserns,uidmaporgidmapsettings (the old baseline asserted this).openshell_gatewayrole (binariesinstaller) adds those settings togateway.tomlbefore the gateway starts. The packaged installers fail on such an environment instead of testing without the mode.default-userns-baseline.yaml,tests.yamland theuserns-*.yamlplaybooks.feature-specific-integration(Branch E2E, Release Dev, Release Qualification; job ID kept) now callse2e-suites.ymlwithfamily: features. Branch E2E's separatee2e-suitesjob is folded into it.driver-specific-integration(Branch E2E) calls it withfamily: drivers.e2e-podmanrow is unchanged and moves to its ownpodman-e2e-integrationjob, which is added to the Core E2E result.e2e-podmanrow.oci-image,provider-refreshanddriver-podmantestsuites, their playbooks, fixtures and test archives.tests/suites/driversis added toALIGNED_WORKSPACES. The fmt/clippy lists (Branch Checks Format and Lint steps,rust:lint,rust:format,rust:format:check) now run once per test workspace (conformance,drivers,features) instead of once per package, so new suite crates need no lint-list edit. This also lintsfeatures/kubernetes-updatesfor the first time; it raises no warnings.TESTING.mdandCI.mdare updated.CI rows after this PR (
e2e-suites.yml):keycloak-provider-refreshubuntu-docker-rootful-keycloakkeycloak-provider-refreshfedora-podman-{rootful,rootless}-keycloakoci-imageubuntu-docker-rootfuloci-imagefedora-podman-{rootful,rootless}podmanfedora-podman-{rootful,rootless}-userns-{default,auto,keep-id,private}The drivers family runs only in Branch E2E, as
driver-specific-integrationdoes today. The Docker Keycloak row now also runs in Release Dev and Release Qualification, because it belongs to the features family.Still to come in this PR: nothing. The last commit deletes
crates/openshell-conformance, itsCargo.lockentry and itsAGENTS.mdarchitecture row. No other code used the crate. tmachine'stestsuiteslist is now justshell(interactive) ande2e-podman.nextest.yamlstays because the runner rows still run nextest in the guest.Cleanup of providers, settings and fixture resources stays inline, as in the old suites, so a panic can leak them. Names are unique and the VMs are throwaway.
Testing
cargo fmtandcargo clippy -D warningsfor every list entry, including each whole test workspace,cargo check --workspace --locked, lockfile checks and alignment,mise run rust:deny:policy,mise run license:check,markdownlint-cli2, actionlint (no new findings), zizmor (no new findings against main),ansible-playbook --syntax-checkon the changed playbooks, andnix build .#tmachine-configtests/supportUID-map normalizationLocal runs:
mise run e2e:oci-imageagainst a local Docker gateway: 6/6 passed, with no images or sandboxes left behind. Changing an expected UID made the test fail and still clean up.gateway-podman.toml.j2template rendered for the default andprivatemodes and parsed as TOML.sudo -n podmancompared with the old playbooks' context, in a Fedora 44 container:sudo -nsetsHOME=/rootand dropsXDG_RUNTIME_DIR, but rootful Podman's graph root, run root, temp directory and socket are identical to the oldHOME=/root XDG_RUNTIME_DIR=/run/user/0context. Rootless runs astmachineover SSH, where PAM setsXDG_RUNTIME_DIR.Not verified locally (tmachine needs an amd64 KVM host): every tmachine row in the table above, including the Podman OCI rows, the eight user-namespace rows, the Fedora Keycloak rows, and Keycloak restarting after a cached-disk boot.
Checklist