fix(operator): 服务器未就绪时改了配置,operator 按新模板重建 pod-0,不再卡在旧镜像上

This commit is contained in:
Lemon-miaow committed 2026-09-27 04:03:15 +08:00
1 parent 0e08fa7ced
commit 2f920cf1c9
6 files changed
+320 -14

No files matched your search

+4 -1
View File
@@ -120,7 +120,10 @@ func cmdOperator(args []string, _, stderr io.Writer) int {
Jobs: mgr.GetAPIReader(),
// Uncached too: RCON Secrets are read by name, so the Role grants
// secrets:get without the list/watch an informer would need.
Secrets: mgr.GetAPIReader(),
Secrets: mgr.GetAPIReader(),
// And pods: pod-0 is read by name, so the Role grants pods:get and no
// namespace-wide pod informer runs.
Pods: mgr.GetAPIReader(),
Recorder: mgr.GetEventRecorderFor("felis-operator"),
Watch: watch,
}
+14 -4
View File
@@ -40,7 +40,7 @@ concrete host.
## 1. Server is stuck in `Starting` and never becomes `Running`
`MinecraftServer.status.phase` stays `Starting`. A start that never succeeds is
requeued every 5s until one of the two startup budgets expires, then escalated
requeued every 2s until one of the two startup budgets expires, then escalated
to `Failed` — `StartupTimeout` if the pod never passed TCP readiness,
`ReadinessTimeout` if the RCON probe never succeeded. Both default to **300s**
when `spec.startup.timeoutSeconds` / `spec.startup.readinessTimeoutSeconds` are
@@ -62,13 +62,16 @@ kubectl get minecraftserver <name> -o jsonpath='{.status.conditions}'
```
`markStarting` writes the same reason to both `Ready=False` and
`RconReached=False`. The reason is exactly one of:
`RconReached=False`. The reason is one of:
| `status.conditions[].reason` | Meaning | Requeue |
|---|---|---|
| `PodNotReady` | Pod not TCP-ready yet (`status.readyReplicas < 1`) | 5s |
| `PodNotReady` | Pod not TCP-ready yet (`status.readyReplicas < 1`) | 2s |
| `SpecChanged` | The spec changed while the pod was not ready; the operator recreated the pod from the new template (§1a) | 2s |
| `AutoRestart` / `StartRetried` | A timed-out start was retried, automatically or from the panel, by recreating the pod | 2s |
| `ServiceAddressPending` | The client Service has no ClusterIP yet | 2s |
| `RconSecretUnavailable` | RCON secret missing or malformed | 10s |
| `RconNotReachable` | RCON dial/auth failed | 5s |
| `RconNotReachable` | RCON dial/auth failed | 2s |
[GO-TESTED for the reason set.]
@@ -88,6 +91,13 @@ kubectl describe pod <pod> # look at Events + container State
not exist, or the registry is unreachable. `spec.image` is copied verbatim into
the container with **zero validation** by the operator. Fix the image
reference, or see §7 (registry reachability) and §6 (build push target).
Editing the spec is enough, even once the server is `Failed` with its retries
spent: the StatefulSet (OrderedReady) never rolls a pod that is not ready, so
the operator deletes a not-ready `<name>-0` made from an older template itself
and the StatefulSet recreates it from the new one. The server goes back to
`Starting` with reason `SpecChanged`, a fresh start timeout and its restart
budget at zero, and the MinecraftServer gets a `PodReplaced` Event.
[GO-TESTED: `TestSpecChangeReplacesAPodThatGaveUp`, `TestStalePodReplacement`.]
- **PVC `Pending`** → `kubectl get pvc -l app.kubernetes.io/name=<name>`. A
nonexistent `spec.storage.storageClassName`, or a request larger than any class
can satisfy, leaves the PVC unbound. The operator does **not** error on this
+68
View File
@@ -130,6 +130,11 @@ type Reconciler struct {
// mirror with the database URL among them) and grow with them. Nil falls back
// to the embedded client.
Secrets client.Reader
// Pods reads a server's pod-0 by name, to replace one left on an old template
// (replaceStalePod). The manager's uncached API reader as well, so the
// operator holds pods:get and no pod informer. Nil falls back to the embedded
// client.
Pods client.Reader
// Recorder puts an Event on the MinecraftServer at each phase change, pod
// recreation, idle stop and RCON Secret provision, so `kubectl describe`
// shows the timeline the status alone overwrites. Nil records none.
@@ -330,6 +335,18 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine
// The pod must first pass its tcpSocket readiness (readyReplicas >= 1).
if current.Status.ReadyReplicas < 1 {
if replaced, err := r.replaceStalePod(ctx, server, &current); err != nil {
return ctrl.Result{}, err
} else if replaced {
// A new spec is a new start: its own timeout and restart budget.
server.Status.AutoRestarts = 0
server.Status.StartRequestedAt = nil
r.markStarting(server, "SpecChanged", "the spec changed while the pod was not ready; recreated the pod from the new spec")
if err := r.patchStatus(ctx, server); err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{RequeueAfter: requeueStarting}, nil
}
r.markStarting(server, "PodNotReady", "waiting for pod TCP readiness")
if r.startupTimedOut(server) {
r.markFailed(server, v1alpha1.ReasonStartupTimeout, "pod did not become ready within startup timeout")
@@ -677,6 +694,57 @@ func (r *Reconciler) secretReader() client.Reader {
return r.Client
}
func (r *Reconciler) podReader() client.Reader {
if r.Pods != nil {
return r.Pods
}
return r.Client
}
// replaceStalePod deletes a pod-0 that is not ready and was made from an older
// template than the StatefulSet's current one, for the StatefulSet to recreate
// from the new spec. The StatefulSet controller does not do it: under
// OrderedReady it rolls a pod only once that pod is Running and Ready, so a
// server that cannot start keeps crash-looping on the image or settings that
// broke it however its spec is corrected, until the next timed-out retry
// deletes the pod, or forever once the retries are spent. A ready pod is left
// to the controller's own rolling update, and one already terminating to its
// deletion. It acts only on a StatefulSet status that has observed the current
// template, so the update revision it compares against is the current one.
func (r *Reconciler) replaceStalePod(ctx context.Context, server *v1alpha1.MinecraftServer, sts *appsv1.StatefulSet) (bool, error) {
update := sts.Status.UpdateRevision
if update == "" || sts.Status.ObservedGeneration < sts.Generation {
return false, nil
}
var pod corev1.Pod
if err := r.podReader().Get(ctx, types.NamespacedName{Namespace: server.Namespace, Name: server.Name + "-0"}, &pod); err != nil {
return false, client.IgnoreNotFound(err)
}
if pod.DeletionTimestamp != nil || pod.Labels[appsv1.ControllerRevisionHashLabelKey] == update || podReady(&pod) {
return false, nil
}
// The UID precondition: a pod the StatefulSet already recreated is not
// deleted in its place.
if err := r.Delete(ctx, &pod, client.Preconditions{UID: &pod.UID}); err != nil {
if apierrors.IsNotFound(err) || apierrors.IsConflict(err) {
return false, nil
}
return false, err
}
log.FromContext(ctx).Info("recreated a pod left on an old spec", "pod", pod.Name, "revision", pod.Labels[appsv1.ControllerRevisionHashLabelKey], "updateRevision", update)
r.event(server, corev1.EventTypeNormal, "PodReplaced", "the spec changed while the pod was not ready; recreated the pod from the new spec")
return true, nil
}
func podReady(pod *corev1.Pod) bool {
for _, c := range pod.Status.Conditions {
if c.Type == corev1.PodReady {
return c.Status == corev1.ConditionTrue
}
}
return false
}
func (r *Reconciler) rconPassword(ctx context.Context, server *v1alpha1.MinecraftServer) (string, error) {
ref := server.Spec.Rcon.SecretRef
if ref.Name == "" || ref.Key == "" {
+217
View File
@@ -0,0 +1,217 @@
package operator_test
import (
"context"
"errors"
"testing"
"time"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"felis.lolicon.best/internal/apis/felis/v1alpha1"
)
// podAt is survival-0 as the StatefulSet controller made it from revision rev.
func podAt(rev string) *corev1.Pod {
p := gamePod()
p.UID = types.UID("pod-" + rev)
p.Labels = map[string]string{appsv1.ControllerRevisionHashLabelKey: rev}
return p
}
func withReady(p *corev1.Pod, status corev1.ConditionStatus) *corev1.Pod {
p.Status.Conditions = []corev1.PodCondition{{Type: corev1.PodReady, Status: status}}
return p
}
// observeTemplate stands in for the StatefulSet controller: it has seen the
// current template, which it names revision rev, and the pod is not ready.
func observeTemplate(t *testing.T, c client.Client, rev string) {
t.Helper()
sts := getSTS(t, c, "survival")
sts.Status.ObservedGeneration = sts.Generation
sts.Status.UpdateRevision = rev
sts.Status.Replicas = 1
sts.Status.ReadyReplicas = 0
if err := c.Status().Update(context.Background(), sts); err != nil {
t.Fatalf("update sts status: %v", err)
}
}
func editImage(t *testing.T, c client.Client, image string) {
t.Helper()
s := getServer(t, c, "survival")
s.Spec.Image = image
if err := c.Update(context.Background(), s); err != nil {
t.Fatalf("edit image: %v", err)
}
}
func readyReason(s *v1alpha1.MinecraftServer) string {
if c := meta.FindStatusCondition(s.Status.Conditions, v1alpha1.ConditionReady); c != nil {
return c.Reason
}
return ""
}
// A server that gave up on a broken image is not stuck on it: once its spec is
// corrected, the pod still crash-looping on the old template is recreated from
// the new one, and the corrected start gets its own timeout and restarts.
func TestSpecChangeReplacesAPodThatGaveUp(t *testing.T) {
srv := runningServer()
srv.Spec.Startup.TimeoutSeconds = 30
r, c := newReconciler(t, fakeProber{}, srv, rconSecret(), podAt("survival-old"))
base := time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC)
clock := base
r.Now = func() metav1.Time { return metav1.NewTime(clock) }
at := func(offset time.Duration) *v1alpha1.MinecraftServer {
clock = base.Add(offset)
reconcile(t, r, "survival")
return getServer(t, c, "survival")
}
// Spend the three automatic restarts (autorestart_test.go pins the schedule),
// each pod coming back on the same broken template.
at(0)
observeTemplate(t, c, "survival-old")
for _, due := range []time.Duration{90 * time.Second, 240 * time.Second, 510 * time.Second} {
at(due)
if err := c.Create(context.Background(), podAt("survival-old")); err != nil {
t.Fatal(err)
}
}
s := at(time.Hour)
if !v1alpha1.StartGaveUp(&s.Status) {
t.Fatalf("setup: not given up: phase=%s restarts=%d", s.Status.Phase, s.Status.AutoRestarts)
}
// The admin corrects the image; the StatefulSet takes the new template but,
// the pod never having been ready, leaves the pod as it is.
editImage(t, c, "registry.internal/felis/paper:fixed")
s = at(time.Hour + time.Second)
if !podPresent(t, c) {
t.Fatal("the pod was deleted before the StatefulSet had observed the new template")
}
if img := getSTS(t, c, "survival").Spec.Template.Spec.Containers[0].Image; img != "registry.internal/felis/paper:fixed" {
t.Fatalf("setup: template image = %q", img)
}
observeTemplate(t, c, "survival-new")
s = at(time.Hour + 2*time.Second)
if podPresent(t, c) {
t.Fatal("the pod on the old template was left in place")
}
if s.Status.Phase != v1alpha1.PhaseStarting || readyReason(s) != "SpecChanged" {
t.Fatalf("phase=%s reason=%q, want Starting SpecChanged", s.Status.Phase, readyReason(s))
}
if s.Status.AutoRestarts != 0 {
t.Fatalf("autoRestarts = %d, want the budget back at 0", s.Status.AutoRestarts)
}
if s.Status.StartRequestedAt == nil || !s.Status.StartRequestedAt.Time.Equal(clock) {
t.Fatalf("startRequestedAt = %v, want the start re-anchored at %v", s.Status.StartRequestedAt, clock)
}
// The StatefulSet recreates the pod from the new revision; that one is left
// to start, and the new start times out on its own clock.
if err := c.Create(context.Background(), podAt("survival-new")); err != nil {
t.Fatal(err)
}
s = at(time.Hour + 20*time.Second)
if !podPresent(t, c) || s.Status.Phase != v1alpha1.PhaseStarting {
t.Fatalf("pod present=%v phase=%s, want the new pod kept while it starts", podPresent(t, c), s.Status.Phase)
}
s = at(time.Hour + 33*time.Second)
if s.Status.Phase != v1alpha1.PhaseFailed || s.Status.AutoRestarts != 0 {
t.Fatalf("phase=%s restarts=%d, want Failed with the budget untouched at the new timeout", s.Status.Phase, s.Status.AutoRestarts)
}
}
// Which pods a Starting server's pass replaces once the StatefulSet has a newer
// template, and which it leaves alone.
func TestStalePodReplacement(t *testing.T) {
terminating := podAt("survival-old")
terminating.DeletionTimestamp = &metav1.Time{Time: time.Date(2026, 6, 25, 11, 59, 0, 0, time.UTC)}
terminating.Finalizers = []string{"test.felis/hold"}
for _, tc := range []struct {
name string
pod *corev1.Pod
observed bool
replaced bool
}{
{"old template, not ready", podAt("survival-old"), true, true},
{"old template, readiness false", withReady(podAt("survival-old"), corev1.ConditionFalse), true, true},
{"current template", podAt("survival-new"), true, false},
{"old template but ready: the StatefulSet rolls it", withReady(podAt("survival-old"), corev1.ConditionTrue), true, false},
{"already terminating", terminating, true, false},
{"template not yet observed", podAt("survival-old"), false, false},
{"no pod", nil, true, false},
} {
t.Run(tc.name, func(t *testing.T) {
objs := []client.Object{runningServer(), rconSecret()}
if tc.pod != nil {
objs = append(objs, tc.pod.DeepCopy())
}
r, c := newReconciler(t, fakeProber{}, objs...)
reconcile(t, r, "survival")
observeTemplate(t, c, "survival-new")
if !tc.observed {
sts := getSTS(t, c, "survival")
sts.Generation = 2
if err := c.Update(context.Background(), sts); err != nil {
t.Fatal(err)
}
sts = getSTS(t, c, "survival")
if sts.Status.ObservedGeneration >= sts.Generation {
t.Fatalf("setup: observedGeneration %d is not behind generation %d", sts.Status.ObservedGeneration, sts.Generation)
}
}
res := reconcile(t, r, "survival")
s := getServer(t, c, "survival")
if gone := tc.pod != nil && !podPresent(t, c); gone != tc.replaced {
t.Fatalf("pod deleted = %v, want %v", gone, tc.replaced)
}
want := "PodNotReady"
if tc.replaced {
want = "SpecChanged"
}
if readyReason(s) != want || s.Status.Phase != v1alpha1.PhaseStarting {
t.Fatalf("phase=%s reason=%q, want Starting %s", s.Status.Phase, readyReason(s), want)
}
if res.RequeueAfter != 2*time.Second {
t.Fatalf("requeue = %v, want the Starting pace", res.RequeueAfter)
}
})
}
}
// noCachedPods stands in for the manager's cached client, which has no pod
// informer: any pod read through it fails.
type noCachedPods struct{ client.Client }
func (c noCachedPods) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
if _, ok := obj.(*corev1.Pod); ok {
return errors.New("pods are not cached")
}
return c.Client.Get(ctx, key, obj, opts...)
}
// The pod is read through Reconciler.Pods alone, so the operator runs with
// pods:get and no pod informer.
func TestStalePodIsReadThroughTheUncachedReader(t *testing.T) {
r, c := newReconciler(t, fakeProber{}, runningServer(), rconSecret(), podAt("survival-old"))
r.Client = noCachedPods{c}
r.Pods = c
reconcile(t, r, "survival")
observeTemplate(t, c, "survival-new")
reconcile(t, r, "survival")
if podPresent(t, c) {
t.Fatal("the pod on the old template was left in place")
}
}
+10 -6
View File
@@ -153,9 +153,12 @@ func APIBuildRole(p Params) *rbacv1.Role {
// call fails closed with a 403), and reads RCON Secrets by name, uncached. Jobs are list-only,
// through the manager's uncached API reader: before scaling a server up from zero
// the operator checks that no restore/backup/file-write Job holds its world
// (internal/maintenance). Pods are delete-only: a start that timed out is retried
// by deleting its pod for the StatefulSet to recreate (bounded, three attempts;
// internal/operator.recoverFailedStart). Events are create/patch only, for the
// (internal/maintenance). Pods are get and delete, by name through the uncached
// reader: a start that timed out is retried by deleting its pod for the
// StatefulSet to recreate (bounded, three attempts;
// internal/operator.recoverFailedStart), and a pod that is not ready and was made
// from an older template is read and replaced the same way
// (internal/operator.replaceStalePod). Events are create/patch only, for the
// timeline it records on each server. It never touches PVCs or finalizers, so
// none appear here.
func OperatorRole(p Params) *rbacv1.Role {
@@ -175,9 +178,10 @@ func OperatorRole(p Params) *rbacv1.Role {
// list only: an uncached List (no informer, so no watch) of the world-volume
// maintenance Jobs; the operator never creates or deletes a Job.
rule([]string{groupBatch}, []string{"jobs"}, []string{"list"}),
// delete only, through the direct client: no read of pods is needed to
// remove the one named <server>-0.
rule([]string{groupCore}, []string{"pods"}, []string{"delete"}),
// get and delete by name of the one named <server>-0: get through the
// uncached API reader (Reconciler.Pods), to see whether it is ready and which
// template revision made it. No list/watch, so there is no pod informer.
rule([]string{groupCore}, []string{"pods"}, []string{"get", "delete"}),
// The Events the reconciler records on MinecraftServers (phase changes,
// pod recreation, idle stop): the recorder creates one and patches its
// count when the same Event repeats.
+7 -3
View File
@@ -215,13 +215,17 @@ func TestOperatorRole_ScopeExact(t *testing.T) {
t.Errorf("operator secrets rule must be get+create only, found %s", v)
}
}
// Pods are delete-only: the bounded retry of a timed-out start.
// Pods are read and deleted by name: the bounded retry of a timed-out start,
// and the replacement of a not-ready pod left on an old template.
if !hasRule(op, groupCore, "pods", "delete") {
t.Error("operator must have pods:delete (auto-restart of a timed-out start)")
}
for _, v := range []string{"get", "list", "watch", "create", "update", "patch", "deletecollection", "*"} {
if !hasRule(op, groupCore, "pods", "get") {
t.Error("operator must have pods:get (replacing a not-ready pod left on an old template)")
}
for _, v := range []string{"list", "watch", "create", "update", "patch", "deletecollection", "*"} {
if hasRule(op, groupCore, "pods", v) {
t.Errorf("operator pods rule must be delete-only, found %s", v)
t.Errorf("operator pods rule must be get+delete only, found %s", v)
}
}
// The world-volume lock check lists Jobs uncached; it never writes one.