Refactor image tree for API usage

[saschagrunert]

Refers to #2791 (comment)

[rhatdan]

@baude PTAL

[vrothberg]

Linter isn't happy yet. Will review now 👍

[+0036s] cmd/podman/tree.go:55:13: SA1006: printf-style function with dynamic first argument and no further arguments should use print-style function instead (staticcheck)
[+0036s] 	fmt.Printf(tree)

[vrothberg]

Looking really good, thanks @saschagrunert!

Can you also extend the swagger docs for the image endpoint? -> https://github.com/containers/libpod/blob/master/pkg/api/server/register_images.go#L581

The syntax is a bit clumsy but copy-pasting from the surrounding endpoints could help.

[saschagrunert]

Linter isn't happy yet. Will review now

Thanks! Please keep in mind that the varlink API does not work yet :)

[vrothberg]

@baude @jwhonce PTAL

[saschagrunert]

Varlink API works now, ready for review.

[vrothberg]

Can you also extend the swagger docs for the image endpoint? -> https://github.com/containers/libpod/blob/master/pkg/api/server/register_images.go#L581

Still open.

[vrothberg]

Two nits.

I am not sure if we want add a Varlink endpoint, though. @baude @jwhonce PTAL.

[vrothberg]

@saschagrunert, can you rebase? This will make local tests easier as we'd fetch podman system service.

[saschagrunert]

@saschagrunert, can you rebase? This will make local tests easier as we'd fetch podman system service.

Rebased on top of the latest master branch and addressed your review comments.

[saschagrunert]

Docs seem to be green now as well :)

[baude]

it LGTM but i want @mheon to peek at this one

[mheon]

/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mheon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mheon]

Can we move this to somewhere in pkg/? It doesn't seem much like an API call, given it only returns a string.

[vrothberg]

I suggest to wait for @mtrmac's new package. We may be able to integrate it there.

[vrothberg]

By "wait" I mean we could merge it as is now and let Miloslav pick it up in his new package.

[mheon]

Hm. It's just libpod/image, not libpod itself, so sure

[mtrmac]

Definitely don’t wait for me, I’m still mostly in learning phase, and I can adjust for whatever happens here.

(Jumping into what I’m sure was a well-considered decision, I’d instinctively prefer to primarily have a well-typed API that returns the structure, and keep the text formatter in the CLI (allowing, at least in principle, a nice GUI to be built on top, for example). OTOH, I guess that exposing the complete tree structure over Varlink/whatever would be a lot of work and pain — and for no benefit at least now that the only consumer is the CLI.)

[mheon]

That's my big hold-up as well - I'd love to return a data structure that represents the tree, with a .String() that returns the output. I'm willing to defer that to followup work, though.

[mheon]

(Honestly, though, I'd be fine keeping the tree local-only, and exposing only a literal output string over the API. There's precedence for this with things like podman top)

[mheon]

(Or was it podman stats... I forget which)

[mheon]

LGTM

[vrothberg]

/lgtm