containerd / containerd/nerdctl

Suggesting we rewrite (parts of) imgutil

Open
#3,531 0 comments 0 reactions 0 assignees View on GitHub
kind/refactor
Dominant language
Go
Stars
10.4k
Forks
826
Avg merge
1d 23h
Merged PRs (30d)
44

Description

### 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_

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.