expose skydns and dnsmasq metrics

[brancz]

The dnsmasq sidecar to expose metrics was recently released. Skydns (within kube-dns) by default exposes Prometheus metrics on a random port, and this defines it to be 8083.

It would be nice to expose those metrics by default, without having to modify the deployment after deploying a new cluster.

[stuart-warren]

your change mentions both port 8082 and 8083?

[brancz]

Port 8082 exposes metrics from dnsmasq and 8083 exposes metrics from skydns

[aaronlevy]

@brancz we typically try to keep the addons in-line with the version of kubernetes the tool is installing. In this case, these changes have been made to upstream master, but not yet in v1.4.x (which this tool currently installs).

We can diverge from the upstream releases, but looks like there are a few differences here compared to the upstream version: https://github.com/kubernetes/kubernetes/pull/36261/files -- and hopefully we can minimize differences / maintenance burden. Is there a reason we can't keep this closer to the upstream changes?

Also, we should probably add a comment to the manifest describing the change / reference PR so down the line when trying to update the manifest - this change is not dropped (e.g. if v1.5.x doesn't ship with that change, it should be clear that we need to maintain this as a patch).

[brancz]

I missed that PR upstream. Of course it makes sense to stay as close to the upstream example as possible. So I guess we wait then until it gets into 1.4.x or 1.5.x?

[aaronlevy]

If this is a useful / low risk change - we can diverge from the upstream. We do in minor ways already (such as using a deployment instead of replication controller - upstream changed in master, but not 1.4/1.5).

Another option would be to propose cherry-picking the upstream change back into v1.4/1.5 releases if that's not planned already.

I'm not super familiar with the needs for this change -- so I could go either way (wait until upstream matches, or carry patch if low-risk + helpful).

[brancz]

The need for this change is simply so we don't have to reconfigure kube-system components on future clusters, just to do something as basic to running a production infrastructure as exposing metrics from all components. All other components already expose metrics through their http servers, just these are missing. It is essential for end-to-end monitoring of a kubernetes cluster.

Regarding diverging from upstream: I have absolutely no hard feeling about the choice of ports, so I would prefer staying as close to upstream as possible, so I will update this PR accordingly. I will probably only get to it early next week, as I want to test it properly.

In addition to that I would also suggest to cherry-pick the change into 1.4 and 1.5.

Does this in general sound good?

[aaronlevy]

Yup, sounds good

[brancz]

Updated Deployment and Service manifests for kube-dns using:

curl -sL https://github.com/kubernetes/kubernetes/raw/545f749a0deb8535656ce93ac1f35a59887839bf/cluster/addons/dns/skydns-rc.yaml.sed | sed s/\$DNS_DOMAIN/cluster\.local/ | tail -n +21
curl -sL https://github.com/kubernetes/kubernetes/raw/545f749a0deb8535656ce93ac1f35a59887839bf/cluster/addons/dns/skydns-svc.yaml.sed | sed s/\$DNS_SERVER_IP/10\.3\.0\.10/ | tail -n +19

(tail to cut off the License part and a TODO, lmk if you want to include either of those)

Looks good to you now @aaronlevy ?

Built and tested with hack/multi-node.

[aaronlevy]

lgtm

[brancz]

for reference kube-dns and it's sidecars have moved to https://github.com/kubernetes/dns