Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,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
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 @@ -179,6 +180,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"`

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 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confirmed by rendering the chart: the Helm CRD contains pauseObservedGeneration but not failedPodUIDs. With the status schema missing this property, Kubernetes prunes the UID provenance, so the next reconciliation sees no recorded identity and the recovery path remains terminal for Helm-installed clusters. Please sync the chart CRD and, if practical, add a generated-CRD parity check to prevent the Helm copy from drifting again.


// 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

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

Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,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
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,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
219 changes: 218 additions & 1 deletion kubernetes/internal/controller/batchsandbox_pause_resume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1603,6 +1603,27 @@ func TestPersistRuntimeView_PreservesPauseFailedConditionFromLatestStatus(t *tes
assert.True(t, foundPauseFailed, "persistRuntimeView should preserve PauseFailed condition once informer cache catches up")
}

func TestUpdateStatus_ClearsPersistedFailedPodUIDs(t *testing.T) {
bs := &sandboxv1alpha1.BatchSandbox{
ObjectMeta: metav1.ObjectMeta{Name: "test-bs", Namespace: "default"},
Status: sandboxv1alpha1.BatchSandboxStatus{
Phase: sandboxv1alpha1.BatchSandboxPhaseFailed,
FailedPodUIDs: []types.UID{"failed-pod-uid"},
},
}
r := newTestReconciler(bs)

desiredStatus := bs.Status.DeepCopy()
desiredStatus.Phase = sandboxv1alpha1.BatchSandboxPhaseSucceed
desiredStatus.FailedPodUIDs = nil
require.NoError(t, r.updateStatus(context.Background(), bs, desiredStatus))

updated := &sandboxv1alpha1.BatchSandbox{}
require.NoError(t, r.Get(context.Background(), types.NamespacedName{Namespace: bs.Namespace, Name: bs.Name}, updated))
assert.Equal(t, sandboxv1alpha1.BatchSandboxPhaseSucceed, updated.Status.Phase)
assert.Nil(t, updated.Status.FailedPodUIDs, "the merge patch must remove persisted failure provenance")
}

func TestPersistRuntimeView_SkipsStatusUpdateWhenRuntimeStatusUnchanged(t *testing.T) {
transitionTime := metav1.NewTime(time.Now().Add(-5 * time.Minute))
bs := &sandboxv1alpha1.BatchSandbox{
Expand Down Expand Up @@ -1810,6 +1831,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 Expand Up @@ -1898,9 +2089,13 @@ func TestBuildRuntimeView_MarksResumeFailedWhenStaleCacheMissesResumingPhase(t *
}

pods := []*corev1.Pod{{
ObjectMeta: metav1.ObjectMeta{Name: "imgpull-0", Namespace: "default"},
ObjectMeta: metav1.ObjectMeta{Name: "imgpull-0", Namespace: "default", UID: types.UID("resume-pod-uid")},
Spec: corev1.PodSpec{
Containers: []corev1.Container{{Name: "main"}},
},
Status: corev1.PodStatus{
ContainerStatuses: []corev1.ContainerStatus{{
Name: "main",
State: corev1.ContainerState{
Waiting: &corev1.ContainerStateWaiting{
Reason: "ErrImagePull",
Expand All @@ -1924,6 +2119,28 @@ func TestBuildRuntimeView_MarksResumeFailedWhenStaleCacheMissesResumingPhase(t *
require.NotNil(t, resumeFailed, "resume in flight must mark ResumeFailed even when the cached phase lags")
assert.Equal(t, sandboxv1alpha1.ConditionTrue, resumeFailed.Status)
assert.Equal(t, "ErrImagePull", resumeFailed.Reason)
assert.Empty(t, view.status.FailedPodUIDs, "resume failures must not record recoverable Pod provenance")

afterFailure := bs.DeepCopy()
afterFailure.Status = *view.status.DeepCopy()
recoveredPod := pods[0].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{},
},
}},
}

recoveredView := buildRuntimeView(afterFailure, []*corev1.Pod{recoveredPod})
assert.Equal(t, sandboxv1alpha1.BatchSandboxPhaseFailed, recoveredView.status.Phase,
"a resume failure observed through stale cache must remain terminal")
}

func TestBuildRuntimeView_SteadyFailureWithoutResumeInFlightKeepsResumeFailedAbsent(t *testing.T) {
Expand Down
Loading
Loading