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

helm: general cleanup #4361

Merged
merged 5 commits into from
Nov 21, 2018
Merged

helm: general cleanup #4361

merged 5 commits into from
Nov 21, 2018

Conversation

derekperkins
Copy link
Member

This is a lot of cleanup work that feels good to have done. TL;DR:

  1. I changed all the imagePullPolicy references to be Always. This was set inconsistently and caused some issues with people trying it out. If this causes too many pulls in production, we can add a user flag for it later.
  2. I switched out a few logging containers that I forgot to handle in helm: logging improvements #4343
  3. The InitShardMaster job wasn't getting the proper labels
  4. Setting init_db_name_override=keyspace.name is the most impactful change since I am changing a default. I did this because many database clients want to prefix the database name on calls. By making keyspace name == schema name, this allows those to work properly. I did add a setting to values.yaml to turn it off for compatibility.
  5. There have been formatting issues where too much whitespace was being removed, so I reduced the amount of whitespace reduction that go templates are handling.

Signed-off-by: Derek Perkins <derek@derekperkins.com>
Signed-off-by: Derek Perkins <derek@derekperkins.com>
Signed-off-by: Derek Perkins <derek@derekperkins.com>
Signed-off-by: Derek Perkins <derek@derekperkins.com>
Signed-off-by: Derek Perkins <derek@derekperkins.com>
@dkhenry
Copy link
Contributor

dkhenry commented Nov 16, 2018

For setting the imagePullPolicy I think we should remove it everywhere and allow the users to use the default behavior of pulling when the tag is set to latest this would allow a user to specify a tag for the images and give them the option of using discreet images

@dkhenry dkhenry self-assigned this Nov 16, 2018
@derekperkins
Copy link
Member Author

There's already support for setting custom image tags, just not the imagePullPolicy. I still think the default for the helm chart should be Always, simply because there have been multiple issues reported that would have been solved by that. With the exception of maybe vtgates in a spiky workload with an HPA, these containers aren't really repulling images all that often anyways.

I agree that imagePullPolicy should be settable, my vote is to do that in a future PR.

Copy link
Contributor

@dkhenry dkhenry left a comment

Choose a reason for hiding this comment

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

LGTM

@dkhenry dkhenry merged commit 8254f43 into vitessio:master Nov 21, 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.

2 participants