Change binfmt flags to ensure setuid is enabled in qemu-user-static supported CI runs #1077
Conversation
|
Here's a link to the successful test of this change: https://github.com/flux-framework/flux-sched/actions/runs/6220263727/job/16879950413?pr=1077 It's not directly accessible from here anymore due to a rebase. Hopefully this will keep similar problems from coming back, and we might want to apply a similar change to the actions file on core. |
|
@grondo, apparently ReadTheDocs is switching what they require in their YAML at the same time. Fix included here as well since it was required to get the tests green. We'll want that on core for sure. https://blog.readthedocs.com/use-build-os-config/ |
|
We've already incorporated th build.os requirement in flux-core (actually, we only added the config file itself recently when it became a requirement), but thanks! |
| targets: aarch64 | ||
| - name: setup qemu-user-static | ||
| run: | | ||
| docker run --rm --privileged aptman/qus -s -- -p --credential aarch64 |
There was a problem hiding this comment.
Question: The commit message references "This adds the --capability yes argument to the script", but I don't see anything in the actual commit that does something like that (this seems to just replace the dbhi/qus/action action with a docker run comand). Just checking if the commit message is outdated.
There was a problem hiding this comment.
Ah, I'll fix that, I meant "--credential" but mistyped it.
|
Also: update PR title for release notes inclusion 😉 |
problem: Arm64 container build still running into issues even avoiding sudo in the docker container, it's hiding in check run, Debian-based distributions by default register binfmt_misc loaders without support for setuid propagating into the container solution: This adds the `--credential` argument to the script in the container, and strips out the github actions indirection so we can maintain it. This should change the flags from F to OC or OCF, either should allow the setuid of the sudo binary to be preserved
I have no idea how I missed this: problem: install-only builds passed `--with-systemdsystemunitdir` which was not supported in the cmake wrapper and caused a failure solution: add support for accepting the option. It currently doesn't *do* anything, but it's pulled in at least so we can use it when and if we want to.
Problem: much like systemdsystemunitdir, we don't support flux-security and caliper arguments to cmake, unlike systemdunitdir I don't see these being useful since we need to match upstream flux-core Solution: remove the flags from docker-run-checks.sh
Problem: RTD decided to require a new key to define the container, see this post: https://blog.readthedocs.com/use-build-os-config/ Solution: add keys to say we want jammy and python 3.10 to appease the yaml demons
Codecov Report
@@ Coverage Diff @@
## master #1077 +/- ##
========================================
- Coverage 71.6% 71.6% -0.1%
========================================
Files 89 89
Lines 11569 11569
========================================
- Hits 8292 8289 -3
- Misses 3277 3280 +3 |
trws
changed the title
trws
added
the
merge-when-passing
Second try to fix the aarch64 container builders. AFAICT this is an issue with the way the binfmt_flags are set up. I actually like this solution if it works, it's a lot easier to maintain than the opaque github action reference, and gives us more control over the behavior.
NOTE: I changed the behavior to build the arm container on push for debugging, ensure that's rebased out before this merges!