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

Move to containers/image v5, support manifest lists #4310

Merged
merged 7 commits into from
Oct 29, 2019

Conversation

nalind
Copy link
Member

@nalind nalind commented Oct 21, 2019

Since containers/storage#283, the storage library's Image type has been able to track multiple digests for an image in storage, so we should support that, as proposed changes in containers/image#400 will start storing manifest lists alongside runnable images. This also extends the exported API and inspection types to provide that information.

When we encounter an image in local storage that we can read as an image library ImageSource but not as an Image, we should handle it gracefully, since it might be an image list with no corresponding runnable image, as containers/buildah#1902 will start creating.

Add some tests to exercise pulling using tags that point to manifest lists, using digests of manifest lists, and digests of images that aren't manifest lists, that we're able to successfully inspect the result, and that when we create an image using the result of one of the pulls, that we get back the name we specified when creating the image. The tests require --override-os and --override-arch flags to pull, which we add but leave hidden, for now at least.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 21, 2019
@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 21, 2019
@nalind nalind force-pushed the manifest-lists branch 3 times, most recently from e2fdd6c to e324fb6 Compare October 21, 2019 16:00
@TomSweeneyRedHat
Copy link
Member

Do you foresee issues with containers and/or images on the system that were created before these changes go into play? i.e. will each pre-existing containers and images need to be migrated/updated?

@nalind
Copy link
Member Author

nalind commented Oct 21, 2019

No, the contents on disk stay the same when we pull an image down using a tag or a digest that doesn't point to a list. For such images, the only visible differences should be in the podman images output, where we stop outputting a digest in the tag column, and in image inspect output, where we likewise don't have names with digests in them in the RepoTags field.

@nalind nalind force-pushed the manifest-lists branch 20 times, most recently from 23e1c4e to 7cd5e67 Compare October 26, 2019 23:47
@nalind nalind force-pushed the manifest-lists branch 3 times, most recently from 4d36fe4 to b5ab257 Compare October 29, 2019 15:53
Move to containers/image v5 and containers/buildah to v1.11.4.

Replace an equality check with a type assertion when checking for a
docker.ErrUnauthorizedForCredentials in `podman login`.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Test that when we pull using tag or digest references from locations
that are manifest lists, that we can inspect using the references that
we used for pulling, that the tags show up in the RepoTag list when we
inspect an image that was pulled using a tag, and that the list and
instance digests always both show up in the RepoDigest list.

Signed-off-by: Nalin Dahyabhai <[email protected]>
When an image can be opened as an ImageSource but not an Image, handle
the case where it's an image list all by itself, the case where it's an
image for a different architecture/OS combination, or the case where
it's both.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Add --override-arch and --override-os as hidden flags, in line with the
global flag names that skopeo uses, so that we can test behavior around
manifest lists without having to conditionalize more of it by arch.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Be prepared to report multiple image digests for images which contain
multiple manifests but, because they continue to have the same set of
layers and the same configuration, are considered to be the same image.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Generate an image's RepoDigests list using all applicable digests, and
refrain from outputting a digest in the tag column of the "images"
output.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Currently podman play kube is not using the system default seccomp.json file.
This PR will use the default or override location for podman play.

Signed-off-by: Daniel J Walsh <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Oct 29, 2019

/lgtm
/hold
hold cancel once tests pass.

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Oct 29, 2019
@nalind
Copy link
Member Author

nalind commented Oct 29, 2019

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 29, 2019
@openshift-merge-robot openshift-merge-robot merged commit e7540d0 into containers:master Oct 29, 2019
@nalind nalind deleted the manifest-lists branch October 29, 2019 19:59
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 26, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants