lints: Add var-tmpfiles #1101
lints: Add var-tmpfiles #1101
Conversation
cgwalters
force-pushed
the
tmpfiles
branch
3 times, most recently
from
114ab01 to
2f8bb31
Compare
|
OK cool! This one is working well! In our own hack/Containerfile without the "provision-derived: Clean lots more stuff" commit: |
|
And indeed taking the updated bootc from this PR and using it in https://gitlab.com/fedora/bootc/base-images/-/tree/wip-baseimage-rework I see And yeah as I noted in the commit message I think we really really want |
|
https://artifacts.dev.testing-farm.io/370c7d10-3914-4736-a2a9-e118805e3b3b/ OK did podman in c9s really just break? |
What does podman info say? Is netavark installed? In general if netavark is installed podman should default to it and not use cni which is likely why cni is not pulled in as dependency. I am not sure what you are doing there but there is some special smart logic in podman to default to cni when it detects existing images in the container store. This is because when we updated to podman 4.0 we had to remain backwards compatible but wanted to default to netavark. So in order to do that the first podman/buildah command tries to create a special flag file when it does not exist yet to record the chosen netavark backend bases on the is netavark installed and are ther eimages in the store conditions. |
Ah hah, that's it. I can reproduce this way:
Yet if I do a |
|
It does seem to work if I invoke just a plain no-op I'm pretty sure this is a regression though in recent podman in c9s, because this job has been passing for a while, and we did just pick up newer changes. Is it worth tracking as a bug on the podman side? The workaround here is trivial but I do worry this is a bit of a canary for other potential failure scenarios. |
|
Or is this just that we need to update skopeo with a newer vendored c/storage maybe? |
See containers#1101 (comment) Basically things break unless "podman" initializes the c/storage instance right now. Signed-off-by: Colin Walters <walters@verbum.org>
|
The behavior has not changed since podman 4.0. If something has changed than not on the podman/buildah side. (just my guess of course) Note using skopeo to import image before running podman/buildah before will bypass the detection because skopeo does not pull in any network code whatsoever so if you do that then yes it is expected to run podman before it. I know the detect logic sucks but it is what it is and we removed CNI support from podman 5.0 upstream and in Centos/RHEL 10 so this is "solved" already as we only use netavark going forward. |
See containers#1101 (comment) Basically things break unless "podman" initializes the c/storage instance right now. Signed-off-by: Colin Walters <walters@verbum.org>
I am 93.7% confident that it was something that changed in the c9s images that triggered this; digging in only slightly: It is notable that both podman and skopeo rev'd. But obviously one would need to try reverting one or both of those individually on top the new base to see.
Yeah, this makes total sense - upgrades are hard and I certainly don't have any better suggestions offhand. My suggestion is: let's just file this issue mentally in the background and if it happens in a way more likely other users or customers may hit it, we may need to think about a stronger workaround (perhaps skopeo detecting if it's the first thing writing to c/storage and running the same podman code?). |
omg the set of files dropped by rpm-related stuff is TOO DAMN HIGH |
|
This adapts code rewritten from rpm-ostree to synthesize tmpfiles.d entries. Signed-off-by: Colin Walters <walters@verbum.org>
This checks for content in `/var` that is missing systemd tmpfiles.d entries. Signed-off-by: Colin Walters <walters@verbum.org>
Because if failures somehow creep in we really want to know. Signed-off-by: Colin Walters <walters@verbum.org>
In some cases we have /var/lib/rhsm too... *cry* Signed-off-by: Colin Walters <walters@verbum.org>
I put these in https://gitlab.com/fedora/bootc/base-images/-/merge_requests/92 too but let's fast track them to our images here so we unblock testing tmpfiles.d translation. Signed-off-by: Colin Walters <walters@verbum.org>
For the same reason we avoid doing this in other code like in lints.rs; it's reasonable for someone to mount a volume on `/var/cache/dnf` for example in a container build, and we don't want to try to convert it to tmpfiles.d. Signed-off-by: Colin Walters <walters@verbum.org>
@Luap99, does the upper statement also apply to Podman 5.0+? I am asking since the issue we ran into here could very well be hit by users on Image Mode going forward. Setting the option in containers.conf could solve the issue, right? |
As long as CNI is compiled in ( containers.conf will work but then I don't know where/how you run your code. Just setting in containers.conf faces the same problem as our auto detection logic, if a user is updating the system and was using CNI before you cannot just hard code netavark in there as this will break all their network configs. But if you 100% know that this is not an update you can set it to netavark. That said you don't have to use containers.conf, the podman detection logic writes I know this is not great bug given this problem is already solved by no longer using cni I rather not touch that logic in podman. |
|
Thanks a lot for elaborating, @Luap99 !
Image Mode is a new offering, so we won't run into the CNI issue other than the one discovered here. So we should be good. Since bootc depends on Podman, a potential fix maybe to ship a containers.conf.d/bootc.conf that sets netavark. |
|
OK lifting draft on this one! |
lints: Add unit test checking pass/fail counts
We have a bug here that this will catch.
Signed-off-by: Colin Walters walters@verbum.org
lints: Actually skip skipped lints
Oops.
Signed-off-by: Colin Walters walters@verbum.org
lint: Add --skip
This allows people to explicitly skip individual lints; especially
useful for warnings. But maybe people have a legitimate reason
to skip ones we're calling fatal too.
Prep especially for supporting
--fix --skip.Signed-off-by: Colin Walters walters@verbum.org
lints: Fix unit tests to use RootType::Alternative
Prep for adding unit tests which hard require the running root
(like tmpfiles.d) - we can't unit test those.
Signed-off-by: Colin Walters walters@verbum.org
utils: Fix compliation standalone
We use
tokio::testso need the macros feature.Signed-off-by: Colin Walters walters@verbum.org
utils: Add a method to split an iterator
Signed-off-by: Colin Walters walters@verbum.org
provision-derived: Clean lots more stuff
Yeah, we're going to need a
dnf clean all --everything...Signed-off-by: Colin Walters walters@verbum.org
tmpfiles: New crate
This adapts code rewritten from rpm-ostree to synthesize
tmpfiles.d entries.
Signed-off-by: Colin Walters walters@verbum.org
lints: Add var-tmpfiles
This checks for content in
/varthat is missing systemd tmpfiles.dentries.
Signed-off-by: Colin Walters walters@verbum.org