Namespace defined templates in _helpers.tpl

[kevinschumacher]

Is this a BUG REPORT or FEATURE REQUEST? (choose one):
FEATURE REQUEST

I'd like to be able to depend on charts in the stable repository without having to inspect the charts' defined templates each version.

In the Helm documentation's Chart Best Practices, there is this to say about names of defined templates:

Defined templates (templates created inside a {{ define }} directive) are globally accessible. That means that a chart and all of its subcharts will have access to all of the templates created with {{ define }}.

For that reason, all defined template names should be namespaced.

Most of the charts in the stable and incubator repositories do not follow this best practice.

As PR's are submitted and merged, I will update this issue to track progress.

Which chart:
All of them except the below 6

List of Charts to be updated

• stable/acs-engine-autoscaler [stable/acs-engine-autoscaler] #1785 namespace defined templates with… #1976
• stable/artifactory [stable/artifactory] #1785 namespace defined templates with chart name #1977
• stable/aws-cluster-autoscaler [stable/aws-cluster-autoscaler] #1785 namespace defined templates wit… #1978 (closed; chart is deprecated)
• stable/buildkite [stable/buildkite] #1785 namespace defined templates with chart name #1979
• stable/centrifugo [stable/centrifugo] #1785 namespace defined templates with chart name #1980
• stable/chaoskube (no templates defined)
• stable/chronograf [stable/chronograf] #1785 namespace defined templates with chart name #1981
• stable/cluster-autoscaler [stable/cluster-autoscaler] #1785 namespace defined templates with ch… #1982
• stable/cockroachdb [stable/cockroachdb] #1785 namespace defined templates with chart name #1984
• stable/concourse [stable/concourse] #1785 namespace defined templates with chart name #1983
• stable/consul [stable/consul] #1785 namespace defined templates with chart name #1985
• stable/coredns [stable/coredns] #1785 namespace defined templates with chart name #1986
• stable/coscale [stable/coscale] #1785 namespace defined templates with chart name #1987
• stable/dask-distributed [stable/dask-distributed] #1785 namespace defined templates with char… #2125
• stable/datadog [stable/datadog] #1785 namespace defined templates with chart name #2126
• stable/dokuwiki [stable/dokuwiki] #1785 namespace defined templates with chart name #2128
• stable/drupal [stable/drupal] #1785 namespace defined templates with chart name #2127
• stable/etcd-operator [stable/etcd-operator] #1785 namespace defined templates with chart name #2129
• stable/external-dns [stable/external-dns] #1785 namespace defined templates with chart name #2130
• stable/factorio [stable/factorio] #1785 namespace defined templates with chart name #2152
• stable/fluent-bit [stable/fluent-bit] #1785 namespace defined templates with chart name #2151
• stable/g2 [stable/g2] #1785 namespace defined templates with chart name #2150
• stable/gcloud-endpoints [stable/gcloud-endpoints] #1785 namespace defined templates with char… #2149
• stable/gcloud-sqlproxy [stable/gcloud-sqlproxy] #1785 namespace defined templates with chart… #2148
• stable/ghost [stable/ghost] #1785 namespace defined templates with chart name #2147
• stable/gitlab-ce [stable/gitlab-ce] #1785 namespace defined templates with chart name #2146
• stable/gitlab-ee [stable/gitlab-ee] #1785 namespace defined templates with chart name #2145
• stable/grafana
• stable/heapster [stable/heapster] #1785 namespace defined templates with chart name #2144
• stable/influxdb [stable/influxdb] #1785 namespace defined templates with chart name #2143
• stable/ipfs [stable/ipfs] #1785 namespace defined templates with chart name #2142
• stable/jasperreports [stable/jasperreports] #1785 namespace defined templates with chart name #2141
• stable/jenkins [stable/jenkins] #1785 namespace defined templates with chart name #2140
• stable/joomla [stable/joomla] #1785 namespace defined templates with chart name #2139
• stable/kapacitor [stable/kapacitor] #1785 namespace defined templates with chart name #2137
• stable/keel [stable/keel] #1785 namespace defined templates with chart name #2136
• stable/kube-lego [stable/kube-lego] #1785 namespace defined templates with chart name #2135
• stable/kube-ops-view [stable/kube-ops-view] #1785 namespace defined templates with chart name #2134
• stable/kube-state-metrics [stable/kube-state-metrics] #1785 namespace defined templates with ch… #2133
• stable/kube2iam [stable/kube2iam] #1785 namespace defined templates with chart name #2132
• stable/kubernetes-dashboard [stable/kubernetes-dashboard] #1785 namespace defined templates with … #2131
• stable/linkerd [stable/linkerd] #1785 namespace defined templates with chart name #2138
• stable/locust
• stable/magento [stable/magento] #1785 namespace defined templates with chart name #2184
• stable/mailhog [stable/mailhog] #1785 namespace defined templates with chart name #2183
• stable/mariadb [stable/mariadb] #1785 namespace defined templates with chart name #2182
• stable/mediawiki [stable/mediawiki] #1785 namespace defined templates with chart name #2181
• stable/memcached [stable/memcached] #1785 namespace defined templates with chart name #2180
• stable/metabase [stable/metabase] #1785 namespace defined templates with chart name #2179
• stable/minecraft [stable/minecraft] #1785 namespace defined templates with chart name #2178
• stable/minio [stable/minio] #1785 namespace defined templates with chart name #2177
• stable/mongodb [stable/mongodb] namespace template helpers with chart name #1779
• stable/mongodb-replicaset [stable/mongodb-replicaset] #1785 namespace defined templates with ch… #2176
• stable/moodle [stable/moodle] #1785 namespace defined templates with chart name #2175
• stable/mysql [stable/mysql] #1785 namespace defined templates with chart name #2174
• stable/namerd [stable/namerd] #1785 namespace defined templates with chart name #2173
• stable/nginx-ingress [stable/nginx-ingress] #1785 namespace defined templates with chart name #2172
• stable/nginx-lego [stable/nginx-lego] #1785 namespace defined templates with chart name #2171
• stable/odoo [stable/odoo] #1785 namespace defined templates with chart name #2170
• stable/opencart [stable/opencart] #1785 namespace defined templates with chart name #2169
• stable/openvpn [stable/openvpn] #1785 namespace defined templates with chart name #2168
• stable/orangehrm [stable/orangehrm] #1785 namespace defined templates with chart name #2167
• stable/osclass [stable/osclass] #1785 namespace defined templates with chart name #2166
• stable/owncloud [stable/owncloud] #1785 namespace defined templates with chart name #2165
• stable/parse [stable/parse] #1785 namespace defined templates with chart name #2164
• stable/percona [stable/percona] #1785 namespace defined templates with chart name #2163
• stable/phabricator [stable/phabricator] #1785 namespace defined templates with chart name #2162
• stable/phpbb [stable/phpbb] #1785 namespace defined templates with chart name #2161
• stable/postgresql [stable/postgresql] namespace template helpers with chart name #1777
• stable/prestashop [stable/prestashop] #1785 namespace defined templates with chart name #2160
• stable/prometheus
• stable/rabbitmq [stable/rabbitmq] namespace template helpers with chart name #1776
• stable/redis [stable/redis] namespace the template helpers with chart name ({{ define "fullname" }} --> {{ define "redis.fullname" }} #1758
• stable/redmine [stable/redmine] #1785 namespace defined templates with chart name #2159
• stable/rethinkdb [stable/rethinkdb] #1785 namespace defined templates with chart name #2158
• stable/risk-advisor [stable/risk-advisor] #1785 namespace defined templates with chart name #2157
• stable/rocketchat [stable/rocketchat] #1785 namespace defined templates with chart name #2156
• stable/sapho [stable/sapho] #1785 namespace defined templates with chart name #2155
• stable/selenium [stable/selenium] #1785 namespace defined templates with chart name #2154
• stable/sensu [stable/sensu] #1785 namespace defined templates with chart name #2153
• stable/sentry
• stable/spark
• stable/spartakus
• stable/spinnaker
• stable/spotify-docker-gc
• stable/stash
• stable/sugarcrm
• stable/suitecrm
• stable/sumokube
• stable/sumologic-fluentd
• stable/sysdig
• stable/telegraf
• stable/testlink
• stable/traefik
• stable/uchiwa
• stable/voyager
• stable/weave-cloud
• stable/wordpress
• stable/zetcd
• incubator/cassandra [incubator/cassandra] #1785 namespace defined templates with chart name #1786
• incubator/check-mk [incubator/check-mk] #1785 namespace defined templates with chart name #1837
• incubator/docker-registry [incubator/docker-registry] #1785 namespace defined templates with ch… #1836
• incubator/elasticsearch [incubator/elasticsearch] #1785 namespace defined templates with chart name #1787
• incubator/etcd [incubator/etcd] #1785 namespace defined templates with chart name #1835
• incubator/gogs [incubator/gogs] #1785 namespace defined templates with chart name #1834
• incubator/istio [incubator/istio] #1785 namespace defined templates with chart name #1839)
• incubator/kafka [incubator/kafka] #1785 namespace defined templates with chart name #1788
• incubator/kube-registry-proxy [incubator/kube-registry-proxy] #1785 namespace defined templates wit… #1833
• incubator/patroni [incubator/patroni] #1785 namespace defined templates with chart name #1832
• incubator/redis-cache [incubator/redis-cache] #1785 namespace defined templates with chart … #1840
• incubator/tensorflow-inception [incubator/tensorflow-inception] #1785 namespace defined templates wi… #1838
• incubator/zookeeper [incubator/zookeeper] #1785 namespace defined templates with chart name #1789

What happened:
What you expected to happen:
How to reproduce it (as minimally and precisely as possible):
Anything else we need to know:

-- Questions for the chart maintainers:

Are there any charts which should not be updated in this way?

Does this change break the chart's API?

• What does that mean for chart versioning?
• Am I correct to increment the patch version, as I have been doing? See [stable/postgresql] namespace template helpers with chart name #1777 for example
  Because this is a distruptive change, I am incrementing major when major > 0, else incrementing minor. A few PRs which were opened/merged prior had the patch version incremented only.

Helper scripts here:
https://gist.github.com/kevinschumacher/a10162f18be905b62a02ed65d36ab2cf

[kevinschumacher]

cc @dhilipkumars

[kevinschumacher]

This is really just some grunt work, and I'm about 90% to automating it already just from doing those 4 PRs earlier. I'm willing to take on the effort as long as no one objects.

[dhilipkumars]

cc: @kubernetes/charts-maintainers
Thanks for the issue @kevinschumacher, if you could make your tool such that, it can be run against any chart (future PRs) to check if the definition is template "<workload>.fullname" .?

I think it would be useful to add that as a port of ci job.

[dhilipkumars]

@kevinschumacher Thanks thats a lot of work.
Once you create a PR link that here with that entry, once the PR is merged 'check' the box.
That way it will be easier to track.

[kevinschumacher]

@dhilipkumars thanks for the issue formatting help.

re: running check in CI -- great idea. Will post gist later.

Just a question of logistics --
One PR per chart is preferred, right?
Do you mind if all PRs get created at once, or would you rather see them spread out over a week?

[dhilipkumars]

All the PRs created at once is totally okay, we could get them in all quickly,
But yes one PR per chart is safer, if you combine all the changes in one PR, then the PR might take a long time to get it.

[ravishivt]

How can a parent chart reference a dependent child chart that has the updated namespace templates?

For instance, my chart declares redis as a dependency. I want to set an environment variable in my parent chart to the redis URL with the redis hostname.

        - name: REDIS_URL
          value: "redis://{{ template "redis.fullname" . }}:6379/0"

This worked previously to the change but no longer does. I tried {{ template "redis.redis.fullname" . }} but that is also undefined. What is the proper syntax to reference nested templates?

[kevinschumacher]

@ravishivt thanks for chiming in.

https://gist.github.com/kevinschumacher/952c8e656f29a85121fd89e80c14c182

I'm using redis 0.7.1 there (which uses "fullname" and "name" templates) and your example actually fails for me.

What version of helm are you using? Perhaps it is a behavior change in the application and not the change to the redis chart?

edit: clarity

[kevinschumacher]

I am seeing some behavior that I didn't expect actually, but, with the redis 0.9.0 chart, you can definitely reference the subchart's defined templates named as they are (the names are global).

In here, I reference "redis.fullname" in my parent chart's NOTES.txt

https://gist.github.com/kevinschumacher/d82507ba67624b761e65c96ca2c21191

However, it seems that the variables referenced in the defined template are bound to the values of the template that's being currently rendered. If I call "redis.fullname" in my parent chart's deployment (or, NOTES.txt), I still get release-parent even though I passed in a redis.nameOverride

This is with helm 2.5.1

[prydonius]

Yes, I believe this doesn't work quite as is expected, as the template is rendered in the context of the chart it's being called from. This means that if the template uses .Chart.Name or other such values redis.fullname would give you the same result as parent.fullname/fullname in the parent chart. This is the reason we currently define redis.fullname in the parent chart (https://github.com/kubernetes/charts/blob/master/stable/sensu/templates/_helpers.tpl#L14). Note that this helper doesn't rely on .Chart.Name and instead hardcodes the name to "redis". It isn't ideal, but this is currently how we get around this.