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

Push manifest lists when releasing docker images #248

Closed
wants to merge 1 commit into from

Conversation

luxas
Copy link
Member

@luxas luxas commented Jan 20, 2017

Instead of pushing the amd64 image to ${registry}/${binary}:${version} we should push a manifest list: https://integratedcode.us/2016/04/22/a-step-towards-multi-platform-docker-images/

More information: https://github.com/kubernetes/community/blob/master/contributors/design-proposals/multi-platform.md

Release for the manifest-tool binary: https://github.com/luxas/manifest-tool/releases/tag/v0.3.0

So basically, when pulling an ${registry}/${binary}:${version} image, docker will check which platform it is on and redirect and pull layers from the ${registry}/${binary}-${arch}:${version} image instead. But the image name will always be ${registry}/${binary}:${version}, so this is a true multi-platform image!

You have to be logged in to a docker registry with docker login so I added that part as well.

I can't test this, but I'm quite confident it will work, I've done the same thing for a lot of other images.

Can we merge this and produce an v1.6.0-alpha.1 release?
Thanks in advance

@ixdy @david-mcmahon @ethernetdan @spxtr @saad-ali @jessfraz

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 20, 2017
@ixdy
Copy link
Member

ixdy commented Jan 21, 2017

So this will set a hard requirement on Docker v1.10+?
What changes are in your fork of manifest-tool that aren't in one of the upstream forks?
Does gcr.io support this?

if [[ -f $(which manifest-tool) ]]; then
MANIFEST_TOOL=$(which manifest-tool)
else
curl -sSL https://github.com/luxas/manifest-tool/releases/download/v0.3.0/manifest-tool > ./manifest-tool
Copy link
Member

Choose a reason for hiding this comment

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

this step should probably go in anago's check_prerequisites function.

and probably rather than automatically downloading, it should present instructions on how to download it.

as-is it'll make the git tree dirty.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, @mkumatag ^

fi

# "docker login" to the gcr.io registry so we can push manifest lists to it
[[ "$registry" =~ gcr.io/ ]] && docker login -u oauth2accesstoken -p "$(gcloud auth print-access-token)" https://gcr.io
Copy link
Member

Choose a reason for hiding this comment

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

would gcloud docker -authorize-only achieve the same thing in a slightly more-readable way?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably, @mkumatag ^

[[ "$registry" =~ gcr.io/ ]] && docker login -u oauth2accesstoken -p "$(gcloud auth print-access-token)" https://gcr.io

# ml_platforms should be in the os1/arch1,os2/arch2,os3/arch3 form
local ml_platforms= $(echo "${KUBE_SERVER_PLATFORMS[@]}" | sed "s/ /,/g")
Copy link
Member

Choose a reason for hiding this comment

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

nits:

  • declare var at top of function
  • local -r

done

# This command will push a manifest list: "${registry}/${binary}-ARCH:${version}" that points to each architecture depending on which platform you're pulling from
$MANIFEST_TOOL push from-args --platforms ${ml_platforms} --template ${registry}/${binary}-ARCH:${version} --target ${registry}/${binary}-ARCH:${version}
Copy link
Member

Choose a reason for hiding this comment

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

is ARCH treated as a special value? should both the template and target contain ARCH?

Copy link
Member Author

Choose a reason for hiding this comment

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

ARCH is a special value yes, it is replaced by the architectures in the platforms list
ugh, no, shouldn't be in --target

@@ -769,23 +783,10 @@ release::docker::release () {
logecho "Release $docker_target:"
logecho -n "- Pushing: "
logrun -r 5 -s ${docker_push_cmd[@]} push "$registry/$docker_target"

# If we have a amd64 docker image. Tag it without -amd64 also
# and push it for compatibility with earlier versions
Copy link
Member

Choose a reason for hiding this comment

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

What version of docker did we use in k8s 1.3? Does it support v2.2 manifests?

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 20, 2017
@luxas
Copy link
Member Author

luxas commented Dec 22, 2017

@ixdy @mkumatag has volunteered to help drive this to completition now that gcr.io supports manifest lists.
cc @vielmetti

@luxas
Copy link
Member Author

luxas commented Dec 22, 2017

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 22, 2017
@vielmetti
Copy link

The most recent manifest-tool is at https://github.com/estesp/manifest-tool/releases/tag/v0.7.0 and I think it has all of the added functions necessary to support this effort.

@nikolas-hermanns
Copy link

Hey,

any news on this? we would like to add functionality to kubeadm to deploy multi architecture platforms see: kubeadm should use multi-architecture images when GCR supports Docker distribution API v2 schema 2 which makes use of this additions.

@mkumatag
Copy link
Member

I'm working on getting k8s infra images manifest and its in-progress., kubernetes/kubernetes#57869

@luxas
Copy link
Member Author

luxas commented Feb 8, 2018

@mkumatag can you make a similar PR (i.e. rebase this PR essentially and submit a new, similar) as this so we can merge it in the v1.10 timeframe please?

@mkumatag
Copy link
Member

created a seperate PR based on this + some additional changes - #516

@luxas
Copy link
Member Author

luxas commented May 1, 2018

So this will set a hard requirement on Docker v1.10+?

Yes

What changes are in your fork of manifest-tool that aren't in one of the upstream forks?

Nothing, @estesp has merged all the stuff to his repo already a long time ago.
Actually, @mkumatag can now in his PR use the official docker client as this is supported upstream in Docker as of lately.

Does gcr.io support this?

Yes, now it does

I'm gonna close this in favor of #516

@luxas luxas closed this May 1, 2018
marpaia pushed a commit to marpaia/release that referenced this pull request Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants