Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed syntax for "and" conditions #2869

Merged
merged 3 commits into from
Jul 7, 2020
Merged

Conversation

8398a7
Copy link
Contributor

@8398a7 8398a7 commented Jul 6, 2020

What problem does this PR solve?

Fixes a deployment error. (#2867)

<(.Values.monitor.create) and (.Values.monitor.grafana.create)>: can't give argument to non-function .Values.monitor.create

I have confirmed that both helm2 and helm3 work.

https://v2.helm.sh/docs/chart_template_guide/#operators-are-functions
https://helm.sh/docs/chart_template_guide/function_list/#and

What is changed and how does it work?

Fixed syntax.

Check List

Tests

  • Manual test (add detailed scripts or steps below)

I used a v1.16 series k8s cluster.
The following command is used to check the operation.

kubectl create ns tidb-admin
helm repo add pingcap https://charts.pingcap.org/
kubectl apply -f https://raw.githubusercontent.com/pingcap/tidb-operator/master/manifests/crd.yaml
helm upgrade tidb-operator pingcap/tidb-operator -i --wait -n tidb-admin
helm upgrade tidb-cluster pingcap/tidb-cluster -i --wait -n tidb-admin \
  --set schedulerName=tidb-scheduler,pd.storageClassName=hostpath,tikv.storageClassName=hostpath,pd.replicas=1,tikv.replicas=1,tidb.replicas=1,monitor.create=true,monitor.grafana.create=true

Code changes

  • Has Helm chart code change

Related changes

None.

Does this PR introduce a user-facing change?:

No.

Fixes a deployment error.

```
<(.Values.monitor.create) and (.Values.monitor.grafana.create)>: can't give argument to non-function .Values.monitor.create
```

We have confirmed that both helm2[1] and helm3[2] work.

[1]: https://v2.helm.sh/docs/chart_template_guide/#operators-are-functions
[2]: https://helm.sh/docs/chart_template_guide/function_list/#and
@CLAassistant
Copy link

CLAassistant commented Jul 6, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@cofyc cofyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Thanks for the fix!

@cofyc
Copy link
Contributor

cofyc commented Jul 7, 2020

/run-all-tests

@cofyc cofyc requested a review from DanielZhangQD July 7, 2020 04:57
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DanielZhangQD
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

Sorry @DanielZhangQD, you don't have permission to trigger auto merge event on this branch. The number of LGTM for this PR is 0 while it needs 2 at least

@DanielZhangQD
Copy link
Contributor

@8398a7 Thanks for your contribution!

@cofyc
Copy link
Contributor

cofyc commented Jul 7, 2020

/run-all-tests

@cofyc cofyc merged commit 863fb80 into pingcap:master Jul 7, 2020
@cofyc
Copy link
Contributor

cofyc commented Jul 7, 2020

/run-cherry-picker

ti-srebot pushed a commit to ti-srebot/tidb-operator that referenced this pull request Jul 7, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-1.1 in PR #2877

ti-srebot added a commit that referenced this pull request Jul 7, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Co-authored-by: 839 <8398a7@gmail.com>
@8398a7 8398a7 deleted the fix-template branch July 7, 2020 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants