banzaicloud / banzai-charts

Curated list of Banzai Cloud Helm charts used by the Pipeline Platform
Apache License 2.0
368 stars 284 forks source link

fix query frontend downstream URL #1214

Closed mstrzele closed 3 years ago

mstrzele commented 3 years ago
Q A
Bug fix? yes
New feature? no
API breaks? no
Deprecations? no
License Apache 2.0

What's in this PR?

The PR contains a change for the query-frontend-downstream-url parameter's value.

Why?

The change removes .svc.cluster.local from the URL, so after the change, it will point to a service without specifying a namespace nor a cluster domain.

Checklist

psandhu79 commented 3 years ago

Should this not include {{ .Release.Namespace }}

    - "--query-frontend.downstream-url=http://{{ include "thanos.componentname" (list $ "query") }}-http.{{ .Release.Namespace }}.svc.cluster.local:{{ .Values.query.http.port }}"
mstrzele commented 3 years ago

Should this not include {{ .Release.Namespace }}

    - "--query-frontend.downstream-url=http://{{ include "thanos.componentname" (list $ "query") }}-http.{{ .Release.Namespace }}.svc.cluster.local:{{ .Values.query.http.port }}"

Both query service and query-frontend deployment are in the same namespace thus adding it to the URL is redundant.

mstrzele commented 3 years ago

Hi @ahma,

Could you take a look at this simple fix? It's been a while since I've created a PR and it would be nice to have it fixed in the upstream chart.

paranoidsp commented 3 years ago

Noticed the same issue, and this should fix it. Needs a bump to the chart version though.

paranoidsp commented 3 years ago

@ahma apologies for tagging you, but you seem to have the last commit on the thanos chart, would it be possible to get a review here and a merge?

paranoidsp commented 3 years ago

Thank you @ahma @mstrzele !