Don’t discard annotations on blob reuse and partial pulls #1892
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mtrmac
added
the
kind/bug
mtrmac
force-pushed
the
TryReuse-annotations
branch
2 times, most recently
from
eb33bbc to
e270b27
Compare
|
CHanges LGTM, but looks like you need a rebase. |
... which was introduced by a previous incorrect refactoring. Signed-off-by: Miloslav Trmač <[email protected]>
…LayerInfo - Introduce our private addedLayerInfo so that we can make changes. - Rename indexToPulledLayerInfo to indexToAddedLayerInfo, to be more precise. - Store values, not unnecessary pointers, in the indexToAddedLayerInfo map. - In queueOrCommit and commitLayer, pass addedLayerInfo as a whole instead of components, and put index before the info. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
... to be a bit more consistent. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
…AsPending We will want to call it without having a full BlobInfo. Signed-off-by: Miloslav Trmač <[email protected]>
... instead of the whole addedLayerInfo, to be more explicit about what is necessary. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
... to make it package-private. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
By construction, every layer for which we add an addedLayerInfo is already recorded in blobDiffIDs, so we don't need the size. So, simplify addedLayerInfo by removing the field, and only provide the value on the code path where it could possibly make a difference. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
This is a subset of types.BlobInfo the transports actually should deal with. Also tighten the semantics a bit, to be much more explicit about what the transports are responsible for. This allows us to stop tracking MIME types in storage.addedLayerInfo, so simplify that. Then fix the generic code so that blob annotations are not discarded on blob reuse. Signed-off-by: Miloslav Trmač <[email protected]>
This is a subset of types.BlobInfo the transports actually should deal with, to be much more explicit about what the transports are responsible for. Should not change behavior, in practice. Signed-off-by: Miloslav Trmač <[email protected]>
This fixes preserving annotations also on that path. Signed-off-by: Miloslav Trmač <[email protected]>
mtrmac
force-pushed
the
TryReuse-annotations
branch
from
e270b27 to
a96a7cf
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Primarily, this modifies the
TryReuseBlob*andPutBlobPartialcode paths to preserve annotations in the source manifest; fixes #1888 .To do that, the private
PutBlob*now return aprivate.UploadedBlob, andTryReusingBlobWithOptionsreturns aReusedBlob, with a much more specific and explicitly documented set of fields and requirements, with the idea that adding a new struct is a small price to pay for the ability to be more precise about field semantics.Also, this fixes a possible infinite recursion (when storageImageDestination is used incorrectly)
See individual commit messages for details.