fix: remove recordReceivedPrematurely check from reconciliation loop#733
Draft
mikenairn wants to merge 2 commits intoKuadrant:mainfrom
Draft
fix: remove recordReceivedPrematurely check from reconciliation loop#733mikenairn wants to merge 2 commits intoKuadrant:mainfrom
mikenairn wants to merge 2 commits intoKuadrant:mainfrom
Conversation
Resolves Kuadrant#664. The premature reconcile check was an anti-pattern that short-circuited reconciliation mid-loop, silently blocking legitimate status updates including WriteCounter tracking. Reconciliation throttling should be handled at the queue level, not by aborting after a reconcile has started. Also removes the now-unused --valid-for flag, validFor global, QueuedAt status field, and reconcileStart global variable. Signed-off-by: Michael Nairn <mnairn@redhat.com>
ProviderEndpointsDeletion() did not account for ProviderEndpointsRemoved being a progression past the deletion stage. This caused the deletion flow to overwrite the Ready condition back to ProviderEndpointsDeletion while waiting for remote controllers to confirm removal, creating a flip loop between the two states. Signed-off-by: Michael Nairn <mnairn@redhat.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
closes #664.
The premature reconcile check was an anti-pattern that short-circuited reconciliation mid-loop, silently blocking legitimate status updates including WriteCounter tracking. Reconciliation throttling should be handled at the queue level, not by aborting after a reconcile has started.
Also removes the now-unused --valid-for flag, validFor global, QueuedAt status field, and reconcileStart global variable.
fix: prevent deletion status flip when awaiting remote controller
ProviderEndpointsDeletion() did not account for ProviderEndpointsRemoved being a progression past the deletion stage. This caused the deletion flow to overwrite the Ready condition back to ProviderEndpointsDeletion while waiting for remote controllers to confirm removal, creating a flip loop between the two states.
Note: The "recordReceivedPrematurely" check was masking this issue hence the reason it's fixed as part of this PR.
Questions:
Having removed
recordReceivedPrematurelytheAwaitingValidationstatus and validation window generally seems pointless. It appears to have been implemented as a reconcile based count, where it will just go fromready=false, msg=AwaitingValidationin one reconcile toready=true, msg=ProviderSuccessin the next. This now happens almost instantly due to the status update, where previously it relied onrecordReceivedPrematurelypreventing an update of the status until at least x amount of time had passed.It could be updated to be time based, using the
LastTransitionTimeon the condition itself, which would make more sense if we were to keep it, but what is really the point of it? What does anyone gain from it existing?