Update watchPods() behavior for Patroni role change #3807
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add an additional check to the existing watchPods function to queue an event when an instance Pod is first given the 'master' role. Issue: PGO-190
cbandy
approved these changes
Dec 19, 2023
Comment on lines
+194
to
+197
| // - https://github.com/zalando/patroni/blob/v3.1.1/patroni/ha.py#L296 | ||
| // - https://github.com/zalando/patroni/blob/v3.1.1/patroni/ha.py#L583 | ||
| // - https://github.com/zalando/patroni/blob/v3.1.1/patroni/ha.py#L782 | ||
| // - https://github.com/zalando/patroni/blob/v3.1.1/patroni/ha.py#L1574 |
There was a problem hiding this comment.
👍🏻 Great to see the moments when/why this happens.
Comment on lines
+63
to
+65
| if len(cluster) != 0 && | ||
| !patroni.PodIsPrimary(e.ObjectOld) && | ||
| patroni.PodIsPrimary(e.ObjectNew) { |
There was a problem hiding this comment.
🌱 I suspect that this also covers the case on L39. Something likely transitions to primary when "may have come out of standby."
I've observed stuff like this in the past using something like:
kubectl get pod --watch --output=json --output-watch-events | jq --unbuffered '
{ _ts: now, type } +
(.object.metadata | { name, creationTimestamp }) +
{ status: (.object.metadata.annotations.status // "{}" | fromjson) }
'There was a problem hiding this comment.
Good note. I think it may be useful to look at our watches holistically in the future, but for now I'll plan to leave both in place. I will mention that my testing showed the reconcile behavior remains similar with this change compared to the existing code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist:
Type of Changes:
What is the current behavior (link to any open issues here)?
In certain circumstances, another reconcile is needed after an instance role change, but there is nothing to trigger it.
What is the new behavior (if this is a feature change)?
This change adds an additional check to the existing watchPods function to queue an event when an instance Pod is first given the 'master' role.
Other Information:
Issue: PGO-190