-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
[bitnami/jaeger] Add support for cassandra existingSecret #26563
Conversation
Signed-off-by: Miguel Ruiz <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Please check my comments.
@@ -142,10 +142,10 @@ ref. https://github.com/jaegertracing/jaeger-operator/issues/1158 | |||
Create the cassandra secret name | |||
*/}} | |||
{{- define "jaeger.cassandra.secretName" -}} | |||
{{- if not .Values.cassandra.enabled -}} | |||
{{- .Values.externalDatabase.existingSecret -}} | |||
{{- if (not .Values.cassandra.enabled) -}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no reason for adding these parenthesis
{{- else -}} | ||
{{- printf "%s-cassandra" (include "common.names.fullname" .) -}} | ||
{{- default (printf "%s-cassandra" .Release.Name) .Values.cassandra.dbUser.existingSecret -}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create another helper that returns the Cassandra fullname:
{{/*
Return the proper Jaeger Cassandra database fullname
*/}}
{{- define "jaeger.cassandra.fullname" -}}
{{- include "common.names.dependency.fullname" (dict "chartName" "cassandra" "chartValues" .Values.cassandra "context" $) -}}
{{- end -}}
Then, use it here:
{{- default (printf "%s-cassandra" .Release.Name) .Values.cassandra.dbUser.existingSecret -}} | |
{{- default (include "jaeger.cassandra.fullname" .) .Values.cassandra.dbUser.existingSecret -}} |
This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution. |
Signed-off-by: Carlos Rodríguez Hernández <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
Signed-off-by: Miguel Ruiz <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description of the change
Fixes an issue where Jaeger would not use the provided Cassandra existingSecret value.
Applicable issues
Checklist
Chart.yaml
according to semver. This is not necessary when the changes only affect README.md files.README.md
using readme-generator-for-helm