search

pingcap / tidb-operator

TiDB operator creates and manages TiDB clusters running in Kubernetes.
https://docs.pingcap.com/tidb-in-kubernetes/
Apache License 2.0
1.22k stars 493 forks source link

[Helm Chart]Cannot properly set string format numeric value from command line #480

Closed aylei closed 5 years ago

aylei commented 5 years ago

Bug Report

What version of Kubernetes are you using? Kubernetes: 1.12.6 tidb-operator: Latest Helm:

$ helm version
Client: &version.Version{SemVer:"v2.9.1", GitCommit:"20adb27c7c5868466912eebdf6664e7390ebe710", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.9.1", GitCommit:"20adb27c7c5868466912eebdf6664e7390ebe710", GitTreeState:"clean"}

What did you do? Try to set some helm variable from command line:

helm install --debug --dry-run -n test ./charts/tidb-cluster --set tidb.tokenLimit='1000'

Which causes the following error:

[debug] Created tunnel using local port: '60216'

[debug] SERVER: "127.0.0.1:60216"

[debug] Original chart version: ""
[debug] CHART PATH: /Users/alei/go/src/github.com/pingcap/tidb-operator/charts/tidb-cluster

Error: render error in "tidb-cluster/templates/tidb-configmap.yaml": template: tidb-cluster/templates/_helpers.tpl:86:3: executing "tidb-configmap.data-digest" at <include "tidb-config...>: error calling include: template: tidb-cluster/templates/_helpers.tpl:81:39: executing "tidb-configmap.data" at <include "helm-toolki...>: error calling include: template: tidb-cluster/templates/_helpers.tpl:23:3: executing "helm-toolkit.utils.template" at <include $wtf $contex...>: error calling include: template: tidb-cluster/templates/config/_tidb-config.tpl:32:60: executing "tidb-cluster/templates/config/_tidb-config.tpl" at <atoi>: wrong type for value; expected string; got float64

According to https://github.com/helm/helm/issues/1694, when set string literal, the quotes have to be escaped:

helm install --debug --dry-run -n test ./charts/tidb-cluster --set tidb.tokenLimit=\"1000\"

The command runs without error, however, the configuration rendered is not as expected:

...
    # The limit of concurrent executed sessions.
    token-limit = 0
...

Other string format numeric values have the same problem as above.

What did you expect?

  1. Should: helm install --set key=\"<int>\" should set the value correctly.
  2. Better: numeric value should accept numeric format too (while keeping the string format for backward compatibility)
aylei commented 5 years ago

According to https://github.com/helm/helm/issues/1707, it should be --set-string, which works for me.

gregwebs commented 5 years ago

I would like to user the style of the Grafana chart where the helm values generate the config file so there is no templating of a config file.