PMM-15070 Set node-exporter port to 9101 to avoid OpenShift conflict#864
PMM-15070 Set node-exporter port to 9101 to avoid OpenShift conflict#864talhabinrizwan wants to merge 3 commits into
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
Updates the pmm-ha Helm chart defaults to run the bundled prometheus-node-exporter on port 9101 (instead of 9100) to avoid port conflicts on OpenShift clusters where the platform node-exporter commonly occupies host port 9100, and bumps the chart patch version accordingly.
Changes:
- Override
prometheus-node-exporter.service.portandtargetPortto9101incharts/pmm-ha/values.yaml. - Bump
charts/pmm-ha/Chart.yamlversion from1.5.0to1.5.1.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| charts/pmm-ha/values.yaml | Changes default node-exporter Service port/targetPort to 9101 to avoid OpenShift host-port collision. |
| charts/pmm-ha/Chart.yaml | Patch version bump to reflect the values change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@4nte this should not be merged like this. |
The previous fix overrode the bundled node-exporter's service port to 9101 to dodge OpenShift's platform node-exporter on host port 9100. That still deploys a second exporter on every node and is OpenShift-specific, and it leaves dashboards/scrape configs expecting the standard 9100. Instead, make node-exporter scraping configurable so an exporter that already exists in the cluster can be reused on any distribution: - Revert the 9101 service port override (back to the upstream 9100 default). - Add a `nodeExporterScrape` section (scrape toggle, namespaces, service/port name regexes) and template the VMAgent node-exporter job from it. Defaults reproduce the previous behaviour exactly, so a fresh install is unchanged. - To reuse an existing exporter, set `prometheus-node-exporter.enabled=false` and point `nodeExporterScrape` at it (no new RBAC needed; VMAgent already has cluster-scoped endpoint discovery). - Warn via NOTES.txt when the bundled exporter is disabled but scraping still targets it, so node metrics don't silently disappear. - Quote interpolated regex and namespace values to avoid YAML injection. - Document the reuse path (incl. the OpenShift kube-rbac-proxy/federation caveat) in the chart README. Signed-off-by: theTibi <tkorocz@gmail.com>
|
@talhabinrizwan @ademidoff I made some changed to this PR, you can enable disable node exporter and you can even tell to it where is the node exporter if it is already installed. what do you think ? |
# Conflicts: # charts/pmm-ha/Chart.yaml
| - source_labels: [__meta_kubernetes_pod_name] | ||
| target_label: pod | ||
| # Scrape node-exporter for node-level metrics | ||
| {{- if .Values.nodeExporterScrape.scrape }} |
There was a problem hiding this comment.
Verified — this is a false positive. {{- if }} trims the whitespace/newline before the directive, but the newline after }} is preserved, so the following line stays on its own line.
I rendered the chart and parsed the resulting inlineScrapeConfig as YAML in all three modes (default; reuse with prometheus-node-exporter.enabled=false; and nodeExporterScrape.scrape=false) — valid in every case, with the node-exporter job rendered correctly.
This is also the exact idiom every other scrape job in this file already uses (haproxy, clickhouse, kube-state-metrics, …). If the left-trim corrupted the block scalar, those jobs would already be broken — they render fine. No change needed.
| {{- if .Values.nodeExporterScrape.namespaces }} | ||
| {{- range .Values.nodeExporterScrape.namespaces }} | ||
| - {{ . | quote }} | ||
| {{- end }} | ||
| {{- else }} | ||
| - {{ .Release.Namespace }} | ||
| {{- end }} |
There was a problem hiding this comment.
Same as the thread above — checked the rendered output. names: is followed by correctly-indented entries (- "pmm" by default; - "monitoring" / - "openshift-monitoring" in reuse mode), and the embedded scrape config parses as valid YAML.
The left-trims only strip leading whitespace before each directive; the line break after each }} is preserved, so there's no line concatenation. No change needed.
| prometheus-node-exporter: | ||
| enabled: true | ||
|
|
| description: A Helm chart for Percona Monitoring and Management (PMM) | ||
| type: application | ||
| version: 1.5.1 | ||
| version: 1.5.2 |
| target_label: namespace | ||
| - source_labels: [__meta_kubernetes_pod_name] | ||
| target_label: pod | ||
| {{- end }} |
There was a problem hiding this comment.
Verified as well — the closing {{- end }} trims the newline before it, but the newline after }} keeps # Scrape kubelet … on its own line. Confirmed by parsing the rendered inlineScrapeConfig as YAML (valid in default / reuse / scrape-disabled modes), and it matches the pattern used by the other jobs in this file. No change needed.
Summary
prometheus-node-exporter.service.port(andtargetPort) to9101incharts/pmm-ha/values.yamlto avoid colliding with OpenShift's platform node-exporter, which is managed by the Cluster Monitoring Operator and binds host port9100on every worker node.charts/pmm-ha/Chart.yamlpatch version1.5.0→1.5.1perCONTRIBUTING.md.Jira
PMM-15070
Test plan
helm templaterenders the pmm-ha chart and the node-exporter Service shows port9101.9100.