[kube-prometheus-stack] support kubelet endpoint slices #4899
Conversation
Signed-off-by: drfaust92 <[email protected]>
DrFaust92
requested review from
andrewgkew,
gianrubio,
gkarthiks,
GMartinez-Sisti,
jkroepke,
QuentinBisson,
scottrigby and
Xtigyro
as code owners
| @@ -103,7 +103,10 @@ rules: | |||
| - endpointslices | |||
| verbs: | |||
| - get | |||
| - create | |||
There was a problem hiding this comment.
Ideally we can use the kubeletEndpointsEnabled and kubeletEndpointSliceEnabled to toggle granting these permissions. will let reviewer decide. i can add this logic if needed
There was a problem hiding this comment.
The toggle would cover only write permissions, right?
There was a problem hiding this comment.
No, I think all of the endpointslice permissions. unelss there is a reason not to (I dont see one)
Signed-off-by: drfaust92 <[email protected]>
| @@ -63,6 +63,12 @@ spec: | |||
| - --kubelet-selector={{ .Values.prometheusOperator.kubeletService.selector }} | |||
| {{- end }} | |||
| {{- end }} | |||
| {{- if .Values.prometheusOperator.kubeletEndpointsEnabled }} | |||
There was a problem hiding this comment.
if in combination with an boolean might be mis-leading.
Since --kubelet-endpoints= is set to true by default, I set the value to false keep the flag still to true, because --kubelet-endpoints=false would be omit.
not sure, if
{{- if (print .Values.prometheusOperator.kubeletEndpointsEnabled) }}
would help here, since it converts the value to a string and "false" is still true-ish, I guess?
There was a problem hiding this comment.
Good point, maybe just omit the ifs entirely and go by the values.yaml values?
| @@ -103,7 +103,10 @@ rules: | |||
| - endpointslices | |||
| verbs: | |||
| - get | |||
| - create | |||
There was a problem hiding this comment.
The toggle would cover only write permissions, right?
|
jkroepke I think (assume) to leave it same as upstream for the "common" case. |
What this PR does / why we need it
Which issue this PR fixes
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged)Special notes for your reviewer
Based one upstream prometheus-operator/prometheus-operator#6882
Checklist
[prometheus-couchdb-exporter])