Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[prometheus-node-exporter] Updated to add k8s service port config #4415

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

arahja
Copy link

@arahja arahja commented Apr 4, 2024

Updated prometheus-node-exporter helm chart to allow you to change the Kubernetes service port separately from the container port.

@gianrubio
@zanhsieh
@zeritti

What this PR does

This PR Makes a small change to the prometheus-node-exporter helm chart to allow you to change the Kubernetes service that the helm chart creates and allow you to set the port that the service presents. This change defaults to the current behavior of the chart. This PR adds a new variable that will allow you change the port that the service presents.

Why we need it

If I change service.port to 80 the pods fail to start because of the security context. I do not want to disable the security context. The current behavior is if I change the service.port it also changes the container port to match the service port. The desire is to be able to change only the port number that the service presents and leave the pod configuration at the defaults.

Which issue this PR fixes

Special notes for your reviewer

Checklist

  • DCO signed
  • Chart Version bumped
  • Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

…e Kubernetes service port seperately from the container port. fixes prometheus-community#4414

Signed-off-by: Adam Rahja <arahja@gmail.com>
@zeritti
Copy link
Member

zeritti commented Apr 25, 2024

Thank you, @arahja, for your PR.

Adding another port in the service would indeed work but would also be a workaround only (and thereby would service allow setting three ports out of which two must generally be equal).
The dependency of the main container port on the service port which in fact does not exist should not be maintained. Instead, we may prefer removing this dependency by providing a field to set the container port as such which can be taken either by the exporter itself or by RBAC Proxy if enabled.

Suppose we chose containerPort for that purpose. This field would then replace service.port in the pod template (for completeness we could also introduce containerPortName for its name). Such a change could be considered major, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[prometheus-node-exporter] Unable to define Service port seperately from container port.
3 participants