att-comdev / halcyon-kubernetes

Ansible playbooks for a kubadm-based kubernetes deployment, on supporting any cloud and any kubeadm-enabled OS.
Apache License 2.0
35 stars 22 forks source link

Provide a simple mechanism for enabling an insecure docker registry. #47

Closed mwgiles closed 7 years ago

mwgiles commented 7 years ago

Allows the user to specify a registry name to be added to docker's list of insecure registries on all hosts.


This change is Reviewable

aric49 commented 7 years ago

Thanks for the PR! This looks like a functionality that will really enhance the flexibility of the Kubernetes deployment.

One clarification, it looks like this deploys the docker insecure registry on all hosts in the cluster. Is there a benefit of having the registry on all hosts, instead of just the master?

mwgiles commented 7 years ago

@aric49 My understanding is that the image pull occurs on whatever kubernetes node on which a pod is assigned, and that the pull will use the local docker settings on that host. Therefore, to allow a container to be pulled from an insecure registry regardless of where it is assigned, the setting needs to be made to all hosts. It's possible my understanding is incorrect, though.

v1k0d3n commented 7 years ago

@mwgiles you are correct. one improvement could be added is something like a kubernetes-registry-proxy (which would actually be a nice improvement).

@mwgiles could you do me a favor and rather than making this a boolean flag, make this a specific variable? this way, we can add a future enhancement of having a single kube-registry-proxy feature.

i'm thinking of flags like:

sample: https://github.com/kubernetes/kubernetes/blob/master/cluster/saltbase/salt/kube-registry-proxy/kube-registry-proxy.yaml

mwgiles commented 7 years ago

@v1k0d3n I'm not sure I understand what you're asking for. Are you suggesting changing the boolean to a variable with an enumerated list of possible values, one of which would indicate a local insecure registry, and other possible values to be implemented later? Or am I totally misunderstanding?

I'm happy to make some changes, but want to make sure I understand what is being requested first. Thanks!

v1k0d3n commented 7 years ago

@mwgiles glad you're open to the changes; i don't think they're that much really.

i see this being implemented like this:

file: group_vars/all.yml

## Registry Type:
# Optional Items: default (no changes), docker_insecure (your changes), kube_pod_proxy (see below)
registry_type: docker_insecure

...then based on a new parameter docker_insecure, your changes are mostly the same. it just expands beyond a simple boolean flag so we can cleanly expand this later. for instance, in the future...we may want to stand up the kubernetes master, and based on our kube_pod_proxy flag configure the nodes to point to a registry (pod) on the master for a single source for proxying/caching images. this part can be captured in another feat:Issue/PR.

hopefully that helps?

mwgiles commented 7 years ago

@v1k0d3n Updated as per your comments. Thanks for the suggestion

aric49 commented 7 years ago

Yes, I just reviewed the changes (sorry for the delay). This looks good to me. Thank you very much for the PR @mwgiles!