concourse / concourse-chart

Helm chart to install Concourse
Apache License 2.0
143 stars 176 forks source link

Fix broken .Values.concourse.worker.garden.dnsProxyEnable helper #257

Closed ericb-summit closed 3 years ago

ericb-summit commented 3 years ago

The helm chart check for concourse.worker.garden.dnsProxyEnable assumes the default value of this variable is false. However, the underlying env var CONCOURSE_GARDEN_DNS_PROXY_ENABLE defaults to true in the container.

In short, changing this value to false has no effect on the worker deployment, and setting to true sets an env var to true that is already default true (in the Dockerfile), so again, no effect.

Changes proposed in this pull request

xtremerui commented 3 years ago

The CONCOURSE_GARDEN_DNS_PROXY_ENABLE is default to false https://github.com/concourse/concourse/blob/0874fc5969abc560af526b4c812147871654e154/worker/workercmd/worker_linux.go#L61-L63

Also there are multiple places we are using the same syntax to check the value of concourse.worker.garden.dnsProxyEnable, i think we could just update the default value to false to make it clear https://github.com/concourse/concourse-chart/blob/4439afefca87733decba826c58d32e6225b085df/values.yaml#L1634

ericb-summit commented 3 years ago

It defaults to true via ENV in the Dockerfile which overrides the golang code.

xtremerui commented 3 years ago

I see. So the intention here would be "we don't want to set this flag to either false or true to override the value in lower stack unless OP wants to do it explicitly here".

Thank you for the PR!