diff --git a/changelog/unreleased/934-statefulset-rollout-recovery.md b/changelog/unreleased/934-statefulset-rollout-recovery.md new file mode 100644 index 000000000..a38d26a95 --- /dev/null +++ b/changelog/unreleased/934-statefulset-rollout-recovery.md @@ -0,0 +1,3 @@ +# Recover a failed server after a corrected rollout + +The operator can replace an obsolete game pod stuck during startup once the StatefulSet has observed a newer revision. Recovery respects lifecycle operations and PodDisruptionBudgets. diff --git a/charts/gameplane/templates/operator.yaml b/charts/gameplane/templates/operator.yaml index 8912750b2..899513986 100644 --- a/charts/gameplane/templates/operator.yaml +++ b/charts/gameplane/templates/operator.yaml @@ -168,6 +168,10 @@ rules: - apiGroups: [""] resources: [pods/attach] verbs: [create] + # Respect disruption budgets when replacing an obsolete failed game pod. + - apiGroups: [""] + resources: ["pods/eviction"] + verbs: ["create"] # Network capture sidecar injection via ephemeral containers (reconcileCapture # in gameserver_controller.go). Namespace-scoped to this managed namespace. - apiGroups: [""] diff --git a/operator/config/rbac/role_namespace.yaml b/operator/config/rbac/role_namespace.yaml index 82416ea3b..a37b46aeb 100644 --- a/operator/config/rbac/role_namespace.yaml +++ b/operator/config/rbac/role_namespace.yaml @@ -66,6 +66,11 @@ rules: - apiGroups: [""] resources: ["pods/attach"] verbs: ["create"] + # Respect disruption budgets when replacing an obsolete failed game pod. + - apiGroups: [""] + resources: ["pods/eviction"] + verbs: ["create"] + # Network capture sidecar injection (reconcileCapture); mirrors the # chart's manage Role. Namespace-scoped only: a cluster-wide grant would # let the operator inject a container into any pod in any namespace. diff --git a/operator/internal/controller/gameserver_controller.go b/operator/internal/controller/gameserver_controller.go index a873b9354..5efcd34b8 100644 --- a/operator/internal/controller/gameserver_controller.go +++ b/operator/internal/controller/gameserver_controller.go @@ -277,6 +277,15 @@ func (r *GameServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) if err := r.Get(ctx, req.NamespacedName, &gs); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } + // Recover the durable eviction decision before any validation that can + // return early (including a missing template or newly requested wipe). + if op, held, err := lifecycleOwner(&gs); held && err == nil && op.Kind == podRecoveryKind { + delay, err := r.resumeFailedPodEviction(ctx, &gs, op.lifecycleIdentity) + if delay == 0 { + delay = reconcileRetryInterval + } + return requeueOnConflict(ctrl.Result{RequeueAfter: delay}, err) + } // Refresh and recover wipe ownership before deriving config or idle state. // A later refresh would mix an older idle calculation with a newer status // patch base and could overwrite a concurrent idle-clock update. @@ -483,6 +492,15 @@ func (r *GameServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) logger.Error(err, "reconcile StatefulSet") return ctrl.Result{}, err } + if replicas == 1 && stopRequeue == 0 { + recoveryRetry, err := r.recoverObsoleteFailedPod(ctx, &gs) + if err != nil { + return ctrl.Result{}, err + } + if recoveryRetry > 0 { + return ctrl.Result{RequeueAfter: recoveryRetry}, nil + } + } // Node placement is a cosmetic annotation for the dashboard; a transient // pod-get hiccup shouldn't stall the rest of reconciliation, so log and go. if err := r.reconcileNodePlacement(ctx, &gs); err != nil { diff --git a/operator/internal/controller/gameserver_rollout_recovery.go b/operator/internal/controller/gameserver_rollout_recovery.go new file mode 100644 index 000000000..2e9b9ad8a --- /dev/null +++ b/operator/internal/controller/gameserver_rollout_recovery.go @@ -0,0 +1,151 @@ +package controller + +import ( + "context" + "errors" + "time" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + gp "github.com/GameplanePanel/gameplane/operator/api/v1alpha1" +) + +const ( + podRecoveryKind = "PodRecovery" + evictionRetry = 5 * time.Second + evictionRetryAnnotation = "operations.gameplane.local/eviction-retry-after" +) + +// OrderedReady can wait indefinitely for a failed ordinal after a correction. +// Admit eviction through the same GameServer resourceVersion CAS as backups +// and restores. The owner identifies the exact old pod across leader changes. +func (r *GameServerReconciler) recoverObsoleteFailedPod(ctx context.Context, gs *gp.GameServer) (time.Duration, error) { + if _, held, err := lifecycleOwner(gs); held || err != nil || !recoveryPowerAllows(gs) { + return 0, err + } + if retry, err := time.Parse(time.RFC3339Nano, gs.Annotations[evictionRetryAnnotation]); err == nil { + if wait := time.Until(retry); wait > 0 && wait <= evictionRetry { + return wait, nil + } + } + var pod corev1.Pod + if err := r.Get(ctx, client.ObjectKey{Namespace: gs.Namespace, Name: gs.Name + "-0"}, &pod); err != nil { + return 0, client.IgnoreNotFound(err) + } + if ok, err := r.failedOrdinal(ctx, gs, &pod); err != nil || !ok { + return 0, err + } + want := lifecycleIdentity{Kind: podRecoveryKind, Name: gs.Name, UID: gs.UID, TargetUID: gs.UID, Generation: gs.Generation, Token: string(pod.UID)} + claimed, err := claimLifecycleOperation(ctx, r.Client, r.wipeReader(), gs.DeepCopy(), want, r.OperationQuiescer) + if err != nil || !claimed { + return 0, err + } + return r.resumeFailedPodEviction(ctx, gs, want) +} + +func recoveryPowerAllows(gs *gp.GameServer) bool { + return gs.DeletionTimestamp.IsZero() && !gs.Spec.Suspend && gs.Annotations[stopRequestedAtAnnotation] == "" && + gs.Annotations[IdleAsleepSinceAnnotation] == "" && !pendingWipe(gs) && !tokenPending(gs, RestartRequestedAnnotation, RestartCompletedAnnotation) +} + +func (r *GameServerReconciler) failedOrdinal(ctx context.Context, gs *gp.GameServer, pod *corev1.Pod) (bool, error) { + rev := pod.Labels[appsv1.StatefulSetRevisionLabel] + if rev == "" || pod.UID == "" || pod.ResourceVersion == "" || !pod.DeletionTimestamp.IsZero() || podReady(pod) { + return false, nil + } + if _, _, failed := startupFailure(pod); !failed { + return false, nil + } + var ss appsv1.StatefulSet + if err := r.wipeReader().Get(ctx, client.ObjectKeyFromObject(gs), &ss); err != nil { + return false, client.IgnoreNotFound(err) + } + if ss.UID == "" || !metav1.IsControlledBy(pod, &ss) || !metav1.IsControlledBy(&ss, gs) || !ss.DeletionTimestamp.IsZero() || + ss.Spec.Replicas == nil || *ss.Spec.Replicas != 1 || ss.Generation < 1 || ss.Status.ObservedGeneration != ss.Generation || + ss.Status.UpdateRevision == "" || ss.Status.UpdateRevision == rev { + return false, nil + } + strategy := ss.Spec.UpdateStrategy + if strategy.Type != appsv1.RollingUpdateStatefulSetStrategyType || (strategy.RollingUpdate != nil && strategy.RollingUpdate.Partition != nil && *strategy.RollingUpdate.Partition > 0) || + (ss.Spec.Ordinals != nil && ss.Spec.Ordinals.Start != 0) { + return false, nil + } + if value := ss.Annotations[lifecycleOperationAnnotation]; value != "" && value != gs.Annotations[lifecycleOperationAnnotation] { + return false, nil + } + for _, key := range []string{backupGuardAnnotation, restoreGuardAnnotation, wipeGuardAnnotation, hostportStopFenceAnnotation} { + if ss.Annotations[key] != "" { + return false, nil + } + } + return true, nil +} + +func (r *GameServerReconciler) resumeFailedPodEviction(ctx context.Context, gs *gp.GameServer, want lifecycleIdentity) (time.Duration, error) { + reader := r.wipeReader() + var live gp.GameServer + if err := reader.Get(ctx, client.ObjectKeyFromObject(gs), &live); err != nil { + return 0, client.IgnoreNotFound(err) + } + op, held, err := lifecycleOwner(&live) + if err != nil || !held || !sameLifecycleOwner(op.lifecycleIdentity, want) { + return 0, err + } + release := func(retry bool) error { + if err := clearLifecycleOperation(ctx, r.Client, reader, &live, want); err != nil { + return err + } + delete(live.Annotations, evictionRetryAnnotation) + if retry { + // Release a refused request for other owners. The metadata update + // itself wakes the controller; persist the delay to avoid a hot loop. + live.Annotations[evictionRetryAnnotation] = time.Now().Add(evictionRetry).UTC().Format(time.RFC3339Nano) + } + return r.Update(ctx, &live) + } + var pod corev1.Pod + err = reader.Get(ctx, client.ObjectKey{Namespace: live.Namespace, Name: live.Name + "-0"}, &pod) + if apierrors.IsNotFound(err) || (err == nil && string(pod.UID) != want.Token) { + return 0, release(false) + } + if err != nil { + return 0, err + } + // An accepted eviction may still be saving during its termination grace. + // Keep the shared owner until this UID has left, including after a restart. + if !pod.DeletionTimestamp.IsZero() { + return evictionRetry, nil + } + if live.Generation != want.Generation || !recoveryPowerAllows(&live) { + return 0, release(false) + } + ok, err := r.failedOrdinal(ctx, &live, &pod) + if err != nil { + return 0, err + } + if !ok { + return 0, release(false) + } + preconditions := metav1.Preconditions{UID: &pod.UID, ResourceVersion: &pod.ResourceVersion} + eviction := &policyv1.Eviction{ObjectMeta: metav1.ObjectMeta{Name: pod.Name, Namespace: pod.Namespace}, DeleteOptions: &metav1.DeleteOptions{Preconditions: &preconditions}} + err = r.SubResource("eviction").Create(ctx, &pod, eviction) + switch { + case err == nil: + return evictionRetry, nil + case apierrors.IsNotFound(err): + return 0, release(false) + case apierrors.IsTooManyRequests(err) || apierrors.IsConflict(err): + return evictionRetry, release(true) + case apierrors.IsForbidden(err) || apierrors.IsInvalid(err) || apierrors.IsBadRequest(err) || apierrors.IsMethodNotSupported(err): + return 0, errors.Join(err, release(true)) + default: + // An indeterminate transport result can represent an accepted request. + // Leave the owner in place and re-read the exact pod on the next pass. + return 0, err + } +} diff --git a/operator/internal/controller/gameserver_rollout_recovery_test.go b/operator/internal/controller/gameserver_rollout_recovery_test.go new file mode 100644 index 000000000..ef37ccdd7 --- /dev/null +++ b/operator/internal/controller/gameserver_rollout_recovery_test.go @@ -0,0 +1,209 @@ +package controller + +import ( + "context" + "errors" + "testing" + + gp "github.com/GameplanePanel/gameplane/operator/api/v1alpha1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" +) + +func TestRecoverObsoleteFailedPod(t *testing.T) { + type fixture struct { + gs *gp.GameServer + ss *appsv1.StatefulSet + pod *corev1.Pod + } + for _, tc := range []struct { + name string + change func(fixture) + evict bool + }{ + {"image pull backoff", func(fixture) {}, true}, + {"repeated crash", func(f fixture) { + s := &f.pod.Status.ContainerStatuses[0] + s.State.Waiting.Reason = "CrashLoopBackOff" + s.RestartCount = crashLoopFailureThreshold + }, true}, + {"still starting", func(f fixture) { f.pod.Status.ContainerStatuses[0].State.Waiting.Reason = "ContainerCreating" }, false}, + {"current revision", func(f fixture) { f.pod.Labels[appsv1.StatefulSetRevisionLabel] = "new" }, false}, + {"missing revision", func(f fixture) { f.pod.Labels = nil }, false}, + {"ready", func(f fixture) { + f.pod.Status.Conditions = []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}} + }, false}, + {"unrelated pod", func(f fixture) { f.pod.OwnerReferences[0].UID = "foreign" }, false}, + {"unrelated set", func(f fixture) { f.ss.OwnerReferences[0].UID = "other-server" }, false}, + {"set not observed", func(f fixture) { f.ss.Status.ObservedGeneration-- }, false}, + {"scaled down", func(f fixture) { f.ss.Spec.Replicas = ptr.To(int32(0)) }, false}, + {"partition", func(f fixture) { + f.ss.Spec.UpdateStrategy.RollingUpdate = &appsv1.RollingUpdateStatefulSetStrategy{Partition: ptr.To(int32(1))} + }, false}, + {"on delete", func(f fixture) { f.ss.Spec.UpdateStrategy.Type = appsv1.OnDeleteStatefulSetStrategyType }, false}, + {"suspended", func(f fixture) { f.gs.Spec.Suspend = true }, false}, + {"backup", func(f fixture) { f.gs.Annotations[backupGuardAnnotation] = backupIdentityJSON("b", "backup") }, false}, + {"restore", func(f fixture) { f.gs.Annotations[restoreGuardAnnotation] = restoreIdentityJSON("r", "restore") }, false}, + {"wipe", func(f fixture) { f.gs.Annotations[WipeRequestedAnnotation] = "1" }, false}, + {"stop", func(f fixture) { f.gs.Annotations[stopRequestedAtAnnotation] = "now" }, false}, + {"restart", func(f fixture) { f.gs.Annotations[RestartRequestedAnnotation] = "1" }, false}, + {"hostport fence", func(f fixture) { f.ss.Annotations = map[string]string{hostportStopFenceAnnotation: "1"} }, false}, + } { + t.Run(tc.name, func(t *testing.T) { + gs, ss, pod := failedPodFixture() + tc.change(fixture{gs, ss, pod}) + evictions := 0 + c := fake.NewClientBuilder().WithScheme(wipeScheme(t)).WithObjects(gs, ss, pod).WithInterceptorFuncs(interceptor.Funcs{ + SubResourceCreate: func(ctx context.Context, c client.Client, sub string, obj client.Object, body client.Object, _ ...client.SubResourceCreateOption) error { + evictions++ + competing := lifecycleIdentity{Kind: "Backup", Name: "snapshot", UID: "backup-uid", TargetUID: gs.UID} + claimed, err := claimLifecycleOperation(ctx, c, c, gs.DeepCopy(), competing, nil) + if err != nil || claimed { + t.Fatalf("backup claimed target during eviction: claimed=%v err=%v", claimed, err) + } + e := body.(*policyv1.Eviction) + if sub != "eviction" || obj.GetUID() != pod.UID || e.Name != pod.Name || e.Namespace != pod.Namespace || e.DeleteOptions == nil || e.DeleteOptions.Preconditions == nil || ptr.Deref(e.DeleteOptions.Preconditions.UID, "") != pod.UID || ptr.Deref(e.DeleteOptions.Preconditions.ResourceVersion, "") != "19" { + t.Fatalf("unfenced eviction: %+v", e) + } + return apierrors.NewTooManyRequests("disruption budget", 1) + }, + }).Build() + r := &GameServerReconciler{Client: c} + retry, err := r.recoverObsoleteFailedPod(context.Background(), gs) + if err != nil || (retry > 0) != tc.evict || (evictions == 1) != tc.evict { + t.Fatalf("retry %v, evictions %d, error %v", retry, evictions, err) + } + }) + } +} + +func failedPodFixture() (*gp.GameServer, *appsv1.StatefulSet, *corev1.Pod) { + gs := wipeGameServer(false, "") + ss := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{Name: gs.Name, Namespace: gs.Namespace, UID: "set", Generation: 2, + OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(gs, gp.GroupVersion.WithKind("GameServer"))}}, + Spec: appsv1.StatefulSetSpec{Replicas: ptr.To(int32(1)), UpdateStrategy: appsv1.StatefulSetUpdateStrategy{Type: appsv1.RollingUpdateStatefulSetStrategyType}}, + Status: appsv1.StatefulSetStatus{UpdateRevision: "new", ObservedGeneration: 2}} + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: gs.Name + "-0", Namespace: gs.Namespace, UID: "old-pod", ResourceVersion: "19", Labels: map[string]string{appsv1.StatefulSetRevisionLabel: "old"}, OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(ss, appsv1.SchemeGroupVersion.WithKind("StatefulSet"))}}, Status: corev1.PodStatus{ContainerStatuses: []corev1.ContainerStatus{{Name: gameContainerName, State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ImagePullBackOff"}}}}}} + return gs, ss, pod +} + +func TestBackupWinsEvictionAdmissionRace(t *testing.T) { + gs, ss, pod := failedPodFixture() + c := fake.NewClientBuilder().WithScheme(wipeScheme(t)).WithObjects(gs, ss, pod).WithInterceptorFuncs(interceptor.Funcs{ + Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + op, _, _ := lifecycleOwner(obj.(*gp.GameServer)) + if op.Kind != podRecoveryKind { + t.Fatal("unexpected owner update") + } + // Win on the shared GameServer while the recovery claimant is + // holding the immediately preceding resourceVersion. + want := lifecycleIdentity{Kind: "Backup", Name: "scheduled", UID: "copy", TargetUID: gs.UID} + ok, err := claimLifecycleOperation(ctx, c, c, gs.DeepCopy(), want, nil) + if err != nil || !ok { + t.Fatalf("backup admission: %t %v", ok, err) + } + return c.Update(ctx, obj, opts...) + }, + SubResourceCreate: func(context.Context, client.Client, string, client.Object, client.Object, ...client.SubResourceCreateOption) error { + t.Fatal("lost claim reached eviction") + return nil + }, + }).Build() + r := GameServerReconciler{Client: c} + if _, err := r.recoverObsoleteFailedPod(t.Context(), gs); !apierrors.IsConflict(err) { + t.Fatalf("losing candidate: %v", err) + } +} + +func TestEvictionOwnerRecoversThroughReconcile(t *testing.T) { + for _, tc := range []string{"accepted", "pdb", "changed intent", "uncertain response"} { + t.Run(tc, func(t *testing.T) { + gs, ss, pod := failedPodFixture() + gs.Generation = 2 + want := lifecycleIdentity{Kind: podRecoveryKind, Name: gs.Name, UID: gs.UID, TargetUID: gs.UID, Generation: 2, Token: string(pod.UID)} + gs.Annotations[lifecycleOperationAnnotation] = lifecycleOperationJSON(lifecycleOperation{lifecycleIdentity: want}) + if tc == "changed intent" { + gs.Generation++ + gs.Spec.Suspend = true + } + pod.Finalizers = []string{"test.gameplane.local/drain"} + evictions := 0 + c := fake.NewClientBuilder().WithScheme(wipeScheme(t)).WithObjects(gs, ss, pod).WithInterceptorFuncs(interceptor.Funcs{ + SubResourceCreate: func(ctx context.Context, c client.Client, _ string, obj client.Object, _ client.Object, _ ...client.SubResourceCreateOption) error { + evictions++ + if tc == "pdb" { + return apierrors.NewTooManyRequests("blocked", 1) + } + if err := c.Delete(ctx, obj); err != nil { + return err + } + if tc == "uncertain response" { + return errors.New("eviction response lost") + } + return nil + }, + }).Build() + r := GameServerReconciler{Client: c} + req := ctrl.Request{NamespacedName: client.ObjectKeyFromObject(gs)} + // A new reconciler has no in-memory state from admission. No + // template is installed, so recovery must run before validation. + _, err := r.Reconcile(t.Context(), req) + if (err != nil) != (tc == "uncertain response") { + t.Fatalf("first pass: %v", err) + } + if tc == "changed intent" && evictions != 0 { + t.Fatal("evicted after suspend") + } + if tc == "pdb" && evictions != 1 { + t.Fatal("missing eviction attempt") + } + if tc == "accepted" || tc == "uncertain response" { + if evictions != 1 { + t.Fatalf("evictions: %d", evictions) + } + r = GameServerReconciler{Client: c} + if _, err = r.Reconcile(t.Context(), req); err != nil { + t.Fatal(err) + } + var live gp.GameServer + if err = c.Get(t.Context(), req.NamespacedName, &live); err != nil { + t.Fatal(err) + } + op, held, err := lifecycleOwner(&live) + if !held || err != nil || !sameLifecycleOwner(op.lifecycleIdentity, want) || evictions != 1 { + t.Fatalf("lost owner during grace: %+v %d %v", op, evictions, err) + } + if err = c.Get(t.Context(), client.ObjectKeyFromObject(pod), pod); err != nil { + t.Fatal(err) + } + pod.Finalizers = nil + if err = c.Update(t.Context(), pod); err != nil { + t.Fatal(err) + } + if _, err = r.Reconcile(t.Context(), req); err != nil { + t.Fatal(err) + } + } + var live gp.GameServer + if err := c.Get(t.Context(), req.NamespacedName, &live); err != nil { + t.Fatal(err) + } + if _, held, err := lifecycleOwner(&live); err != nil || held { + t.Fatalf("owner retained after release: %v", err) + } + if tc == "pdb" { + if wait, err := r.recoverObsoleteFailedPod(t.Context(), &live); err != nil || wait <= 0 || evictions != 1 { + t.Fatalf("immediate retry: %s %d %v", wait, evictions, err) + } + } + }) + } +} diff --git a/operator/internal/controller/lifecycle_operation.go b/operator/internal/controller/lifecycle_operation.go index 261c06cee..c69078143 100644 --- a/operator/internal/controller/lifecycle_operation.go +++ b/operator/internal/controller/lifecycle_operation.go @@ -65,7 +65,7 @@ func parseLifecycleOperation(value string) (lifecycleOperation, error) { return op, fmt.Errorf("decode lifecycle owner: %w", err) } for _, id := range append([]lifecycleIdentity{op.lifecycleIdentity}, op.Recovery...) { - if id.Name == "" || id.UID == "" || id.TargetUID == "" || (id.Kind != "Backup" && id.Kind != "Restore" && id.Kind != "Wipe" && id.Kind != "Rollout") || (id.Kind == "Wipe" && id.Token == "") || (id.Kind == "Rollout" && !validTemplateRevision(id.Token)) { + if id.Name == "" || id.UID == "" || id.TargetUID == "" || (id.Kind != "Backup" && id.Kind != "Restore" && id.Kind != "Wipe" && id.Kind != "Rollout" && id.Kind != podRecoveryKind) || ((id.Kind == "Wipe" || id.Kind == podRecoveryKind) && id.Token == "") || (id.Kind == "Rollout" && !validTemplateRevision(id.Token)) { return op, fmt.Errorf("invalid lifecycle owner identity") } if id.TargetUID != op.TargetUID { @@ -149,7 +149,7 @@ func lifecycleOwnerExists(ctx context.Context, reader client.Reader, namespace s obj = &gameplanev1alpha1.Backup{} case "Restore": obj = &gameplanev1alpha1.Restore{} - case "Wipe", "Rollout": + case "Wipe", "Rollout", podRecoveryKind: // Only the GameServer reconciler may cancel/replace a wipe token after // its workers drain. A removed request is not proof of worker absence. return true, nil @@ -175,7 +175,7 @@ func claimLifecycleOperation(ctx context.Context, c client.Client, reader client if live.UID != want.TargetUID || !live.DeletionTimestamp.IsZero() { return false, fmt.Errorf("lifecycle target changed identity or is being deleted") } - if want.Kind == "Rollout" && live.Generation != want.Generation { + if (want.Kind == "Rollout" || want.Kind == podRecoveryKind) && live.Generation != want.Generation { return false, errLifecycleGenerationChanged } *gs = live @@ -187,6 +187,10 @@ func claimLifecycleOperation(ctx context.Context, c client.Client, reader client return false, err } if occupied && !sameLifecycleOwner(op.lifecycleIdentity, want) { + // A failed ordinal can wait for an existing operation's own cleanup. + if want.Kind == podRecoveryKind { + return false, nil + } exists, err := lifecycleOwnerExists(ctx, reader, gs.Namespace, op.lifecycleIdentity) if err != nil || exists { return false, err @@ -214,7 +218,7 @@ func claimLifecycleOperation(ctx context.Context, c client.Client, reader client } // A stop command already sent may still save data and exit. Waiting // here also lets pending restarts finish instead of freezing mid-stop. - if want.Kind == "Backup" || want.Kind == "Rollout" { + if want.Kind == "Backup" || want.Kind == "Rollout" || want.Kind == podRecoveryKind { pending, err := backupLifecycleTransitionPending(ctx, reader, gs) if err != nil || pending { return false, err diff --git a/operator/specs.md b/operator/specs.md index 10c9b06d7..046fdcb95 100644 --- a/operator/specs.md +++ b/operator/specs.md @@ -843,3 +843,13 @@ RecoveryIntervention is removed only after cleanup has durably released lifecycl ownership and its compatibility guards; unresolved owners retain the condition. See [the recovery contract](../specs/033-stopped-multivolume-recovery/contracts/recovery.md) and [operator operations](../docs/operator-operations.md) for defaults and failures. + +### Failed StatefulSet rollout recovery + +When a corrected rolling update stalls on a failed older ordinal, the GameServer controller requests a pod eviction with UID and resource-version preconditions. It checks live workload ownership, desired replicas, revision and lifecycle fences. The StatefulSet recreates the pod. Disruption budgets can delay recovery. + +Recovery first claims the GameServer's shared lifecycle owner for the exact +failed pod UID and GameServer generation. Backups, restores, wipes and template +rollouts wait through eviction and termination of that UID. On restart the +controller resumes the claim before validating new render inputs. A disruption +budget refusal releases the claim so other operations can proceed during the retry delay. diff --git a/test/e2e/buckets.sh b/test/e2e/buckets.sh index aaa36ecd7..346db975a 100755 --- a/test/e2e/buckets.sh +++ b/test/e2e/buckets.sh @@ -64,6 +64,7 @@ TestGameServer_ExplicitStorageClassOverridesDefault TestGameServer_NonexistentStorageClassSurfacesError TestGameServer_VersionSwitch TestGameServer_TemplateOnRequestRetainsData +TestGameServer_CorrectedImageRecoversFailedOrdinal TestGameServer_WipeReportsFailureOnPermissionDenied TestGameServer_CascadingDelete TestGameServer_TunnelCredentialRefusalSurfaced diff --git a/test/e2e/template_rollout_e2e_test.go b/test/e2e/template_rollout_e2e_test.go index 0f6e7c046..615fcd148 100644 --- a/test/e2e/template_rollout_e2e_test.go +++ b/test/e2e/template_rollout_e2e_test.go @@ -119,3 +119,54 @@ func TestGameServer_TemplateOnRequestRetainsData(t *testing.T) { t.Fatalf("persisted contents after adoption: %s %v", out, err) } } + +// Start from a healthy server, deploy a broken image, then restore the +// original spec. OrderedReady must replace the failed ordinal after rollback. +func TestGameServer_CorrectedImageRecoversFailedOrdinal(t *testing.T) { + t.Parallel() + ctx := context.Background() + const ns, name = "gameplane-games", "e2e-corrected-image" + applyBusyboxTemplate(t, name) + applyBusyboxGameServer(t, ns, name, name) + pods := envInstance.K8s.CoreV1().Pods(ns) + servers := envInstance.Dyn.Resource(gameServerGVR).Namespace(ns) + patch := func(body string) { + t.Helper() + if _, err := servers.Patch(ctx, name, types.MergePatchType, []byte(body), metav1.PatchOptions{}); err != nil { + t.Fatal(err) + } + } + waitPodReady := func(exclude types.UID) { + t.Helper() + envInstance.Eventually(t, 4*time.Minute, func() (bool, string) { + pod, err := pods.Get(ctx, name+"-0", metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + for _, condition := range pod.Status.Conditions { + if condition.Type == "Ready" && condition.Status == "True" && pod.UID != exclude { + return true, "" + } + } + return false, "waiting for ready ordinal" + }) + } + waitPodReady("") + patch(`{"spec":{"image":"127.0.0.1:1/gameplane/unavailable:missing"}}`) + var broken types.UID + envInstance.Eventually(t, 4*time.Minute, func() (bool, string) { + pod, err := pods.Get(ctx, name+"-0", metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + for _, c := range pod.Status.ContainerStatuses { + if c.Name == "game" && c.State.Waiting != nil && c.State.Waiting.Reason == "ImagePullBackOff" { + broken = pod.UID + return true, "" + } + } + return false, "waiting for image pull backoff" + }) + patch(`{"spec":{"image":null}}`) + waitPodReady(broken) +}