Skip to content
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

Copy: Support MIME type changes, two-step updates #1147

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Feb 12, 2021

See #1138 for the exploratory discussion, and commit messages for details.

@jomeier
Copy link

jomeier commented Feb 18, 2021

Hi, any progress on that?

@rhatdan
Copy link
Member

rhatdan commented Mar 2, 2021

@mtrmac This needs a rebase I believe.
@vrothberg PTAL

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mtrmac
Copy link
Collaborator Author

mtrmac commented Mar 2, 2021

Not just a rebase, this is an alternative to #1138 and should, if pursued, revert/replace that.

@rhatdan
Copy link
Member

rhatdan commented Mar 2, 2021

Ok let's close this and concentrate on whether or not the merged fix solves the issue.

@rhatdan rhatdan closed this Mar 2, 2021
@mtrmac
Copy link
Collaborator Author

mtrmac commented Mar 2, 2021

I need some way to track investigating this avenue.

- Remove a nonsensical comment
- Remove a redundant variable
- Move one line closer to its user.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Currently, horribly, types.UpdatedImage is _documented_ to ignore
ManifestUpdateOptions.LayerInfos.MediaType; so if LayerInfosForCopy
replaces a layer with a differently-compressed one, that MIME type
update is never performed.

First, work around that by adding updatedImageWithMIME as a
hopefully temporary hack that implements the edit using CompressionOperation
+ CompressionAlgorithm (which are not documented to be ignored).

Then, use it to process LayerInfosForCopy.  Because LayerInfosForCopy's
MediaType values are manifest-schema specific, we must call UpdatedImage(WithMime)
twice: once just to update the layers with LayerInfosForCopy information
using the original schema, and later to possibly convert the manifest and apply
CompressionOperation/... from results of TryReusingBlob/PutBlob.  So,
move the LayerInfosForCopy processing before determineManifestConversion
requests a MIME type change.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Only call ic.src.LayerInfos() if necessary to
evaluate UpdatedLayerInfosForCopy().

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants