util/estargz: simplify implementation and define consts for labels#5933
util/estargz: simplify implementation and define consts for labels#5933thaJeztah wants to merge 2 commits intomoby:masterfrom
Conversation
| // [estargz.StoreUncompressedSizeAnnotation]: https://pkg.go.dev/github.com/containerd/stargz-snapshotter/estargz@v0.16.3#StoreUncompressedSizeAnnotation | ||
| StoreUncompressedSizeAnnotation = "io.containers.estargz.uncompressed-size" |
There was a problem hiding this comment.
Curious why this uses a different prefix (io.containers.estargz) than the other ones, which all use containerd.io/.. 😅
There was a problem hiding this comment.
This is because this annotation is necessary to make the filesystem work with github.com/containers/storage library which is used by Podman/CRI-O. c.f. https://github.com/containerd/stargz-snapshotter/blob/2f71f9ea81620e302c5ec6384597fc630fbc6656/estargz/types.go#L78-L82
| if err := labels.Validate(TargetImageLayersLabel, layers+l.Digest.String()); err != nil { | ||
| break | ||
| } | ||
| layers += ls | ||
| layers += l.Digest.String() + "," |
There was a problem hiding this comment.
Slight change here; previously this would check the length including the trailing ,, but when setting the label, we trim the trailing comma, so there would be a potential off-by-one here, where we'd skip the label for being too long, even if it wasn't?
| // This annotation is valid only when it is specified in `.[]layers.annotations` | ||
| // of an image manifest. | ||
| // | ||
| // This is a copy of [estargz.TOCJSONDigestAnnotation] |
There was a problem hiding this comment.
Why isn't this imported (or upstream constant just used instead). Same for next.
There was a problem hiding this comment.
I went back-and-forth a bit; initially I had these as alias (TOCJSONDigestAnnotation = estargz.TOCJSONDigestAnnotation), but as we already would have to define the non-exported ones locally (and the estargz package containing much more than just the consts), and these consts to not change, I thought it would be OK to just have our own copy, as long as we refer to where they originate from.
|
Needs rebase |
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
4 participants
No description provided.