podman-bootc images #62
Conversation
Add a new `images` command that lists bootc-enabled images in the local store. This is a first step toward working with local images rather than always pulling them. Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
|
@germag @cgwalters @rhatdan WDYT? My desired UX is to do a |
|
Friendly ping. I can add some e2e test. Shall we expose more flags as |
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Overall, this looks totally sane to me.
I will say though (and this touches on #61 ) is that it's honestly not clear to me we gain significant value with another implementation of basically just forking off a run of podman images --filter label=containers.bootc=1.
| Short: "List bootc images in the local containers store", | ||
| Long: "List bootc images in the local container store", |
There was a problem hiding this comment.
Why the difference in plurality?
(Do we actually need to duplicate short/long at all with cobra? In short can't we just have Short:?)
| machine, err := utils.GetMachineContext() | ||
| if err != nil { | ||
| println(utils.PodmanMachineErrorMessage) | ||
| logrus.Errorf("failed to connect to podman machine. Is podman machine running?\n%s", err) |
There was a problem hiding this comment.
I'm not sure if we do this elsewhere in the code but IMO logging and returning errors often leads to unnecessary double printing.
There was a problem hiding this comment.
Now I remembered, that println message is a banner that shows that podman machine should be running
| return rpt.Execute(imgs) | ||
| } | ||
|
|
||
| func sortImages(imageS []*entities.ImageSummary) ([]imageReporter, error) { |
|
(Will trade my review of this for review of #61 ) |
Yes, it seems that forking |
|
Thanks! I will play a bit with calling podman directly. But I probably won't find time before Friday. |
|
We chatted about this and are good with it as is! |
3 participants
Add a new
imagescommand that lists bootc-enabled images in the local store.