Allow the nginx-ingress-controller to be configured by a configmap

[axinojolais]

This resolves juju-solutions/bundle-canonical-kubernetes#186

[mbruzek]

Thanks for the contribution! I see you are adding a configmap flag here. How are you going to get the Nginx configuration to the worker?

[axinojolais]

Easy : you just create a ConfigMap object. For example :

apiVersion: v1
data:
  body-size: 1024m
kind: ConfigMap
metadata:
  name: nginx-load-balancer-conf

You can see the list of all the options you can tweak at https://github.com/kubernetes/contrib/blob/master/ingress/controllers/nginx/configuration.md#allowed-parameters-in-configuration-configmap

Using the ConfigMap allows to deploy a "generic" ingress controller that anyone can tweak to their own needs later on.

[mbruzek]

The above link 404's for me so I can't see the options.

[axinojolais]

@mbruzek https://github.com/kubernetes/ingress/blob/master/controllers/nginx/configuration.md#allowed-parameters-in-configuration-configmap

[mbruzek]

I have not yet deployed this change, but it looks fine to me. I would like to add information in the README about how to configure or how a user would configure this option.

[chuckbutler]

This change LGTM. The default behavior didn't appear to change when the configmap was missing, and I like how easy this makes it tuning the ingress bits.

More to @mbruzek's point though we should at bare minimum land a simple change to the readme along with this so it's clear to end users that this is a configurable option. I'm not going to block this PR on that, however.

This also needs to be piggybacked into github.com/kubernetes/kubernetes as our source destination has moved from here to there as our large divergent branch has been caught up.

With this being a single file change it should be trivial to do so. If you don't mind @axinojolais would you point this PR at the upstream destination? If not, I can certainly carry this forward on your behalf by pulling this into a branch based on tip and denote that you've made the contribution. (they love squashed commits upstream)

+1 LGTM barring some minor doc updates.

[axinojolais]

Will update the README and retarget

[chuckbutler]

Do we know if axino was able to retarget this against upstream before heading out on vacation?

I tried grepping the pull request list but didn't see it... however that might have just been me (considering its 7:30 in the morning and i have no coffee)

[Cynerva]

Doesn't look like it. @chuckbutler ☕️

[mbruzek]

This PR needs to be rebased to the upstream repositry github.com/kubernetes/kubernetes.git as @chuckbutler pointed out #100 (comment).

I checked our addons directories and I don't see a file named "nginx-load-balancer-conf" and I worry about adding this configuration will cause a file not found error somewhere. @axinojolais gave me an "easy" example of what to put there, but without the file in this same PR I am not comfortable merging this.

Here is the directory listing of the kubernetes-worker file system:

$ ls -al /var/lib/juju/agents/unit-kubernetes-worker-0/charm/templates/
total 44
drwxr-xr-x  2 root root  276 Mar  6 17:16 .
drwxr-xr-x 15 root root 4096 Mar  7 17:31 ..
-rw-r--r--  1 root root  965 Mar  6 17:16 default-http-backend.yaml
-rw-r--r--  1 root root  134 Mar  6 17:16 docker.defaults
-rw-r--r--  1 root root 1349 Mar  6 17:16 docker.systemd
-rw-r--r--  1 root root 1319 Mar  6 17:16 ingress-replication-controller.yaml
-rw-r--r--  1 root root  657 Mar  6 17:16 kube-default
-rw-r--r--  1 root root  469 Mar  6 17:16 kubelet.defaults
-rw-r--r--  1 root root  524 Mar  6 17:16 kubelet.service
-rw-r--r--  1 root root   40 Mar  6 17:16 kube-proxy.defaults
-rw-r--r--  1 root root  426 Mar  6 17:16 kube-proxy.service
-rw-r--r--  1 root root 1177 Mar  6 17:16 microbot-example.yaml
$ ls -al /etc/kubernetes/addons/ 
total 8
drwxr-xr-x 2 root root   82 Mar  6 17:17 .
drwxr-xr-x 3 root root   20 Mar  6 17:17 ..
-r--r--r-- 1 root root  964 Mar  6 17:17 default-http-backend.yaml
-r--r--r-- 1 root root 1318 Mar  6 17:17 ingress-replication-controller.yaml

I don't see the configuration file that will create this configmap and I am not comfortable merging this at this time.

[axinojolais]

@mbruzek FWIW, the official example from Google does have the --nginx-configmap option : https://github.com/kubernetes/ingress/blob/master/examples/customization/configuration-snippets/nginx/nginx-ingress-controller.yaml

I think it works just fine if the file doesn't exist.

[mbruzek]

In the linked example the nginx configuration definition file exists in the directory so the ConfigMap object would exist in that example cluster. I was confused because in this PR, the ConfigMap is not defined but the ingress controller is configured to use a ConfigMap if/when it exists. Meaning the users would have to add their own ConfigMap object (with a specific name) to the Kubernetes system after install.

If the users are expected to add their own custom ConfigMap after install is it not reasonable to expect them to configure the ingress controller to point to that object? If this PR added the nginx-load-balancer-conf.yaml file to the template directory the charm would render of both these objects for the users when the charm re-renders the templates on charm upgrade or other hook events that might re-render the templates.

I spoke with @axinojolais on IRC and he explained that every users ingress config would be different and confirmed that this PR would be setting the ingress controller to use an undefined object. If that is the recommended way to do it I am OK with this approach it just seems unusual to me and I am worried about supporting this in the field.

[axinojolais]

This is deprecated, closing.