-
Notifications
You must be signed in to change notification settings - Fork 339
ci: Sync bootc-ubuntu-setup action from bootc-dev/infra #3556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The CI was failing because we were pulling podman/crun/skopeo from Debian testing which has become unreliable. The bootc-dev/infra repository maintains a reusable action that uses Ubuntu's plucky repository instead, which is more appropriate for ubuntu-24.04 runners. This also brings in additional improvements from the shared action: - Disk space cleanup on the runner - Unprivileged /dev/kvm access setup - Optional libvirt stack support Assisted-by: ClaudeCode (Claude Opus 4.5)
There was a problem hiding this 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 GitHub Action, bootc-ubuntu-setup, which aims to streamline the setup of bootc development environments on Ubuntu runners. The action addresses previous CI failures by switching to Ubuntu's 'plucky' repository for core packages like Podman, crun, and Skopeo. Additionally, it incorporates several improvements, including disk space cleanup on runners, enabling unprivileged /dev/kvm access, and providing optional support for the libvirt virtualization stack. The changes are well-motivated and enhance the reliability and functionality of the CI pipeline.
| - name: Enable unprivileged /dev/kvm access | ||
| shell: bash | ||
| run: | | ||
| set -xeuo pipefail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting MODE="0666" for /dev/kvm makes it world-writable, which is a significant security concern. It's best practice to restrict access to the kvm group by using MODE="0660" and ensuring the user running the CI is part of that group. This minimizes the attack surface.
echo 'KERNEL=="kvm", GROUP="kvm", MODE="0660", OPTIONS+="static_node=kvm"' | sudo tee /etc/udev/rules.d/99-kvm4all.rulesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing is done on bootc-dev/infra and is just for the ephemeral runners. @cgwalters Is this valid to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah I think /dev/kvm is fine to be world accessible, it's a pretty hardened kernel interface overall.
| done | ||
| # Apt removals in foreground, as we can't parallelize these | ||
| for x in ${unwanted_pkgs[@]}; do | ||
| /bin/time -f '%E %C' sudo apt-get remove -y $x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good shell scripting practice to quote variables when expanding them, especially in commands, to prevent word splitting or glob expansion. While apt-get remove package names typically don't contain spaces, using "$x" ensures robustness against unexpected characters.
/bin/time -f '%E %C' sudo apt-get remove -y "$x"| rm -rf "$td" | ||
|
|
||
| # Also bump the default fd limit as a workaround for https://github.com/bootc-dev/bcvk/issues/65 | ||
| sudo sed -i -e 's,^\* hard nofile 65536,* hard nofile 524288,' /etc/security/limits.conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current sed command relies on the exact line * hard nofile 65536 being present in /etc/security/limits.conf. If this line is missing or differs, the file descriptor limit will not be updated, potentially leaving the workaround for bootc-dev/bcvk#65 unapplied. A more robust and less intrusive approach is to create a new file in /etc/security/limits.d/ to set the limit, which will be respected by systemd and login.
echo "* hard nofile 524288" | sudo tee /etc/security/limits.d/99-bcvk.conf
The CI was failing because we were pulling podman/crun/skopeo from Debian testing which has become unreliable. The bootc-dev/infra repository maintains a reusable action that uses Ubuntu's plucky repository instead, which is more appropriate for ubuntu-24.04 runners.
This also brings in additional improvements from the shared action:
Assisted-by: ClaudeCode (Claude Opus 4.5)