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
Added init container which applies config file once by setting --watch-interval
equals to 0
#3955
Added init container which applies config file once by setting --watch-interval
equals to 0
#3955
Conversation
…iner names used in initContainers and Containers.
At the point that the init container runs, the primary containers have not started. So prometheus won't actually be up yet for this to trigger a reload on. |
--watch-interval
equals to 0
Good call. Updated to skip making reload request to :locahost:9090/-/reload when --watch-interval==0; please see thanos-io/thanos#4032 |
Would you please help to review the changes? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you update the description of the --watch-interval
flag to explain that when 0, the program runs only once and exits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few suggestions but great work!
--watch-interval
equals to 0
--watch-interval
equals to 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution, the PR looks quite good so far.
As one last improvement, could you also add tests to the prometheus and alertmanager test suites to make sure the init container is added to the respective statefulsets?
@fpetkovski Are these changes good enough and would you please give another review? |
Hey @zsuzhengdu, this looks good to me now. I also tested the changes locally and they look solid. Thanks for the contribution! |
Thank you for the review and local try out, @fpetkovski! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm from my side, leaving final say to @simonpasquier
Thanks a lot for your perseverance and sorry about the delayed review from my side! Also thanks to @fpetkovski for taking over :) I have 2 last comments:
|
…tainers doc string with containers doc string in PrometheusSpec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 thanks a lot! 🎉
@zsuzhengdu it took longer than expected but finally it's merged! Thanks a lot for your patience 🙇 |
Fix #3061
Depends on a new Thanos release that includes thanos-io/thanos#3994