-
Notifications
You must be signed in to change notification settings - Fork 71
Systemd-unit based runner (new) #2155
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
Minor: remove pointless imports
Minor: mark systemd based runner as experimental
eb56348 to
03b5dcb
Compare
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (80.48%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2155 +/- ##
==========================================
- Coverage 53.34% 53.02% -0.32%
==========================================
Files 399 395 -4
Lines 42907 42698 -209
Branches 7945 7928 -17
==========================================
- Hits 22887 22641 -246
- Misses 19214 19255 +41
+ Partials 806 802 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
db96314 to
16d4fb6
Compare
pieqq
left a comment
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.
Good stuff! I left a few inline suggestions, mainly on naming stuff.
This PR includes plz-run for the snap version, but not for the Debian packages. Do we need to package plz-run in our PPA and pull it when installing Checkbox?
Also document what feature flags are
|
Even though I have ideas as to the future of this and how it may become the default executor for checkbox eventually, currently it is experimental. The only purpose of it right now is to escape snapd/aa sandbox, so we don't need it on debian at all. I have added an error message if one tries to enable it without having the required plz-run binary though |
pieqq
left a comment
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.
Thanks for the fix and the additional logging information!
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.
Pull Request Overview
This PR introduces an experimental systemd-based job runner for Checkbox that addresses permission issues with rfkill and provides a foundation for escaping the snap/AppArmor sandbox. The new runner executes jobs as systemd units rather than child processes, which can help bypass certain permission restrictions in strict snap environments.
- Adds a new systemd-based job execution mechanism using plz-run utility
- Implements feature flag system to enable/disable the experimental runner
- Adds comprehensive test coverage for both execution methods
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| checkbox-ng/plainbox/impl/execution.py | Core implementation of systemd unit execution logic and refactored job execution methods |
| checkbox-ng/plainbox/impl/session/state.py | Added feature flag for systemd-based runner with dependency validation |
| checkbox-ng/plainbox/impl/session/assistant.py | Integration of systemd runner flag into job execution flow |
| checkbox-ng/plainbox/impl/test_execution.py | Comprehensive unit tests for both execution methods |
| checkbox-ng/plainbox/impl/config.py | Configuration option for systemd-based runner feature |
| checkbox-ng/checkbox_ng/launcher/subcommands.py | Minor refactoring to use normal user utility function |
| checkbox-core-snap/*/snap/snapcraft.yaml | Added plz-run dependency to all snap configurations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # See https://bugs.launchpad.net/snapd/+bug/2003955 | ||
| env["SYSTEMD_IGNORE_CHROOT"] = "1" | ||
| # run the command | ||
|
|
Copilot
AI
Nov 3, 2025
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.
[nitpick] Empty line should be removed to improve code readability.
| from argparse import ArgumentTypeError | ||
| from argparse import RawDescriptionHelpFormatter | ||
| from argparse import SUPPRESS | ||
| from argparse import ArgumentTypeError, RawDescriptionHelpFormatter, SUPPRESS |
Copilot
AI
Nov 3, 2025
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.
[nitpick] The import statement was changed from multiple lines to a single line. While this works, the original multi-line format may be more readable for long import lists.
| from argparse import ArgumentTypeError, RawDescriptionHelpFormatter, SUPPRESS | |
| from argparse import ( | |
| ArgumentTypeError, | |
| RawDescriptionHelpFormatter, | |
| SUPPRESS, | |
| ) |
Description
This introduces a new experimental systemd-based runner. This new runner adresses in the short term the rfkill permission denied situation but the objective is to make it, in the long run, the default runner for checkbox strict frontends.
For now this new runner is disabled by default and clearly marked as experimental when enabled.
Resolved issues
Fixes: #1211
Fixes: https://warthogs.atlassian.net/browse/CHECKBOX-2003
Documentation
New runner clearly documented in the docstring
Tests
New function is unit tested.
Tested on RPI4 on core24, This can be tested in any environment that has plz-run installed. Currently, only snap packaging is supported.
To test this use the following launcher:
The result must be unchanged, the runner should be completely transparent to the user.
Snap builds: