Create Docker images for master components #6326
Conversation
|
assigning to Dawn for want of a better assignee, git blame wasn't too helpful |
ArtfulCoder
force-pushed
the
docker_image_creation
branch
from
6ee8300
to
77dc5c2
Compare
| fi; | ||
| printf " FROM scratch \n ADD ${binary_name} /${binary_name} \n ENTRYPOINT [ \"/${binary_name}\" ]\n" >> ${docker_file_path}; | ||
| local md5_sum=$(md5sum ${binary_file_path} | awk '{print $1}') | ||
| local docker_image_tag=kubernetes/$binary_name:$md5_sum |
There was a problem hiding this comment.
s/kubernetes/gcr.io/google_containers?
There was a problem hiding this comment.
Could you elaborate what you are recommending ? I didnt completely follow..
the images are kubernetes-specific..
There was a problem hiding this comment.
I mean replace kubernetes with gcr.io/google_containers, so that the image is going to push to gcr.io, instead of docker hub.
There was a problem hiding this comment.
We dont push at all to docker hub or gcr.io
There was a problem hiding this comment.
Yes, you are not pushing in this PR, but kubernetes here represents repository which should be gcr.io.
|
@dchen1107 thanks for reviewing the PR! I added some comments.. |
| @@ -569,6 +578,30 @@ function kube::release::package_server_tarballs() { | |||
| done | |||
| } | |||
|
|
|||
| # This will take binaries that run on master and creates Docker images | |||
| # that wrap the binary in them. (One docker image per binary) | |||
| function kube::release::create_docker_images_for_master() { | |||
There was a problem hiding this comment.
nit: create_docker_images_for_wrapped_binaries() (since they don't necessarily have to be master, but are for now)
ArtfulCoder
force-pushed
the
docker_image_creation
branch
2 times, most recently
from
a6538de
to
4a3b4e2
Compare
ArtfulCoder
force-pushed
the
docker_image_creation
branch
from
4a3b4e2
to
b1b779a
Compare
|
LGTM. |
Create Docker images for master components
| printf " FROM scratch \n ADD ${binary_name} /${binary_name} \n ENTRYPOINT [ \"/${binary_name}\" ]\n" >> ${docker_file_path}; | ||
| local md5_sum=$(md5sum ${binary_file_path} | awk '{print $1}') | ||
| local docker_image_tag=gcr.io/google_containers/$binary_name:$md5_sum | ||
| docker build -t "${docker_image_tag}" ${1}; |
There was a problem hiding this comment.
this doesn't work unless the machine you are provisioning on has docker installed by default - i don't think this is the case for all salt-based providers. this also doesn't make sense if we have salt scripts that are later responsible for installing docker as part of salt provisioning.
|
So this PR broke the Vagrant cluster... are we requiring docker to be installed before doing a cluster/kube-up now? |
|
We do need docker to be installed to build docker images which then go into the server tarballs. Optionally, I could consider doing this as part of the deployment process, once salt-scripts have installed docker. |
|
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/cluster/saltbase/salt/top.sls#L55 Looks to me that each kubernetes-master uses salt to install docker on the master , which is where this is erroring for me. |
|
Ah, yes On Mon, Apr 6, 2015 at 10:18 AM, Derek Carr notifications@github.com
|
|
@ArtfulCoder: I think one easy thing to do is just defer the "docker load" to a later Salt step. You can populate the pillar data as you're doing today, and just load after docker is ensured. |
|
FYI I have both the master and node dockerize here https://github.com/mingfang/docker-kubernetes-master and here https://github.com/mingfang/docker-kubernetes-node in case you're interested. |
Automatic merge from submit-queue Use munged semantic version for side-loaded docker tag **What this PR does / why we need it**: rather than using the md5sum of the dockerized binary for each side-loaded docker image, use the semantic version (with `+`s replaced with `_`s) for the side-loaded docker images. The use of the md5sum for the docker tag dates to #6326 2 years ago. I'm not sure why that was chosen, short of it being fairly unique. My main motivation for changing this is that it makes building the docker images using Bazel's docker rules easier, since the semantic version doesn't depend on the build output. An added benefit is that the list of images on a running kubernetes cluster is also more straightfoward; rather than a list of opaque, meaningless hexadecimal strings, you get something that indicates the provenance of the image. It'd also be clearer that all of the images came from the same build. I was able to start a cluster with this change on GCE using both `make quick-release` and `make bazel-release`. Note that this change has no effect on the tag that's pushed to gcr.io during releases; that's still controlled via `KUBE_IMAGE_DOCKER_TAG`, though we may want to merge this functionality at some point. @kubernetes/sig-node-pr-reviews is there any reason to stick with using the md5sum strategy? @dchen1107 do you remember why we went with md5sums originally? cc @spxtr @mikedanese **Release note**: ```release-note ```
This is a step towards containerization of the various master components that run directly on host today.