[Jaeger] fix .allinone.replicas helm value - Githubissues

jaegertracing / helm-charts

Helm Charts for Jaeger backend

Apache License 2.0

269 stars 347 forks source link

[Jaeger] fix .allinone.replicas helm value #499

Closed micolun closed 1 year ago

micolun commented 1 year ago

This PR fixes the bug created after PR#494 when helm chart value `.allInOne.replicas` is `0` but the deployment is still set to 1.

When the below if statement is computed for `replicas: 0`, the result is `false` and as a consequence the deployment manifest doesn't contain `replicas` field, which in turn forces it to use the default value, which is `1`.

spec:
  {{- if .Values.allInOne.replicas }}
  replicas: {{ .Values.allInOne.replicas }}
  {{- end }}

fixes # Rather than checking value of replicas, we can check if .allInOne.replicas key has been declared. This would create a deployment with the specified number of replicas in the values file.

Checklist

[x] DCO signed
[x] Commits are GPG signed
[x] Chart Version bumped
[x] Title of the PR starts with chart name ([jaeger] or [jaeger-operator])
[x] README.md has been updated to match version/contain new values

pavelnikolov commented 1 year ago

@Mihail-blip not all commit have been signed. Could you, please, squash all the two commits into one and sign them?

micolun commented 1 year ago

One of my first contributions, I accidentally closed the PR. Think I managed to gpg sign the commits, please review

pavelnikolov commented 1 year ago

The PR is still not DCO signed. Thank you for reporting. Fixed in https://github.com/jaegertracing/helm-charts/commit/28ab98bfb2b2c5ab4f024b7337c3bb76871a0768