Skip to content
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

Suggesting we rewrite (parts of) imgutil #3531

Open
apostasie opened this issue Oct 12, 2024 · 0 comments
Open

Suggesting we rewrite (parts of) imgutil #3531

apostasie opened this issue Oct 12, 2024 · 0 comments

Comments

@apostasie
Copy link
Contributor

apostasie commented Oct 12, 2024

What is the problem you're trying to solve

In light of #3516 it is clear there are issues with code that likely pre-dates the introduction of multi-architecture.

There is also a certain amount of duplication - a lot of what pkg/cmd/image/list.go does should really be provided by imgutil.

Also, imgutil is making assumptions that the current native platform version of the image is necessarily available - or that other platforms variants simply do not matter (especially wrt labels).

Finally, the imagewalker (see #3502) might be part of the problem - along with the added abstraction layers (EnsuredImage) on top / mixed with the already crowded and confusing containerd API (Image aka provides the model for how containerd views container images. vs. Image aka describes an image used by containers - client.ImageService().List(ctx) vs. client.ListImages(ctx) 🤦‍♂️).

Describe the solution you'd like

Suggesting someone motivated starts from scratch:

  • cleanup the confusion between referenceutil and distributionref, and come-up with one simple abstraction to represent image references (and carry that around instead of re-parsing the rawref over and over again)
  • review all the places where we are currently manipulating Images
  • list what they need
  • come-up with a multi-platform-aware abstraction for "NerdImage" to cover all of these use-cases (maybe that is an evolution of EnsuredImage) and seal the implementation away so that we stop shooting ourselves in the foot with containerd types
  • re-evaluate all helpers inside imgutil against use cases - simplify them, make them solidly multi-platform aware
  • remove most of the code from pkg/cmd/image/list.go and other places that do access low-level details directly
  • come-up with serious test - we have close to 0 multi-platform tests for images

Additional context

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants