Fix #858 Add support for digests in sync #880
Conversation
|
Depends on containers/image#883 |
|
I'll need to address the integration-tests but in the meantime, I'd like to get feedback on changing |
Both Some registries in the wild don't accept manifest lists, though, so I think we need to decide what should happen if we're trying to sync a tag that points to a manifest list, but find that the destination can't accept lists. |
Does it make sense to expose the |
|
@mtrmac PTAL |
|
@muff1nman Needs a rebase. |
muff1nman
force-pushed
the
sync-digests
branch
3 times, most recently
from
83cbc07 to
6654bf4
Compare
|
@mtrmac @vrothberg @QiWang19 PTAL |
|
Supporting digests makes sense. The second change to copy all instances could be a breaking change to existing users, so I prefer to make this opt in. One way or another, the two changes should be in two separate commits. This will make maintenance easier (e.g., bisecting, testing, backporting, reverting etc.). |
|
@muff1nman Could you split this PR in two? If you are still working on it? |
|
Split commits and added |
muff1nman
force-pushed
the
sync-digests
branch
2 times, most recently
from
2967e75 to
e5d788d
Compare
|
Just fixed a missing signature. No code change. |
|
@mtrmac @vrothberg PTAL |
|
Any chance this can get reviewed. |
|
@mtrmac PTAL |
|
LGTM |
muff1nman
force-pushed
the
sync-digests
branch
from
14faf6b to
2979e59
Compare
|
Rebased with no changes. Can we get this one over the finish line? |
|
@vrothberg @mtrmac PTAL |
|
@vrothberg @mtrmac Pretty PTAL? |
|
Apologies! I'll take a look on Monday morning. |
|
@muff1nman could you do a last rebase? While there are no merge conflicts, the state is quite old. |
Signed-off-by: Andrew DeMaria <[email protected]>
This replicates the --all copy flag to sync to perform the same behavior. Namely, the default is CopySystemImage unless --all is passed which changes the behavior to CopyAllImages. While it is probably desirable for --all to be the default as there is no option to override ones architecture with the sync command, --all can potentially break existing sync incantations depending on registry support. Hence CopySystemImage remains the default. Signed-off-by: Andrew DeMaria <[email protected]>
muff1nman
force-pushed
the
sync-digests
branch
from
2979e59 to
873fbee
Compare
|
@vrothberg rebased with no changes. |
|
Thanks, @muff1nman! @rhatdan PTAL |
|
LGTM |
In addition, this changes the copy options to pass
CopyAllImages such that any digests referencing manifest
lists work as expected. While this is potentially a change
in behavior for previous sync incantations, it is most likely
desired as there is no option to override ones architecture
with the sync command anyways.
Signed-off-by: Andrew DeMaria [email protected]