Skip to content

Commit 2176f60

Browse files
haoqing0110claude
andcommitted
fix: Check Applied condition before evaluating rollout status
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Qing Hao <qhao@redhat.com>
1 parent 8f8cd01 commit 2176f60

3 files changed

Lines changed: 555 additions & 32 deletions

File tree

pkg/work/hub/controllers/manifestworkreplicasetcontroller/manifestworkreplicaset_deploy_reconcile.go

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -218,15 +218,10 @@ func (d *deployReconciler) clusterRolloutStatusFunc(clusterName string, manifest
218218
// Get all relevant conditions
219219
progressingCond := apimeta.FindStatusCondition(manifestWork.Status.Conditions, workv1.WorkProgressing)
220220
degradedCond := apimeta.FindStatusCondition(manifestWork.Status.Conditions, workv1.WorkDegraded)
221+
appliedCond := apimeta.FindStatusCondition(manifestWork.Status.Conditions, workv1.WorkApplied)
221222

222-
// Return ToApply if:
223-
// - No Progressing condition exists yet (work hasn't been reconciled by agent)
224-
// - Progressing condition hasn't observed the latest spec
225-
// - Degraded condition exists but hasn't observed the latest spec
226-
// (Degraded is optional, but if it exists, we wait for it to catch up)
227-
if progressingCond == nil ||
228-
progressingCond.ObservedGeneration != manifestWork.Generation ||
229-
(degradedCond != nil && degradedCond.ObservedGeneration != manifestWork.Generation) {
223+
// Check if the work should be in ToApply status
224+
if shouldReturnToApply(manifestWork.Generation, appliedCond, progressingCond, degradedCond) {
230225
return clsRolloutStatus, nil
231226
}
232227

@@ -262,6 +257,60 @@ func (d *deployReconciler) clusterRolloutStatusFunc(clusterName string, manifest
262257
return clsRolloutStatus, nil
263258
}
264259

260+
// shouldReturnToApply determines if the ManifestWork should be in ToApply status
261+
// based on the state of its conditions.
262+
//
263+
// Returns true if:
264+
// - The Applied condition is not ready (missing, hasn't observed latest spec, or not True)
265+
// - The Progressing condition is not ready (missing or hasn't observed latest spec)
266+
// - The Degraded condition exists but hasn't observed the latest spec
267+
//
268+
// IMPORTANT: Applied condition is checked FIRST to ensure the work has been properly applied
269+
// by the spoke agent before checking other agent-side conditions (Progressing/Degraded).
270+
// This prevents using stale timestamps from previous generations when conditions update
271+
// their ObservedGeneration without changing Status.
272+
func shouldReturnToApply(generation int64, appliedCond, progressingCond, degradedCond *metav1.Condition) bool {
273+
// Check Applied condition first - work must be applied by spoke agent
274+
if !isConditionReady(appliedCond, generation, true) {
275+
return true
276+
}
277+
278+
// Check Progressing condition - work must be reconciled by spoke agent
279+
if !isConditionReady(progressingCond, generation, false) {
280+
return true
281+
}
282+
283+
// Check Degraded condition if it exists - it must have observed the latest spec
284+
// Degraded is optional, but if it exists, we wait for it to catch up to avoid
285+
// using stale status information
286+
if degradedCond != nil && degradedCond.ObservedGeneration != generation {
287+
return true
288+
}
289+
290+
return false
291+
}
292+
293+
// isConditionReady checks if a condition is ready for rollout status evaluation.
294+
// A condition is ready if:
295+
// - It exists (not nil)
296+
// - It has observed the latest generation
297+
// - If requireTrue is set, it must also have Status=True
298+
func isConditionReady(cond *metav1.Condition, generation int64, requireTrue bool) bool {
299+
if cond == nil {
300+
return false
301+
}
302+
303+
if cond.ObservedGeneration != generation {
304+
return false
305+
}
306+
307+
if requireTrue && cond.Status != metav1.ConditionTrue {
308+
return false
309+
}
310+
311+
return true
312+
}
313+
265314
// GetManifestworkApplied return only True status if there all clusters have manifests applied as expected
266315
func GetManifestworkApplied(reason string, message string) metav1.Condition {
267316
if reason == workapiv1alpha1.ReasonAsExpected {

0 commit comments

Comments
 (0)