-
Notifications
You must be signed in to change notification settings - Fork 411
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
fix successThreshold, validate server protocol, deprecate scheme #792
Conversation
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
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.
Verified on OCP 4.10.x using verification steps, works as expected, thanks @weisdd 👍 !
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
@HubertStefanski @pb82 As discussed over the call, I've brought the |
@NissesSenap thanks for the update. Once your PR is merged, I can update mine :) |
Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
Updated tests, PR is up-to-date. |
Description
PeriodSeconds
which would led to unexpected results. E.g. if someone setsPeriodSeconds
to2
, kube-apiserver will decline the spec, becausesuccessThreshold
forlivenessProbes
cannot be anything but1
(docs).In logs, it would be something like this:
grafana-operator-544bbbbf5b-bw6c6 grafana-operator 2022-07-19T08:27:11.077Z DEBUG controller-runtime.manager.events Warning {"object": {"kind":"Grafana","namespace":"monitoring","name":"grafana-operator-grafana","uid":"e42ba143-2787-41cd-a2f7-6d1f2371235f","apiVersion":"integreatly.org/v1alpha1","resourceVersion":"390657"}, "reason": "ProcessingError", "message": "Deployment.apps \"grafana-deployment\" is invalid: spec.template.spec.containers[0].livenessProbe.successThreshold: Invalid value: 104: must be 1"}
;scheme
is now ignored as it's always tied to theconfig.server.protocol
section of grafana config, thus should not have been configurable in the first place;http
andhttps
).Type of change
scheme
field ofreadinessProbeSpec
andlivenessProbeSpec
is ignored, the actual value depends onconfig.server.protocol
.Checklist
Verification steps
Probes in the resulting deployment should be set to the same values as in the code.
All probes settings in the resulting deployment should match the values from the spec except for
scheme
.scheme
is ignored in favor ofconfig.server.protocol
, and since it's not present in the spec,scheme: HTTP
is set by default.