node-images: enable copr repo#93
Conversation
Enable the rhcontainerbot/bootc COPR repo before building the rootfs so that the composefs imageDigest fix from bootc-dev/bootc#2277 gets picked up once it merges and COPR builds resume. Signed-off-by: HarshwardhanPatil07 <[email protected]>
ae9cdd3 to
7ae4c0e
Compare
| # https://github.com/bootc-dev/bink/issues/52 | ||
| ARG KERNEL_VERSION=6.19.14-300.fc44 | ||
| RUN dnf -y install 'dnf5-command(copr)' && \ | ||
| dnf -y copr enable rhcontainerbot/bootc fedora-44-x86_64 |
There was a problem hiding this comment.
I do understand why you directly add here. However, I'm reluctant to include this by default. Possibly, what we could is to add a new flavor of the disk image for composefs and this fix for the time it lands in fedora.
We could introduce a new argument that if sets will enable the copr repository and then we add a new GHA that will build the composefs disk image including bootc with this fix. This could helps us to eventually add the reverse dependencies job and test. See bootc-dev/bootc-operator#70
There was a problem hiding this comment.
Interested in your hesitation to just unconditionally add this. Yes, we're special-casing bootc here, but... bootc is special for us so it seems warranted.
Note also that this only affects the buildroot; the COPR repo is not enabled by default in the final node image built.
There was a problem hiding this comment.
We are overwriting the default bootc from fedora. So, does it mean we want always to rely from one of the latest bootc version instead of the default one?
There was a problem hiding this comment.
IMO, yes.
I think what's muddling the waters here is the fact that the node image definition lives in this repo even though bink itself is designed to be decoupled from the actual node image. It makes sense for bink to have a "reference default image", but the main consumer of bink and that node image today is the bootc-operator. And there I think it makes sense for CI to always test against latest bootc. So that in-tree node image definition plays double-duty.
I wouldn't be against having a separate set of node images specifically tailored for bootc-operator (which could e.g. live in that repo instead), but I don't think we're at that point yet where it's worth maintaining separate sets.
So my strawman is to do this and accommodate the bootc-operator use case. If we keep mutating the node image for bootc-operator's goals in a way that differs from what we think the reference default bink node image should be, we can always pay that cost later. WDYT?
There was a problem hiding this comment.
A deeper thing going on here is that it's not clear to me what that "reference node image" should even prioritize/value. The main goal of bink today is to easily provide a K8s cluster based on real bootc systems.
There's already so many mini-K8s tools out there that exist for the different gradations of how real enough you want it to be. And we're not trying to compete with all those for the "general easy K8s" app development flow. Worth noting also that minikube supports VMs (and perhaps one angle long-term is to try to somehow add a new e.g. bcvk backend there...). So "real bootc systems" is really the main differentiator here.
And therefore, the choice of such a reference image tracking latest bootc doesn't seem obviously wrong to me. At least not yet. (I can imagine bink users that would value slower bootc to be clear; this is more about what the default should be.)
Definitely interested in your take on this! But anyway, my main take overall is that this is something we can easily tweak as we discover more where we want it to go.
There was a problem hiding this comment.
My major concern is that every build could pull a different bootc version which might break the CI. Hence, a change in bootc might let the CI failing on new code changes, but that's not th fault of that particular PR. It introduces a new variant.
I'm not against building bink images with latest bootc, but I don't want to put it as bink default. Having said that, we can always set the image in bootc-operator to point to the latest bootc images. Or have a job that test both the stable one in fedora and the latest
There was a problem hiding this comment.
My major concern is that every build could pull a different bootc version which might break the CI. Hence, a change in bootc might let the CI failing on new code changes, but that's not th fault of that particular PR.
This is a valid concern, but it's already something we face today. There's lots of other packages in the current image that may break us on any update (and in fact did).
Definitely not opposed to adding some locking + PR bump workflow if we'd like (I think an easy way to do this is probably to use the versioned Fedora compose URLs and Renovate to bump it, similar to what e.g. is done for some of the bootc images building in Konflux today -- though that wouldn't work as easily for the bootc COPR). It's always a matter of tradeoffs with these things.
I'm also not opposed to blocking this PR on doing that first. But at the same time, because it's bootc, and we're a part of that project now, I think any issues we hit will in practice be dealt with quite quickly (either by other bootc members or us! :)). And if not, we can always freeze if needed.
Enable the copr repo before building the rootfs so that the composefs imageDigest fix from bootc-dev/bootc#2277 gets picked up once it merges and copr builds resume.