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

UPSTREAM: 33014: Kubelet: Use RepoDigest for ImageID when available #11674

Merged

Conversation

DirectXMan12
Copy link
Contributor

Previously, we used the docker config digest (also called "image ID"
by Docker) for the value of the ImageID field in the container status.
This was not particularly useful, since the config manifest is not
what's used to identify the image in a registry, which uses the manifest
digest instead. Docker 1.12+ always populates the RepoDigests field
with the manifest digests, and Docker 1.10 and 1.11 populate it when
images are pulled by digest.

This commit changes ImageID to point to the the manifest digest when
available, using the prefix docker-pullable:// (instead of
docker://)

@DirectXMan12
Copy link
Contributor Author

cc @derekwaynecarr

@derekwaynecarr
Copy link
Member

[test]

Previously, we used the docker config digest (also called "image ID"
by Docker) for the value of the `ImageID` field in the container status.
This was not particularly useful, since the config manifest is not
what's used to identify the image in a registry, which uses the manifest
digest instead.  Docker 1.12+ always populates the RepoDigests field
with the manifest digests, and Docker 1.10 and 1.11 populate it when
images are pulled by digest.

This commit changes `ImageID` to point to the the manifest digest when
available, using the prefix `docker-pullable://` (instead of
`docker://`)
@derekwaynecarr
Copy link
Member

can you check if we also needed upstream 29316 which we pulled previously, or is it already in?

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 8a78ecf

@derekwaynecarr derekwaynecarr self-assigned this Oct 31, 2016
@derekwaynecarr
Copy link
Member

noting that 29316 is already in, so this is sufficient. LGTM

@derekwaynecarr
Copy link
Member

[merge]

@derekwaynecarr
Copy link
Member

re [merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 8a78ecf

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 2, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10971/) (Base Commit: 90d8c62) (Image: devenv-rhel7_5296)

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10969/) (Base Commit: 90d8c62)

@openshift-bot openshift-bot merged commit fc8773f into openshift:master Nov 2, 2016
@DirectXMan12 DirectXMan12 deleted the cherry-pick/repodigest branch November 4, 2016 18:59
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.

3 participants