-
Notifications
You must be signed in to change notification settings - Fork 354
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
Require podman #5285
Require podman #5285
Conversation
Hi @cgwalters looks good to me. However, don't we want to add the new dependency together with the new code? Just to be clear, I'm not opposed to merge this if it will improve your life even like this. |
/kickstart-test --testtype smoke |
/kickstart-test --testtype ostree |
I guess this is related change to Accepted Fedora 40 change. |
@KKoukiou do you know why the icon is broken on pixel tests? I don't think it's happening because of these change. |
Yep, up to you! Though as I said in the commit message I think it'd likely be very generally useful to be able to run |
We did not had a request so far but can't say it will not be :D |
I'm thinking we might have to bump the advertised RAM requirements a bit or add a disclaimer there as I would guess that running a container could result in more resource usage. Like, of course it was already possible to hit OOM with regular scriptlets if you are crazy enough, but running a container will likely move the baseline. |
Adding podman itself will not run a containers so until we do that (bootc support) we probably don't want to raise the requirements. |
/build-image --boot.iso |
Images built based on commit 036646c:
Download the images from the bottom of the job status page. |
Currently the ostree-container stack depends on |
@AdamWill I wonder how we should proceed to find out if the size change is an issue. Do you remember what is the process? |
@jkonecny12 Btw. our dependency on skopeo is defined in the |
I can do a scratch build and run it through openQA to see the size change for an x86_64 netinst image. |
although, building your own netinst image is pretty easy (much easier than live image) - basically you do |
AIUI a (the?) difference is that dependencies of |
@cgwalters if we do it this way, then it's dependency of For Anaconda I think this is correct, however, I'm not that sure about |
@AdamWill I already did the comparation by our automation and local build from the latest Anaconda sources: However, yeah, maybe we should also try to do this without our modifications to be sure. |
@AdamWill verified in the latest fedora Rawhide container:
With podman:
So the size increase is 20 MB as mentioned above. Now I wonder who to ask or where to verify that such a size increase is fine. Also we might need to move this to |
@jkonecny12 Maybe I am missing something, but I don't understand the reasoning behind having a hard dependency on podman via |
Yes, I agree it should be part of other package. However, the main question here is if the 20MB of image size is an issue or no, that will not change with the move of the package. |
Well, there isn't really a simple answer to that, it's a judgment call. 20M is a big chunk, relatively speaking, so the functionality bonus should be quite large to justify it. This will make it very difficult to keep netinst images under the 700M size limit we still have for x86_64 at present (though possibly removing yelp will help counterbalance that for now). Is there any way we could do a |
In the long term at least, having podman and the
That already happens when |
@AdamWill I wonder if I should create a FESCO ticket to get the final decision about this? @cgwalters IIRC |
Hum, maybe. Oh, in your result above - was that with or without the yelp removal PR? |
Oh, right, the size limit increase went through, so that is no longer an immediate concern. Of course, it's still always worthwhile to consider whether the value gained by increasing the image size is worth it. |
@cgwalters can you rebase this? |
036646c
to
8694f75
Compare
Done, though there appeared to be no conflicts? |
I was just hoping it might make the tests pass. :D But then today was branching, so maybe not the best time... |
/kickstart-test --testtype smoke |
Any movement on this? |
I think it is worth it and it is also inevitable if we want to embrace bootc in the future (what we want to do). |
@cgwalters would it be fine to move this to F41 or you would rather want F40? |
/packit build |
Also @cgwalters could you please move this dependency to |
This is prep for supporting bootc: rhinstaller#5197 A big part of the idea with bootc is that with `bootc install`, a container image can *install itself*: https://github.com/containers/bootc/blob/main/docs/install.md This will longer term replace the `ostreecontainer` verb. However even beyond that, having podman on the (smaller netinst, as well as bigger DVD) installer ISO will just generally be useful for a variety of things. For example, one can now do e.g.: ``` %post --no-chroot podman run ... ``` Which unblocks a lot of things! (Some of which admittedly will be hacks, but that's what a lot of `%post` is...) Having a container runtime in the Live ISO is a very key feature of the Fedora CoreOS (and RHEL CoreOS) Live ISOs, and this brings Anaconda closer to parity too.
Thanks, done and rebased 🏄 |
/kickstart-test --testtype smoke |
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.
Looks great to me. Thanks!
/build-image --boot.iso |
Images built based on commit ccb3475:
Download the images from the bottom of the job status page. |
@cgwalters so I tested this change and I found out that
I'm inclined to merge this anyway because this seems to me like something we need to resolve in Lorax or special way to execute podman. Do you agree @cgwalters? |
It should work with So we could either:
|
I also wonder if this shouldn't be a fallback? I mean not fail on podman but print warning and fallback to |
Anyway, tested with Merging. P.S.: I just realized that this is great tool for debugging. Basically it will add |
That'd be insecure in the general case. But, it would clearly make sense to have a more useful error message.
Right. Probably the immediately most interesting will be documenting use of
And similar for |
This is prep for supporting bootc:
#5197
A big part of the idea with bootc is that with
bootc install
, a container image can install itself:https://github.com/containers/bootc/blob/main/docs/install.md
This will longer term replace the
ostreecontainer
verb.However even beyond that, having podman on the (smaller netinst, as well as bigger DVD) installer ISO will just generally be useful for a variety of things. For example,
one can now do e.g.:
Which unblocks a lot of things! (Some of which admittedly will be hacks, but that's what a lot of
%post
is...)Having a container runtime in the Live ISO is a very key feature of the Fedora CoreOS (and RHEL CoreOS) Live ISOs, and this brings Anaconda closer to parity too.