[kube-prometheus-stack] Add prometheus.thanosService.omitClusterIP #864
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yurrriq
requested review from
bismarck,
gianrubio,
gkarthiks,
scottrigby,
vsliouniaev and
Xtigyro
as code owners
yurrriq
force-pushed
the
thanos-service-omit-cluster-ip
branch
2 times, most recently
from
4d9ff1e to
839d63f
Compare
|
rebased. need anything else from me here? this is pretty much a blocker for us |
Signed-off-by: Eric Bailey <eric@ericb.me>
yurrriq
force-pushed
the
thanos-service-omit-cluster-ip
branch
from
839d63f to
411ee39
Compare
|
rebased again. |
monotek
requested changes
Apr 25, 2021
| @@ -1597,6 +1597,7 @@ prometheus: | |||
| portName: grpc | |||
| port: 10901 | |||
| targetPort: "grpc" | |||
| omitClusterIP: false | |||
There was a problem hiding this comment.
Why is this needed?
Imho you could just set clusterIP: "" to get rid of the error.
There was a problem hiding this comment.
I'll try that and report back.
There was a problem hiding this comment.
Same result.
UPGRADE FAILED: cannot patch "prometheus-operator-thanos-discovery" with kind Service: Service "prometheus-operator-thanos-discovery" is invalid: spec.clusterIP: Invalid value: "": field is immutable
yurrriq
commented
Apr 27, 2021
| @@ -16,7 +16,9 @@ metadata: | |||
| {{- end }} | |||
| spec: | |||
| type: {{ .Values.prometheus.thanosService.type }} | |||
| {{- if not .Values.prometheus.thanosService.omitClusterIP }} | |||
| clusterIP: {{ .Values.prometheus.thanosService.clusterIP }} | |||
There was a problem hiding this comment.
As discussed in the linked tickets (see links in #863) regarding the nginx chart, the ideal solution here would be something like this, but if we want to maintain backward compatibility, we'll probably need something like omitClusterIP.
Suggested change
| clusterIP: {{ .Values.prometheus.thanosService.clusterIP }} | |
| {{- with .Values.prometheus.thanosService.clusterIP }} | |
| clusterIP: {{ . }} | |
| {{- end }} |
3 tasks
|
Superseded by #916 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Support omission of the Thanos service
clusterIP.Which issue this PR fixes
Special notes for your reviewer:
Similar to helm/charts#13646
Checklist
[prometheus-couchdb-exporter])