-
Notifications
You must be signed in to change notification settings - Fork 0
introduce GetImageWithDetails to manage image inspect data in backend #27
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
Conversation
3236817 to
3e175c0
Compare
image/image.go
Outdated
| Details *Details | ||
| } | ||
|
|
||
| // Details provides additional image data | ||
| type Details struct { | ||
| Size int64 | ||
| Metadata map[string]string | ||
| Driver string | ||
| LastUpdated time.Time |
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.
For the new fields, make sure they are not serialised to JSON (json:"-")
not sure if it works to add that to Details, or it if's needed for the individual fields (or both) (haven't tested)
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.
makes me wonder: why do we define json rules for an engine type? This one is not part of the API
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.
I think (but would have to double-check) these types are also used to serialise image information to disk (and as such could be part of, e.g., save/load. So in that case it's not for the API, but to prevent persisting this data to disk.
IIRC, this is also the reason why the V1Image is embedded, as that's what's used for save/load
fcdcb46 to
bb4c1bb
Compare
bb43aa3 to
fbb0432
Compare
| // Details provides additional image data | ||
| type Details struct { | ||
| Size int64 | ||
| Metadata map[string]string |
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.
Just realised that this is to store graphdriver metadata; wondering if that's clear enough from this type (at least could use a GoDoc), or was the intent to re-purpose this field for something else following the containerd integration?
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.
I actually wonder why graphdriver metadata are exposed to the API, seems like debugging data as a user would hardly use image's mergedir
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.
Yes, I agree. It looks like this was one of the Red Hat contributions, and I can only "guess" how they wanted to use this (possibly as part of some systemd hackery); moby#13198 (also related; moby#13130)
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
2ef8a54 to
50401b4
Compare
- What I did
Introduce GetImageWithDetails so that "image inspect" logic is not implement by router but backend, and we can re-implement using containerd snapshotter
relates to moby#43680