Skip to content

refactor(sandbox): extract syncStatusFromPod as struct field with syncReadyCondition flag #672

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.

Sign up for GitHub

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

Merged
furykerry merged 5 commits into from
Jul 21, 2026
Merged

refactor(sandbox): extract syncStatusFromPod as struct field with syncReadyCondition flag #672

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ee8148d
refactor(sandbox): extract syncStatusFromPod as struct field with syn…
zmberg Jul 20, 2026
549be21
api(sandbox): add enum validation for SandboxUpgradePolicyType
zmberg Jul 20, 2026
9e86be6
api(sandboxupdateops): add enum validation for SandboxUpdateOpsStrate…
zmberg Jul 20, 2026
bedb299
fix(sandbox): use DeepCopy for PostUpgrade hook to resolve correct PodIP
zmberg Jul 21, 2026
2043f41
fix: align constructor parameter name to syncReadyCondition
zmberg Jul 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion api/v1alpha1/sandbox_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ const (
type SandboxUpgradePolicy struct {
// Type specifies the upgrade policy type.
// When empty (default), upgrading is disabled.
// Supported values: Recreate.
// Supported values: Recreate, CheckpointRestore.
// +kubebuilder:validation:Enum=Recreate;CheckpointRestore
// +optional
Type SandboxUpgradePolicyType `json:"type,omitempty"`
}
Expand Down
1 change: 1 addition & 0 deletions api/v1alpha1/sandboxupdateops_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ type SandboxUpdateOpsStrategy struct {
// Type specifies the update strategy type.
// When empty, defaults to Recreate.
// Supported values: Recreate, CheckpointRestore.
// +kubebuilder:validation:Enum=Recreate;CheckpointRestore
// +optional
Type SandboxUpdateOpsStrategyType `json:"type,omitempty"`

Expand Down
5 changes: 4 additions & 1 deletion config/crd/bases/agents.kruise.io_sandboxes.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,10 @@ spec:
description: |-
Type specifies the upgrade policy type.
When empty (default), upgrading is disabled.
Supported values: Recreate.
Supported values: Recreate, CheckpointRestore.
enum:
- Recreate
- CheckpointRestore
type: string
type: object
volumeClaimTemplates:
Expand Down
3 changes: 3 additions & 0 deletions config/crd/bases/agents.kruise.io_sandboxupdateops.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ spec:
Type specifies the update strategy type.
When empty, defaults to Recreate.
Supported values: Recreate, CheckpointRestore.
enum:
- Recreate
- CheckpointRestore
type: string
type: object
required:
Expand Down
18 changes: 12 additions & 6 deletions pkg/controller/sandbox/core/common_control.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ type commonControl struct {
initializer SandboxInitializer
recycleControl *SandboxRecycleControl
upgradeControl *UpgradeControl
syncStatusFromPod func(pod *corev1.Pod, newStatus *agentsv1alpha1.SandboxStatus, syncReadyCondition bool)
}

func NewCommonControl(args SandboxControlArgs) SandboxControl {
Expand All @@ -86,8 +87,9 @@ func NewCommonControl(args SandboxControlArgs) SandboxControl {
lifecycleHookFunc: ExecuteLifecycleHook,
initializer: initializer,
recycleControl: NewSandboxRecycleControl(args.Client, args.Recorder, args.RecycleConfig),
upgradeControl: NewUpgradeControl(args.Client, args.CheckpointControl, args.PodControl, ExecuteLifecycleHook, initializer),
syncStatusFromPod: defaultSyncStatusFromPod,
}
control.upgradeControl = NewUpgradeControl(args.Client, args.CheckpointControl, args.PodControl, ExecuteLifecycleHook, initializer, control.syncStatusFromPod)
return control
}

Expand All @@ -109,7 +111,7 @@ func (r *commonControl) EnsureSandboxRunning(ctx context.Context, args EnsureFun
// pod status running
if pod.Status.Phase == corev1.PodRunning {
newStatus.Phase = agentsv1alpha1.SandboxRunning
syncSandboxStatusFromPod(pod, newStatus)
r.syncStatusFromPod(pod, newStatus, true)
return 0, nil
}

Expand Down Expand Up @@ -151,20 +153,24 @@ func (r *commonControl) EnsureSandboxUpdated(ctx context.Context, args EnsureFun
return nil
}
}
syncSandboxStatusFromPod(pod, newStatus)
r.syncStatusFromPod(pod, newStatus, true)
return nil
}

// syncSandboxStatusFromPod updates sandbox status from pod info and syncs the Ready condition
// with container startup failure detection.
func syncSandboxStatusFromPod(pod *corev1.Pod, newStatus *agentsv1alpha1.SandboxStatus) {
// defaultSyncStatusFromPod is the default implementation of syncStatusFromPod.
// It syncs sandbox status from pod info and, when syncReadyCondition is true, also
// syncs the Ready condition and detects container startup failures.
func defaultSyncStatusFromPod(pod *corev1.Pod, newStatus *agentsv1alpha1.SandboxStatus, syncReadyCondition bool) {
newStatus.NodeName = pod.Spec.NodeName
newStatus.SandboxIp = pod.Status.PodIP
newStatus.PodInfo = agentsv1alpha1.PodInfo{
PodIP: pod.Status.PodIP,
NodeName: pod.Spec.NodeName,
PodUID: pod.UID,
}
if !syncReadyCondition {
return
}
pCond := utils.GetPodCondition(&pod.Status, corev1.PodReady)
cond := utils.GetSandboxCondition(newStatus, string(agentsv1alpha1.SandboxConditionReady))
if cond == nil {
Expand Down
18 changes: 14 additions & 4 deletions pkg/controller/sandbox/core/common_control_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ func TestCommonControl_EnsureSandboxRunning(t *testing.T) {
inplaceUpdateControl: inplaceupdate.NewInPlaceUpdateControl(fakeClient, inplaceupdate.DefaultGeneratePatchBodyFunc),
rateLimiter: rl,
podControl: NewPodControl(fakeClient, record.NewFakeRecorder(10), GeneratePodFromSandbox),
syncStatusFromPod: defaultSyncStatusFromPod,
}

requeue, err := control.EnsureSandboxRunning(context.TODO(), tt.args)
Expand Down Expand Up @@ -450,6 +451,7 @@ func TestCommonControl_EnsureSandboxUpdated(t *testing.T) {
recorder: record.NewFakeRecorder(10),
inplaceUpdateControl: inplaceupdate.NewInPlaceUpdateControl(fc, inplaceupdate.DefaultGeneratePatchBodyFunc),
podControl: NewPodControl(fc, record.NewFakeRecorder(10), GeneratePodFromSandbox),
syncStatusFromPod: defaultSyncStatusFromPod,
}

err := control.EnsureSandboxUpdated(context.TODO(), tt.args)
Expand Down Expand Up @@ -644,6 +646,7 @@ func TestCommonControl_EnsureSandboxUpdated_InitializePath(t *testing.T) {
inplaceUpdateControl: inplaceupdate.NewInPlaceUpdateControl(fc, inplaceupdate.DefaultGeneratePatchBodyFunc),
initializer: tt.initializer,
podControl: NewPodControl(fc, record.NewFakeRecorder(10), GeneratePodFromSandbox),
syncStatusFromPod: defaultSyncStatusFromPod,
}

newStatus := &agentsv1alpha1.SandboxStatus{
Expand Down Expand Up @@ -796,6 +799,7 @@ func TestCommonControl_EnsureSandboxPaused(t *testing.T) {
recorder: record.NewFakeRecorder(10),
inplaceUpdateControl: inplaceupdate.NewInPlaceUpdateControl(fc, inplaceupdate.DefaultGeneratePatchBodyFunc),
podControl: NewPodControl(fc, record.NewFakeRecorder(10), GeneratePodFromSandbox),
syncStatusFromPod: defaultSyncStatusFromPod,
}

err := control.EnsureSandboxPaused(context.TODO(), tt.args)
Expand Down Expand Up @@ -1335,6 +1339,7 @@ func TestCommonControl_EnsureSandboxTerminated(t *testing.T) {
recorder: record.NewFakeRecorder(10),
inplaceUpdateControl: inplaceupdate.NewInPlaceUpdateControl(fc, inplaceupdate.DefaultGeneratePatchBodyFunc),
podControl: NewPodControl(fc, record.NewFakeRecorder(10), GeneratePodFromSandbox),
syncStatusFromPod: defaultSyncStatusFromPod,
}

err := control.EnsureSandboxTerminated(context.TODO(), tt.args)
Expand Down Expand Up @@ -2214,6 +2219,7 @@ func TestCommonControl_EnsureSandboxUpdated_InplaceNotDone(t *testing.T) {
recorder: record.NewFakeRecorder(10),
inplaceUpdateControl: inplaceupdate.NewInPlaceUpdateControl(fakeClient, inplaceupdate.DefaultGeneratePatchBodyFunc),
podControl: NewPodControl(fakeClient, record.NewFakeRecorder(10), GeneratePodFromSandbox),
syncStatusFromPod: defaultSyncStatusFromPod,
}

newStatus := &agentsv1alpha1.SandboxStatus{
Expand Down Expand Up @@ -2261,6 +2267,7 @@ func TestCommonControl_EnsureSandboxResumed_TerminatingPod(t *testing.T) {
recorder: record.NewFakeRecorder(10),
inplaceUpdateControl: inplaceupdate.NewInPlaceUpdateControl(fakeClient, inplaceupdate.DefaultGeneratePatchBodyFunc),
podControl: NewPodControl(fakeClient, record.NewFakeRecorder(10), GeneratePodFromSandbox),
syncStatusFromPod: defaultSyncStatusFromPod,
}

newStatus := &agentsv1alpha1.SandboxStatus{
Expand Down Expand Up @@ -2302,6 +2309,7 @@ func TestCommonControl_EnsureSandboxResumed_SetResumedCondition(t *testing.T) {
recorder: record.NewFakeRecorder(10),
inplaceUpdateControl: inplaceupdate.NewInPlaceUpdateControl(fakeClient, inplaceupdate.DefaultGeneratePatchBodyFunc),
podControl: NewPodControl(fakeClient, record.NewFakeRecorder(10), GeneratePodFromSandbox),
syncStatusFromPod: defaultSyncStatusFromPod,
}

newStatus := &agentsv1alpha1.SandboxStatus{
Expand Down Expand Up @@ -2446,6 +2454,7 @@ func TestCommonControl_EnsureSandboxTerminated_PodNotExist_NoFinalizer(t *testin
recorder: record.NewFakeRecorder(10),
inplaceUpdateControl: inplaceupdate.NewInPlaceUpdateControl(fakeClient, inplaceupdate.DefaultGeneratePatchBodyFunc),
podControl: NewPodControl(fakeClient, record.NewFakeRecorder(10), GeneratePodFromSandbox),
syncStatusFromPod: defaultSyncStatusFromPod,
}

err := control.EnsureSandboxTerminated(context.TODO(), EnsureFuncArgs{Pod: nil, Box: box, NewStatus: &agentsv1alpha1.SandboxStatus{}})
Expand Down Expand Up @@ -2473,6 +2482,7 @@ func TestCommonControl_EnsureSandboxPaused_AlreadyPaused(t *testing.T) {
recorder: record.NewFakeRecorder(10),
inplaceUpdateControl: inplaceupdate.NewInPlaceUpdateControl(fakeClient, inplaceupdate.DefaultGeneratePatchBodyFunc),
podControl: NewPodControl(fakeClient, record.NewFakeRecorder(10), GeneratePodFromSandbox),
syncStatusFromPod: defaultSyncStatusFromPod,
}

newStatus := &agentsv1alpha1.SandboxStatus{
Expand Down Expand Up @@ -2543,7 +2553,7 @@ func TestCommonControl_performRecreateUpgrade_PodTerminating(t *testing.T) {
podControl: podCtrl,
checkpointControl: checkpointCtrl,
initializer: initializer,
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, ExecuteLifecycleHook, initializer),
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, ExecuteLifecycleHook, initializer, defaultSyncStatusFromPod),
}

newStatus := &agentsv1alpha1.SandboxStatus{
Expand Down Expand Up @@ -2604,7 +2614,7 @@ func TestCommonControl_performRecreateUpgrade_NewPodNotReady(t *testing.T) {
podControl: podCtrl,
checkpointControl: checkpointCtrl,
initializer: initializer,
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, ExecuteLifecycleHook, initializer),
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, ExecuteLifecycleHook, initializer, defaultSyncStatusFromPod),
}

newStatus := &agentsv1alpha1.SandboxStatus{
Expand Down Expand Up @@ -2849,7 +2859,7 @@ func TestCommonControl_performRecreateUpgrade_PodReadyFalse(t *testing.T) {
podControl: podCtrl,
checkpointControl: checkpointCtrl,
initializer: initializer,
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, ExecuteLifecycleHook, initializer),
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, ExecuteLifecycleHook, initializer, defaultSyncStatusFromPod),
}

newStatus := &agentsv1alpha1.SandboxStatus{
Expand Down Expand Up @@ -3292,7 +3302,7 @@ func TestCommonControl_performRecreateUpgrade_InitializerPath(t *testing.T) {
podControl: podCtrl,
checkpointControl: checkpointCtrl,
initializer: initializer,
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, ExecuteLifecycleHook, initializer),
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, ExecuteLifecycleHook, initializer, defaultSyncStatusFromPod),
}

newStatus := &agentsv1alpha1.SandboxStatus{
Expand Down
15 changes: 5 additions & 10 deletions pkg/controller/sandbox/core/upgrade_control.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type UpgradeControl struct {
podControl *PodControl
lifecycleHookFunc LifecycleHookFunc
initializer SandboxInitializer
syncStatusFromPod func(pod *corev1.Pod, newStatus *agentsv1alpha1.SandboxStatus, syncReadyCondition bool)
}

// NewUpgradeControl creates a new UpgradeControl.
Expand All @@ -54,13 +55,15 @@ func NewUpgradeControl(
podControl *PodControl,
lifecycleHookFunc LifecycleHookFunc,
initializer SandboxInitializer,
syncStatusFromPod func(pod *corev1.Pod, newStatus *agentsv1alpha1.SandboxStatus, syncCondition bool),

@furykerry furykerry Jul 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

) *UpgradeControl {
return &UpgradeControl{
Client: cli,
checkpointControl: checkpointControl,
podControl: podControl,
lifecycleHookFunc: lifecycleHookFunc,
initializer: initializer,
syncStatusFromPod: syncStatusFromPod,
}
}

Expand Down Expand Up @@ -269,16 +272,6 @@ func (r *UpgradeControl) performRecreateUpgrade(ctx context.Context, args Ensure
return false, nil
}

if pod.Status.PodIP != "" && pod.Spec.NodeName != "" {
newStatus.NodeName = pod.Spec.NodeName
newStatus.SandboxIp = pod.Status.PodIP
newStatus.PodInfo = agentsv1alpha1.PodInfo{
PodIP: pod.Status.PodIP,
NodeName: pod.Spec.NodeName,
PodUID: pod.UID,
}
}

// Step 3: Wait for new Pod to be running and ready
pCond := utils.GetPodCondition(&pod.Status, corev1.PodReady)
cond := utils.GetSandboxCondition(newStatus, string(agentsv1alpha1.SandboxConditionUpgrading))
Expand Down Expand Up @@ -310,6 +303,8 @@ func (r *UpgradeControl) performRecreateUpgrade(ctx context.Context, args Ensure
return false, nil
}

r.syncStatusFromPod(pod, newStatus, false)

// Step 4: Perform post-recreate-upgrade initialization (re-init runtime, re-mount CSI).
if err := r.initializer.Initialize(ctx, box, newStatus); err != nil {
return false, err
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/sandbox/core/upgrade_control_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func newTestCommonControl(hookFunc LifecycleHookFunc, objects ...client.Object)
podControl: podCtrl,
lifecycleHookFunc: hookFunc,
initializer: initializer,
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, hookFunc, initializer),
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, hookFunc, initializer, defaultSyncStatusFromPod),
}
}

Expand Down Expand Up @@ -841,7 +841,7 @@ func newTestCommonControlWithCheckpointIndex(hookFunc LifecycleHookFunc, objects
podControl: podCtrl,
lifecycleHookFunc: hookFunc,
initializer: initializer,
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, hookFunc, initializer),
upgradeControl: NewUpgradeControl(fakeClient, checkpointCtrl, podCtrl, hookFunc, initializer, defaultSyncStatusFromPod),
}
}

Expand Down
Loading