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

[stable/locust]: Updates configmap for locustfile and lib to not be wrapped in explictstring "example" #634

Merged

Conversation

byronmansfield
Copy link
Contributor

@byronmansfield byronmansfield commented Nov 21, 2024

Description

Updates configmap for locustfile and lib to not be wrapped in explictstring "example"

Checklist

  • Title of the PR starts with chart name (e.g. [stable/mychartname])
  • I have read the contribution instructions, bumped chart version and regenerated the docs
  • Github actions are passing

@byronmansfield byronmansfield requested a review from a team as a code owner November 21, 2024 18:24
@byronmansfield
Copy link
Contributor Author

Hey @max-rocket-internet I hope you are not getting sick of me yet. 😆

I made this one because I have been using helmfile and made my own locustfile ConfigMap I deployed from a local chart in the same repo. But I always had to roll the locust pods in order for them to get pick up for the locustfile change. Which was annoying. Then I tried to add the checksum annotation, and after re-reading the helm chart, I realized, that only works for the same helm deployment. You can't do that natively in kubernetes without something like reloader.

The current helm chart locustfile (and lib) CM's only will deploy if they are the default "example". I hope it's ok we just default it to that in the values.yaml and still deploy the same CM if someone overrides those values.

I believe the README.md explicitly calls out you have to create your own CM. If you would prefer I update the README.md to reflect these changes. I am happy to add that. Thoughts?

Copy link
Member

@max-rocket-internet max-rocket-internet left a comment

Choose a reason for hiding this comment

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

Nice, thanks @byronmansfield 💜

@max-rocket-internet max-rocket-internet merged commit c349de1 into deliveryhero:master Nov 22, 2024
7 checks passed
@byronmansfield byronmansfield deleted the locust-updates branch November 27, 2024 21:25
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