Skip to content

Conversation

@cgwalters
Copy link
Collaborator

No description provided.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new CI workflow for containerized testing on CentOS Stream 10, which is a great addition for improving test coverage. The changes include a new .dockerignore file, a GitHub Action for setting up Ubuntu runners, a Containerfile for the test image, and updates to the Justfile. The implementation is solid, but I have a few suggestions to improve the efficiency and maintainability of the Containerfile and the GitHub Action.

@cgwalters
Copy link
Collaborator Author

This is aiming to add test coverage for #120 and also test coverage for bcvk-in-container.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new CI workflow for containerized testing on CentOS Stream 10. The changes include a new .dockerignore file, a GitHub Action for setting up an Ubuntu runner, a Containerfile for the test environment, and Justfile recipes to run the tests. The overall approach is solid. The GitHub Action is well-structured, with good practices like freeing up disk space and caching. The Containerfile correctly sets up a multi-stage build. I've provided a few suggestions to improve the Containerfile for better efficiency and maintainability, such as optimizing layer caching and avoiding redundant installations. I also have some minor suggestions for the bash scripting in the GitHub Action to improve robustness and adhere to modern practices.

Add a complete end-to-end test workflow for running ephemeral integration
tests in a CentOS Stream 10 container environment. This tests two things
very different from the default main.yml workflow:

- Running in a nested container environment
- bcvk on RHEL-based systems where qemu is at /usr/libexec/qemu-kvm

The implementation provides both GitHub Actions CI and local testing:

**CI Workflow (.github/workflows/main-c10s.yml):**
- Simplified single-job workflow that builds and runs the test container
- Uses the ubuntu-24.04 runner with privileged podman
- All 13 ephemeral integration tests run in the container

**Local Testing (just test-ephemeral-c10s):**
- Builds the test container from tests/fixtures/Containerfile
- Runs with KVM device access and container storage volume
- Provides quick iteration for C10S-specific testing

**Container Structure (tests/fixtures/Containerfile):**
- Multi-stage build: compile bcvk and create nextest archive in build stage
- Runtime stage: C10S base with qemu/libvirt/podman dependencies
- Includes full workspace structure for nextest archive execution
- Pulls test images and runs ephemeral tests by default

**Supporting Files:**
- .dockerignore: Includes .config/ for nextest configuration
- Justfile: Adds build-image-c10s and test-ephemeral-c10s targets

Successfully tested locally with all 13 ephemeral tests passing.

Assisted-by: Claude Code (Sonnet 4.5)
Signed-off-by: Colin Walters <walters@verbum.org>
Copy the bootc-ubuntu-setup action from bootc-dev/bootc and integrate
it into both the main CI and C10S workflows. This action provides:

- Disk space cleanup on GHA runners
- Updated podman/just from Ubuntu plucky
- Unprivileged /dev/kvm access via udev rules
- Optional libvirt stack installation
- Rust cache configuration

The main workflow now uses this action with libvirt: 'true' to install
the full virtualization stack, then removes the pre-installed bcvk
binary to ensure we test the locally built version. The C10S workflow
uses it for basic setup (podman, KVM access). This consolidates setup
logic and ensures consistent environment configuration across workflows.

Assisted-by: Claude Code (Sonnet 4.5)
Signed-off-by: Colin Walters <walters@verbum.org>
The bootc-ubuntu-setup action doesn't include go-md2man, which is
needed by the Makefile to generate man pages during the build step.
Add explicit installation step for go-md2man after the environment setup.

Assisted-by: Claude Code (Sonnet 4.5)
Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters
Copy link
Collaborator Author

Hmm, this works locally, not yet sure why it's timing out in the GHA run.

@cgwalters cgwalters marked this pull request as draft November 4, 2025 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant