Skip to content

add toggle to turn off readiness probes #2004

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

Merged
merged 3 commits into from
Oct 5, 2022
Merged

Conversation

FxKu
Copy link
Member

@FxKu FxKu commented Aug 22, 2022

Addresses: #2003, #1978

Having statefulsets with readiness probes was requested a couple of times in the past. We merged it with #1825. Now they are enabled by default, which turns out to be not the greatest idea when pod_management_policy is set to ordered_ready. What could happen?

Imagine a cluster with pod-0 and pod-1 where the first instance (replica) is broken (e.g. missing a WAL file to restore, disk full etc.) but the master is doing fine. Now the master pod must be replaced (maintenance or it dies for whatever reason). The first pod is failing the readiness probe blocking the second pod from coming back and restore the former master.

Without the readiness probe, it's enough for K8s to see that the first pod is in Running state ignoring the actual state of the cluster member. This helped us in the past that masters could repair themselves.

Changing the pod_management_policy is not as easy as it seems, because the operator does not compare it on sync. This PR will include the policy as well as the readiness probe of the container into the stateful set comparison to trigger a replacement of the stateful set (and rolling update of pods if the probe changes).

With ordered_ready being the default policy, the readiness probe should be disabled by default although this is a breaking change to v1.8. Therefore, a new toggle is added to keep the existing behavior.

@FxKu FxKu added this to the 1.9 milestone Aug 22, 2022
@FxKu
Copy link
Member Author

FxKu commented Sep 21, 2022

👍

@OlleLarsson
Copy link
Contributor

Any updates regarding this @FxKu?

@idanovinda
Copy link
Member

idanovinda commented Oct 5, 2022

👍

1 similar comment
@FxKu
Copy link
Member Author

FxKu commented Oct 5, 2022

👍

@FxKu FxKu merged commit a119772 into master Oct 5, 2022
@FxKu FxKu deleted the readiness-probe-toggle branch October 5, 2022 16:25
@zlcnju
Copy link

zlcnju commented Jul 16, 2023

why not use parallel as the default policy?

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.

4 participants