[Additional Layer Store] Use TOCDigest as ID of each layer (patch for c/image) #2416
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -308,7 +308,7 @@ func (s *storageImageSource) LayerInfosForCopy(ctx context.Context, instanceDige | |
| return nil, fmt.Errorf("reading layer %q in image %q: %w", layerID, s.image.ID, err) | ||
| } | ||
| if layer.UncompressedSize < 0 { | ||
| layer.UncompressedSize = -1 | ||
mtrmac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
mtrmac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| blobDigest := layer.UncompressedDigest | ||
|
|
@@ -452,9 +452,12 @@ func (s *storageImageSource) getSize() (int64, error) { | |
| if err != nil { | ||
| return -1, err | ||
| } | ||
| if (layer.TOCDigest == "" && layer.UncompressedDigest == "") || (layer.TOCDigest == "" && layer.UncompressedSize < 0) { | ||
| return -1, fmt.Errorf("size for layer %q is unknown, failing getSize()", layerID) | ||
| } | ||
| if layer.UncompressedSize < 0 { | ||
|
There was a problem hiding this comment. If a TOC-identified layer can have … but, also, note to self: is a new behavior of the There was a problem hiding this comment. The commit says
but we collect all metadata at the time of layer creation ( Actually, in https://github.com/containerd/stargz-snapshotter/pull/1673/files , it is now reported always as There was a problem hiding this comment.
That’s actually fine because that is already conditional on There was a problem hiding this comment. … and I don’t know whether |
||
| sum = 0 | ||
|
There was a problem hiding this comment. This means that if there 3 layers, and the middle one has an unknown size, the total returned size of the image is… some value larger than the true size, but the particular value can AFAICS not be reasonably used for anything. Why is that better than returning There was a problem hiding this comment. #2428 modifies this to treat the unknown-size layer as zero-size, while returning the size of the other components. |
||
| } | ||
| sum += layer.UncompressedSize | ||
| if layer.Parent == "" { | ||
| break | ||
|
|
||
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.
We have passed down the expected TOC digest in
LookupAdditionalLayer. Would this ever return a different value? If so, what does that mean / what is the guaranteed relationship between the two TOCs?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.
Looking at the implementation in containerd/stargz-snapshotter#1673 , this seems to always return the digest that is used in the FUSE request, i.e. it is exactly the value we passed in … unless the FUSE server is old and does not return the value at all; and in that case we don’t want to use any layers in the ALS — the FUSE server would be looking up a layer with compressed (non-TOC) digest
options.TOCDigest.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.
#2428 modifies this to only trust
options.TOCDigest(but to require the one from the ALS to match).