Skip to content

Preserve CRP scheduled status across non-scheduling spec updates#730

Draft
Simon Waight (sjwaight) with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-cluster-resource-placement-scheduled
Draft

Preserve CRP scheduled status across non-scheduling spec updates#730
Simon Waight (sjwaight) with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-cluster-resource-placement-scheduled

Conversation

Copilot AI commented May 29, 2026

Copy link
Copy Markdown
Contributor

Updating CRPs at scale with spec changes that do not affect scheduling could reset ClusterResourcePlacementScheduled to Unknown, which in turn drove the placement controller into the backup requeue path and inflated workqueue depth. This was most visible on bulk updates such as rollout-strategy changes across thousands of CRPs.

  • Root cause

    • The placement controller rewrote the existing policy snapshot's CRPGenerationAnnotation even when the scheduling policy itself was unchanged.
    • That made the snapshot look stale to buildScheduledCondition(), which reset ClusterResourcePlacementScheduled to SchedulePending / Unknown.
    • Once that happened, reconciliation fell into the "Scheduler has not scheduled any cluster yet and requeue the request as a backup" path.
  • What changed

    • Stop mutating the latest scheduling policy snapshot solely because the CRP generation changed.
    • Stop treating ObservedCRPGeneration < placement.Generation as sufficient reason to reset the CRP scheduled condition to Unknown.
    • Preserve the last scheduler-produced scheduled result for same-policy updates, while still allowing real scheduling-input changes to invalidate and refresh scheduling state.
  • Behavioral impact

    • Non-scheduling CRP updates no longer flip ClusterResourcePlacementScheduled back to Unknown.
    • Existing scheduling decisions remain stable until the scheduler actually needs to recompute them.
    • The backup requeue path is no longer triggered just because a CRP generation changed.
  • Targeted coverage

    • Added coverage for same-policy generation bumps leaving the policy snapshot unchanged.
    • Added coverage for preserving the last known scheduled condition when scheduling inputs did not change.
    • Updated edge-case coverage so older snapshots missing CRPGenerationAnnotation are tolerated in this path.
func buildScheduledCondition(placementObj fleetv1beta1.PlacementObj, latestSchedulingPolicySnapshot fleetv1beta1.PolicySnapshotObj) metav1.Condition {
	scheduledCondition := latestSchedulingPolicySnapshot.GetCondition(string(fleetv1beta1.PolicySnapshotScheduled))

	if scheduledCondition == nil ||
		scheduledCondition.ObservedGeneration < latestSchedulingPolicySnapshot.GetGeneration() ||
		scheduledCondition.Status == metav1.ConditionUnknown {
		return metav1.Condition{
			Status:             metav1.ConditionUnknown,
			Type:               getPlacementScheduledConditionType(placementObj),
			Reason:             condition.SchedulingUnknownReason,
			Message:            "Scheduling has not completed",
			ObservedGeneration: placementObj.GetGeneration(),
		}
	}
	return metav1.Condition{
		Status:             scheduledCondition.Status,
		Type:               getPlacementScheduledConditionType(placementObj),
		Reason:             scheduledCondition.Reason,
		Message:            scheduledCondition.Message,
		ObservedGeneration: placementObj.GetGeneration(),
	}
}

Copilot AI changed the title [WIP] Fix resetting of ClusterResourcePlacementScheduled condition in CRP spec Preserve CRP scheduled status across non-scheduling spec updates May 29, 2026
@sjwaight

Copy link
Copy Markdown
Member

michaelawyu (@michaelawyu) can you please review when you get time.

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.

ClusterResourcePlacementScheduled condition gets reset in CRP spec updates

2 participants