Skip to content

test(features): add CDI injection scenarios for Docker and Podman - #4461

Merged
elezar merged 4 commits into
mainfrom
test/4460-tmachine-cdi/elezar
Nov 11, 2026
Merged

elezar merged 4 commits into
mainfrom
test/4460-tmachine-cdi/elezar

Conversation

@elezar

@elezar elezar commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Summary

Add hardware-free CDI injection tests to the features suite and tmachine environments that provide the fixtures. Each test creates a sandbox that selects one scenario device through the Docker or Podman driver and checks what the workload observes against expectations recorded in a spec. This PR builds on #4289, which has merged.

Related Issue

Related to #4460. This PR supplies the initial CDI test infrastructure; the issue remains open to track follow-up coverage.

Changes

  • Compose tmachine environments from base environments with a shared helper, and add a features table so a feature's setup playbooks and inputs apply to each base it supports. The Keycloak and Podman user-namespace variants are built the same way. The Ubuntu Docker Keycloak environment now also runs the base's test-tools playbook.
  • Apply the CDI environments' gateway override through the mechanism fix(tmachine): restore package-installed Podman qualification #4289 added: the cdi_fixtures role writes [openshell.drivers.<driver>] allow_driver_config = true to /etc/openshell-test/gateway-overrides.toml, and every installer merges it into the gateway's own configuration, so the rpm keeps its seeded defaults. Each base in tests/tmachine/config.nix passes its compute driver to setup playbooks as environment_driver.
  • Add a cdi feature with environments ubuntu-docker-rootful-cdi, fedora-podman-rootful-cdi and fedora-podman-rootless-cdi. The cdi_fixtures role creates one CDI device per scenario of the tmachine.example/cdi-scenario kind, installs the spec in /etc/cdi, publishes the expectations, and enables caller driver config, which the gateway requires to accept a per-sandbox CDI device.
  • Keep the fixtures usable by containers on SELinux hosts. The fixture files live under /usr/local/share, whose default type containers can read, and the socket's directory is labeled container_file_t. A systemd service serves the echo socket with systemd-socket-activate --inetd. It opens the socket to any user, because the process creates it root-only, and runs as container_t on SELinux hosts, because a container may not connect to a socket owned by a service's default unconfined_service_t. The playbook connects to the socket after starting the service and fails if it does not echo. Another service creates the host node /dev/tmachine-cdi-null on every boot with the SELinux type of /dev/null, so a rootless runtime that bind-mounts it can use it.
  • Publish the path of the expectations to the tests as OPENSHELL_TEST_CDI_SCENARIOS through playbooks/test-env.yaml. The expectations name the device kind, so the suite assumes no kind or path of its own, fails when the variable is unset, and runs in any environment that creates matching devices and sets it.
  • Add the cdi features package with a scenario wrapper (env, exists, contents, filetype, read, write, non-root, in-group-of and echo-socket checks) and 10 tests, one per scenario. echo-socket connects to a unix socket from the workload and expects its payload back; it needs perl in the workload image, which the default image has, and reports no-client without it. The write check applies to a file that exists: it opens it for append without writing, and a path that does not exist reports missing, so a check on the wrong path fails. Nothing creates files on the host. There is no check for creating an entry in a mounted directory yet, so the scenarios no longer cover that. Run it from e2e-suites.yml in the three environments above, with the deb installer for Docker and the rpm installer for Podman.

Removed scenario: device-node-with-gid

This scenario is not in this PR. The Podman driver rejects a CDI-added supplementary group at sandbox startup (#5305), and the scenario is recorded there and below so it can be restored once the driver is fixed. Its original definition in the cdi_fixtures role spec (vars/scenarios.yaml), with a group openshell-cdi-fixture created in the guest and the node made with mknod:

device-node-with-gid scenario (verbatim from fb39ac1)
# The device is mode 0660 with a group that only `additionalGids` grants.
- name: device-node-with-gid
  fixtures:
    - {path: group-device, type: chardev, major: 1, minor: 3, mode: "0660", group: openshell-cdi-fixture}
  edits:
    additionalGids: ["{{ cdi_fixture_group.gid | int }}"]
    deviceNodes:
      - path: /dev/tmachine-cdi-group-null
        hostPath: "{{ cdi_scenario_dir }}/group-device"
        permissions: rw
        uid: 0
        gid: "{{ cdi_fixture_group.gid | int }}"
        fileMode: 432
  expect:
    checks:
      - {type: non-root}
      - {type: group-of, path: /dev/tmachine-cdi-group-null}
      - {type: write, path: /dev/tmachine-cdi-group-null, expect: denied}

The group-of check was later renamed in-group-of. The role's tasks/scenarios.yaml creates the system group openshell-cdi-fixture (registered as cdi_fixture_group) and makes the node with mknod ... c 1 3.

Release Notes

Testing

  • Checks appropriate to the affected code and behavior pass: the repository pre-commit hooks pass on each commit, and Clippy for the features workspace is clean.
  • Unit tests added/updated: 14 tests in the cdi package cover check parsing, comparison and the generated probe shell, including a probe against a real unix socket and the device kind read from the expectations.
  • E2E tests added/updated: the cdi package passed all 24 tests (the 14 unit tests and the 10 scenarios) in ubuntu-docker-rootful-cdi (deb) and fedora-podman-rootless-cdi (rpm) on this stack. fedora-podman-rootful-cdi (rpm) passed 22 tests (12 unit and 10 scenarios) at an earlier revision and was not re-run locally; CI runs all three. Each ran with nix run .#openshell-test-runner -- --environment <environment> --installer <installer> --family features --package cdi on a local Apple Silicon Mac, using locally built arm64 artifacts.

The expectations in the spec record the sandbox's behavior today, so they will change as CDI grant derivation changes. x86_64 was not run locally. Rootless Podman cannot create device nodes, so the device-node scenario uses a host node at the same path as the container path.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Testing documentation updated; runtime architecture is unchanged

@copy-pr-bot

copy-pr-bot Bot commented Oct 13, 2026

Copy link
Copy Markdown

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.

@github-actions

Copy link
Copy Markdown

@elezar
elezar changed the base branch from main to codex/cdi-name-parser October 14, 2026 14:39
@elezar
elezar marked this pull request as ready for review October 15, 2026 12:09
@github-actions

Copy link
Copy Markdown

Compatibility review (advisory)

Needs review

No source-level SDK, CLI, configuration, policy, Helm, or state contract changes were found. Qualification remains incomplete because release VM drivers embed newly hashed Linux runtime archives whose semantic delta is unavailable in the supplied source.

Not inspected: api-behavior

1. api-behavior — NOT ACKED

The pinned x86_64 and aarch64 Linux VM runtime archive hashes changed, but the archive contents and semantic differences are not present in the supplied source.

  • Impact: Consumers of Linux VM sandboxes may receive changed libkrun, firmware, or umoci behavior that cannot be checked for compatibility from this repository.
  • Action: Provide an immutable provenance or source diff for both replacement archives and confirm runtime compatibility, or restore the baseline archives for this train.
  • PR: test(features): add CDI injection scenarios for Docker and Podman #4461
Review details
  • Execution: incomplete
  • Candidate: PR #4461 (782cf8bdcab7138914bfdd859ae6a3f904c4ead6)
  • Baseline: 360f6d5cc2ad707bf84cbcda239e08086a527cac (360f6d5cc2ad707bf84cbcda239e08086a527cac)
  • Reviewed: rust-sdk, python-sdk, go-sdk, typescript-sdk, cli, configuration, policy, helm, state

Advisory review; publication gates are unchanged.

Branch check run

@SDAChess SDAChess left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some questions, I'm surprised there are changes related to the VM driver, are these on purpose?

Comment thread nix/pkgs/vm-runtime.nix Resolved
@elezar
elezar force-pushed the test/4460-tmachine-cdi/elezar branch from 844e44f to e9fa277 Compare October 15, 2026 14:51
@elezar
elezar force-pushed the codex/cdi-name-parser branch from 360f6d5 to 01b0b55 Compare October 15, 2026 15:01
@elezar
elezar force-pushed the test/4460-tmachine-cdi/elezar branch from e9fa277 to fb39ac1 Compare October 15, 2026 15:01
@github-actions

Copy link
Copy Markdown

This pull request has had no activity for 14 days and is now marked stale.

@github-actions github-actions Bot added the state:stale Inactive item at risk of automatic closure. label Oct 30, 2026
@elezar
elezar force-pushed the codex/cdi-name-parser branch 2 times, most recently from 1228858 to 22db4b6 Compare November 3, 2026 21:03
@github-actions github-actions Bot removed the state:stale Inactive item at risk of automatic closure. label Nov 4, 2026
@elezar
elezar force-pushed the test/4460-tmachine-cdi/elezar branch from fb39ac1 to 82fed66 Compare November 5, 2026 12:26
@copy-pr-bot

copy-pr-bot Bot commented Nov 5, 2026

Copy link
Copy Markdown

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.

@elezar elezar changed the title test(tmachine): add Docker CDI injection coverage and local DEB builds test(features): add CDI injection scenarios for the Docker driver Nov 5, 2026
@elezar
elezar changed the base branch from codex/cdi-name-parser to main November 5, 2026 12:26
@elezar
elezar force-pushed the test/4460-tmachine-cdi/elezar branch from 82fed66 to 3f0a69c Compare November 5, 2026 14:17
@elezar elezar changed the title test(features): add CDI injection scenarios for the Docker driver test(features): add CDI injection scenarios for Docker and Podman Nov 5, 2026
@elezar elezar added the test:e2e Requires end-to-end coverage label Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Label test:e2e applied, but pull-request/4461 is at fb39ac1 while the PR head is 3f0a69c. A maintainer needs to comment /ok to test 3f0a69cde952a3939965efdee9d5c3bacc6689ac to refresh the mirror. Once the mirror catches up, re-run Branch E2E Checks from the Actions tab.

@elezar

elezar commented Nov 5, 2026

Copy link
Copy Markdown
Member Author

/ok-to-test 3f0a69c

@elezar elezar added area:build Related to CI/CD and builds topic:testing labels Nov 5, 2026

@SDAChess SDAChess left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the shell is fine with me. One small fix for the echo fixture and a non-blocking parser nit below.

Comment thread tests/suites/features/cdi/src/lib.rs Resolved
@elezar
elezar force-pushed the test/4460-tmachine-cdi/elezar branch from 3f0a69c to 2f2907b Compare November 6, 2026 15:01

@matthewgrossman matthewgrossman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, Evan. The layering (withSetup/features), one crate per feature, explicit CI rows, and expect_compute_driver all fit how we're structuring features suites. My comments are about how inputs reach the tests, not the scenarios themselves.

1. Scenario expectations should come through OPENSHELL_TEST_* and test-env.yaml.
The suite reads a hard-coded guest path (/var/lib/openshell-test-inputs/cdi/scenarios.json), with an override named OPENSHELL_CDI_SCENARIOS. The pattern from the Keycloak example is that setup includes playbooks/test-env.yaml to write OPENSHELL_TEST_* into /etc/openshell-test/env.d/<name>.env, the runner sources those into the test environment, and the suite reads plain variables. Reading a fixed guest path means the suite doesn't run unchanged against any deployment that supplies the same variables, and the openshell-test-inputs file-path style is the one we're moving away from (OPENSHELL_TEST_INPUT_DIR in the old Podman playbooks). Could the cdi_fixtures role publish something like OPENSHELL_TEST_CDI_SCENARIOS (and the device class, if useful) this way?

2. /etc/openshell-test/driver.toml is a new general mechanism.
It's slurped in detect.yaml and templated into the Docker and Podman gateway.toml. It fits "one environment per gateway config", but it isn't one of the channels we've settled on, and other features will likely start using it. Is this meant to be the shared way for an environment to add driver settings? If so I'd like to agree on the name/contract and document it in TESTING.md; if it's CDI-specific, it may be better scoped that way.

3. Packaged gateway override.
openshell_packaged_gateway now writes a fresh operator gateway.toml containing only the extra driver settings. Please confirm nothing the generated config previously supplied for the rpm Podman rootless path (socket path, openshell_gateway_podman_driver_settings, etc.) is dropped when this file exists. The PR notes validation was arm64-only, so CI on x86_64 will be the first real check.

4. Hidden perl dependency.
echo-socket needs perl in the workload image. Fine for the default image, but it'd be good for that to be explicit rather than inferred. For example, tie it to OPENSHELL_TEST_BASE_IMAGE, or state it in the suite docs.

Smaller notes, non-blocking:

  • The write probe uses : >> path, so on a path that doesn't exist yet (.../directory/probe, .../new-file) it creates a file on the host if the write is allowed. The comment says nothing is written.
  • rejected isn't exercised by any scenario after device-node-with-gid was dropped, and an empty rejected string matches any failure.
  • The expectations record current behavior (e.g. read: denied on read-only mounts). A short "characterization, not spec" note would keep them from being read as desired behavior.

@elezar
elezar force-pushed the test/4460-tmachine-cdi/elezar branch 3 times, most recently from 0bf9937 to 06125b4 Compare November 10, 2026 13:26
@elezar
elezar changed the base branch from main to test/3454-package-driver-validation/elezar November 10, 2026 13:26
SDAChess
SDAChess previously approved these changes Nov 10, 2026
@elezar

elezar commented Nov 10, 2026

Copy link
Copy Markdown
Member Author

Thanks for the review. I've reworked some things here to address things. This includes stacking this PR on #4289 -- making the driver config changes there since that would be required anyway for a switch to package installers for all CI lanes.

Thanks for this, Evan. The layering (withSetup/features), one crate per feature, explicit CI rows, and expect_compute_driver all fit how we're structuring features suites. My comments are about how inputs reach the tests, not the scenarios themselves.

1. Scenario expectations should come through OPENSHELL_TEST_* and test-env.yaml. The suite reads a hard-coded guest path (/var/lib/openshell-test-inputs/cdi/scenarios.json), with an override named OPENSHELL_CDI_SCENARIOS. The pattern from the Keycloak example is that setup includes playbooks/test-env.yaml to write OPENSHELL_TEST_* into /etc/openshell-test/env.d/<name>.env, the runner sources those into the test environment, and the suite reads plain variables. Reading a fixed guest path means the suite doesn't run unchanged against any deployment that supplies the same variables, and the openshell-test-inputs file-path style is the one we're moving away from (OPENSHELL_TEST_INPUT_DIR in the old Podman playbooks). Could the cdi_fixtures role publish something like OPENSHELL_TEST_CDI_SCENARIOS (and the device class, if useful) this way?

I've updated this. The path to the scenarios file is now published, and the CDI device kind is included in the scenario instead of as a separate constant / variable.

2. /etc/openshell-test/driver.toml is a new general mechanism. It's slurped in detect.yaml and templated into the Docker and Podman gateway.toml. It fits "one environment per gateway config", but it isn't one of the channels we've settled on, and other features will likely start using it. Is this meant to be the shared way for an environment to add driver settings? If so I'd like to agree on the name/contract and document it in TESTING.md; if it's CDI-specific, it may be better scoped that way.

This is what is addressed in #4289. I changed the implementation there to define a gateway config override as this would be required for non-driver configs at some point. I also created #5446 to track adding "merged" configs / overrides as a first-class entity.

3. Packaged gateway override. openshell_packaged_gateway now writes a fresh operator gateway.toml containing only the extra driver settings. Please confirm nothing the generated config previously supplied for the rpm Podman rootless path (socket path, openshell_gateway_podman_driver_settings, etc.) is dropped when this file exists. The PR notes validation was arm64-only, so CI on x86_64 will be the first real check.

The new merge logic should maintain existing keys since we're doing a deep merge.

4. Hidden perl dependency. echo-socket needs perl in the workload image. Fine for the default image, but it'd be good for that to be explicit rather than inferred. For example, tie it to OPENSHELL_TEST_BASE_IMAGE, or state it in the suite docs.

Documented. For what it's worth, I chose perl because it is available in the base image instead of requiring that these tests use python.

Smaller notes, non-blocking:

  • The write probe uses : >> path, so on a path that doesn't exist yet (.../directory/probe, .../new-file) it creates a file on the host if the write is allowed. The comment says nothing is written.
  • rejected isn't exercised by any scenario after device-node-with-gid was dropped, and an empty rejected string matches any failure.
  • The expectations record current behavior (e.g. read: denied on read-only mounts). A short "characterization, not spec" note would keep them from being read as desired behavior.

Base automatically changed from test/3454-package-driver-validation/elezar to main November 10, 2026 15:25
@elezar
elezar dismissed SDAChess’s stale review November 10, 2026 15:25

The base branch was changed.

Define the base environments once and build the Keycloak and
user-namespace variants from them with a shared helper, instead of
listing the Ubuntu Docker variants by hand. Add a features table so a
feature's setup playbooks and inputs apply to each base it supports.

The Ubuntu Docker Keycloak environment now also gets the base's
test-tools playbook, as the Fedora Keycloak environments already do.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Add the cdi_fixtures role, which defines one CDI device per scenario of
the tmachine.example/cdi-scenario class together with the host files each
references, so CDI injection can be tested without GPU hardware.

Run it in a cdi feature environment on each Docker and Podman base,
such as ubuntu-docker-rootful-cdi, instead of the base environments.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Add the cdi features package, which creates a sandbox that selects one
CDI scenario device on the gateway's Docker or Podman compute driver and
checks what the workload sees against the scenario's published
expectations. The first test covers a mount under a directory the
sandbox already makes read-only, and the merge of spec and device edits.

Run the suite in ubuntu-docker-rootful-cdi with the deb installer, and in
fedora-podman-rootful-cdi and fedora-podman-rootless-cdi with the rpm
installer.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Add the scenarios for read-only and read-write mounts, library parent
derivation, a device node, and a unix socket, with one test per scenario
in the cdi suite. Each test delegates to the scenario wrapper, which
checks the workload against the expectations recorded in the spec.

Add the fixture machinery the scenarios need: a pipe fixture type, a
host device node that a systemd service creates and labels on every
boot, and the echo socket, which a systemd service serves on every
boot. The node has the SELinux type of /dev/null so that a rootless
runtime, which bind-mounts it, can use it.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar
elezar force-pushed the test/4460-tmachine-cdi/elezar branch from 06125b4 to e7776d2 Compare November 10, 2026 15:54
@elezar
elezar added this pull request to the merge queue Nov 11, 2026
Merged via the queue into main with commit 1580589 Nov 11, 2026
258 of 261 checks passed
@elezar
elezar deleted the test/4460-tmachine-cdi/elezar branch November 11, 2026 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:build Related to CI/CD and builds test:e2e Requires end-to-end coverage topic:testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants