Expose a lot of params - mostly to ease deployments in a different overlay network range #82
Conversation
…ng more hosts to the api-server's cert.
…oxy (the recommended/default now), and easing initial access to kube-controller-manager
|
Forgot to mention I also removed the kube-proxy |
|
@mrwulf Thanks for the PR. There are a lot of changes here and we will need to do some testing internally then get back to you. By any chance have you tested this with Weave or older versions of Kubernetes |
|
@mrwulf Can you also update the README for all the changes |
|
@mrwulf An update on this PR, we have sign off internally how we will manage backwards compatibility of K8 versions. So you will see movement on this PR this week |
tooling/kube_tool/other_params.rb
Outdated
| @@ -14,17 +14,19 @@ def OtherParams.create(os, version, container_runtime, cni_provider, bootstrap_c | |||
| kubernetes_package_version = version | |||
| end | |||
|
|
|||
| cni_cluster_cidr = false | |||
There was a problem hiding this comment.
These values need to be empty for Weave. When using weave this cause the Puppet run to fail.
==> kube-master: Error: Evaluation Error: Error while evaluating a Resource Statement, Class[Kubernetes]: parameter 'cni_cluster_cidr' expects a value of type Undef or String, got Boolean (file: /tmp/modules/role/manifests/kubernetes/master.pp, line: 3, column: 3) on node kube-master.lan
There was a problem hiding this comment.
@mrwulf This PR looks good, I have tested it across multiple OS and network providers. I just need the above issue fixed for Weave and it will be ready to merge.
templates/kube-apiserver.yaml.erb
Outdated
| @@ -15,7 +15,7 @@ spec: | |||
| - --allow-privileged=true | |||
| <% if @kubernetes_version =~ /1[.](8|9)[.]\d/ -%>- --enable-bootstrap-token-auth=true<% end %> | |||
| <% if @kubernetes_version =~ /1[.](6|7)[.]\d/ -%>- --experimental-bootstrap-token-auth=true<% end %> | |||
| - --service-cluster-ip-range=10.96.0.0/12 | |||
| - --service-cluster-ip-range=<%= @cni_cluster_cidr %> | |||
There was a problem hiding this comment.
This should not be the cluster cidr pool, this should be the service cidr pool. Can you please change that as it breaks the calico networking
There was a problem hiding this comment.
I added a cluster_service_cidr param to the puppet class to handle this. It isn't obvious which networking solutions require this, though. Maybe it should be added to kube_tool too.
There was a problem hiding this comment.
@mrwulf I agree it's not straightforward. Weave, Flannel and Calico act quite differently and need different configurations. End users usually only know how to configure one of the providers. We are trying to abstract the difference in configuration as much as possible in Kubetool
…i plugin doesn't override it
templates/kube-apiserver.yaml.erb
Outdated
| - --experimental-bootstrap-token-auth=true | ||
| <%- end -%> | ||
| <%- if (@cluster_service_cidr and not @cluster_service_cidr.empty?) -%> | ||
| - --service-cluster-ip-range=<%= @cni_cluster_cidr %> |
There was a problem hiding this comment.
I think this should be @cluster_service_cidr not @cni_cluster_cidr
There was a problem hiding this comment.
Sorry found just that last bug and it should be ready to merge
There was a problem hiding this comment.
doh- sorry! I changed the condition, but forgot to change the actual value. Fixed now
|
LGTM |
|
@mrwulf Thanks for a great PR !!!! |
(MODULES-7192) - Improved error message for incorrect formatting
New Parameters in kube_tool:
New Parameters in module:
Other changes:
kubectl createfor add-ons to bekubectl applyTested with:
centos 7.4
kubernetes 1.9.4
flannel 0.10.0
kube-dns 1.14.8
etcd 3.1.10
cni 0.6.0
docker 17.05.0.ce