-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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/thanos] feature: allow configuring labels fir query ingress #32202
[bitnami/thanos] feature: allow configuring labels fir query ingress #32202
Conversation
Thank you for the PR! As in the other case, your fork doesn't allow @bitnami-bot to access it as a maintainer, which is required for the GitHub Actions to succeed. Could you please check this? |
Thanks for your quick response, @carrodher! We have now added @bitnami-bot to the project with the maintainer role. Is there something else we need to do to re-launch the github-actions involved in the PR approval? |
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 contribution! Please check my comments
{{- if or .Values.query.ingress.labels .Values.commonLabels }} | ||
{{- $labels := include "common.tplvalues.merge" ( dict "values" ( list .Values.query.ingress.labels .Values.commonLabels ) "context" . ) }} | ||
labels: {{- include "common.tplvalues.render" ( dict "value" $labels "context" $) | nindent 4 }} | ||
{{- end }} |
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.
Labels should be set regardless they're .Values.query.ingress.labels
OR .Values.commonLabels
are set:
{{- if or .Values.query.ingress.labels .Values.commonLabels }} | |
{{- $labels := include "common.tplvalues.merge" ( dict "values" ( list .Values.query.ingress.labels .Values.commonLabels ) "context" . ) }} | |
labels: {{- include "common.tplvalues.render" ( dict "value" $labels "context" $) | nindent 4 }} | |
{{- end }} | |
{{- $labels := include "common.tplvalues.merge" ( dict "values" ( list .Values.query.ingress.labels .Values.commonLabels ) "context" . ) }} | |
labels: {{- include "common.labels.standard" ( dict "customLabels" $labels "context" $ ) | nindent 4 }} | |
app.kubernetes.io/component: query |
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, you are right.
I've applied the proposed changes
bitnami#31607) * [bitnami/elasticsearch] fix: 🐛 Mount emptyDir in /bitnami/elasticsearch to allow init scripts Signed-off-by: leoarry <[email protected]> --------- Signed-off-by: leoarry <[email protected]> Signed-off-by: Bitnami Containers <[email protected]> Signed-off-by: Carlos Rodríguez Hernández <[email protected]> Co-authored-by: leoarry <[email protected]> Co-authored-by: Bitnami Containers <[email protected]> Co-authored-by: Carlos Rodríguez Hernández <[email protected]> Signed-off-by: Angel Abella <[email protected]>
* [bitnami/concourse] Release 5.1.15 updating components versions Signed-off-by: Bitnami Containers <[email protected]> * Update CHANGELOG.md Signed-off-by: Bitnami Containers <[email protected]> --------- Signed-off-by: Bitnami Containers <[email protected]> Signed-off-by: Angel Abella <[email protected]>
Signed-off-by: Angel Abella <[email protected]>
…ress.labels Signed-off-by: Angel Abella <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]> Signed-off-by: Angel Abella <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]> Signed-off-by: Angel Abella <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]> Signed-off-by: Angel Abella <[email protected]>
14261b4
to
1c270fd
Compare
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.
Hi @viyullas
It seems your fork's branch isn't properly rebased with latest changes on main
branch given now several changes affecting other charts appear
…sticsear… (bitnami#31607)" This reverts commit 59d7edd.
This reverts commit 89bbbae.
…uery.ingress.labels" This reverts commit 1efc025.
…ting .Values.query.ingress.labels Signed-off-by: Angel Abella <[email protected]>
I've closed this PR and started again on #32240 |
Allow setting labels for the query ingress just as annotations are allowed
Description of the change
Add the labels on the ingress template along with the common labels already present
Benefits
Now you can set labels on the query ingress if you need
Possible drawbacks
Applicable issues
Additional information
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