helm-charts icon indicating copy to clipboard operation
helm-charts copied to clipboard

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

Open arahja opened this issue 1 year ago • 1 comments

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

  • fixes #4414

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])

arahja avatar Apr 04 '24 15:04 arahja

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.

zeritti avatar Apr 25 '24 19:04 zeritti