refactor(sandbox): extract syncStatusFromPod as struct field with syncReadyCondition flag#672
Conversation
…cReadyCondition flag - Extract syncStatusFromPod as a func field on commonControl and UpgradeControl to enable pluggable behavior for different control structs - Add syncReadyCondition bool parameter to control whether Ready condition sync is performed (upgrade scenario skips it, using upgrade-specific conditions) - Replace manual pod info sync in performRecreateUpgrade with r.syncStatusFromPod(pod, newStatus, false), called after Pod Ready check - NewCommonControl passes syncStatusFromPod to NewUpgradeControl for consistent behavior injection Signed-off-by: liheng <liheng.zms@alibaba-inc.com>
Add +kubebuilder:validation:Enum=Recreate;CheckpointRestore to the Type field and update the comment to list both supported values. Signed-off-by: liheng <liheng.zms@alibaba-inc.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
…gyType Add +kubebuilder:validation:Enum=Recreate;CheckpointRestore to the Type field of SandboxUpdateOpsStrategy. Signed-off-by: liheng <liheng.zms@alibaba-inc.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #672 +/- ##
==========================================
- Coverage 80.34% 80.33% -0.01%
==========================================
Files 231 231
Lines 18057 18057
==========================================
- Hits 14507 14506 -1
- Misses 2963 2964 +1
Partials 587 587
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| podControl *PodControl, | ||
| lifecycleHookFunc LifecycleHookFunc, | ||
| initializer SandboxInitializer, | ||
| syncStatusFromPod func(pod *corev1.Pod, newStatus *agentsv1alpha1.SandboxStatus, syncCondition bool), |
There was a problem hiding this comment.
Issue: The struct field declares the func type with parameter name syncReadyCondition bool, but the NewUpgradeControl constructor parameter uses syncCondition bool:
Recommendation: Align the constructor parameter name to syncReadyCondition for consistency and documentation clarity
The refactoring in commit ee8148d moved PodInfo sync from before the Pod Ready check to after it. This caused box.Status.PodInfo.PodIP to remain stale when performRecreateUpgrade returns true and falls through to PostUpgrade in the same reconcile cycle. GetRuntimeURL(box) would then connect to an empty/wrong address, causing a 30s timeout. Fix: create a DeepCopy of box with the latest newStatus before passing it to executeUpgradeAction. This preserves the original box for updateSandboxStatus DeepEqual comparison while ensuring the hook receives the correct PodIP. Signed-off-by: liheng <liheng.zms@alibaba-inc.com>
Address review feedback: the struct field declares the func type parameter as syncReadyCondition but the NewUpgradeControl constructor used syncCondition. Align for consistency and documentation clarity. Signed-off-by: liheng <liheng.zms@alibaba-inc.com>
…cReadyCondition flag (openkruise#672)
3 participants
Summary
Refactor from a package-level function into a struct field on both and , enabling pluggable behavior for different control structs.
Key Changes
Test Coverage
Files Changed