helm-charts
helm-charts copied to clipboard
[kube-prometheus-stack] Default port on which ETCD exposes its metrics
By default ETCD exposes its metrics on port 2379 and not 2381
What this PR does / why we need it
By default ETCD exposes its metrics on port 2379 and not 2381
Which issue this PR fixes
No service (by default) is listening on port 2381 on the cluster, so after installing the chart, the metrics for etcd would not be monitored unless the port in values.yaml (or ETCD configuration) is changed.
(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)
- fixes # Changing eh default port for etcd metrics from 2381 to 2379 in the following path kubeEtcd.service. port AND kubeEtcd.service. targetPort
Special notes for your reviewer
Checklist
- [x] DCO signed
- [x] Chart Version bumped
- [x] Title of the PR starts with chart name (e.g.
[prometheus-couchdb-exporter])
Hi @xogoodnow, thank you for the contribution. Please check https://github.com/prometheus-community/helm-charts/pull/2264 regarding the ETCD ports.
TLDR: https://github.com/kubernetes/kubernetes/blob/8b132ea40a250e9acf08ab2fadb9a8c2fd811d74/cmd/kubeadm/app/constants/constants.go#L95-L96
Hi @QuentinBisson, The version of my cluster at the moment is 1.28 which is the latest. as for the security concern, by default etcd exposes its metrics on 2379 and in the official docs of etcd itself there are no mentions of any security risk for doing this. https://etcd.io/docs/v3.1/op-guide/monitoring/ There is no mention of security risk of doing this anywhere (since it uses the certificates) Also I believe the default value must be compatible with default configuration of other services
Hi @GMartinez-Sisti , Just deploy the latest version of the cluster and install the chart, it does not work. I just tested it on our stage and it did not work. (if you have the time I recommend you just test this with the latest version of k8s 1.28 and see if it works by default) cheers
My impression is that introducing 2381 as an Etcd metrics port in the chart was based on a specific internal change in kubeadm, judging from the link to its constants above and in the original PR. From Etcd metrics-endpoint
Each etcd server exports metrics under the /metrics path on its client port and optionally on locations given by --listen-metrics-urls.
The client port is indeed 2379/tcp and supposed to always be protected by TLS.
kubeadm introduced port 2381 in a change to access the Etcd's /health endpoint for health checks/liveness probes without TLS replacing thus the previous health check implementation. It makes use of the Etcd flag above that can enable additional URLs serving /metrics and /health. Furthermore, as the health check is insecure, it opens the port at localhost - it is not supposed to be accessed from outside.
The changes in kubeadm have no effect on how Etcd primarily serves its metrics: through the client port.
IMHO if we decide to go ahead and change the port, it should be considered a breaking change to ensure we don't break existing installations.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.
Hi, considering that this would be a breaking change, do we have any estimation on how and when to make these changes on the chart?
Bump the helm major chart version + upgrade notes on the README.md is the way to go for an breaking change and needs to be part of the PR itself.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.
Hi Should something be done on my end?
Hi Should something be done on my end?
Yes, please take note of the review comment from @GMartinez-Sisti + resolve MR conflicts
hi @jkroepke, the changes have been made Please check and if there is something that I must do , let me know
Hi @xogoodnow
currently, I'm missing everything what is requested.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.