-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[feat] Add prometheus config reloader livenessProbe & readinessProbe #5449
[feat] Add prometheus config reloader livenessProbe & readinessProbe #5449
Conversation
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.
Out of curiosity, do you any concrete use case for adding liveness/readiness probes to the config reloader? In other words, did you face situations where probes would have helped?
One problem I can see with the current approach is that the next operator's version requires to use a config reloader image with the new health endpoint. This would make upgrades more complicated than needed IMHO.
To smooth the upgrade, we can introduce a new operator's flag to enable liveness/readiness probes for the config reloader (e.g. --enable-config-reloader-probes
).
THKS for U review! The Open Policy Agent is installed in the online kubernetes cluster. All containers in the pod must have livenessProbe or ReadinessProbe. like:
add |
Ack, I think that being an opt-in feature is the best approach because from past experience, liveness probes can create accidental problems (e.g. probe timeouts triggering container restarts). |
I agree! It is very necessary to add |
Thank you @dongjiang1989 , Looks good to me. Will defer to @simonpasquier 's to review and approve |
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
@simonpasquier Please review and approve |
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.
from my side lgtm
Sure! this is clean code. Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Description
#4730
#3587
#5294
Type of change
What type of changes does your code introduce to the Prometheus operator? Put an
x
in the box that apply.CHANGE
(fix or feature that would cause existing functionality to not work as expected)FEATURE
(non-breaking change which adds functionality)BUGFIX
(non-breaking change which fixes an issue)ENHANCEMENT
(non-breaking change which improves existing functionality)NONE
(if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)Changelog entry
Please put a one-line changelog entry below. This will be copied to the changelog file during the release process.