From aa009b788b76ee8d825ba0d2bafcbe88934886bd Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Tue, 13 Oct 2026 16:17:20 +0400 Subject: [PATCH 1/3] fix(controllers): recreate adopted Pods stuck on a removed legacy ConfigMap In-place adoption deletes the legacy cluster-state ConfigMap but leaves the running Pods untouched, and their etcd container keeps an envFrom on it. The next container start (node reboot, kubelet restart, stand power cycle) fails with CreateContainerConfigError. The Pod stays Running with RestartCount 0, so neither the terminal-phase replacement nor the crash-loop trigger fires, and ensurePod treats the existing Pod as healthy: the member stays down until someone deletes the Pod by hand. Treat such a Pod the same way as a terminal one: when the etcd container is waiting on CreateContainerConfigError and references a ConfigMap or Secret that buildPod does not emit, delete it so the next pass recreates it from buildPod against the same PVC and member ID. A config error on an env source buildPod itself emits is left alone, because recreating would fail the same way forever. Fixes #373 Assisted-by: LLM Signed-off-by: Andrey Kolkov --- controllers/etcdmember_controller.go | 115 +++++++++++++--- controllers/etcdmember_controller_test.go | 157 +++++++++++++++++++++- 2 files changed, 246 insertions(+), 26 deletions(-) diff --git a/controllers/etcdmember_controller.go b/controllers/etcdmember_controller.go index f3e87112..7ed1bf53 100644 --- a/controllers/etcdmember_controller.go +++ b/controllers/etcdmember_controller.go @@ -20,7 +20,9 @@ import ( "context" "crypto/tls" "fmt" + "sort" "strconv" + "strings" "time" corev1 "k8s.io/api/core/v1" @@ -161,24 +163,22 @@ func (r *EtcdMemberReconciler) Reconcile(ctx context.Context, req ctrl.Request) } } - // The kubelet never restarts a Pod in a terminal phase (a graceful node - // shutdown leaves etcd Succeeded) and nothing else replaces a bare Pod. - // Delete it so the next pass recreates it: PVC-backed resumes with the - // same member ID, memory-backed takes the pod-loss path above. Not - // quorum-gated: a whole-cluster reboot lands every member here at once. - // Ready=False is written before the delete so a failed write leaves the - // Pod in place as the retry trigger; PodUID stays for the pod-loss gate. - if pod, err := r.terminalPod(ctx, member); err != nil { + // Some Pods can never run again as they are, and nothing else replaces a + // bare Pod: see podToRecreate. Delete it so the next pass recreates it: + // PVC-backed resumes with the same member ID, memory-backed takes the + // pod-loss path above. Not quorum-gated: a whole-cluster reboot lands + // every member here at once. Ready=False is written before the delete so + // a failed write leaves the Pod in place as the retry trigger; PodUID + // stays for the pod-loss gate. + if pod, msg, err := r.podToRecreate(ctx, member); err != nil { return ctrl.Result{}, err } else if pod != nil { - if setMemberCondition(member, lll.MemberReady, metav1.ConditionFalse, "PodReplacing", - terminalPodMessage(pod)) { + if setMemberCondition(member, lll.MemberReady, metav1.ConditionFalse, "PodReplacing", msg) { if err := r.Status().Update(ctx, member); err != nil { return ctrl.Result{}, err } } - log.Info("deleting terminal-phase pod for recreation", - "phase", pod.Status.Phase, "reason", pod.Status.Reason, "podUID", pod.UID) + log.Info("deleting pod for recreation", "why", msg, "podUID", pod.UID) if err := r.Delete(ctx, pod); err != nil && !errors.IsNotFound(err) { return ctrl.Result{}, err } @@ -219,21 +219,96 @@ func podInTerminalPhase(pod *corev1.Pod) bool { return pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed } -// terminalPod returns the member's own Pod when it sits in a terminal phase -// and is not already terminating (drain, eviction, manual delete), else nil. -func (r *EtcdMemberReconciler) terminalPod(ctx context.Context, member *lll.EtcdMember) (*corev1.Pod, error) { +// podToRecreate returns the member's own Pod, with the reason, when the +// kubelet can never get it running as it is, else nil. A Pod already +// terminating (drain, eviction, manual delete) is left to finish. +// +// Two shapes qualify: +// - a terminal phase, which the kubelet never restarts (a graceful node +// shutdown leaves etcd Succeeded); +// - an etcd container stuck in CreateContainerConfigError on an env source +// buildPod does not emit. An adopted legacy Pod references the legacy +// cluster-state ConfigMap that adoption deletes, so its next container +// start fails with RestartCount 0 and a Running phase — invisible to both +// the terminal-phase check and the crash-loop trigger. Recreating from +// buildPod drops the reference; a source buildPod itself emits is +// excluded, since recreating would hit the same error forever. +func (r *EtcdMemberReconciler) podToRecreate(ctx context.Context, member *lll.EtcdMember) (*corev1.Pod, string, error) { pod := &corev1.Pod{} err := r.Get(ctx, types.NamespacedName{Namespace: member.Namespace, Name: member.Name}, pod) if errors.IsNotFound(err) { - return nil, nil + return nil, "", nil } if err != nil { - return nil, err + return nil, "", err + } + if !podOwnedBy(pod, member) || pod.DeletionTimestamp != nil { + return nil, "", nil + } + if podInTerminalPhase(pod) { + return pod, terminalPodMessage(pod), nil + } + if !etcdContainerConfigError(pod) { + return nil, "", nil + } + foreign := foreignEnvSources(pod, r.buildPod(member, false)) + if len(foreign) == 0 { + return nil, "", nil + } + return pod, fmt.Sprintf("etcd container cannot be created (CreateContainerConfigError) and references "+ + "env sources the operator does not manage: %s; deleted for recreation", strings.Join(foreign, ", ")), nil +} + +func etcdContainerConfigError(pod *corev1.Pod) bool { + for _, cs := range pod.Status.ContainerStatuses { + if cs.Name == "etcd" { + return cs.State.Waiting != nil && cs.State.Waiting.Reason == "CreateContainerConfigError" + } + } + return false +} + +// foreignEnvSources lists the ConfigMaps and Secrets the live Pod's etcd +// container pulls env from that the desired Pod's does not, sorted. +func foreignEnvSources(live, desired *corev1.Pod) []string { + want := envSources(desired) + var out []string + for src := range envSources(live) { + if !want[src] { + out = append(out, src) + } } - if !podOwnedBy(pod, member) || pod.DeletionTimestamp != nil || !podInTerminalPhase(pod) { - return nil, nil + sort.Strings(out) + return out +} + +func envSources(pod *corev1.Pod) map[string]bool { + srcs := map[string]bool{} + for _, c := range pod.Spec.Containers { + if c.Name != "etcd" { + continue + } + for _, ef := range c.EnvFrom { + if ef.ConfigMapRef != nil { + srcs["ConfigMap/"+ef.ConfigMapRef.Name] = true + } + if ef.SecretRef != nil { + srcs["Secret/"+ef.SecretRef.Name] = true + } + } + for _, e := range c.Env { + if e.ValueFrom == nil { + continue + } + if ref := e.ValueFrom.ConfigMapKeyRef; ref != nil { + srcs["ConfigMap/"+ref.Name] = true + } + if ref := e.ValueFrom.SecretKeyRef; ref != nil { + srcs["Secret/"+ref.Name] = true + } + } } - return pod, nil + return srcs } // terminalPodMessage keeps the phase and reason the Pod died with; the diff --git a/controllers/etcdmember_controller_test.go b/controllers/etcdmember_controller_test.go index 41d35bcb..0649aa61 100644 --- a/controllers/etcdmember_controller_test.go +++ b/controllers/etcdmember_controller_test.go @@ -22,6 +22,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -2409,7 +2410,7 @@ func TestReconcile_MemoryMemberTerminalPodTriggersReplacement(t *testing.T) { } // A Pod already terminating is left to finish. -func TestTerminalPod_SkipsPodBeingDeleted(t *testing.T) { +func TestPodToRecreate_SkipsPodBeingDeleted(t *testing.T) { ctx := context.Background() tru := true @@ -2434,9 +2435,9 @@ func TestTerminalPod_SkipsPodBeingDeleted(t *testing.T) { } r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} - got, err := r.terminalPod(ctx, member) + got, _, err := r.podToRecreate(ctx, member) if err != nil { - t.Fatalf("terminalPod: %v", err) + t.Fatalf("podToRecreate: %v", err) } if got != nil { t.Fatalf("a Pod already terminating must not be reported as terminal") @@ -2444,7 +2445,7 @@ func TestTerminalPod_SkipsPodBeingDeleted(t *testing.T) { } // A same-name terminal Pod owned by another EtcdMember is not ours. -func TestTerminalPod_SkipsPodOwnedByAnotherMember(t *testing.T) { +func TestPodToRecreate_SkipsPodOwnedByAnotherMember(t *testing.T) { ctx := context.Background() tru := true @@ -2464,15 +2465,159 @@ func TestTerminalPod_SkipsPodOwnedByAnotherMember(t *testing.T) { c, _ := newTestClient(t, member, pod) r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} - got, err := r.terminalPod(ctx, member) + got, _, err := r.podToRecreate(ctx, member) if err != nil { - t.Fatalf("terminalPod: %v", err) + t.Fatalf("podToRecreate: %v", err) } if got != nil { t.Fatalf("a terminal Pod owned by another EtcdMember must not be reported") } } +// adoptedLegacyFixture is a PVC-backed member whose Pod still carries the +// legacy operator's envFrom on the cluster-state ConfigMap, with the etcd +// container in the given state. +func adoptedLegacyFixture(t *testing.T, state corev1.ContainerState) (*lll.EtcdMember, *corev1.Pod, *corev1.PersistentVolumeClaim) { + t.Helper() + tru := true + owner := []metav1.OwnerReference{{ + APIVersion: "etcd-operator.cozystack.io/v1alpha2", Kind: "EtcdMember", + Name: "etcd-1", UID: types.UID("member-uid"), Controller: &tru, BlockOwnerDeletion: &tru, + }} + member := &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{ + Name: "etcd-1", Namespace: "ns", UID: types.UID("member-uid"), + Labels: memberLabels("etcd", "etcd-1"), + Finalizers: []string{MemberFinalizer}, + }, + Spec: lll.EtcdMemberSpec{ + ClusterName: "etcd", Version: "3.5.12", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + InitialCluster: "x", ClusterToken: "ns-etcd", + }, + Status: lll.EtcdMemberStatus{PodName: "etcd-1", PodUID: "old-uid", PVCName: "data-etcd-1"}, + } + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "etcd-1", Namespace: "ns", UID: types.UID("old-uid"), OwnerReferences: owner}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Name: "etcd", + EnvFrom: []corev1.EnvFromSource{{ + ConfigMapRef: &corev1.ConfigMapEnvSource{LocalObjectReference: corev1.LocalObjectReference{Name: "etcd-cluster-state"}}, + }}, + }}}, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{{Name: "etcd", State: state}}, + }, + } + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "data-etcd-1", Namespace: "ns", OwnerReferences: owner}, + } + return member, pod, pvc +} + +var createContainerConfigError = corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{ + Reason: "CreateContainerConfigError", + Message: `configmap "etcd-cluster-state" not found`, +}} + +// An adopted Pod whose container cannot be created because of the legacy +// ConfigMap reference is recreated from buildPod against the same PVC. +func TestReconcile_RecreatesPodStuckOnForeignEnvSource(t *testing.T) { + ctx := context.Background() + member, pod, pvc := adoptedLegacyFixture(t, createContainerConfigError) + c, _ := newTestClient(t, member, pod, pvc) + r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factoryReturning(newFakeEtcd(0xdead))} + req := ctrl.Request{NamespacedName: types.NamespacedName{Name: "etcd-1", Namespace: "ns"}} + + if _, err := r.Reconcile(ctx, req); err != nil { + t.Fatalf("Reconcile (pass 1): %v", err) + } + if err := c.Get(ctx, types.NamespacedName{Namespace: "ns", Name: "etcd-1"}, &corev1.Pod{}); !apierrors.IsNotFound(err) { + t.Fatalf("stuck adopted Pod must be deleted; got err=%v", err) + } + got := mustGet(t, c, "etcd-1", "ns", &lll.EtcdMember{}) + cond := meta.FindStatusCondition(got.Status.Conditions, lll.MemberReady) + if cond == nil || cond.Status != metav1.ConditionFalse || cond.Reason != "PodReplacing" || + !strings.Contains(cond.Message, "ConfigMap/etcd-cluster-state") { + t.Fatalf("MemberReady must be False/PodReplacing naming the foreign source; got %+v", cond) + } + + if _, err := r.Reconcile(ctx, req); err != nil { + t.Fatalf("Reconcile (pass 2): %v", err) + } + fresh := mustGet(t, c, "etcd-1", "ns", &corev1.Pod{}) + if fresh.UID == types.UID("old-uid") { + t.Fatalf("Pod must be recreated with a new UID; still old-uid") + } + for _, ctr := range fresh.Spec.Containers { + if len(ctr.EnvFrom) != 0 { + t.Fatalf("recreated Pod must not carry the legacy envFrom; got %+v", ctr.EnvFrom) + } + } + if !pvcOwnedBy(mustGet(t, c, "data-etcd-1", "ns", &corev1.PersistentVolumeClaim{}), member) { + t.Fatalf("PVC must be preserved and still owned by the member") + } +} + +// An adopted Pod that is running is left alone: the stale reference only +// matters once the container has to be created again. +func TestPodToRecreate_KeepsRunningAdoptedPod(t *testing.T) { + member, pod, pvc := adoptedLegacyFixture(t, corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}) + c, _ := newTestClient(t, member, pod, pvc) + r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} + + got, _, err := r.podToRecreate(context.Background(), member) + if err != nil { + t.Fatalf("podToRecreate: %v", err) + } + if got != nil { + t.Fatalf("a running adopted Pod must not be recreated") + } +} + +// A config error on a Pod the operator itself would build again is not +// recreated: the replacement would fail the same way, forever. +func TestPodToRecreate_SkipsConfigErrorWithoutForeignEnvSource(t *testing.T) { + member, pod, pvc := adoptedLegacyFixture(t, createContainerConfigError) + pod.Spec.Containers[0].EnvFrom = nil + c, _ := newTestClient(t, member, pod, pvc) + r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} + + got, _, err := r.podToRecreate(context.Background(), member) + if err != nil { + t.Fatalf("podToRecreate: %v", err) + } + if got != nil { + t.Fatalf("a config error with no foreign env source must not trigger recreation") + } +} + +func TestForeignEnvSources(t *testing.T) { + cmRef := func(name string) corev1.EnvFromSource { + return corev1.EnvFromSource{ConfigMapRef: &corev1.ConfigMapEnvSource{LocalObjectReference: corev1.LocalObjectReference{Name: name}}} + } + secretKey := func(name string) corev1.EnvVar { + return corev1.EnvVar{Name: "V", ValueFrom: &corev1.EnvVarSource{SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: name}, Key: "k", + }}} + } + podWith := func(name string, envFrom []corev1.EnvFromSource, env []corev1.EnvVar) *corev1.Pod { + return &corev1.Pod{Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: name, EnvFrom: envFrom, Env: env}}}} + } + + live := podWith("etcd", []corev1.EnvFromSource{cmRef("legacy"), cmRef("shared")}, []corev1.EnvVar{secretKey("creds")}) + desired := podWith("etcd", []corev1.EnvFromSource{cmRef("shared")}, nil) + got := foreignEnvSources(live, desired) + want := []string{"ConfigMap/legacy", "Secret/creds"} + if !equality.Semantic.DeepEqual(got, want) { + t.Fatalf("foreignEnvSources = %v, want %v", got, want) + } + + if got := foreignEnvSources(podWith("sidecar", []corev1.EnvFromSource{cmRef("legacy")}, nil), desired); len(got) != 0 { + t.Fatalf("only the etcd container counts; got %v", got) + } +} + // Replacing the terminal Pod flips MemberReady=False in the same pass, so a // member whose re-creation then fails (missing TLS Secret) does not sit at // Ready=True with no Pod. From 48472ded70fe08cdcf64094274599d81712d4ab2 Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Tue, 13 Oct 2026 17:01:42 +0400 Subject: [PATCH 2/3] fix(controllers): cover init containers and bound adopted-Pod recreation A legacy Pod bootstrapped from a restore also runs a restore-agent init container with envFrom on the cluster-state ConfigMap. After a reboot that init container waits in CreateContainerConfigError while etcd waits in PodInitializing, so checking only the etcd container missed it. Scan every init and main container, for both the waiting state and the env sources, and compare against every container buildPod emits, so the S3 credentials Secret of a restore seed is never treated as foreign. Limit the branch to Pods that still carry the StatefulSet pod-name label. Only adopted legacy Pods have it, and buildPod never sets it, so each Pod is recreated at most once. A mutating webhook that injects a missing env source into an operator-built Pod therefore cannot cause a recreate loop. Adoption is PVC-only, so memory-backed members never take this branch. Assisted-by: LLM Signed-off-by: Andrey Kolkov --- controllers/etcdmember_controller.go | 92 +++++++----- controllers/etcdmember_controller_test.go | 173 +++++++++++++++------- 2 files changed, 171 insertions(+), 94 deletions(-) diff --git a/controllers/etcdmember_controller.go b/controllers/etcdmember_controller.go index 7ed1bf53..9633dd40 100644 --- a/controllers/etcdmember_controller.go +++ b/controllers/etcdmember_controller.go @@ -226,13 +226,17 @@ func podInTerminalPhase(pod *corev1.Pod) bool { // Two shapes qualify: // - a terminal phase, which the kubelet never restarts (a graceful node // shutdown leaves etcd Succeeded); -// - an etcd container stuck in CreateContainerConfigError on an env source -// buildPod does not emit. An adopted legacy Pod references the legacy -// cluster-state ConfigMap that adoption deletes, so its next container -// start fails with RestartCount 0 and a Running phase — invisible to both -// the terminal-phase check and the crash-loop trigger. Recreating from -// buildPod drops the reference; a source buildPod itself emits is -// excluded, since recreating would hit the same error forever. +// - an adopted legacy StatefulSet Pod with a container, init or main, stuck +// in CreateContainerConfigError on an env source buildPod does not emit. +// Adoption deletes the legacy cluster-state ConfigMap those containers +// pull env from, so the next start after a reboot fails with RestartCount +// 0 and a non-terminal phase, invisible to both the terminal-phase check +// and the crash-loop trigger. Recreating from buildPod drops the +// reference. Only legacy Pods qualify, because buildPod never sets the +// StatefulSet label: a replacement is recreated at most once, even if a +// mutating webhook keeps injecting a missing source into it. A source +// buildPod itself emits is excluded, since a replacement would hit the +// same error. func (r *EtcdMemberReconciler) podToRecreate(ctx context.Context, member *lll.EtcdMember) (*corev1.Pod, string, error) { pod := &corev1.Pod{} err := r.Get(ctx, types.NamespacedName{Namespace: member.Namespace, Name: member.Name}, pod) @@ -248,28 +252,43 @@ func (r *EtcdMemberReconciler) podToRecreate(ctx context.Context, member *lll.Et if podInTerminalPhase(pod) { return pod, terminalPodMessage(pod), nil } - if !etcdContainerConfigError(pod) { + if pod.Labels[legacyStatefulSetPodLabel] == "" { + return nil, "", nil + } + stuck := containerConfigErrors(pod) + if len(stuck) == 0 { return nil, "", nil } foreign := foreignEnvSources(pod, r.buildPod(member, false)) if len(foreign) == 0 { return nil, "", nil } - return pod, fmt.Sprintf("etcd container cannot be created (CreateContainerConfigError) and references "+ - "env sources the operator does not manage: %s; deleted for recreation", strings.Join(foreign, ", ")), nil + return pod, fmt.Sprintf("adopted pod cannot create container %s (CreateContainerConfigError) and references "+ + "env sources the operator does not manage: %s; deleted for recreation", + strings.Join(stuck, ", "), strings.Join(foreign, ", ")), nil } -func etcdContainerConfigError(pod *corev1.Pod) bool { - for _, cs := range pod.Status.ContainerStatuses { - if cs.Name == "etcd" { - return cs.State.Waiting != nil && cs.State.Waiting.Reason == "CreateContainerConfigError" +// legacyStatefulSetPodLabel is set by the StatefulSet controller on every Pod +// it creates, so only Pods adopted from the legacy operator carry it. +const legacyStatefulSetPodLabel = "statefulset.kubernetes.io/pod-name" + +// containerConfigErrors names the init and main containers waiting on +// CreateContainerConfigError, sorted. +func containerConfigErrors(pod *corev1.Pod) []string { + var out []string + for _, statuses := range [][]corev1.ContainerStatus{pod.Status.InitContainerStatuses, pod.Status.ContainerStatuses} { + for _, cs := range statuses { + if w := cs.State.Waiting; w != nil && w.Reason == "CreateContainerConfigError" { + out = append(out, cs.Name) + } } } - return false + sort.Strings(out) + return out } -// foreignEnvSources lists the ConfigMaps and Secrets the live Pod's etcd -// container pulls env from that the desired Pod's does not, sorted. +// foreignEnvSources lists the ConfigMaps and Secrets the live Pod's init and +// main containers pull env from that the desired Pod's do not, sorted. func foreignEnvSources(live, desired *corev1.Pod) []string { want := envSources(desired) var out []string @@ -284,27 +303,26 @@ func foreignEnvSources(live, desired *corev1.Pod) []string { func envSources(pod *corev1.Pod) map[string]bool { srcs := map[string]bool{} - for _, c := range pod.Spec.Containers { - if c.Name != "etcd" { - continue - } - for _, ef := range c.EnvFrom { - if ef.ConfigMapRef != nil { - srcs["ConfigMap/"+ef.ConfigMapRef.Name] = true - } - if ef.SecretRef != nil { - srcs["Secret/"+ef.SecretRef.Name] = true - } - } - for _, e := range c.Env { - if e.ValueFrom == nil { - continue - } - if ref := e.ValueFrom.ConfigMapKeyRef; ref != nil { - srcs["ConfigMap/"+ref.Name] = true + for _, containers := range [][]corev1.Container{pod.Spec.InitContainers, pod.Spec.Containers} { + for _, c := range containers { + for _, ef := range c.EnvFrom { + if ef.ConfigMapRef != nil { + srcs["ConfigMap/"+ef.ConfigMapRef.Name] = true + } + if ef.SecretRef != nil { + srcs["Secret/"+ef.SecretRef.Name] = true + } } - if ref := e.ValueFrom.SecretKeyRef; ref != nil { - srcs["Secret/"+ref.Name] = true + for _, e := range c.Env { + if e.ValueFrom == nil { + continue + } + if ref := e.ValueFrom.ConfigMapKeyRef; ref != nil { + srcs["ConfigMap/"+ref.Name] = true + } + if ref := e.ValueFrom.SecretKeyRef; ref != nil { + srcs["Secret/"+ref.Name] = true + } } } } diff --git a/controllers/etcdmember_controller_test.go b/controllers/etcdmember_controller_test.go index 0649aa61..881a4f25 100644 --- a/controllers/etcdmember_controller_test.go +++ b/controllers/etcdmember_controller_test.go @@ -2474,9 +2474,9 @@ func TestPodToRecreate_SkipsPodOwnedByAnotherMember(t *testing.T) { } } -// adoptedLegacyFixture is a PVC-backed member whose Pod still carries the -// legacy operator's envFrom on the cluster-state ConfigMap, with the etcd -// container in the given state. +// adoptedLegacyFixture is a PVC-backed member whose adopted StatefulSet Pod +// still carries the legacy operator's envFrom on the cluster-state ConfigMap, +// with the etcd container in the given state. func adoptedLegacyFixture(t *testing.T, state corev1.ContainerState) (*lll.EtcdMember, *corev1.Pod, *corev1.PersistentVolumeClaim) { t.Helper() tru := true @@ -2497,12 +2497,13 @@ func adoptedLegacyFixture(t *testing.T, state corev1.ContainerState) (*lll.EtcdM Status: lll.EtcdMemberStatus{PodName: "etcd-1", PodUID: "old-uid", PVCName: "data-etcd-1"}, } pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: "etcd-1", Namespace: "ns", UID: types.UID("old-uid"), OwnerReferences: owner}, + ObjectMeta: metav1.ObjectMeta{ + Name: "etcd-1", Namespace: "ns", UID: types.UID("old-uid"), OwnerReferences: owner, + Labels: map[string]string{legacyStatefulSetPodLabel: "etcd-1"}, + }, Spec: corev1.PodSpec{Containers: []corev1.Container{{ - Name: "etcd", - EnvFrom: []corev1.EnvFromSource{{ - ConfigMapRef: &corev1.ConfigMapEnvSource{LocalObjectReference: corev1.LocalObjectReference{Name: "etcd-cluster-state"}}, - }}, + Name: "etcd", + EnvFrom: []corev1.EnvFromSource{legacyStateEnvFrom}, }}}, Status: corev1.PodStatus{ Phase: corev1.PodRunning, @@ -2515,10 +2516,30 @@ func adoptedLegacyFixture(t *testing.T, state corev1.ContainerState) (*lll.EtcdM return member, pod, pvc } -var createContainerConfigError = corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{ - Reason: "CreateContainerConfigError", - Message: `configmap "etcd-cluster-state" not found`, -}} +var ( + legacyStateEnvFrom = corev1.EnvFromSource{ + ConfigMapRef: &corev1.ConfigMapEnvSource{LocalObjectReference: corev1.LocalObjectReference{Name: "etcd-cluster-state"}}, + } + createContainerConfigError = corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{ + Reason: "CreateContainerConfigError", + Message: `configmap "etcd-cluster-state" not found`, + }} + podInitializing = corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "PodInitializing"}} +) + +func assertPodToRecreate(t *testing.T, member *lll.EtcdMember, pod *corev1.Pod, pvc *corev1.PersistentVolumeClaim, want bool) string { + t.Helper() + c, _ := newTestClient(t, member, pod, pvc) + r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} + got, msg, err := r.podToRecreate(context.Background(), member) + if err != nil { + t.Fatalf("podToRecreate: %v", err) + } + if (got != nil) != want { + t.Fatalf("podToRecreate recreate = %v, want %v (msg %q)", got != nil, want, msg) + } + return msg +} // An adopted Pod whose container cannot be created because of the legacy // ConfigMap reference is recreated from buildPod against the same PVC. @@ -2549,72 +2570,110 @@ func TestReconcile_RecreatesPodStuckOnForeignEnvSource(t *testing.T) { if fresh.UID == types.UID("old-uid") { t.Fatalf("Pod must be recreated with a new UID; still old-uid") } - for _, ctr := range fresh.Spec.Containers { - if len(ctr.EnvFrom) != 0 { - t.Fatalf("recreated Pod must not carry the legacy envFrom; got %+v", ctr.EnvFrom) - } + if len(envSources(fresh)) != 0 { + t.Fatalf("recreated Pod must not carry the legacy env sources; got %v", envSources(fresh)) + } + if fresh.Labels[legacyStatefulSetPodLabel] != "" { + t.Fatalf("recreated Pod must not carry the StatefulSet label, or it could be recreated again") } if !pvcOwnedBy(mustGet(t, c, "data-etcd-1", "ns", &corev1.PersistentVolumeClaim{}), member) { t.Fatalf("PVC must be preserved and still owned by the member") } } -// An adopted Pod that is running is left alone: the stale reference only -// matters once the container has to be created again. +// A legacy Pod bootstrapped from a restore also runs a restore-agent init +// container with the same envFrom; after a reboot that init container is the +// one stuck, while etcd waits in PodInitializing. +func TestPodToRecreate_StuckInitContainer(t *testing.T) { + member, pod, pvc := adoptedLegacyFixture(t, podInitializing) + pod.Spec.InitContainers = []corev1.Container{{Name: "restore-agent", EnvFrom: []corev1.EnvFromSource{legacyStateEnvFrom}}} + pod.Spec.Containers[0].EnvFrom = nil + pod.Status.Phase = corev1.PodPending + pod.Status.InitContainerStatuses = []corev1.ContainerStatus{{Name: "restore-agent", State: createContainerConfigError}} + + msg := assertPodToRecreate(t, member, pod, pvc, true) + if !strings.Contains(msg, "restore-agent") || !strings.Contains(msg, "ConfigMap/etcd-cluster-state") { + t.Fatalf("reason must name the stuck init container and the foreign source; got %q", msg) + } +} + func TestPodToRecreate_KeepsRunningAdoptedPod(t *testing.T) { member, pod, pvc := adoptedLegacyFixture(t, corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}) - c, _ := newTestClient(t, member, pod, pvc) - r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} + assertPodToRecreate(t, member, pod, pvc, false) +} - got, _, err := r.podToRecreate(context.Background(), member) - if err != nil { - t.Fatalf("podToRecreate: %v", err) - } - if got != nil { - t.Fatalf("a running adopted Pod must not be recreated") - } +// Any other wait is left to the existing paths: a crash loop belongs to the +// restart-count trigger. +func TestPodToRecreate_KeepsAdoptedPodInOtherWait(t *testing.T) { + member, pod, pvc := adoptedLegacyFixture(t, corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "CrashLoopBackOff"}}) + assertPodToRecreate(t, member, pod, pvc, false) +} + +// A Pod the operator built carries no StatefulSet label, so a mutating +// webhook injecting a missing source cannot drive a recreate loop. +func TestPodToRecreate_KeepsOperatorBuiltPod(t *testing.T) { + member, pod, pvc := adoptedLegacyFixture(t, createContainerConfigError) + pod.Labels = nil + assertPodToRecreate(t, member, pod, pvc, false) } -// A config error on a Pod the operator itself would build again is not -// recreated: the replacement would fail the same way, forever. +// A config error on a source the operator itself emits is not recreated: the +// replacement would fail the same way. func TestPodToRecreate_SkipsConfigErrorWithoutForeignEnvSource(t *testing.T) { member, pod, pvc := adoptedLegacyFixture(t, createContainerConfigError) pod.Spec.Containers[0].EnvFrom = nil - c, _ := newTestClient(t, member, pod, pvc) - r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} - - got, _, err := r.podToRecreate(context.Background(), member) - if err != nil { - t.Fatalf("podToRecreate: %v", err) - } - if got != nil { - t.Fatalf("a config error with no foreign env source must not trigger recreation") - } + assertPodToRecreate(t, member, pod, pvc, false) } func TestForeignEnvSources(t *testing.T) { - cmRef := func(name string) corev1.EnvFromSource { - return corev1.EnvFromSource{ConfigMapRef: &corev1.ConfigMapEnvSource{LocalObjectReference: corev1.LocalObjectReference{Name: name}}} - } - secretKey := func(name string) corev1.EnvVar { - return corev1.EnvVar{Name: "V", ValueFrom: &corev1.EnvVarSource{SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, Key: "k", - }}} + ref := corev1.LocalObjectReference{Name: "x"} + cases := []struct { + name string + c corev1.Container + want []string + }{ + {"envFrom ConfigMap", corev1.Container{EnvFrom: []corev1.EnvFromSource{{ConfigMapRef: &corev1.ConfigMapEnvSource{LocalObjectReference: ref}}}}, []string{"ConfigMap/x"}}, + {"envFrom Secret", corev1.Container{EnvFrom: []corev1.EnvFromSource{{SecretRef: &corev1.SecretEnvSource{LocalObjectReference: ref}}}}, []string{"Secret/x"}}, + {"ConfigMap key", corev1.Container{Env: []corev1.EnvVar{{Name: "V", ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{LocalObjectReference: ref, Key: "k"}}}}}, []string{"ConfigMap/x"}}, + {"Secret key", corev1.Container{Env: []corev1.EnvVar{{Name: "V", ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: ref, Key: "k"}}}}}, []string{"Secret/x"}}, + {"plain value", corev1.Container{Env: []corev1.EnvVar{{Name: "V", Value: "v"}}}, nil}, } - podWith := func(name string, envFrom []corev1.EnvFromSource, env []corev1.EnvVar) *corev1.Pod { - return &corev1.Pod{Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: name, EnvFrom: envFrom, Env: env}}}} + for _, tc := range cases { + for _, init := range []bool{false, true} { + live := &corev1.Pod{} + if init { + live.Spec.InitContainers = []corev1.Container{tc.c} + } else { + live.Spec.Containers = []corev1.Container{tc.c} + } + if got := foreignEnvSources(live, &corev1.Pod{}); !equality.Semantic.DeepEqual(got, tc.want) { + t.Fatalf("%s (init=%v): foreignEnvSources = %v, want %v", tc.name, init, got, tc.want) + } + if got := foreignEnvSources(live, live); len(got) != 0 { + t.Fatalf("%s (init=%v): a source the desired Pod also uses is not foreign; got %v", tc.name, init, got) + } + } } +} - live := podWith("etcd", []corev1.EnvFromSource{cmRef("legacy"), cmRef("shared")}, []corev1.EnvVar{secretKey("creds")}) - desired := podWith("etcd", []corev1.EnvFromSource{cmRef("shared")}, nil) - got := foreignEnvSources(live, desired) - want := []string{"ConfigMap/legacy", "Secret/creds"} - if !equality.Semantic.DeepEqual(got, want) { - t.Fatalf("foreignEnvSources = %v, want %v", got, want) +// The restore init container buildPod emits pulls S3 credentials from a +// Secret; a live Pod with the same container must not count it as foreign. +func TestForeignEnvSources_RestoreSeedIsNotForeign(t *testing.T) { + member, _, _ := adoptedLegacyFixture(t, createContainerConfigError) + member.Spec.Bootstrap = true + member.Spec.Restore = &lll.RestoreSpec{Source: lll.SnapshotLocation{S3: &lll.S3SnapshotLocation{ + Endpoint: "https://s3", Bucket: "b", Key: "k", + CredentialsSecretRef: corev1.LocalObjectReference{Name: "s3-creds"}, + }}} + r := &EtcdMemberReconciler{OperatorImage: "op:img"} + desired := r.buildPod(member, false) + if !envSources(desired)["Secret/s3-creds"] { + t.Fatalf("fixture must exercise a restore Pod whose init container uses the S3 Secret; got %v", envSources(desired)) } - - if got := foreignEnvSources(podWith("sidecar", []corev1.EnvFromSource{cmRef("legacy")}, nil), desired); len(got) != 0 { - t.Fatalf("only the etcd container counts; got %v", got) + if got := foreignEnvSources(r.buildPod(member, false), desired); len(got) != 0 { + t.Fatalf("buildPod's own sources must not be foreign; got %v", got) } } From 3ac0e7b2be8583a5936e433aa956aecbfc7adeb6 Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Tue, 13 Oct 2026 17:50:00 +0400 Subject: [PATCH 3/3] fix(controllers): never recreate an adopted Pod whose etcd is running The recreation branch is not quorum-gated, which is safe only while etcd in the Pod is down anyway. Counting every container's config error broke that: a legacy sidecar, or a native sidecar init container, stuck on a missing env source would delete a Pod whose etcd still serves, and losing that member with another one already down costs quorum. Count only the etcd container and init containers, and leave the Pod alone while etcd is running. buildPod now also drops the StatefulSet pod-name label, so additionalMetadata cannot make an operator-built Pod look adopted and re-enable recreation on it. Assisted-by: LLM Signed-off-by: Andrey Kolkov --- controllers/etcdmember_controller.go | 35 ++++++++--- controllers/etcdmember_controller_test.go | 74 +++++++++++++++++++---- 2 files changed, 88 insertions(+), 21 deletions(-) diff --git a/controllers/etcdmember_controller.go b/controllers/etcdmember_controller.go index 9633dd40..59c511f9 100644 --- a/controllers/etcdmember_controller.go +++ b/controllers/etcdmember_controller.go @@ -226,13 +226,16 @@ func podInTerminalPhase(pod *corev1.Pod) bool { // Two shapes qualify: // - a terminal phase, which the kubelet never restarts (a graceful node // shutdown leaves etcd Succeeded); -// - an adopted legacy StatefulSet Pod with a container, init or main, stuck -// in CreateContainerConfigError on an env source buildPod does not emit. +// - an adopted legacy StatefulSet Pod whose etcd container is not running +// and whose etcd or init container is stuck in CreateContainerConfigError +// on an env source buildPod does not emit. // Adoption deletes the legacy cluster-state ConfigMap those containers // pull env from, so the next start after a reboot fails with RestartCount // 0 and a non-terminal phase, invisible to both the terminal-phase check // and the crash-loop trigger. Recreating from buildPod drops the -// reference. Only legacy Pods qualify, because buildPod never sets the +// reference. A stuck sidecar never qualifies on its own: deleting a Pod +// whose etcd still serves would cost a member, and this branch is not +// quorum-gated. Only legacy Pods qualify, because buildPod never sets the // StatefulSet label: a replacement is recreated at most once, even if a // mutating webhook keeps injecting a missing source into it. A source // buildPod itself emits is excluded, since a replacement would hit the @@ -272,15 +275,25 @@ func (r *EtcdMemberReconciler) podToRecreate(ctx context.Context, member *lll.Et // it creates, so only Pods adopted from the legacy operator carry it. const legacyStatefulSetPodLabel = "statefulset.kubernetes.io/pod-name" -// containerConfigErrors names the init and main containers waiting on -// CreateContainerConfigError, sorted. +// containerConfigErrors names the init containers and the etcd container +// waiting on CreateContainerConfigError, sorted. It returns nil while etcd is +// running, whatever state the other containers are in. func containerConfigErrors(pod *corev1.Pod) []string { var out []string - for _, statuses := range [][]corev1.ContainerStatus{pod.Status.InitContainerStatuses, pod.Status.ContainerStatuses} { - for _, cs := range statuses { - if w := cs.State.Waiting; w != nil && w.Reason == "CreateContainerConfigError" { - out = append(out, cs.Name) - } + for _, cs := range pod.Status.ContainerStatuses { + if cs.Name != "etcd" { + continue + } + if cs.State.Running != nil { + return nil + } + if w := cs.State.Waiting; w != nil && w.Reason == "CreateContainerConfigError" { + out = append(out, cs.Name) + } + } + for _, cs := range pod.Status.InitContainerStatuses { + if w := cs.State.Waiting; w != nil && w.Reason == "CreateContainerConfigError" { + out = append(out, cs.Name) } } sort.Strings(out) @@ -844,6 +857,8 @@ func (r *EtcdMemberReconciler) buildPod(member *lll.EtcdMember, clusterFormed bo labels[LabelRole] = RoleVoter } labels, annotations := applyAdditionalMetadata(labels, nil, member.Spec.AdditionalMetadata) + // podToRecreate treats this label as proof of an adopted legacy Pod. + delete(labels, legacyStatefulSetPodLabel) // Data dir defaults to the volume root; adopted legacy members carry an // AnnDataDirSubPath annotation (e.g. "default.etcd") so a replacement diff --git a/controllers/etcdmember_controller_test.go b/controllers/etcdmember_controller_test.go index 881a4f25..c70d6a2f 100644 --- a/controllers/etcdmember_controller_test.go +++ b/controllers/etcdmember_controller_test.go @@ -2658,22 +2658,74 @@ func TestForeignEnvSources(t *testing.T) { } } -// The restore init container buildPod emits pulls S3 credentials from a -// Secret; a live Pod with the same container must not count it as foreign. -func TestForeignEnvSources_RestoreSeedIsNotForeign(t *testing.T) { - member, _, _ := adoptedLegacyFixture(t, createContainerConfigError) +// A restore seed's own restore-agent pulls S3 credentials from a Secret +// buildPod emits too, so a config error on it alone is not foreign: a +// replacement would fail the same way. +func TestPodToRecreate_SkipsRestoreSeedOwnSecret(t *testing.T) { + member, pod, pvc := adoptedLegacyFixture(t, podInitializing) member.Spec.Bootstrap = true member.Spec.Restore = &lll.RestoreSpec{Source: lll.SnapshotLocation{S3: &lll.S3SnapshotLocation{ Endpoint: "https://s3", Bucket: "b", Key: "k", CredentialsSecretRef: corev1.LocalObjectReference{Name: "s3-creds"}, }}} - r := &EtcdMemberReconciler{OperatorImage: "op:img"} - desired := r.buildPod(member, false) - if !envSources(desired)["Secret/s3-creds"] { - t.Fatalf("fixture must exercise a restore Pod whose init container uses the S3 Secret; got %v", envSources(desired)) - } - if got := foreignEnvSources(r.buildPod(member, false), desired); len(got) != 0 { - t.Fatalf("buildPod's own sources must not be foreign; got %v", got) + pod.Spec.Containers[0].EnvFrom = nil + pod.Spec.InitContainers = []corev1.Container{{Name: "restore-agent", Env: []corev1.EnvVar{{ + Name: "AWS_ACCESS_KEY_ID", ValueFrom: &corev1.EnvVarSource{SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "s3-creds"}, Key: "AWS_ACCESS_KEY_ID", + }}, + }}}} + pod.Status.Phase = corev1.PodPending + pod.Status.InitContainerStatuses = []corev1.ContainerStatus{{Name: "restore-agent", State: createContainerConfigError}} + assertPodToRecreate(t, member, pod, pvc, false) + + // Control: the same Pod with a source buildPod does not emit is recreated. + pod.Spec.InitContainers[0].EnvFrom = []corev1.EnvFromSource{legacyStateEnvFrom} + assertPodToRecreate(t, member, pod, pvc, true) +} + +// A legacy sidecar stuck on a missing source never deletes a Pod whose etcd +// still serves: the branch is not quorum-gated. +func TestPodToRecreate_KeepsPodWithRunningEtcdAndStuckSidecar(t *testing.T) { + member, pod, pvc := adoptedLegacyFixture(t, corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}) + pod.Spec.Containers = append(pod.Spec.Containers, corev1.Container{Name: "exporter", EnvFrom: []corev1.EnvFromSource{{ + SecretRef: &corev1.SecretEnvSource{LocalObjectReference: corev1.LocalObjectReference{Name: "exporter-creds"}}, + }}}) + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, + corev1.ContainerStatus{Name: "exporter", State: createContainerConfigError}) + assertPodToRecreate(t, member, pod, pvc, false) + + // Control: once etcd is stuck as well, the Pod is recreated. + pod.Status.ContainerStatuses[0].State = createContainerConfigError + assertPodToRecreate(t, member, pod, pvc, true) +} + +// A native sidecar is an init container that keeps running beside etcd; its +// config error must not delete a Pod whose etcd still serves either. +func TestPodToRecreate_KeepsPodWithRunningEtcdAndStuckNativeSidecar(t *testing.T) { + always := corev1.ContainerRestartPolicyAlways + member, pod, pvc := adoptedLegacyFixture(t, corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}) + pod.Spec.InitContainers = []corev1.Container{{Name: "proxy", RestartPolicy: &always, EnvFrom: []corev1.EnvFromSource{legacyStateEnvFrom}}} + pod.Status.InitContainerStatuses = []corev1.ContainerStatus{{Name: "proxy", State: createContainerConfigError}} + assertPodToRecreate(t, member, pod, pvc, false) +} + +// Only etcd and init containers can trigger recreation: a stuck sidecar next +// to an etcd that is down for another reason is left to the other paths. +func TestPodToRecreate_IgnoresStuckSidecarWhenEtcdDownOtherwise(t *testing.T) { + member, pod, pvc := adoptedLegacyFixture(t, corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "CrashLoopBackOff"}}) + pod.Spec.Containers = append(pod.Spec.Containers, corev1.Container{Name: "exporter", EnvFrom: []corev1.EnvFromSource{legacyStateEnvFrom}}) + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, + corev1.ContainerStatus{Name: "exporter", State: createContainerConfigError}) + assertPodToRecreate(t, member, pod, pvc, false) +} + +// additionalMetadata cannot make an operator-built Pod look adopted. +func TestBuildPod_DropsLegacyStatefulSetLabel(t *testing.T) { + member, _, _ := adoptedLegacyFixture(t, createContainerConfigError) + member.Spec.AdditionalMetadata = &lll.AdditionalMetadata{Labels: map[string]string{legacyStatefulSetPodLabel: "etcd-1"}} + pod := (&EtcdMemberReconciler{}).buildPod(member, false) + if _, ok := pod.Labels[legacyStatefulSetPodLabel]; ok { + t.Fatalf("buildPod must never emit %s; got labels %v", legacyStatefulSetPodLabel, pod.Labels) } }