Skip to content

fix(kubernetes): recover BatchSandbox after Pod restart #1523

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

Open
luppyw wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(kubernetes): recover BatchSandbox after Pod restart #1523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions docs/kubernetes/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,8 @@ For a BatchSandbox with multiple replicas, `Succeed` also does not mean that eve
| `Resuming` | The controller is restoring runtime resources after a pause. |
| `Failed` | The controller detected a sandbox runtime failure. Inspect conditions and Pod events for details. |

When a transient Pod failure clears, the controller returns `Failed` to `Succeed` only if the Pods recorded at failure time are still the same Kubernetes objects (matching UIDs) and are Running and Ready. A replacement Pod does not count as recovery, even if it reuses the same name. Lifecycle failures such as a failed resume remain terminal.

The controller records active conditions with `status: "True"`:

| Condition | Meaning when `True` |
Expand Down
7 changes: 7 additions & 0 deletions kubernetes/apis/sandbox/v1alpha1/batchsandbox_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
runtime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
)

// +kubebuilder:validation:Enum=Pending;Succeed;Pausing;Paused;Resuming;Failed
Expand Down Expand Up @@ -177,6 +178,12 @@ type BatchSandboxStatus struct {
// +optional
Phase BatchSandboxPhase `json:"phase,omitempty"`

// FailedPodUIDs records the Pods whose transient runtime failures caused the
// current Failed phase. The controller uses these UIDs to distinguish an
// in-place recovery from a replacement Pod that reuses the same name.
// +optional
FailedPodUIDs []types.UID `json:"failedPodUIDs,omitempty"`

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep the Helm CRD schema in sync

This status field is added to the Go type and the Kustomize CRD, but the bundled Helm CRD copy at kubernetes/charts/opensandbox-controller/templates/crds/batchsandboxes.yaml still lacks status.failedPodUIDs. In Helm-installed clusters, that unknown status field is pruned by the CRD schema, so the controller cannot persist pod UID provenance and the transient Pod recovery path remains ineffective for Helm users.

AGENTS.md reference: kubernetes/AGENTS.md:L164-L164

Useful? React with 👍 / 👎.


// PauseObservedGeneration is the generation most recently ACKed by the Controller
// when entering pause/resume dispatch logic. Written immediately to prevent reentry (idempotent gating).
// +optional
Expand Down
6 changes: 6 additions & 0 deletions kubernetes/apis/sandbox/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions kubernetes/config/crd/bases/sandbox.opensandbox.io_batchsandboxes.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,18 @@ spec:
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
failedPodUIDs:
description: |-
FailedPodUIDs records the Pods whose transient runtime failures caused the
current Failed phase. The controller uses these UIDs to distinguish an
in-place recovery from a replacement Pod that reuses the same name.
items:
description: |-
UID is a type that holds unique ID values, including UUIDs. Because we
don't ONLY use UUIDs, this is an alias to string. Being a type captures
intent and helps make sure that UIDs and names do not get conflated.
type: string
type: array
observedGeneration:
description: |-
ObservedGeneration is the most recent generation observed for this BatchSandbox. It corresponds to the
Expand Down
170 changes: 170 additions & 0 deletions kubernetes/internal/controller/batchsandbox_pause_resume_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1807,6 +1807,176 @@ func TestBuildRuntimeView_AggregatesPodFailuresInSteadyState(t *testing.T) {
assert.Equal(t, "3/4 observed pods failed; primary reason=ErrImagePull; sample pod=err-image-0", podFailed.Message)
}

func TestBuildRuntimeView_RecoversOnlySameFailedPod(t *testing.T) {
bs := &sandboxv1alpha1.BatchSandbox{
ObjectMeta: metav1.ObjectMeta{
Name: "test-bs",
Namespace: "default",
},
Status: sandboxv1alpha1.BatchSandboxStatus{
Phase: sandboxv1alpha1.BatchSandboxPhasePending,
},
}
failedPod := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test-bs-0",
Namespace: "default",
UID: types.UID("original-pod-uid"),
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{{Name: "main"}},
},
Status: corev1.PodStatus{
Phase: corev1.PodPending,
ContainerStatuses: []corev1.ContainerStatus{{
Name: "main",
State: corev1.ContainerState{
Waiting: &corev1.ContainerStateWaiting{
Reason: "CreateContainerConfigError",
Message: "temporary container creation failure",
},
},
}},
},
}

failedView := buildRuntimeView(bs, []*corev1.Pod{failedPod})
require.Equal(t, sandboxv1alpha1.BatchSandboxPhaseFailed, failedView.status.Phase)
require.Equal(t, []types.UID{failedPod.UID}, failedView.status.FailedPodUIDs)

tests := []struct {
name string
podUID types.UID
mainContainerRunning bool
wantPhase sandboxv1alpha1.BatchSandboxPhase
}{
{
name: "same Pod recovers",
podUID: failedPod.UID,
mainContainerRunning: true,
wantPhase: sandboxv1alpha1.BatchSandboxPhaseSucceed,
},
{
name: "same Pod is Ready but main container is not running",
podUID: failedPod.UID,
mainContainerRunning: false,
wantPhase: sandboxv1alpha1.BatchSandboxPhaseFailed,
},
{
name: "replacement Pod reuses name",
podUID: types.UID("replacement-pod-uid"),
mainContainerRunning: true,
wantPhase: sandboxv1alpha1.BatchSandboxPhaseFailed,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
afterFailure := bs.DeepCopy()
afterFailure.Status = *failedView.status.DeepCopy()
recoveredPod := failedPod.DeepCopy()
recoveredPod.UID = tt.podUID
mainContainerState := corev1.ContainerState{}
if tt.mainContainerRunning {
mainContainerState.Running = &corev1.ContainerStateRunning{}
}
recoveredPod.Status = corev1.PodStatus{
Phase: corev1.PodRunning,
Conditions: []corev1.PodCondition{{
Type: corev1.PodReady,
Status: corev1.ConditionTrue,
}},
ContainerStatuses: []corev1.ContainerStatus{{
Name: "main",
State: mainContainerState,
}},
}

view := buildRuntimeView(afterFailure, []*corev1.Pod{recoveredPod})
assert.Equal(t, tt.wantPhase, view.status.Phase)
if tt.wantPhase == sandboxv1alpha1.BatchSandboxPhaseSucceed {
assert.Empty(t, view.status.FailedPodUIDs)
} else {
assert.Equal(t, []types.UID{failedPod.UID}, view.status.FailedPodUIDs)
}
})
}

t.Run("failure without recorded Pod identity remains terminal", func(t *testing.T) {
afterFailure := bs.DeepCopy()
afterFailure.Status = *failedView.status.DeepCopy()
afterFailure.Status.FailedPodUIDs = nil
recoveredPod := failedPod.DeepCopy()
recoveredPod.Status = corev1.PodStatus{
Phase: corev1.PodRunning,
Conditions: []corev1.PodCondition{{
Type: corev1.PodReady,
Status: corev1.ConditionTrue,
}},
ContainerStatuses: []corev1.ContainerStatus{{
Name: "main",
State: corev1.ContainerState{
Running: &corev1.ContainerStateRunning{},
},
}},
}

view := buildRuntimeView(afterFailure, []*corev1.Pod{recoveredPod})
assert.Equal(t, sandboxv1alpha1.BatchSandboxPhaseFailed, view.status.Phase)
})
}

func TestBuildRuntimeView_DoesNotRecoverResumeFailure(t *testing.T) {
bs := &sandboxv1alpha1.BatchSandbox{
ObjectMeta: metav1.ObjectMeta{Name: "test-bs", Namespace: "default"},
Status: sandboxv1alpha1.BatchSandboxStatus{
Phase: sandboxv1alpha1.BatchSandboxPhaseResuming,
},
}
pod := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test-bs-0",
Namespace: "default",
UID: types.UID("original-pod-uid"),
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{{Name: "main"}},
},
Status: corev1.PodStatus{
Phase: corev1.PodPending,
ContainerStatuses: []corev1.ContainerStatus{{
Name: "main",
State: corev1.ContainerState{
Waiting: &corev1.ContainerStateWaiting{Reason: "ImagePullBackOff"},
},
}},
},
}

failedView := buildRuntimeView(bs, []*corev1.Pod{pod})
require.Equal(t, sandboxv1alpha1.BatchSandboxPhaseFailed, failedView.status.Phase)
require.Empty(t, failedView.status.FailedPodUIDs)

afterFailure := bs.DeepCopy()
afterFailure.Status = *failedView.status.DeepCopy()
pod.Status = corev1.PodStatus{
Phase: corev1.PodRunning,
Conditions: []corev1.PodCondition{{
Type: corev1.PodReady,
Status: corev1.ConditionTrue,
}},
ContainerStatuses: []corev1.ContainerStatus{{
Name: "main",
State: corev1.ContainerState{
Running: &corev1.ContainerStateRunning{},
},
}},
}

view := buildRuntimeView(afterFailure, []*corev1.Pod{pod})
assert.Equal(t, sandboxv1alpha1.BatchSandboxPhaseFailed, view.status.Phase)
}

func TestBuildRuntimeView_AggregatesResumeFailures(t *testing.T) {
bs := &sandboxv1alpha1.BatchSandbox{
ObjectMeta: metav1.ObjectMeta{
Expand Down
42 changes: 41 additions & 1 deletion kubernetes/internal/controller/batchsandbox_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ type podFailureSummary struct {
failed int
primaryReason string
samplePod string
podUIDs []types.UID
}

func summarizePodFailures(pods []*corev1.Pod) (podFailureSummary, bool) {
Expand All @@ -140,6 +141,9 @@ func summarizePodFailures(pods []*corev1.Pod) (podFailureSummary, bool) {
}

summary.failed++
if pod.UID != "" {
summary.podUIDs = append(summary.podUIDs, pod.UID)
}
if _, exists := firstPodByReason[reason]; !exists {
firstPodByReason[reason] = pod.Name
}
Expand Down Expand Up @@ -203,28 +207,64 @@ func applyResumingRuntimePhase(status *sandboxv1alpha1.BatchSandboxStatus, pods
if summary, hasFailures := summarizePodFailures(pods); hasFailures {
setConditionInStatus(status, sandboxv1alpha1.BatchSandboxConditionResumeFailed, sandboxv1alpha1.ConditionTrue, summary.primaryReason, summary.message(true))
setConditionInStatus(status, sandboxv1alpha1.BatchSandboxConditionPodFailed, sandboxv1alpha1.ConditionTrue, summary.primaryReason, summary.message(false))
status.FailedPodUIDs = nil
status.Phase = sandboxv1alpha1.BatchSandboxPhaseFailed
return
}
if status.Ready > 0 {
status.Phase = sandboxv1alpha1.BatchSandboxPhaseSucceed
status.FailedPodUIDs = nil
setConditionInStatus(status, sandboxv1alpha1.BatchSandboxConditionPodFailed, sandboxv1alpha1.ConditionFalse, "", "")
}
}

func failedPodsRecovered(failedPodUIDs []types.UID, pods []*corev1.Pod) bool {
// Without provenance, recovery is unsafe because a replacement Pod may reuse the same name.
if len(failedPodUIDs) == 0 {
return false
}

recoveredUIDs := make(map[types.UID]struct{}, len(failedPodUIDs))
for _, pod := range pods {
if pod.DeletionTimestamp != nil || !utils.IsPodReady(pod) || len(pod.Spec.Containers) == 0 {
continue
}

// The first container is the sandbox's main runtime container by convention.
mainContainerName := pod.Spec.Containers[0].Name
for _, containerStatus := range pod.Status.ContainerStatuses {
if containerStatus.Name == mainContainerName && containerStatus.State.Running != nil {
recoveredUIDs[pod.UID] = struct{}{}
break
}
}
}

for _, uid := range failedPodUIDs {
if _, recovered := recoveredUIDs[uid]; !recovered {
return false
}
}
return true
}

func applySteadyRuntimePhase(batchSbx *sandboxv1alpha1.BatchSandbox, status *sandboxv1alpha1.BatchSandboxStatus, pods []*corev1.Pod) {
if summary, hasFailures := summarizePodFailures(pods); hasFailures {
if batchSbx.Status.Phase != sandboxv1alpha1.BatchSandboxPhaseFailed {
setConditionInStatus(status, sandboxv1alpha1.BatchSandboxConditionPodFailed, sandboxv1alpha1.ConditionTrue, summary.primaryReason, summary.message(false))
status.FailedPodUIDs = append([]types.UID(nil), summary.podUIDs...)
status.Phase = sandboxv1alpha1.BatchSandboxPhaseFailed
}
return
}

if status.Phase == sandboxv1alpha1.BatchSandboxPhaseFailed {
return
if !failedPodsRecovered(status.FailedPodUIDs, pods) {
return
}
}

status.FailedPodUIDs = nil

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clear failedPodUIDs in the merge patch

When a failed pod recovers, setting FailedPodUIDs to nil is not enough to remove the persisted status field because updateStatus marshals the struct into a JSON merge patch and the field is tagged omitempty, so failedPodUIDs is omitted rather than sent as null. After recovery, the API server keeps the old UID list, the desired status keeps comparing unequal to the stored status, and the reconciler will keep trying to patch the same object instead of becoming idempotent.

AGENTS.md reference: kubernetes/AGENTS.md:L166-L166

Useful? React with 👍 / 👎.

setConditionInStatus(status, sandboxv1alpha1.BatchSandboxConditionPodFailed, sandboxv1alpha1.ConditionFalse, "", "")
if status.Ready > 0 {
status.Phase = sandboxv1alpha1.BatchSandboxPhaseSucceed
Expand Down
Loading