Skip to content

default helm vtgate maxReplicas to replicas#3710

Merged
sougou merged 1 commit intovitessio:masterfrom
gflarity:fix_vtgate_helm_tpl
May 2, 2018
Merged

default helm vtgate maxReplicas to replicas#3710
sougou merged 1 commit intovitessio:masterfrom
gflarity:fix_vtgate_helm_tpl

Conversation

@gflarity
Copy link
Copy Markdown

@gflarity gflarity commented Mar 1, 2018

Using the Unsharded keyspace example from:
https://github.com/vitessio/vitess/tree/master/helm/vitess

Fails:
Error: render error in "vitess/templates/vitess.yaml": template: vitess/templates/vitess.yaml:51:3: executing "vitess/templates/vitess.yaml" at <include "vtgate" (tu...>: error calling include: template: vitess/templates/_vtgate.tpl:141:6: executing "vtgate" at <gt .maxReplicas .rep...>: error calling gt: invalid type for comparison
On inspection it looks like .maxReplicas is undefined and so you can't compare it to an int. This fix assumes that maxReplicas isn't required, and defaults it to .replicas. Alternatively .maxReplicas could be required and the documentation should be updated instead.

Cheers,
Geoff

@enisoc
Copy link
Copy Markdown
Member

enisoc commented Mar 13, 2018

@derekperkins Can you take a look at this if you have a chance?

@derekperkins
Copy link
Copy Markdown
Member

Yep, I'll be dealing with this in the next day or two.

@gflarity
Copy link
Copy Markdown
Author

gflarity commented Mar 15, 2018

Sorry, I didn't realize I need to git commit -s. I can close and reopen a signed version if my PR makes sense.

@enisoc
Copy link
Copy Markdown
Member

enisoc commented Mar 15, 2018

@gflarity Not your fault. I think we switched to DCO after you opened this PR. :)

If you're comfortable with rebases, you could fix this PR without having to open a new one, with something like:

git rebase --signoff master
git push -f

Signed-off-by: Geoff Flarity <geoff@squareup.com>
@gflarity gflarity force-pushed the fix_vtgate_helm_tpl branch from 7b0be21 to e5e619a Compare March 26, 2018 15:52
@gflarity
Copy link
Copy Markdown
Author

Done!

@sougou
Copy link
Copy Markdown
Contributor

sougou commented May 1, 2018

Now that I understand how helms work, I like this change :)

@derekperkins
Copy link
Copy Markdown
Member

LGTM. Let's go ahead and merge it

@sougou sougou merged commit 9e3189e into vitessio:master May 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants