Launch a cluster-local registry. #12833

uluyol · 2015-08-17T23:44:35Z

This registry can be accessed through proxies that run on each node listening on port 5000. We send the proxy images to the nodes directly to avoid requests that hit the network during cluster launch. For now, we continue to pull the registry itself over the network, especially given its large size (we should be able to dramatically shrink the image). On GCE we create a PD and use that for storage, otherwise we use an emptyDir. The registry is not enabled outside of GCE. All communication is currently plain HTTP. In order to use SSL, we will need to be able to request a certificate/key from the apiserver signed by the apiserver's CA cert.

@thockin

uluyol · 2015-08-17T23:44:52Z

Some discussion for the cert issue is in #11725.

uluyol · 2015-08-17T23:46:14Z

A quick note from an earlier discussion with @thockin, this does not currently use PV/PVCs.

k8s-bot · 2015-08-17T23:48:38Z

GCE e2e build/test failed for commit 2b545583be7d06051f8c021542e1d2dd718b9459.

k8s-bot · 2015-08-18T00:17:12Z

GCE e2e build/test passed for commit f53586977dfbf4cde60cda4e2008110ae2875fd3.

k8s-bot · 2015-08-18T00:28:18Z

GCE e2e build/test passed for commit b8b5b2c14ada082222a763635d336f2273004334.

thockin · 2015-08-18T22:24:42Z

Could use help on thsi review - Bueller? Bueller?

Firing randomly: @eparis @a-robinson @fgrzadkowski

thockin · 2015-08-18T22:25:01Z

Ohh, @zmerlynn

thockin · 2015-08-18T22:25:34Z

build/common.sh

+# The set of addons images that should be prepopulated
+readonly KUBE_ADDON_PATHS=(
+  gcr.io/google_containers/pause:0.8.0
+  uluyol/kube-registry-proxy:0.2.3


we'll have to make this a gcr.io image

Is this going to get addressed before you leave?

(Or before we merge.)

thockin · 2015-08-19T00:45:34Z

I like the feature. I am concerned about adding more salt (to an open wound). Looking to @zmerlynn for a reality check. Other than general distaste with salt/jinja I did not find much to pick at.

LGTM, pending consult

zmerlynn · 2015-08-19T14:55:54Z

I'll take a look today.

uluyol · 2015-08-19T18:22:13Z

Ref #1319

uluyol · 2015-08-19T22:54:05Z

Pushed a new commit that uses a PV for storage.

k8s-bot · 2015-08-19T23:17:18Z

GCE e2e build/test failed for commit 406d7503459e5479c2225456c67ce25b79a148c7.

k8s-bot · 2015-08-20T00:49:49Z

GCE e2e build/test passed for commit 0a9d6f85ecb29eed0b2ed77502460d5132082f43.

justinsb · 2015-08-20T15:55:03Z

cluster/addons/registry/images/run_proxy.sh

+	-e "s/%PORT%/$REGISTRY_PORT/g" \
+	-e "s/%FWDPORT%/$FORWARD_PORT/g" \
+	-e "s|%CA_FILE%|$REGISTRY_CA|g" \
+	</proxy.conf.in >/proxy.conf


Should proxy.conf be in the commit then - seems like it is auto-generated here?

We generate it inside of the docker container at runtime.

Right - my point is that it is in this PR, but I think we're agreed that it should not be.

Oh, thanks. Removed.

justinsb · 2015-08-20T16:05:44Z

Cool, though I don't understand the purpose of the proxy? I've done something similar in the past, though I set it up using a service. I set it up using a known IP, and then just used that IP to access the registry, though our DNS has improved so much since then that pure DNS might work now.

(I also set it up using S3 as the backing store; not sure which is better. My guess is that a persistent volume is more portable and doesn't require embedding credentials, so I like this approach.)

From an AWS implementation point of view, I agree that there should be a persistent backend for it too. I suggest that once this merges we open an issue to do the same thing for AWS & I will perform the equivalent changes for AWS.

thockin · 2015-08-20T17:54:18Z

Using anything but localhost requires extra flags to docker or requires TLS
certs. We were trying to get a dead-simple case working. It's a place to
build from.

On Thu, Aug 20, 2015 at 9:06 AM, Justin Santa Barbara <
notifications@github.com> wrote:

Cool, though I don't understand the purpose of the proxy? I've done
something similar in the past, though I set it up using a service. I set it
up using a known IP, and then just used that IP to access the registry,
though our DNS has improved so much since then that pure DNS might work now.

(I also set it up using S3 as the backing store; not sure which is better.
My guess is that a persistent volume is more portable and doesn't require
embedding credentials, so I like this approach.)

From an AWS implementation point of view, I agree that there should be a
persistent backend for it too. I suggest that once this merges we open an
issue to do the same thing for AWS & I will perform the equivalent changes
for AWS.

—
Reply to this email directly or view it on GitHub
#12833 (comment)
.

justinsb · 2015-08-20T17:57:04Z

Ah, I didn't realize there was an exception for localhost. That is great then!

thockin · 2015-08-20T18:00:16Z

Well, we WANT to use TLS, but it was easier to start without it :)

On Thu, Aug 20, 2015 at 10:57 AM, Justin Santa Barbara <
notifications@github.com> wrote:

Ah, I didn't realize there was an exception for localhost. That is great
then!

—
Reply to this email directly or view it on GitHub
#12833 (comment)
.

zmerlynn · 2015-08-20T21:11:51Z

Okay, I'm just now getting around to reviewing due to ${STUFF} getting in the way, sorry. :/

zmerlynn · 2015-08-20T21:17:24Z

cluster/addons/registry/registry-pv.yaml.in

+  labels:
+    kubernetes.io/cluster-service: "true"
+spec:
+{% if pillar.get('cluster_registry_disk_type', '') == 'gce' %}


So if cluster_registry_disk_type isn't gce, this yaml blob just trails off at spec:?

I wasn't sure what the best way to do this is. This template should only be evaluated if the cluster registry is enabled which should only be permitted if one of the branches here is taken (for non-gce, they'd need to have their own config branch). If somehow this is evaluated otherwise, that's a bug and we should see a failure.

There's a pattern to do this e.g. http://stackoverflow.com/questions/21778252/how-to-raise-an-exception-in-a-jinja2-macro, but I'm not sure we have any of the pieces elsewhere in our Salt. This looks fine for now, though, it'll error in a different way, I think.

zmerlynn · 2015-08-20T22:28:11Z

In general looks good, I like the approach. It's salty, but meh, that's the system we have. :/

uluyol · 2015-08-20T23:25:21Z

Pushed new version and rebased. Nits should have been addressed.

k8s-bot · 2015-08-20T23:34:29Z

GCE e2e build/test failed for commit eef53a0de7cf01370b4f87056034e18e9ffae5e0.

k8s-bot · 2015-08-20T23:53:16Z

GCE e2e build/test passed for commit 957b3f68eb23a000f3191a6efecaa0a1dcbda5b4.

zmerlynn · 2015-08-21T00:01:08Z

cluster/saltbase/salt/kube-addons/kube-addons.sh

+    for image in "$1/"*; do
+      timeout 30 docker load -i "${image}" &>/dev/null
+      rc=$?
+      if [[ $rc == 124 ]]; then


Sorry, I didn't notice the minor style nits in this file. Can you "${rc}", ${success}, ${restart_docker}, and quote "true"?

Sure, though it really isn't necessary with the double brackets.

zmerlynn · 2015-08-21T00:03:25Z

LGTM except one minor other nit and a question. @thockin for final buyoff. I'll be oncall tomorrow morning to merge, so if it LGTY, we can see how it does with Jenkins.

Oh, and I forgot to add before: Great work!

thockin · 2015-08-21T00:08:18Z

I'm good with it all, but we'll switch the image to a gcr.io image tomorrow

On Thu, Aug 20, 2015 at 5:03 PM, Zach Loafman notifications@github.com
wrote:

LGTM except one minor other nit and a question. @thockin
https://github.com/thockin for final buyoff. I'll be oncall tomorrow
morning to merge, so if it LGTY, we can see how it does with Jenkins.

Oh, and I forgot to add before: Great work!

—
Reply to this email directly or view it on GitHub
#12833 (comment)
.

This registry can be accessed through proxies that run on each node listening on port 5000. We send the proxy images to the nodes directly to avoid requests that hit the network during cluster launch. For now, we continue to pull the registry itself over the network, especially given its large size (we should be able to dramatically shrink the image). On GCE we create a PD and use that for storage, otherwise we use an emptyDir. The registry is not enabled outside of GCE. All communication is currently plain HTTP. In order to use SSL, we will need to be able to request a certificate/key from the apiserver signed by the apiserver's CA cert.

uluyol · 2015-08-21T01:58:00Z

Fixed remaining nits and removed extraneous file.

k8s-bot · 2015-08-21T03:23:09Z

GCE e2e build/test passed for commit 2fb4e7b.

k8s-bot · 2015-08-21T18:40:53Z

GCE e2e build/test failed for commit 55825b3f78fb24a15e3b33f89cea2dd3a5931cd4.

zmerlynn · 2015-08-21T18:48:13Z

@uluyol: Happy to merge on green, but that @k8s-bot complaint looks real. (Also we're having a rough ride on the Jenkins side this morning, but I think the seas will be calmer by lunch.)

thockin · 2015-08-21T19:35:48Z

The image for this is pushed to gcr now.

On Fri, Aug 21, 2015 at 11:48 AM, Zach Loafman notifications@github.com
wrote:

@uluyol https://github.com/uluyol: Happy to merge on green, but that
@k8s-bot https://github.com/k8s-bot complaint looks real. (Also we're
having a rough ride on the Jenkins side this morning, but I think the seas
will be calmer by lunch.)

—
Reply to this email directly or view it on GitHub
#12833 (comment)
.

k8s-bot · 2015-08-21T20:56:01Z

GCE e2e build/test passed for commit 3dc10a2.

zmerlynn · 2015-08-21T20:58:13Z

Now I'm waiting for even a vaguely yellowing e2e-gce run. :(

zmerlynn · 2015-08-21T21:58:35Z

Supergreen, I'll watch Jenkins.

Launch a cluster-local registry.

googlebot added the cla: yes label Aug 17, 2015

uluyol force-pushed the insecure-reg branch 2 times, most recently from f535869 to b8b5b2c Compare August 18, 2015 00:01

roberthbailey assigned thockin Aug 18, 2015

thockin reviewed Aug 18, 2015
View reviewed changes

uluyol force-pushed the insecure-reg branch from 6d02ac8 to 406d750 Compare August 19, 2015 22:53

justinsb reviewed Aug 20, 2015
View reviewed changes

zmerlynn reviewed Aug 20, 2015
View reviewed changes

uluyol force-pushed the insecure-reg branch 2 times, most recently from eef53a0 to 957b3f6 Compare August 20, 2015 23:22

zmerlynn reviewed Aug 21, 2015
View reviewed changes

uluyol-goog added 4 commits August 20, 2015 18:44

Use a persistent volume for the docker registry.

14b554c

Add conversion function from GCE storage units to kubernetes units.

82638f8

Use quotes more consistently

2fb4e7b

uluyol force-pushed the insecure-reg branch from 957b3f6 to 2fb4e7b Compare August 21, 2015 01:56

Use gcr.io for proxy image

3dc10a2

uluyol force-pushed the insecure-reg branch from 55825b3 to 3dc10a2 Compare August 21, 2015 20:32

thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 21, 2015

zmerlynn added a commit that referenced this pull request Aug 21, 2015

Merge pull request #12833 from uluyol/insecure-reg

185b5af

Launch a cluster-local registry.

zmerlynn merged commit 185b5af into kubernetes:master Aug 21, 2015

uluyol mentioned this pull request Aug 21, 2015

Run a Docker registry #1319

Closed

pwais mentioned this pull request Aug 27, 2015

run kube-proxy on master node #9623

Closed

ikehz mentioned this pull request Mar 30, 2016

don't ship kube-registry-proxy and pause images in tars. #23605

Merged

Launch a cluster-local registry. #12833

Launch a cluster-local registry. #12833

Conversation

uluyol commented Aug 17, 2015

uluyol commented Aug 17, 2015

uluyol commented Aug 17, 2015

k8s-bot commented Aug 17, 2015

k8s-bot commented Aug 18, 2015

k8s-bot commented Aug 18, 2015

thockin commented Aug 18, 2015

thockin commented Aug 18, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin commented Aug 19, 2015

zmerlynn commented Aug 19, 2015

uluyol commented Aug 19, 2015

uluyol commented Aug 19, 2015

k8s-bot commented Aug 19, 2015

k8s-bot commented Aug 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinsb commented Aug 20, 2015

thockin commented Aug 20, 2015

justinsb commented Aug 20, 2015

thockin commented Aug 20, 2015

zmerlynn commented Aug 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zmerlynn commented Aug 20, 2015

uluyol commented Aug 20, 2015

k8s-bot commented Aug 20, 2015

k8s-bot commented Aug 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zmerlynn commented Aug 21, 2015

thockin commented Aug 21, 2015

uluyol commented Aug 21, 2015

k8s-bot commented Aug 21, 2015

k8s-bot commented Aug 21, 2015

zmerlynn commented Aug 21, 2015

thockin commented Aug 21, 2015

k8s-bot commented Aug 21, 2015

zmerlynn commented Aug 21, 2015

zmerlynn commented Aug 21, 2015