tests: Add bootc end-to-end validation test #58
Conversation
Reviewer's GuideThis PR integrates an end-to-end bootc validation test suite into CI by upgrading tox-lsr, pinning Podman for compatibility, adjusting test invocations to exclude bootc tests by default, adding a dedicated QEMU job for bootc-image validation, and introducing a new Ansible playbook for bootc end-to-end testing. Sequence Diagram for CI Bootc E2E Test Execution FlowsequenceDiagram
actor User
participant GHA as "GitHub Actions"
participant TOX as "Tox-LSR"
participant QEMU as "QEMU Environment"
participant ANSIBLE as "Ansible (tests_bootc_e2e.yml)"
User->>GHA: Push code / Open PR
activate GHA
GHA->>GHA: Install dependencies (updated tox-lsr, pinned Podman)
GHA->>TOX: Run standard tests (bootc tests skipped)
activate TOX
TOX-->>GHA: Standard tests complete
deactivate TOX
GHA->>TOX: Run bootc-e2e tests (dedicated step)
activate TOX
TOX->>QEMU: Setup and trigger bootc tests
activate QEMU
QEMU->>ANSIBLE: Execute bootc validation
activate ANSIBLE
ANSIBLE-->>QEMU: Validation results
deactivate ANSIBLE
QEMU-->>TOX: Test results
deactivate QEMU
TOX-->>GHA: Bootc e2e tests complete
deactivate TOX
GHA-->>User: CI Results
deactivate GHA
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
=======================================
Coverage ? 52.31%
=======================================
Files ? 1
Lines ? 346
Branches ? 0
=======================================
Hits ? 181
Misses ? 165
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @martinpitt - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
codespell lol -- why does that only hit now? Presumably just a new version? I'll add a new commit to ignore "passt". |
There was a problem hiding this comment.
Hey @martinpitt - I've reviewed your changes - here's some feedback:
- The
Update podman to 5.xhack block is tightly coupled to Ubuntu ‘plucky’—consider refactoring it into a reusable step or adding a clear TODO with removal criteria and target milestone. - In the
Run bootc validation testsstep, consider uploading both PASS and FAIL logs as artifacts (not only on failure) to simplify debugging of successful runs. - The test loop uses
$TOX_ARGSwithout a default—add a fallback or an explicit check to avoid undefined variable errors during CI execution.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
The removal criterion is clearly documented both in the commit message and a comment. Once this fails, it's a good reminder to revisit if it's still even applicable.
We don't currently do this, it's a waste. If/when we want that, let's do that in a separate PR.
No, the workflow does set it by default. |
This comment was marked as outdated.
This comment was marked as outdated.
| run: >- | ||
| tox -e ${{ matrix.scenario.env }} -- --image-name ${{ matrix.scenario.image }} --make-batch | ||
| --log-level debug --skip-tags tests::infiniband,tests::nvme,tests::scsi | ||
| --log-level debug $TOX_ARGS --skip-tags tests::bootc-e2e |
There was a problem hiding this comment.
Why do you remove previous skip tags here?
There was a problem hiding this comment.
We should also use TOX_ARGS: "--skip-tags tests::infiniband,tests::nvme,tests::scsi" for this.
There was a problem hiding this comment.
Because that's the exact value of the defined TOX_ARGS. The workflow defines that at the top but never actually used it. And it seems sensible to be able to parameterize it. Otherwise we could drop the unused definition at the top and repeat the value (or not, it's not super important for bootc e2e tests)
| for t in $(grep -wl tests::bootc-e2e tests/tests_*.yml); do | ||
| test=$(basename ${t%.yml}) | ||
| image_file="$(pwd)/tests/tmp/$test/qcow2/disk.qcow2" |
There was a problem hiding this comment.
Ah - this is where the /tmp/ directory is used. Hmm - not sure how to get the $LSR_TOX_ENV_TMP_DIR in this context - this may also be problematic when testing the collections format, because the test directory is tests/$rolename/ and not ....$rolename/tests/ there
There was a problem hiding this comment.
This isn't $LSR_TOX_ENV_TMP_DIR at the moment, but OUTPUT="./tmp/$BUILDAH_ID" in bootc-buildah-qcow.sh (in the tox-lsr PR). We can of course change it on both sides.
|
I updated the workflow to the version in #59 which supports both new exclusive bootc-e2e tests as well as modifying existing tests. |
This *is* the correct name for our test dependency. The most recent codespell version started to complain about it.
martinpitt
force-pushed
the
qemu-bootc
branch
2 times, most recently
from
d995c25 to
2ed8ec2
Compare
|
Meh, now F41 fails with "Installing to filesystem: Creating ostree deployment: invalid reference format". This is reproducible locally. Reported as osbuild/bootc-image-builder#949 , and it is fixed in https://bodhi.fedoraproject.org/updates/FEDORA-2025-f68c3fced6 |
Ubuntu 24.04's podman 4.9.3 does not work with the bootc-image-builder container: Bind-mounting the container storage into it makes the container's podman 5 fail with a storage error. Thus install podman 5.4 from Ubuntu 25.04 to regain compatibility. This is rather hackish, but as that is a stable release, it should not break in the future (wrt. library dependencies and such).
These tests run the role during a bootc container image build, deploy the container into a QEMU VM, boot that, and validate the expected configuration there. They run in two different tox environments, and thus have to be run in two steps (preparation in buildah, validation in QEMU). The preparation is expected to output a qcow2 image in `tests/tmp/TESTNAME/qcow2/disk.qcow2`, i.e. the output structure of <https://github.com/osbuild/bootc-image-builder>. There are two possibilities: * Have separate bootc end-to-end tests. These are tagged with `tests::bootc-e2` and are skipped in the normal qemu-* scenarios. They run as part of the container-* ones. * Modify an existing test: These need to build a qcow2 image exactly *once* (via calling `bootc-buildah-qcow.sh`) and skip setup/cleanup and role invocations in validation mode, i.e. when `__bootc_validation` is true. In the container scenario, run the QEMU validation as a separate step in the workflow. See https://issues.redhat.com/browse/RHEL-88396
|
Updated to tox 3.10.0. Most probably we'll land #59 instead, but then this at least serves as a worked example how to do a new test. |
There was a problem hiding this comment.
Hey @martinpitt - I've reviewed your changes - here's some feedback:
- The APT pinning hack for Podman is pretty complex and brittle—consider isolating it into a reusable script or container image and clearly marking its removal checkpoint once Ubuntu 26.04 LTS is in use.
- In the QEMU bootc-e2e step, add an explicit check or log if no image files are found so it doesn’t silently skip all tests when
tests/tmp/*/qcow2/disk.qcow2is empty. - You’ve repeated the tox-lsr version bump across four workflows—consider using a shared variable or workflow template to DRY up that change.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Landed #59. |
3 participants
See https://issues.redhat.com/browse/RHEL-88396 and linux-system-roles/tox-lsr#202. As mentioned in the latter, I also have branches for less trivial roles, firewall (passing) and postgresql (spotted a bug). But I'd like to get this reviewed first, as iterating is cheap.
After we agree to the structure, I'll submit the general structure (first four commits) to .github.git, and send PRs for firewall and postgresql.
Summary by Sourcery
Add end-to-end bootc validation testing and update CI to support bootc-image scenarios
New Features:
CI:
Tests:
Summary by Sourcery
Add bootc end-to-end validation testing and enhance CI workflows to install updated dependencies, pin Podman, and run bootc-e2e tests separately in QEMU
New Features:
CI:
Tests: