-
Notifications
You must be signed in to change notification settings - Fork 4.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
Prometheus config reload livenessprobe #18391
Prometheus config reload livenessprobe #18391
Conversation
@aweiteka: GitHub didn't allow me to request PR reviews from the following users: simonpasquier. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
examples/prometheus/prometheus.yaml
Outdated
exec: | ||
command: | ||
- /bin/sh | ||
- -i |
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.
Our convention for this is:
command:
- /bin/bash
- -c
args:
- |
#!/bin/bash
set -euo pipefail
....
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.
It appears the exec liveness probe only supports 'command', not 'args'.
examples/prometheus/prometheus.yaml
Outdated
failureThreshold: 3 | ||
initialDelaySeconds: 60 | ||
periodSeconds: 60 | ||
successThreshold: 1 |
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.
Remove any of these which are defaults.
You have to call hack/update-generated-bindata.sh when you change these configs. |
@aweiteka could you please PR here https://github.com/openshift/openshift-ansible/blob/master/roles/openshift_prometheus/templates/prometheus.j2#L83 once this PR gets merged to keep the installation aligned with this template? |
@zgalor yes, that is the plan. We want to keep these in sync. My understanding is we try stuff out in examples, port to openshift-ansible. Not clear if that's the best approach. |
FYI, log output of prometheus container:
|
7aad1a2
to
ef682b5
Compare
Signed-off-by: Aaron Weitekamp <aweiteka@redhat.com>
ef682b5
to
058c982
Compare
@spadgett can you take a look? This is the formatting I found I needed to make this work. Tough to debug. :( |
I don't know of another way :/ |
/retest |
@smarterclayton ready for review/merge |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aweiteka, pgier, smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Automatic merge from submit-queue (batch tested with PRs 18780, 18802, 18391, 18832, 18808). |
Use case: config updated via configmap. When the change lands in the container (~60 seconds delay) the config directory hash changes and the process is killed. This does not kill the pod but results in a silent reload of config with a corresponding metric timestamp,
prometheus_config_last_reload_success_timestamp_seconds
.Signed-off-by: Aaron Weitekamp aweiteka@redhat.com