Skip to content

feat: Support this role in container builds#243

Merged
martinpitt merged 1 commit intolinux-system-roles:mainfrom
martinpitt:bootc
Jun 25, 2025
Merged

feat: Support this role in container builds#243
martinpitt merged 1 commit intolinux-system-roles:mainfrom
martinpitt:bootc

Conversation

@martinpitt
Copy link
Copy Markdown
Contributor

@martinpitt martinpitt commented Jun 23, 2025

Feature: Support running the cockpit role during container builds.

Reason: This is particularly useful for building bootc derivative OSes.

Result: The role now works during container builds. The bootc container scenarios run in CI, which ensures that the role works in buildah build environment. This allows us to officially support this role for image mode builds.

Detect if the system is booted (with systemd), and skip all runtime operations and checks if not. Also use firewall-offline-cmd which works also in non-booted environments -- we are only/primarily interested in the persistent firewall config anyway.

Make tests_verify_fullstack.yml a full bootc end-to-end test, as that covers the most code paths.

https://issues.redhat.com/browse/RHEL-98911


Requirements:

Summary by Sourcery

Enable the cockpit role to be executed during container (Buildah) image builds by detecting systemd boot status, skipping unsupported runtime operations, using persistent firewall tooling, and extending CI tests to cover buildah QEMU scenarios.

New Features:

  • Detect systemd boot status and skip runtime operations in non-booted container build environments
  • Switch firewall checks to use firewall-offline-cmd for persistent configuration

Enhancements:

  • Guard tasks and tests on the new __metrics_is_booted flag to enable containerbuild support
  • Convert tests_verify_fullstack.yml into a bootc end-to-end test with a Buildah QEMU deployment
  • Update galaxy metadata tags to include containerbuild
  • Introduce sudo_password vault variable for CI tests
  • Conditionally adjust PCP command usage when systemd is not available

Summary by Sourcery

Enable the metrics Ansible role to run in container (Buildah) image builds by detecting when systemd isn’t running, skipping unsupported runtime tasks, and anchoring firewall changes to persistent configuration.

New Features:

  • Detect systemd boot status and skip runtime operations in non-booted container environments
  • Switch firewall management to firewall-offline-cmd for persistent configuration

Enhancements:

  • Guard tasks and tests on a __metrics_is_booted flag to enable containerbuild support
  • Convert fullstack tests into a bootc end-to-end Buildah QEMU scenario
  • Adjust PCP command usage when systemd is unavailable

Build:

  • Add containerbuild tag to Galaxy metadata

CI:

  • Extend CI workflows to run bootc container build scenarios and update tests to conditionally run on __metrics_is_booted

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai bot commented Jun 23, 2025

Reviewer's Guide

This PR enables container build support by detecting if the system is booted under systemd and gating all runtime tasks and tests behind a new __metrics_is_booted flag, switches firewall commands to offline mode, adapts PCP invocation, extends CI full-stack tests with a Buildah QEMU scenario, and updates metadata to reflect containerbuild support.

Sequence diagram for role execution in container build environments

sequenceDiagram
    actor CI as CI Pipeline
    participant Buildah as Buildah Container
    participant Role as Cockpit Role
    CI->>Buildah: Start container build
    Buildah->>Role: Execute role tasks
    Role->>Role: detect_systemd_boot()
    alt System is booted (systemd present)
        Role->>Role: run runtime operations
    else System is not booted (container build)
        Role->>Role: skip runtime operations
    end
Loading

ER diagram for new __metrics_is_booted flag in role data model

erDiagram
    METRICS_ROLE {
        bool __metrics_is_booted
    }
    METRICS_ROLE ||--o{ TASKS : guards
    TASKS {
        string name
        bool runtime_only
    }
Loading

Class diagram for booted system detection and runtime gating

classDiagram
    class MetricsRole {
        +__metrics_is_booted : bool
        +detect_systemd_boot()
        +run_runtime_tasks()
    }
    MetricsRole : +detect_systemd_boot() sets __metrics_is_booted
    MetricsRole : +run_runtime_tasks() only if __metrics_is_booted == true
Loading

File-Level Changes

Change Details Files
Detect systemd boot status and set __metrics_is_booted flag
  • Invoke systemctl is-system-running to probe boot state
  • Fail early if systemd is not installed
  • Set __metrics_is_booted fact based on command output
tasks/main.yml
Guard role execution and tests on boot detection flag
  • Wrap service operations and restores behind __metrics_is_booted checks
  • Add `when: __metrics_is_booted
bool` to various test tasks to skip in non-booted environments
Use offline firewall commands
  • Replace firewall-cmd with firewall-offline-cmd in firewall verification tasks
tests/check_firewall_selinux.yml
Conditional PCP command invocation
  • Invoke pcp when booted, fall back to pcp --version when not
tests/check_pcp.yml
Enhance full-stack tests for container builds
  • Add a QEMU Buildah deployment step in tests_verify_fullstack.yml
  • Set missing vars (__metrics_is_booted, metrics_provider) for bootc validation
tests/tests_verify_fullstack.yml
Update metadata for container build support
  • Add containerbuild tag to Galaxy metadata
meta/main.yml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@martinpitt
Copy link
Copy Markdown
Contributor Author

Ah, our new favourite "unique name:" warning. Sent to performancecopilot/ansible-pcp#85 and included here.

@martinpitt
Copy link
Copy Markdown
Contributor Author

[citest]

@martinpitt
Copy link
Copy Markdown
Contributor Author

martinpitt commented Jun 24, 2025

Both CentOS 7 and CentOS 8 fail in TF with

TASK [Ensure the service and the port status with the firewall role] ***********
task path: /tmp/collections-ZvT/ansible_collections/fedora/linux_system_roles/roles/metrics/tasks/firewall.yml:45
Tuesday 24 June 2025  02:08:16 -0400 (0:00:00.033)       0:00:27.997 ********** 
ERROR! the role 'fedora.linux_system_roles.firewall' was not found in fedora.linux_system_roles:ansible.legacy:/tmp/collections-ZvT/ansible_collections/fedora/linux_system_roles/tests/metrics/roles:/root/.ansible/roles:/usr/share/ansible/roles:/etc/ansible/roles:/tmp/collections-ZvT/ansible_collections/fedora/linux_system_roles/tests/metrics

This also reproduces locally with

tox -e qemu-ansible-2.9 -- --image-name centos-8 --log-level=debug tests/tests_verify_basic.yml 

(note: search for "ERROR!", not "fatal:" -- that's just the follow-up "ansible_failed_result is undefined" error, but that's not interesting).

The failure also happens on main, and on c8c8fd4 (rewound main to the state from a few days ago).

@spetrosi @richm does that ring a bell? Did something go wrong with yesterday's release?

@martinpitt
Copy link
Copy Markdown
Contributor Author

@sourcery-ai review

Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @martinpitt - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@martinpitt

This comment was marked as outdated.

@martinpitt
Copy link
Copy Markdown
Contributor Author

martinpitt commented Jun 24, 2025

Back to metrics, locally tox -e qemu-ansible-core-2.17 -- --image-name centos-10 --log-level=debug tests/tests_verify_basic.yml does succeed, and installs ./.tox/ansible_collections/fedora/linux_system_roles/roles/firewall and all the other bits.

However, even after that, tox -e qemu-ansible-2.9 -- --image-name centos-8 --log-level=debug tests/tests_verify_basic.yml still fails -- it doesn't seem to look at .tox/ansible_collections/. So something changed somewhere (galaxy? our test driver? some meta declarations for supported OSes?) that broke the collection on CentOS 7 and 8.

At least after git clean -fdx, the centos-8 run does install .tox/ansible-collections/, it just doesn't use it.

This seems to be specific to Ansible 2.9. It works with 2.16:

tox -e qemu-ansible-core-2.16 -- --image-name centos-8 --log-level=debug tests/tests_verify_basic.yml

Feature: Support running the cockpit role during container builds.

Reason: This is particularly useful for building bootc derivative OSes.

Result: The role now works during container builds. The bootc container
scenarios run in CI, which ensures that the role works in buildah build
environment. This allows us to officially support this role for image
mode builds.

Detect if the system is booted (with systemd), and skip all runtime
operations and checks if not. Also use `firewall-offline-cmd` which
works also in non-booted environments -- we are only/primarily
interested in the persistent firewall config anyway.

Make tests_verify_fullstack.yml a full bootc end-to-end test, as that
covers the most code paths.

https://issues.redhat.com/browse/RHEL-98911
@martinpitt
Copy link
Copy Markdown
Contributor Author

[citest]

@martinpitt martinpitt marked this pull request as ready for review June 24, 2025 14:00
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @martinpitt - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@martinpitt
Copy link
Copy Markdown
Contributor Author

WTH -- it's now doing this role dep failure thing on C10 as well 😢 What is happening here?

@richm
Copy link
Copy Markdown
Collaborator

richm commented Jun 24, 2025

[citest]

@martinpitt
Copy link
Copy Markdown
Contributor Author

[citest]

@martinpitt
Copy link
Copy Markdown
Contributor Author

phew, the mysterious collection dependency failure from yesterday magically resolved itself again. All green except for the F42 spark SELinux failure which isn't new (see #173).

@martinpitt martinpitt merged commit 791a1bb into linux-system-roles:main Jun 25, 2025
35 of 38 checks passed
@martinpitt martinpitt deleted the bootc branch June 25, 2025 09:50
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.

2 participants