-
Notifications
You must be signed in to change notification settings - Fork 203
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: Resource doesn't re-reconcile after completed update #3468
Conversation
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.
That came out really cleanly.
Hmm, the added test seems to fail? |
Codecov Report
@@ Coverage Diff @@
## main #3468 +/- ##
==========================================
- Coverage 54.31% 54.25% -0.07%
==========================================
Files 1545 1573 +28
Lines 651527 653631 +2104
==========================================
+ Hits 353882 354600 +718
- Misses 240078 241456 +1378
- Partials 57567 57575 +8
|
@@ -65,7 +67,7 @@ func Test_Latest_Reconciled_Generation_Reconciles_AllEvents(t *testing.T) { | |||
old := agentPool.DeepCopy() | |||
agentPool.Spec.OrchestratorVersion = to.Ptr("1.27.1") | |||
|
|||
tc.Patch(old, agentPool) | |||
tc.PatchResourceAndWaitForState(old, agentPool, metav1.ConditionFalse, conditions.ConditionSeverityInfo) |
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.
Did you confirm that doing this was still able to trigger the test on the behavior we wanted (that 2 PUTs happen before the first reconcile is finished)?
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.
Yep, It does take that - generation mismatch path on re-runs.
* Add latest reconciled generation check * Add tests and minor refactor * Add header * Fix test race --------- Co-authored-by: Bevan Arps <bevan.arps@microsoft.com>
Closes #3451
What this PR does / why we need it:
This PR adds a latest-reconciled-generation annotation to the resource every time the reconciler receives a createOrUpdate event and further is checked in
MonitorResourceCreation
. We need this check inMonitorResourceCreation
since when a subsequent update is received, it always has aresume-poller-token
annotation which makes the reconciler think that it's monitoring the resource and the update never happened.The generation check runs once the poller is done or runs into an error, and requeues the resource if generation set by API server does not match with the annotation or else returns successfully.