From e81d7ba4549f394ce9a7785ec7a44afc35373fbc Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Tue, 28 Jul 2026 17:56:04 +0200 Subject: [PATCH 1/2] feat(controllers): stop cascade-deleting data volumes with their member MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A member's data PVC carried a controller owner reference, which does two jobs at once: it marks the owner, and it makes the volume cascade-delete with it. The second is a liability for the only copy of a database — every route that removes an EtcdMember takes the data with it, irreversibly on a Delete-reclaim StorageClass: a stray kubectl delete, a GitOps prune, a CRD replacement during an upgrade, a chart rollback that drops the CRs. None of those say anything about whether the data still matters. Separate identity from lifetime, following the model CloudNativePG uses for its instance volumes: - Identity moves to an etcd-operator.cozystack.io/member-uid annotation. Volumes stamped by an older build or by the migration tool are adopted on the first reconcile: annotation stamped, owner reference stripped. - Deletion becomes explicit, in the two places the operator itself retires a member and the data is replicated or unusable: scale-down (contents live on the remaining peers, MemberRemove already ran) and crash-loop replacement (member cannot boot on its data dir, quorum-gated). Everything else leaves the volume in place, including deletion of the EtcdCluster itself. What is left behind is visible rather than silent: each volume carries etcd-operator.cozystack.io/pvc-status with initializing / ready / detached, so a leftover is one kubectl get away instead of a cross-reference against the member list. Assisted-By: Claude Signed-off-by: Andrei Kvapil --- controllers/etcdcluster_controller.go | 13 ++ controllers/etcdcluster_controller_test.go | 104 +++++++++ controllers/etcdmember_controller.go | 132 +++++++++-- controllers/etcdmember_controller_test.go | 244 ++++++++++++++++++++- controllers/helpers.go | 91 ++++++++ docs/concepts.md | 38 +++- docs/installation.md | 2 +- docs/operations.md | 12 +- 8 files changed, 597 insertions(+), 39 deletions(-) diff --git a/controllers/etcdcluster_controller.go b/controllers/etcdcluster_controller.go index 32ab7632..58bf9741 100644 --- a/controllers/etcdcluster_controller.go +++ b/controllers/etcdcluster_controller.go @@ -1309,6 +1309,19 @@ func (r *EtcdClusterReconciler) scaleDown( if err := r.Delete(ctx, &victim); err != nil && !errors.IsNotFound(err) { return ctrl.Result{}, err } + // One of the two places the operator reclaims a data volume. Deleting + // the member above runs MemberRemove through its finalizer, so by the + // time this volume is dropped its contents are already replicated on + // the remaining peers — this is the deliberate shrink the user asked + // for, not an accident, so the storage goes back to the pool. + // + // Note this is skipped for the 1→0 pause handled above: that member is + // parked dormant, CR and volume intact, and resume mounts it again. + if victim.Spec.Storage.Medium != lll.StorageMediumMemory { + if err := deleteMemberPVC(ctx, r.Client, victim.Namespace, victim.Name); err != nil { + return ctrl.Result{}, err + } + } return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } diff --git a/controllers/etcdcluster_controller_test.go b/controllers/etcdcluster_controller_test.go index dcf8bc38..6ab0cd1e 100644 --- a/controllers/etcdcluster_controller_test.go +++ b/controllers/etcdcluster_controller_test.go @@ -961,6 +961,110 @@ func TestScaleUp_WaitsForInFlightDeletion(t *testing.T) { } } +// scaleDownFixture builds an n-member cluster ready to shrink by one, with a +// data volume per member. +func scaleDownFixture(t *testing.T, replicas int32, members int) (*lll.EtcdCluster, []client.Object) { + t.Helper() + cluster := &lll.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}, + Spec: lll.EtcdClusterSpec{ + Replicas: ptrInt32(replicas), Version: "3.5.17", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + }, + Status: lll.EtcdClusterStatus{ + ClusterToken: "ns-test-x", ClusterID: "deadbeef", + Observed: &lll.ObservedClusterSpec{ + Replicas: replicas, Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + }, + ProgressDeadline: &metav1.Time{Time: metav1.Now().Add(time.Hour)}, + }, + } + objs := []client.Object{cluster} + now := metav1.Now() + for i := 0; i < members; i++ { + name := fmt.Sprintf("test-%d", i) + objs = append(objs, &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, Namespace: "ns", Labels: memberLabels("test", name), + CreationTimestamp: metav1.NewTime(now.Add(time.Duration(i) * time.Minute)), + }, + Spec: lll.EtcdMemberSpec{ + ClusterName: "test", InitialCluster: "x", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + }, + Status: lll.EtcdMemberStatus{ + PodName: name, MemberID: fmt.Sprintf("id-%d", i), PVCName: memberPVCName(name), + Conditions: []metav1.Condition{{ + Type: lll.MemberReady, Status: metav1.ConditionTrue, Reason: "PodReady", LastTransitionTime: now, + }}, + }, + }) + objs = append(objs, &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: memberPVCName(name), Namespace: "ns", Labels: memberLabels("test", name), + }, + }) + } + return cluster, objs +} + +// TestScaleDown_ReclaimsTheVictimsVolume: shrinking a cluster is a deliberate +// instruction, and the departing member's contents are already replicated on +// the peers that remain (its finalizer runs MemberRemove first). Leaving the +// volume behind would quietly accumulate storage nobody asked to keep, so this +// is one of the two paths that reclaims it — explicitly, not by cascade. +func TestScaleDown_ReclaimsTheVictimsVolume(t *testing.T) { + ctx := context.Background() + _, objs := scaleDownFixture(t, 2, 3) + c, _ := newTestClient(t, objs...) + fe := newFakeEtcd(0xdeadbeef) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factoryReturning(fe)} + + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}); err != nil { + t.Fatalf("Reconcile: %v", err) + } + + // Victim selection is newest-first, so test-2 goes. + if err := c.Get(ctx, types.NamespacedName{Name: memberPVCName("test-2"), Namespace: "ns"}, + &corev1.PersistentVolumeClaim{}); !apierrors.IsNotFound(err) { + t.Fatalf("victim's volume should have been reclaimed; got %v", err) + } + // The survivors keep theirs. + for _, name := range []string{"test-0", "test-1"} { + if err := c.Get(ctx, types.NamespacedName{Name: memberPVCName(name), Namespace: "ns"}, + &corev1.PersistentVolumeClaim{}); err != nil { + t.Fatalf("surviving member %s lost its volume: %v", name, err) + } + } +} + +// TestScaleDown_PauseKeepsTheVolume: scaling to zero parks the last member +// dormant rather than deleting it — the whole point being that the data +// survives until the cluster is resumed. The reclaim path must not fire here. +func TestScaleDown_PauseKeepsTheVolume(t *testing.T) { + ctx := context.Background() + _, objs := scaleDownFixture(t, 0, 1) + c, _ := newTestClient(t, objs...) + fe := newFakeEtcd(0xdeadbeef) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factoryReturning(fe)} + + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}); err != nil { + t.Fatalf("Reconcile: %v", err) + } + + if err := c.Get(ctx, types.NamespacedName{Name: memberPVCName("test-0"), Namespace: "ns"}, + &corev1.PersistentVolumeClaim{}); err != nil { + t.Fatalf("paused cluster must keep its data volume; got %v", err) + } + member := &lll.EtcdMember{} + if err := c.Get(ctx, types.NamespacedName{Name: "test-0", Namespace: "ns"}, member); err != nil { + t.Fatalf("Get member: %v", err) + } + if !member.Spec.Dormant { + t.Fatalf("last member should have been parked dormant, not deleted") + } +} + // TestScaleDown_PicksMostRecentlyCreatedVictim: with apiserver-assigned // names there is no ordinal to scale down by; victim selection sorts by // CreationTimestamp (newest first). This naturally retires the most diff --git a/controllers/etcdmember_controller.go b/controllers/etcdmember_controller.go index c87c69ab..7ebb0498 100644 --- a/controllers/etcdmember_controller.go +++ b/controllers/etcdmember_controller.go @@ -116,6 +116,12 @@ func (r *EtcdMemberReconciler) Reconcile(ctx context.Context, req ctrl.Request) log.Error(err, "failed to delete pod for dormant member") return ctrl.Result{}, err } + // The volume is intact but nothing mounts it while paused. Marking + // it detached is what makes "paused cluster" distinguishable from + // "leftover volume" in a plain `kubectl get pvc -o custom-columns`. + if err := r.setPVCStatus(ctx, member, PVCStatusDetached); err != nil { + log.Error(err, "failed to mark PVC detached") + } // Persist the cleared PodName + a Paused condition. updateStatus() // is for the running flow (reads pod, derives Ready); for dormant // we know the answer directly. Idempotent — setMemberCondition @@ -357,19 +363,24 @@ func (r *EtcdMemberReconciler) ensurePVC(ctx context.Context, member *lll.EtcdMe return nil } - pvcName := "data-" + member.Name + pvcName := memberPVCName(member.Name) pvc := &corev1.PersistentVolumeClaim{} err := r.Get(ctx, types.NamespacedName{Namespace: member.Namespace, Name: pvcName}, pvc) if err == nil { - // The PVC stays owned by this EtcdMember across pause/resume - // (the cluster controller flips Spec.Dormant rather than - // deleting the member CR), so ownership never moves. If the - // PVC's controller-owner doesn't match this member's UID, it - // belongs to something else and we must refuse — silently - // inheriting another member's data dir would crashloop the - // pod when etcd notices a removed memberID in the WAL. - if !pvcOwnedBy(pvc, member) { - return fmt.Errorf("PVC %q is owned by a different EtcdMember; awaiting GC before reuse", pvcName) + // The volume stays with this EtcdMember across pause/resume (the + // cluster controller flips Spec.Dormant rather than deleting the + // member CR), so the marker never moves. A volume carrying a + // different member's marker belongs to something else and must be + // refused — silently inheriting another member's data dir would + // crash-loop the pod when etcd notices a removed memberID in the WAL. + if !pvcBelongsTo(pvc, member) { + return fmt.Errorf("PVC %q belongs to a different EtcdMember; awaiting cleanup before reuse", pvcName) + } + // Migrate volumes that predate the annotation: stamp the marker and + // drop the controller owner reference, which is what made a deleted + // member take its data with it. + if err := r.adoptLegacyPVC(ctx, pvc, member); err != nil { + return err } member.Status.PVCName = pvcName return nil @@ -378,11 +389,16 @@ func (r *EtcdMemberReconciler) ensurePVC(ctx context.Context, member *lll.EtcdMe return err } - // The PVC is operator-created and operator-owned, so it carries the - // cluster's additionalMetadata like every other child object (backup - // tooling and cost-allocation selectors target PVCs specifically). + // The PVC is operator-created, so it carries the cluster's + // additionalMetadata like every other child object (backup tooling and + // cost-allocation selectors target PVCs specifically). pvcLabels, pvcAnnotations := applyAdditionalMetadata( memberLabels(member.Spec.ClusterName, member.Name), nil, member.Spec.AdditionalMetadata) + if pvcAnnotations == nil { + pvcAnnotations = map[string]string{} + } + pvcAnnotations[AnnPVCMemberUID] = string(member.UID) + pvcAnnotations[AnnPVCStatus] = PVCStatusInitializing pvc = &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Name: pvcName, @@ -400,9 +416,9 @@ func (r *EtcdMemberReconciler) ensurePVC(ctx context.Context, member *lll.EtcdMe }, }, } - if err := controllerutil.SetControllerReference(member, pvc, r.Scheme); err != nil { - return err - } + // No controller owner reference on purpose: the volume must outlive + // anything that removes its EtcdMember without meaning to discard the + // data. See AnnPVCMemberUID and deleteMemberPVC. if err := r.Create(ctx, pvc); err != nil { return err } @@ -410,19 +426,69 @@ func (r *EtcdMemberReconciler) ensurePVC(ctx context.Context, member *lll.EtcdMe return nil } -// pvcOwnedBy returns true only if the PVC carries an EtcdMember owner -// reference whose UID matches this member — i.e. we created it. PVCs with -// no owner refs, or with owner refs pointing at anything else, are refused. -func pvcOwnedBy(pvc *corev1.PersistentVolumeClaim, member *lll.EtcdMember) bool { +// adoptLegacyPVC brings a pre-existing volume onto the annotation-based +// ownership model: stamp the member-UID marker, and strip the controller +// owner reference so the volume no longer cascade-deletes with its member. +// +// Runs against volumes created by an older operator build and against those +// stamped by the in-place migration tool. Idempotent, and a no-op once a +// volume has been migrated. +func (r *EtcdMemberReconciler) adoptLegacyPVC( + ctx context.Context, + pvc *corev1.PersistentVolumeClaim, + member *lll.EtcdMember, +) error { + kept := make([]metav1.OwnerReference, 0, len(pvc.OwnerReferences)) for _, o := range pvc.OwnerReferences { if o.Kind == "EtcdMember" && o.UID == member.UID { - return true + continue } + kept = append(kept, o) } - return false + _, marked := pvc.Annotations[AnnPVCMemberUID] + if marked && len(kept) == len(pvc.OwnerReferences) { + return nil + } + + original := pvc.DeepCopy() + if pvc.Annotations == nil { + pvc.Annotations = map[string]string{} + } + pvc.Annotations[AnnPVCMemberUID] = string(member.UID) + pvc.OwnerReferences = kept + if err := r.Patch(ctx, pvc, client.MergeFrom(original)); err != nil { + return fmt.Errorf("migrate PVC %q to annotation ownership: %w", pvc.Name, err) + } + return nil } -// podOwnedBy mirrors pvcOwnedBy: true only when the Pod's owner refs +// setPVCStatus records the volume's lifecycle marker (AnnPVCStatus). Purely +// observational — see the constant's doc comment. Best-effort: a failure here +// must never block the member's reconcile, so callers log and continue. +func (r *EtcdMemberReconciler) setPVCStatus(ctx context.Context, member *lll.EtcdMember, status string) error { + if member.Spec.Storage.Medium == lll.StorageMediumMemory { + return nil + } + pvc := &corev1.PersistentVolumeClaim{} + name := types.NamespacedName{Namespace: member.Namespace, Name: memberPVCName(member.Name)} + if err := r.Get(ctx, name, pvc); err != nil { + if errors.IsNotFound(err) { + return nil + } + return err + } + if pvc.Annotations[AnnPVCStatus] == status { + return nil + } + original := pvc.DeepCopy() + if pvc.Annotations == nil { + pvc.Annotations = map[string]string{} + } + pvc.Annotations[AnnPVCStatus] = status + return r.Patch(ctx, pvc, client.MergeFrom(original)) +} + +// podOwnedBy: true only when the Pod's owner refs // point at this EtcdMember by UID. Less load-bearing than the PVC // check (Pod corruption is recoverable; replacing one is cheap), but // adopting a leftover Pod from a prior cluster generation would leave @@ -1064,6 +1130,14 @@ func (r *EtcdMemberReconciler) updateStatus(ctx context.Context, member *lll.Etc if err := r.Delete(ctx, member); err != nil { return ctrl.Result{}, err } + // One of the two places the operator reclaims a data volume: + // this member's data dir is broken enough to crash-loop it, and + // the quorum gate above proved the rest of the cluster is + // healthy, so its contents are both unusable and redundant. The + // replacement joins with an empty volume and syncs from peers. + if err := deleteMemberPVC(ctx, r.Client, member.Namespace, member.Name); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } if setMemberCondition(member, lll.MemberReady, metav1.ConditionFalse, "PodNotReady", @@ -1135,6 +1209,18 @@ func (r *EtcdMemberReconciler) updateStatus(ctx context.Context, member *lll.Etc } } + // Track the volume's lifecycle marker alongside member readiness: a + // member that is serving means its volume is in use, anything else means + // it is still filling. Best-effort — never fail a reconcile over an + // observability annotation. + pvcStatus := PVCStatusInitializing + if ready { + pvcStatus = PVCStatusReady + } + if err := r.setPVCStatus(ctx, member, pvcStatus); err != nil { + log.Error(err, "failed to update PVC status annotation") + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } diff --git a/controllers/etcdmember_controller_test.go b/controllers/etcdmember_controller_test.go index 56096f8a..6fbd6a15 100644 --- a/controllers/etcdmember_controller_test.go +++ b/controllers/etcdmember_controller_test.go @@ -359,6 +359,159 @@ func TestEnsurePVC_AcceptsOwnPVC(t *testing.T) { } } +// TestEnsurePVC_CreatesVolumeWithoutOwnerReference pins the ownership model +// that keeps etcd data alive through events nobody intended as "discard the +// data". +// +// A controller owner reference does two jobs at once: it says "this is mine" +// and it says "delete me with my owner". For a data volume the second job is +// a liability — every route that removes an EtcdMember (a stray kubectl +// delete, a GitOps prune, a CRD replacement, a chart rollback that drops the +// CRs) takes the volume with it, silently and irreversibly. Identity moves to +// an annotation so recognition survives without the coupling. +func TestEnsurePVC_CreatesVolumeWithoutOwnerReference(t *testing.T) { + ctx := context.Background() + member := &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{Name: "test-0", Namespace: "ns", UID: types.UID("member-uid")}, + Spec: lll.EtcdMemberSpec{ + ClusterName: "test", Version: "3.5.17", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, InitialCluster: "x", ClusterToken: "test", + }, + } + c, _ := newTestClient(t, member) + r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} + + if err := r.ensurePVC(ctx, member); err != nil { + t.Fatalf("ensurePVC: %v", err) + } + + pvc := mustGet(t, c, "data-test-0", "ns", &corev1.PersistentVolumeClaim{}) + if len(pvc.OwnerReferences) != 0 { + t.Fatalf("data volume must carry no owner reference; got %+v", pvc.OwnerReferences) + } + if got := pvc.Annotations[AnnPVCMemberUID]; got != "member-uid" { + t.Fatalf("member-uid annotation = %q, want %q", got, "member-uid") + } + if got := pvc.Annotations[AnnPVCStatus]; got != PVCStatusInitializing { + t.Fatalf("fresh volume status = %q, want %q", got, PVCStatusInitializing) + } + // The annotation must be enough to recognise the volume on the next pass. + if !pvcBelongsTo(pvc, member) { + t.Fatalf("member must recognise its own volume by annotation") + } +} + +// TestEnsurePVC_MigratesLegacyOwnerReference covers the upgrade path: volumes +// created by an older build — and those stamped by the in-place migration +// tool — carry a controller owner reference and no annotation. Leaving that +// reference in place would keep exactly the cascade this change removes, so +// the first reconcile after the upgrade must strip it and stamp the marker. +func TestEnsurePVC_MigratesLegacyOwnerReference(t *testing.T) { + ctx := context.Background() + uid := types.UID("legacy-uid") + yes := true + legacy := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "data-test-0", Namespace: "ns", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "etcd-operator.cozystack.io/v1alpha2", + Kind: "EtcdMember", + Name: "test-0", + UID: uid, + Controller: &yes, + BlockOwnerDeletion: &yes, + }}, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceStorage: resource.MustParse("1Gi")}, + }, + }, + } + member := &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{Name: "test-0", Namespace: "ns", UID: uid}, + Spec: lll.EtcdMemberSpec{ + ClusterName: "test", Version: "3.5.17", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, InitialCluster: "x", ClusterToken: "test", + }, + } + c, _ := newTestClient(t, member, legacy) + r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} + + if err := r.ensurePVC(ctx, member); err != nil { + t.Fatalf("ensurePVC on legacy volume: %v", err) + } + + pvc := mustGet(t, c, "data-test-0", "ns", &corev1.PersistentVolumeClaim{}) + if len(pvc.OwnerReferences) != 0 { + t.Fatalf("legacy owner reference must be stripped; got %+v", pvc.OwnerReferences) + } + if got := pvc.Annotations[AnnPVCMemberUID]; got != string(uid) { + t.Fatalf("member-uid annotation = %q, want %q", got, uid) + } + if member.Status.PVCName != "data-test-0" { + t.Fatalf("PVCName not recorded: %q", member.Status.PVCName) + } +} + +// TestMemberDeletion_LeavesTheVolumeBehind is the regression this whole change +// exists for: an EtcdMember going away must not take its data with it. +// +// The finalizer removes the member from etcd and lets the CR go, and the +// volume stays — no owner reference to cascade through, and nothing in the +// deletion path reclaims it. Recovering or discarding what is left is then a +// human's call. +func TestMemberDeletion_LeavesTheVolumeBehind(t *testing.T) { + ctx := context.Background() + now := metav1.Now() + uid := types.UID("doomed-uid") + member := &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-0", Namespace: "ns", UID: uid, + Labels: memberLabels("test", "test-0"), + DeletionTimestamp: &now, + Finalizers: []string{MemberFinalizer}, + }, + Spec: lll.EtcdMemberSpec{ + ClusterName: "test", Version: "3.5.17", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, InitialCluster: "x", ClusterToken: "test", + }, + Status: lll.EtcdMemberStatus{PVCName: "data-test-0", MemberID: "abc"}, + } + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "data-test-0", Namespace: "ns", + Annotations: map[string]string{AnnPVCMemberUID: string(uid)}, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceStorage: resource.MustParse("1Gi")}, + }, + }, + } + cluster := &lll.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}, + Status: lll.EtcdClusterStatus{ClusterID: "deadbeef", ReadyMembers: 3}, + } + c, _ := newTestClient(t, member, pvc, cluster) + fe := newFakeEtcd(0xdeadbeef) + r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factoryReturning(fe)} + + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test-0", Namespace: "ns"}}); err != nil { + t.Fatalf("Reconcile of deleting member: %v", err) + } + + got := &corev1.PersistentVolumeClaim{} + if err := c.Get(ctx, types.NamespacedName{Name: "data-test-0", Namespace: "ns"}, got); err != nil { + t.Fatalf("data volume must survive its member's deletion; got %v", err) + } + if got.DeletionTimestamp != nil { + t.Fatalf("data volume must not be marked for deletion by the member's finalizer") + } +} + // TestEnsurePVC_AppliesStorageClassName covers the wiring of // spec.storage.storageClassName onto the created PVC. The propagation // is what makes per-cluster StorageClass overrides actually take effect @@ -974,6 +1127,77 @@ func TestUpdateStatus_ReplacesStuckMember(t *testing.T) { } } +// TestUpdateStatus_ReplacingStuckMemberReclaimsItsVolume: the second of the +// two paths that deliberately destroy data. The member crash-loops on its own +// data dir while the quorum gate proves the rest of the cluster is healthy, so +// its contents are both unusable and already replicated — the replacement is +// meant to start clean and sync from peers. Leaving the volume would also +// strand it: replacements get fresh apiserver-assigned names, so nothing would +// ever mount it again. +func TestUpdateStatus_ReplacingStuckMemberReclaimsItsVolume(t *testing.T) { + ctx := context.Background() + cluster := &lll.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}, + Spec: lll.EtcdClusterSpec{Replicas: ptrInt32(3)}, + } + member := &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{Name: "test-1", Namespace: "ns", UID: types.UID("stuck-uid"), Labels: memberLabels("test", "test-1")}, + Spec: lll.EtcdMemberSpec{ClusterName: "test", Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, InitialCluster: "x", ClusterToken: "test"}, + } + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "data-test-1", Namespace: "ns", + Annotations: map[string]string{AnnPVCMemberUID: "stuck-uid"}, + }, + } + c, _ := newTestClient(t, cluster, member, pvc, crashLoopPod("test-1", "ns")) + clusterWithReady(t, c, "test", "ns", 2) // 2/3 ready → quorum without test-1 + + r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} + if _, err := r.updateStatus(ctx, member); err != nil { + t.Fatalf("updateStatus: %v", err) + } + + if err := c.Get(ctx, types.NamespacedName{Name: "data-test-1", Namespace: "ns"}, + &corev1.PersistentVolumeClaim{}); !apierrors.IsNotFound(err) { + t.Fatalf("replaced member's volume should have been reclaimed; got %v", err) + } +} + +// TestUpdateStatus_KeepingStuckMemberKeepsItsVolume is the mirror image: when +// the quorum gate refuses the replacement, neither the member nor its data may +// be touched. A cluster-wide outage looks exactly like this on every member at +// once, and reclaiming there would turn an outage into data loss. +func TestUpdateStatus_KeepingStuckMemberKeepsItsVolume(t *testing.T) { + ctx := context.Background() + cluster := &lll.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}, + Spec: lll.EtcdClusterSpec{Replicas: ptrInt32(3)}, + } + member := &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{Name: "test-1", Namespace: "ns", UID: types.UID("stuck-uid"), Labels: memberLabels("test", "test-1")}, + Spec: lll.EtcdMemberSpec{ClusterName: "test", Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, InitialCluster: "x", ClusterToken: "test"}, + } + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "data-test-1", Namespace: "ns", + Annotations: map[string]string{AnnPVCMemberUID: "stuck-uid"}, + }, + } + c, _ := newTestClient(t, cluster, member, pvc, crashLoopPod("test-1", "ns")) + clusterWithReady(t, c, "test", "ns", 1) // 1/3 ready → no quorum without test-1 + + r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} + if _, err := r.updateStatus(ctx, member); err != nil { + t.Fatalf("updateStatus: %v", err) + } + + if err := c.Get(ctx, types.NamespacedName{Name: "data-test-1", Namespace: "ns"}, + &corev1.PersistentVolumeClaim{}); err != nil { + t.Fatalf("volume must survive when the replacement is refused; got %v", err) + } +} + // TestUpdateStatus_KeepsStuckMemberWithoutQuorum: the same crash-looping member // is NOT deleted when the rest of the cluster lacks quorum — self-heal must // never cascade a cluster-wide outage into mass deletion. @@ -1832,11 +2056,16 @@ func TestReconcile_DormantMemberDeletesPod(t *testing.T) { } else if !apierrors.IsNotFound(err) { t.Fatalf("unexpected error fetching Pod: %v", err) } - // PVC must still exist with the EtcdMember as its owner-controller — - // nothing reparented anything. + // PVC must still exist, still marked as this member's — nothing + // reparented anything. gotPVC := mustGet(t, c, "data-test-saved1", "ns", &corev1.PersistentVolumeClaim{}) - if !pvcOwnedBy(gotPVC, dormant) { - t.Fatalf("PVC owner-controller must still be the EtcdMember; got %+v", gotPVC.OwnerReferences) + if !pvcBelongsTo(gotPVC, dormant) { + t.Fatalf("PVC must still be marked as the EtcdMember's; got annotations %+v owner %+v", + gotPVC.Annotations, gotPVC.OwnerReferences) + } + // A paused member's volume is intact but unmounted: detached. + if got := gotPVC.Annotations[AnnPVCStatus]; got != PVCStatusDetached { + t.Fatalf("paused member's PVC should be marked %q; got %q", PVCStatusDetached, got) } // Status.PodName cleared so /status reflects reality. gotMember := mustGet(t, c, "test-saved1", "ns", &lll.EtcdMember{}) @@ -1901,10 +2130,11 @@ func TestReconcile_WakeFromDormantCreatesPod(t *testing.T) { if gotPod.Name != "test-saved1" { t.Fatalf("expected Pod test-saved1 to exist after wake") } - // PVC must still exist with the same owner. + // PVC must still exist, still marked as the woken member's. gotPVC := mustGet(t, c, "data-test-saved1", "ns", &corev1.PersistentVolumeClaim{}) - if !pvcOwnedBy(gotPVC, woken) { - t.Fatalf("PVC owner-controller must still be the woken EtcdMember; got %+v", gotPVC.OwnerReferences) + if !pvcBelongsTo(gotPVC, woken) { + t.Fatalf("PVC must still be marked as the woken EtcdMember's; got annotations %+v owner %+v", + gotPVC.Annotations, gotPVC.OwnerReferences) } } diff --git a/controllers/helpers.go b/controllers/helpers.go index 087038a7..5d0853e0 100644 --- a/controllers/helpers.go +++ b/controllers/helpers.go @@ -1,13 +1,16 @@ package controllers import ( + "context" "fmt" "path" "regexp" "strings" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" lll "github.com/cozystack/etcd-operator/api/v1alpha2" ) @@ -43,6 +46,39 @@ const ( // turn data-dir-subpath into a user-controllable path into --data-dir. ReservedAnnotationPrefix = "etcd-operator.cozystack.io/" + // AnnPVCMemberUID records the UID of the EtcdMember a data PVC was + // created for. It is how the member controller recognises its own + // volume, replacing the controller owner reference that used to serve + // that purpose. + // + // The distinction matters because an owner reference does two jobs at + // once: it identifies the owner AND it makes the object cascade-delete + // with it. For a data volume the second job is a liability — every + // route that removes an EtcdMember (an operator's kubectl delete, a + // GitOps prune, a CRD replacement, a Helm rollback that drops the CRs) + // silently takes the data with it, whether or not anyone intended to + // discard it. Identity via annotation keeps the recognition and drops + // the coupling: volumes are deleted only where the operator explicitly + // decides to (see deleteMemberPVC). + AnnPVCMemberUID = ReservedAnnotationPrefix + "member-uid" + + // AnnPVCStatus mirrors the lifecycle marker CloudNativePG puts on its + // instance volumes (cnpg.io/pvcStatus). It is observability, not + // control flow: nothing branches on it, but it answers "is this volume + // in use, still filling, or left behind?" with a kubectl get -o custom-columns + // instead of a cross-reference against the member list. + AnnPVCStatus = ReservedAnnotationPrefix + "pvc-status" + + // PVCStatusInitializing — the volume exists, its member has not reported + // Ready yet (fresh member joining, or a restore still populating it). + PVCStatusInitializing = "initializing" + // PVCStatusReady — the volume is mounted by a member that is Ready. + PVCStatusReady = "ready" + // PVCStatusDetached — the volume is intact but no Pod is using it: a + // paused (dormant) member, or a volume whose member is gone. Detached + // volumes are never reclaimed automatically. + PVCStatusDetached = "detached" + // AnnHeadlessServiceName overrides the headless Service name a member's // DNS identity keys off: its Pod subdomain and every peer/client URL // the operator constructs for it. Absent ⇒ the cluster's own name @@ -572,6 +608,61 @@ func setMemberCondition(member *lll.EtcdMember, condType string, status metav1.C return true } +// memberPVCName is the data volume name for a member. Kept in one place +// because both controllers derive it: the member controller to create and +// recognise the volume, the cluster controller to reclaim it. +func memberPVCName(memberName string) string { + return "data-" + memberName +} + +// pvcBelongsTo reports whether a data PVC was created for this member. +// +// Primary check is the member-UID annotation. The owner-reference fallback +// covers volumes created before the ownership model changed, and volumes +// stamped by the in-place migration tool — both carry a controller owner ref +// and no annotation until ensurePVC migrates them. +// +// A volume with neither marker is refused rather than adopted: silently +// inheriting another member's data dir crash-loops the pod when etcd finds a +// removed member ID in the WAL. +func pvcBelongsTo(pvc *corev1.PersistentVolumeClaim, member *lll.EtcdMember) bool { + if uid, ok := pvc.Annotations[AnnPVCMemberUID]; ok { + return uid == string(member.UID) + } + for _, o := range pvc.OwnerReferences { + if o.Kind == "EtcdMember" && o.UID == member.UID { + return true + } + } + return false +} + +// deleteMemberPVC reclaims a member's data volume. +// +// This is the ONLY path that destroys etcd data, and it is called exclusively +// from places where the operator itself decided to retire a member and the +// data is either replicated elsewhere or already unusable: +// +// - scale-down, where the removed member's contents live on the remaining +// peers (MemberRemove has already run via the finalizer), and +// - self-heal replacement of a member whose data dir is broken enough to +// crash-loop it, gated on the rest of the cluster holding quorum. +// +// Deliberately NOT wired to EtcdMember deletion in general. A member CR can +// disappear for reasons that say nothing about the data's value — a stray +// kubectl delete, a GitOps prune, a CRD replacement, a chart rollback — and +// none of those should reach the volume. What is left behind is visible as a +// detached PVC (AnnPVCStatus) and reclaimed by a human. +func deleteMemberPVC(ctx context.Context, c client.Client, namespace, memberName string) error { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: memberPVCName(memberName), Namespace: namespace}, + } + if err := c.Delete(ctx, pvc); err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("delete PVC %q: %w", pvc.Name, err) + } + return nil +} + // setCondition inserts or updates a condition, preserving LastTransitionTime // when the status has not changed. func setCondition(conditions *[]metav1.Condition, c metav1.Condition) { diff --git a/docs/concepts.md b/docs/concepts.md index 6cd535ba..c6153038 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -10,7 +10,7 @@ Two custom resources, one of them user-facing. **`EtcdCluster`** — the user-facing object. It captures cluster-wide intent: replica count, etcd version, per-member storage size, a progress deadline. This is the only resource users normally touch. -**`EtcdMember`** — one per etcd member. Created and deleted by the cluster controller. Each `EtcdMember` owns its Pod and PVC. Users should not create or edit these directly. +**`EtcdMember`** — one per etcd member. Created and deleted by the cluster controller. Each `EtcdMember` owns its Pod; its data PVC is deliberately **not** owned by it (see [data volume lifecycle](#data-volume-lifecycle)). Users should not create or edit these directly. There is **no StatefulSet**. Each member's Pod and PVC are reconciled independently by the member controller. The motivation is protocol awareness: scale-up adds a member as a learner first and only promotes once it's caught up; scale-down runs `MemberRemove` via a finalizer before reclaiming the Pod; pod restarts reuse the existing data dir and rejoin with the same etcd-side member ID. None of these flows fit StatefulSet's "all replicas are one fungible workload" model. @@ -98,7 +98,7 @@ If the seed's pod hasn't been created yet (between Create and Pod-up), the contr ### Pause (1→0) -When the cluster controller's `scaleDown` observes `desired==0 && len(running)==1`, it Patches `spec.dormant=true` on the surviving member. The CR is **not** deleted. On the next reconcile of that member, the member controller observes `spec.dormant=true` and runs `ensurePodAbsent` — deletes the Pod, clears `status.podName`, surfaces `Ready=False/Paused`. The PVC is not touched. It keeps its existing owner-ref to the `EtcdMember`, which still exists. So nothing reparents, nothing cascade-deletes. +When the cluster controller's `scaleDown` observes `desired==0 && len(running)==1`, it Patches `spec.dormant=true` on the surviving member. The CR is **not** deleted. On the next reconcile of that member, the member controller observes `spec.dormant=true` and runs `ensurePodAbsent` — deletes the Pod, clears `status.podName`, surfaces `Ready=False/Paused`. The volume is not touched — it keeps its member-UID marker and is flagged `pvc-status: detached` while nothing mounts it. So nothing reparents, and nothing is reclaimed. Intermediate steps of a multi-member descent (3→2, 2→1) are normal scale-downs: pick newest, Delete CR, finalizer runs `MemberRemove`. Only the final 1→0 step flips dormant. @@ -121,6 +121,36 @@ An earlier iteration of this feature deleted the CR, reparented the PVC to the ` The single exception is the steady-state call to `updateStatus`, which receives the full active set (including dormant). `updateStatus`'s Paused branch uses `findDormantMember(members)` to name the parked PVC in the `Available=False/Paused` message. Stripping the dormant member at that call site would silently fall back to the fresh-zero "no data has been written" message even on real dormant clusters. The asymmetry is deliberate and the call site is commented. +## Data volume lifecycle + +A member's data PVC carries **no owner reference**. Nothing in Kubernetes garbage-collects it: not deleting the `EtcdMember`, not deleting the `EtcdCluster`, not removing the CRDs. + +This is a deliberate departure from the usual "child object owned by its CR" pattern, because an owner reference does two jobs at once — it marks the owner, and it makes the object cascade-delete with that owner. The first is wanted, the second is a liability for a volume holding the only copy of a database. Every route that removes an `EtcdMember` would take the data with it: a stray `kubectl delete`, a GitOps prune, a CRD replacement during an upgrade, a chart rollback that drops the CRs. None of those say anything about whether the data still matters, and all of them are irreversible against a `Delete`-reclaim StorageClass. + +So identity and lifetime are separated: + +- **Identity** is the `etcd-operator.cozystack.io/member-uid` annotation. The member controller mounts a volume only when the annotation matches its own UID; anything else is refused rather than adopted (inheriting a foreign data dir crash-loops the pod once etcd finds a removed member ID in the WAL). Volumes from older operator versions, and those stamped by the migration tool, carry a controller owner reference instead — the first reconcile after upgrading migrates them: annotation stamped, owner reference stripped. +- **Lifetime** is the operator's explicit decision. Exactly two paths delete a volume, both places where the operator itself retired the member and the data is either replicated or unusable: + - **scale-down** — the departing member's contents live on the remaining peers (its finalizer ran `MemberRemove` first). The shrink was asked for, so the storage goes back to the pool. + - **crash-loop replacement** — the member cannot boot on its own data dir, and the quorum gate proved the rest of the cluster is healthy. The replacement starts clean and syncs from peers. + +Everything else leaves the volume in place. A paused (`spec.replicas: 0`) cluster keeps its member CR and its volume; a member deleted by anything other than the two paths above leaves a volume behind that no future member will mount, because replacements get fresh apiserver-assigned names. + +Leftover volumes are visible rather than silent. Each carries `etcd-operator.cozystack.io/pvc-status`, mirroring the marker CloudNativePG puts on its instance volumes: + +| Value | Meaning | +|---|---| +| `initializing` | The volume exists; its member has not reported `Ready` yet (joining, or a restore still populating it). | +| `ready` | Mounted by a member that is serving. | +| `detached` | Intact, nothing mounting it: a paused member, or a volume whose member is gone. | + +```sh +kubectl get pvc -l etcd-operator.cozystack.io/cluster= -n \ + -o custom-columns='NAME:.metadata.name,STATUS:.metadata.annotations.etcd-operator\.cozystack\.io/pvc-status' +``` + +Nothing branches on this annotation — it is observability. Reclaiming a detached volume is a human's call. + ## Storage Each member's data dir is configured via `spec.storage`, a struct with `size`, `medium`, and an optional `storageClassName`. The medium chooses between a PVC and a tmpfs `emptyDir`; the locking pattern protects size and medium just like `replicas` and `version` — a mid-flight flip is locked out until the current target is reached or the deadline expires. @@ -149,7 +179,7 @@ On every reconcile of a memory-backed member the controller stamps `Status.PodUI - Pod present, UID matches → steady state. - Pod absent (or UID differs) with a previously recorded UID → loss confirmed. -The member controller self-deletes the `EtcdMember`. The existing finalizer runs `MemberRemove` against quorum-reachable peers and the Pod / PVC owner-refs handle the rest of GC. The cluster controller's normal `current < desired` arm then scales up: a fresh `EtcdMember` is created with a new `GenerateName` and a new etcd-side member ID. There is no in-place "rejoin with empty data dir" — that path would require lying to raft. +The member controller self-deletes the `EtcdMember`. The existing finalizer runs `MemberRemove` against quorum-reachable peers and the Pod's owner reference handles its GC; the memory-backed member has no volume to reclaim. The cluster controller's normal `current < desired` arm then scales up: a fresh `EtcdMember` is created with a new `GenerateName` and a new etcd-side member ID. There is no in-place "rejoin with empty data dir" — that path would require lying to raft. If quorum is already lost across multiple simultaneous failures, `MemberRemove` will fail and the dying members stay in `Terminating` until quorum returns. That is the correct outcome: the cluster is dead and the user has to recreate it. The operator does not try to be clever about restoring a quorum from inconsistent half-states. @@ -163,7 +193,7 @@ The member controller detects this and replaces the member: - **Trigger.** The etcd container is not ready and has restarted at least `dataLossRestartThreshold` (5) times. `OOMKilled` is excluded (whether it's the current or the last termination) — that's a resource problem re-creating the member would not fix — and a Pod that is itself being deleted (drain/eviction/manual restart) is never treated as stuck. - **Quorum gate.** The operator deletes the member only when the *rest* of the cluster still has quorum, so a cluster-wide outage (many members crashing at once) never cascades into mass deletion. The count is read from `Status.ReadyMembers`, which the cluster controller maintains and which can lag; if the stuck member is still counted ready, the gate subtracts it. As a second line of defence the finalizer's `MemberRemove` is itself quorum-gated, so even a stale-high count cannot delete data below quorum. -- **Replacement.** Deleting the `EtcdMember` runs the finalizer's clean `MemberRemove`, the member-owned PVC is GC'd (discarding the corrupt data dir), and the cluster controller gap-fills a fresh `GenerateName` member with a current `--initial-cluster` and a **new** etcd member ID — not a same-ID rejoin. +- **Replacement.** Deleting the `EtcdMember` runs the finalizer's clean `MemberRemove`, the operator explicitly reclaims that member's volume (discarding the corrupt data dir — one of the two paths allowed to do so, see [data volume lifecycle](#data-volume-lifecycle)), and the cluster controller gap-fills a fresh `GenerateName` member with a current `--initial-cluster` and a **new** etcd member ID — not a same-ID rejoin. - **Latency.** `CrashLoopBackOff` caps its backoff at 5 minutes, so reaching 5 restarts takes on the order of **tens of minutes**, not the ~5s of the memory Pod-loss path. A deliberately-deleted-and-replaced member during this window is expected operator behavior, not a fault. A slow restore or slow learner join on the *replacement* can itself trip the threshold and be replaced again; this is quorum-gated and self-limiting, but expect it on a struggling cluster. ### What is missing from memory clusters today diff --git a/docs/installation.md b/docs/installation.md index d8c6ab4c..945cba4a 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -318,7 +318,7 @@ kubectl delete crd etcdclusters.etcd-operator.cozystack.io \ etcdsnapshots.etcd-operator.cozystack.io ``` -Deleting an `EtcdCluster` while it's running cascades through every owned resource: the operator's finalizer on each `EtcdMember` calls `MemberRemove` (when the cluster itself is also being deleted, the operator detects this and skips `MemberRemove` to avoid a deadlock — see `handleDeletion` in `controllers/etcdmember_controller.go`). Pods and PVCs are then GC'd via owner-refs. +Deleting an `EtcdCluster` while it's running cascades through every owned resource: the operator's finalizer on each `EtcdMember` calls `MemberRemove` (when the cluster itself is also being deleted, the operator detects this and skips `MemberRemove` to avoid a deadlock — see `handleDeletion` in `controllers/etcdmember_controller.go`). Pods are then GC'd via owner-refs. **Data volumes are not** — they carry no owner reference by design ([data volume lifecycle](concepts.md#data-volume-lifecycle)), so deleting a cluster leaves its `data-` PVCs behind, marked `pvc-status: detached`. Reclaim them yourself once you are sure the data is no longer needed. If the operator is uninstalled while `EtcdCluster` resources still exist, they're stranded — the finalizers won't run because no controller is reading the queue. Recovery is to either re-install the operator, or `kubectl patch ... --type=merge -p '{"metadata":{"finalizers":null}}'` on each `EtcdMember` (manual, leaves PVCs and Pods in place — clean them up by label). diff --git a/docs/operations.md b/docs/operations.md index 48649db8..55acfa7e 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -57,7 +57,7 @@ kubectl patch etcdcluster.etcd-operator.cozystack.io -n --type=merge -p '{"spec":{"replicas":3}}' ``` -Picks the most-recently-created member as the victim (`CreationTimestamp` DESC, name DESC tiebreak). The finalizer calls `MemberRemove` against the remaining peers before the Pod and PVC are garbage-collected. No special seed-protection — the seed (the original bootstrap member) has no permanent special role and can be removed like any other member. +Picks the most-recently-created member as the victim (`CreationTimestamp` DESC, name DESC tiebreak). The finalizer calls `MemberRemove` against the remaining peers, the Pod is garbage-collected with its member, and the operator explicitly reclaims that member's data volume — its contents are already replicated on the peers that remain. No special seed-protection — the seed (the original bootstrap member) has no permanent special role and can be removed like any other member. ### Pause (scale to 0) @@ -66,7 +66,7 @@ kubectl patch etcdcluster.etcd-operator.cozystack.io -n --type=merge -p '{"spec":{"replicas":0}}' ``` -For an N>1 cluster this is a staged descent: each intermediate step (`MemberRemove` + Pod/PVC GC) until one member remains, then a 1→0 "pause" — the surviving member's `spec.dormant` is patched to `true`. The Pod goes away; the PVC stays owned by the `EtcdMember`, which itself stays alive. `etcdctl` from outside is no longer reachable (no Pod) but the data is intact. +For an N>1 cluster this is a staged descent: each intermediate step (`MemberRemove`, Pod GC, volume reclaimed) until one member remains, then a 1→0 "pause" — the surviving member's `spec.dormant` is patched to `true`. The Pod goes away; the volume stays, marked `pvc-status: detached`, and its `EtcdMember` stays alive. `etcdctl` from outside is no longer reachable (no Pod) but the data is intact. Observable state once paused: @@ -161,7 +161,11 @@ kubectl get etcdmember,pvc -l etcd-operator.cozystack.io/cluster= -n .yaml ``` -The PVC GC step is important: re-creating before the prior PVCs are gone causes the new EtcdMember to refuse to adopt them (`pvcOwnedBy` UID check fails — see [concepts](concepts.md#api-model)). The operator's check is a safety feature; the right answer is to wait. +Volumes from the deleted cluster are **not** garbage-collected (see [data volume lifecycle](concepts.md#data-volume-lifecycle)) — they remain as `detached` PVCs. A recreated cluster never adopts them: its members get fresh names and fresh UIDs, and the member-UID check refuses anything that is not theirs. Delete the leftovers yourself once you have confirmed the data is not needed: + +```sh +kubectl get pvc -l etcd-operator.cozystack.io/cluster= -n +``` ### `Available=False/DeadlineExceeded` @@ -488,7 +492,7 @@ If a non-bootstrap PVC member's etcd cannot start — classically because its da - **Detection**: the etcd container is not ready and has restarted at least 5 times (`dataLossRestartThreshold`), excluding `OOMKilled` (a resource problem, not a lost data dir — raising `spec.resources.limits.memory` is the fix there, not replacement). A Pod that is being deleted (drain, eviction, manual restart) is never treated as stuck. - **Quorum gate**: the operator deletes the member only when the *rest* of the cluster still has quorum, so a cluster-wide outage never cascades into mass deletion. The gate reads `Status.ReadyMembers` (maintained by the cluster controller, and possibly lagging) and subtracts the stuck member if it is still counted; the finalizer's `MemberRemove` is independently quorum-gated as a backstop. -- **Replacement**: the `EtcdMember` CR is deleted → finalizer `MemberRemove` → the member-owned `data-` PVC is GC'd (discarding the corrupt data dir) → the cluster controller gap-fills a fresh `GenerateName` member with a current `--initial-cluster` and a **new** etcd member ID. +- **Replacement**: the `EtcdMember` CR is deleted → finalizer `MemberRemove` → the operator explicitly reclaims `data-` (discarding the corrupt data dir) → the cluster controller gap-fills a fresh `GenerateName` member with a current `--initial-cluster` and a **new** etcd member ID. **Detection latency is much longer than the Pod-loss path.** `CrashLoopBackOff` caps backoff at 5 minutes, so reaching 5 restarts takes **tens of minutes**, not ~5 s. Budget for that before concluding the operator is misbehaving — a member that vanishes and is replaced by a fresh-named one after a long crash-loop is the operator working as designed, not flapping. Note also that a replacement which is itself slow to come up (slow restore, slow learner join) can trip the same threshold and be replaced again; this is quorum-gated and harmless to the cluster, but expect repeated replacement on a genuinely unhealthy member. From 1c83290643f952a609779f3468e8c71cf202e39a Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Tue, 28 Jul 2026 22:43:45 +0200 Subject: [PATCH 2/2] feat(controllers): reclaim data volumes when the cluster is deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing the owner reference stopped a member's disappearance from taking its data, but it also left deleting the EtcdCluster incomplete: the volumes outlived the cluster they belonged to, and reclaiming them fell to whoever noticed. Deleting an EtcdCluster is the user declaring the cluster finished, so its storage goes with it. Add a cluster finalizer that removes the data volumes and is released only once they are actually gone — not merely asked to go. A volume stays Terminating while a Pod still mounts it, so releasing early would let the EtcdCluster disappear with storage still allocated behind it, and would let a namespace delete look complete while PVCs were draining. Reclaiming is scoped by the cluster label, so a second cluster sharing the namespace is untouched. The cluster controller gains update on etcdclusters (for the finalizer) and list/delete on persistentvolumeclaims; RBAC regenerated. Assisted-By: Claude Signed-off-by: Andrei Kvapil --- .../files/manager-role-rules.yaml | 2 + controllers/etcdcluster_controller.go | 91 +++++++++++- controllers/etcdcluster_controller_test.go | 139 +++++++++++++++++- controllers/helpers.go | 10 ++ docs/concepts.md | 7 +- docs/installation.md | 2 +- docs/operations.md | 4 +- 7 files changed, 242 insertions(+), 13 deletions(-) diff --git a/charts/etcd-operator/files/manager-role-rules.yaml b/charts/etcd-operator/files/manager-role-rules.yaml index 4ad4b945..11c60323 100644 --- a/charts/etcd-operator/files/manager-role-rules.yaml +++ b/charts/etcd-operator/files/manager-role-rules.yaml @@ -74,6 +74,8 @@ verbs: - get - list + - patch + - update - watch - apiGroups: - etcd-operator.cozystack.io diff --git a/controllers/etcdcluster_controller.go b/controllers/etcdcluster_controller.go index 58bf9741..4d481869 100644 --- a/controllers/etcdcluster_controller.go +++ b/controllers/etcdcluster_controller.go @@ -75,9 +75,10 @@ type EtcdClusterReconciler struct { ClusterDomain string } -//+kubebuilder:rbac:groups=etcd-operator.cozystack.io,resources=etcdclusters,verbs=get;list;watch +//+kubebuilder:rbac:groups=etcd-operator.cozystack.io,resources=etcdclusters,verbs=get;list;watch;update;patch //+kubebuilder:rbac:groups=etcd-operator.cozystack.io,resources=etcdclusters/status,verbs=get;update;patch //+kubebuilder:rbac:groups=etcd-operator.cozystack.io,resources=etcdclusters/finalizers,verbs=update +//+kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list;watch;delete //+kubebuilder:rbac:groups=etcd-operator.cozystack.io,resources=etcdmembers,verbs=get;list;watch;create;delete //+kubebuilder:rbac:groups=etcd-operator.cozystack.io,resources=etcdmembers/status,verbs=get;update;patch //+kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;create;update;patch @@ -102,11 +103,23 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } - // If the cluster is being deleted, don't keep reconciling it. Owned - // resources are cascaded out via owner refs; recreating a Service for a - // Terminating cluster races against the GC and pollutes logs. + // If the cluster is being deleted, stop reconciling it and run cleanup. + // Owner-referenced resources cascade out on their own; data volumes do + // not (they carry no owner reference — see deleteMemberPVC) and are + // reclaimed here, before the finalizer is released. if !cluster.DeletionTimestamp.IsZero() { - return ctrl.Result{}, nil + return r.handleClusterDeletion(ctx, cluster) + } + + // Claim the cleanup finalizer before anything else creates state worth + // cleaning up. + if controllerutil.AddFinalizer(cluster, ClusterFinalizer) { + if err := r.Update(ctx, cluster); err != nil { + if errors.IsConflict(err) { + return ctrl.Result{Requeue: true}, nil + } + return ctrl.Result{}, err + } } // Terminal-config gate. spec.tls.{client,peer}.certManager requires @@ -1964,6 +1977,74 @@ func (r *EtcdClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { // that happens we sit in DeadlineExceeded. // // We never auto-pivot during bootstrap, and never silently in steady state. +// handleClusterDeletion reclaims what k8s garbage collection will not, then +// releases the finalizer. +// +// Members, Pods and Services are owner-referenced to the cluster and cascade +// out on their own. Data volumes deliberately are not (see deleteMemberPVC): +// they must survive a member disappearing for reasons that say nothing about +// the data's value. Deleting the EtcdCluster is not one of those reasons — it +// is the user declaring the cluster finished — so this is where the volumes +// are reclaimed, explicitly. +// +// The finalizer is held until they are actually gone rather than merely asked +// to go. A volume stays Terminating while a Pod still mounts it, so releasing +// early would let the EtcdCluster vanish with storage still allocated behind +// it, and a namespace delete would look complete while PVCs were still +// draining. Holding the finalizer makes "the EtcdCluster is gone" mean "its +// storage is gone too". +func (r *EtcdClusterReconciler) handleClusterDeletion( + ctx context.Context, + cluster *lll.EtcdCluster, +) (ctrl.Result, error) { + log := log.FromContext(ctx) + + if !controllerutil.ContainsFinalizer(cluster, ClusterFinalizer) { + return ctrl.Result{}, nil + } + + pvcs := &corev1.PersistentVolumeClaimList{} + if err := r.List(ctx, pvcs, + client.InNamespace(cluster.Namespace), + client.MatchingLabels{LabelCluster: cluster.Name}, + ); err != nil { + return ctrl.Result{}, fmt.Errorf("list data volumes of deleted cluster: %w", err) + } + + remaining := 0 + for i := range pvcs.Items { + pvc := &pvcs.Items[i] + if !pvc.DeletionTimestamp.IsZero() { + // Already draining — most likely still mounted by a Pod that is + // itself being GC'd. Wait it out rather than declaring the + // cluster gone while its storage is not. + remaining++ + continue + } + if err := r.Delete(ctx, pvc); err != nil { + if errors.IsNotFound(err) { + continue + } + return ctrl.Result{}, fmt.Errorf("delete data volume %q: %w", pvc.Name, err) + } + log.Info("reclaiming data volume of deleted cluster", "pvc", pvc.Name) + remaining++ + } + + if remaining > 0 { + return ctrl.Result{RequeueAfter: 5 * time.Second}, nil + } + + controllerutil.RemoveFinalizer(cluster, ClusterFinalizer) + if err := r.Update(ctx, cluster); err != nil { + if errors.IsConflict(err) { + return ctrl.Result{Requeue: true}, nil + } + return ctrl.Result{}, err + } + return ctrl.Result{}, nil +} + func (r *EtcdClusterReconciler) handleDeadlineExceeded( ctx context.Context, cluster *lll.EtcdCluster, diff --git a/controllers/etcdcluster_controller_test.go b/controllers/etcdcluster_controller_test.go index 6ab0cd1e..7fe916cc 100644 --- a/controllers/etcdcluster_controller_test.go +++ b/controllers/etcdcluster_controller_test.go @@ -36,6 +36,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" lll "github.com/cozystack/etcd-operator/api/v1alpha2" ) @@ -1008,6 +1009,136 @@ func scaleDownFixture(t *testing.T, replicas int32, members int) (*lll.EtcdClust return cluster, objs } +// TestClusterDeletion_ReclaimsVolumesBeforeReleasingFinalizer: deleting the +// EtcdCluster is the user declaring the cluster finished, so its storage goes +// with it. Volumes carry no owner reference (that is what keeps a member +// disappearing from destroying data), so nothing reclaims them implicitly — +// the finalizer does, explicitly, and is held until they are actually gone. +func TestClusterDeletion_ReclaimsVolumesBeforeReleasingFinalizer(t *testing.T) { + ctx := context.Background() + now := metav1.Now() + cluster := &lll.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "ns", + DeletionTimestamp: &now, + Finalizers: []string{ClusterFinalizer}, + }, + Spec: lll.EtcdClusterSpec{Replicas: ptrInt32(3)}, + } + objs := []client.Object{cluster} + for _, name := range []string{"test-a", "test-b"} { + objs = append(objs, &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: memberPVCName(name), Namespace: "ns", Labels: memberLabels("test", name), + }, + }) + } + c, _ := newTestClient(t, objs...) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t)} + + // First pass: volumes are asked to go, finalizer still held. + res, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}) + if err != nil { + t.Fatalf("Reconcile: %v", err) + } + if res.RequeueAfter == 0 { + t.Fatalf("expected a requeue while volumes drain; got %+v", res) + } + for _, name := range []string{"test-a", "test-b"} { + if err := c.Get(ctx, types.NamespacedName{Name: memberPVCName(name), Namespace: "ns"}, + &corev1.PersistentVolumeClaim{}); !apierrors.IsNotFound(err) { + t.Fatalf("volume of %s should have been reclaimed; got %v", name, err) + } + } + + // Second pass: nothing left, so the cluster is allowed to go. + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}); err != nil { + t.Fatalf("Reconcile (second pass): %v", err) + } + got := &lll.EtcdCluster{} + if err := c.Get(ctx, types.NamespacedName{Name: "test", Namespace: "ns"}, got); err == nil { + if controllerutil.ContainsFinalizer(got, ClusterFinalizer) { + t.Fatalf("finalizer must be released once the volumes are gone") + } + } else if !apierrors.IsNotFound(err) { + t.Fatalf("Get cluster: %v", err) + } +} + +// TestClusterDeletion_HoldsFinalizerWhileAVolumeDrains: a volume stays +// Terminating while a Pod still mounts it. Releasing the finalizer then would +// let the EtcdCluster vanish with storage still allocated behind it — and a +// namespace delete would look complete while PVCs were still draining. +func TestClusterDeletion_HoldsFinalizerWhileAVolumeDrains(t *testing.T) { + ctx := context.Background() + now := metav1.Now() + cluster := &lll.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "ns", + DeletionTimestamp: &now, + Finalizers: []string{ClusterFinalizer}, + }, + Spec: lll.EtcdClusterSpec{Replicas: ptrInt32(1)}, + } + draining := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: memberPVCName("test-a"), Namespace: "ns", Labels: memberLabels("test", "test-a"), + DeletionTimestamp: &now, + Finalizers: []string{"kubernetes.io/pvc-protection"}, + }, + } + c, _ := newTestClient(t, cluster, draining) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t)} + + res, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}) + if err != nil { + t.Fatalf("Reconcile: %v", err) + } + if res.RequeueAfter == 0 { + t.Fatalf("expected a requeue while the volume drains; got %+v", res) + } + got := mustGet(t, c, "test", "ns", &lll.EtcdCluster{}) + if !controllerutil.ContainsFinalizer(got, ClusterFinalizer) { + t.Fatalf("finalizer must be held until the volume is actually gone") + } +} + +// TestClusterDeletion_LeavesAnotherClustersVolumes: the reclaim is scoped by +// the cluster label. Two clusters in one namespace is ordinary, and deleting +// one must not touch the other's data. +func TestClusterDeletion_LeavesAnotherClustersVolumes(t *testing.T) { + ctx := context.Background() + now := metav1.Now() + cluster := &lll.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "ns", + DeletionTimestamp: &now, + Finalizers: []string{ClusterFinalizer}, + }, + Spec: lll.EtcdClusterSpec{Replicas: ptrInt32(1)}, + } + mine := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: memberPVCName("test-a"), Namespace: "ns", Labels: memberLabels("test", "test-a"), + }, + } + theirs := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: memberPVCName("other-a"), Namespace: "ns", Labels: memberLabels("other", "other-a"), + }, + } + c, _ := newTestClient(t, cluster, mine, theirs) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t)} + + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}); err != nil { + t.Fatalf("Reconcile: %v", err) + } + if err := c.Get(ctx, types.NamespacedName{Name: memberPVCName("other-a"), Namespace: "ns"}, + &corev1.PersistentVolumeClaim{}); err != nil { + t.Fatalf("another cluster's volume must be untouched; got %v", err) + } +} + // TestScaleDown_ReclaimsTheVictimsVolume: shrinking a cluster is a deliberate // instruction, and the departing member's contents are already replicated on // the peers that remain (its finalizer runs MemberRemove first). Leaving the @@ -1831,7 +1962,9 @@ func TestDeadlineExceeded_NoChurnWhenSteady(t *testing.T) { now := metav1.Now() past := metav1.NewTime(now.Add(-time.Hour)) cluster := &lll.EtcdCluster{ - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}, + // A settled cluster already carries the cleanup finalizer; it is + // claimed once on the first reconcile, not on every pass. + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns", Finalizers: []string{ClusterFinalizer}}, Spec: lll.EtcdClusterSpec{ Replicas: ptrInt32(3), Version: "3.5.17", @@ -2478,7 +2611,9 @@ func TestClusterUpdateStatus_NoChurnInSteadyState(t *testing.T) { ctx := context.Background() now := metav1.Now() cluster := &lll.EtcdCluster{ - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}, + // A settled cluster already carries the cleanup finalizer; it is + // claimed once on the first reconcile, not on every pass. + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns", Finalizers: []string{ClusterFinalizer}}, Spec: lll.EtcdClusterSpec{ Replicas: ptrInt32(3), Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, }, diff --git a/controllers/helpers.go b/controllers/helpers.go index 5d0853e0..629d096a 100644 --- a/controllers/helpers.go +++ b/controllers/helpers.go @@ -37,6 +37,16 @@ const ( // graceful removal from the etcd cluster before deletion. MemberFinalizer = "etcd-operator.cozystack.io/member-cleanup" + // ClusterFinalizer is placed on EtcdCluster resources so deleting one + // also reclaims its data volumes. Those volumes carry no owner + // reference (see AnnPVCMemberUID), which is what keeps a member + // disappearing from destroying data — but a deleted EtcdCluster is the + // user declaring the cluster finished, so its storage must go too. The + // finalizer is held until the volumes are actually gone, not merely + // asked to go, so "the EtcdCluster is gone" implies "its storage is + // gone". + ClusterFinalizer = "etcd-operator.cozystack.io/cluster-cleanup" + // ReservedAnnotationPrefix namespaces the operator-interpreted // annotations below. additionalMetadata must never be able to set a // key under this prefix (applyAdditionalMetadata strips it): the diff --git a/docs/concepts.md b/docs/concepts.md index c6153038..fa4e36b6 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -123,18 +123,19 @@ The single exception is the steady-state call to `updateStatus`, which receives ## Data volume lifecycle -A member's data PVC carries **no owner reference**. Nothing in Kubernetes garbage-collects it: not deleting the `EtcdMember`, not deleting the `EtcdCluster`, not removing the CRDs. +A member's data PVC carries **no owner reference**. Nothing in Kubernetes garbage-collects it: not deleting the `EtcdMember`, not deleting the `EtcdCluster`, not removing the CRDs. Reclaiming is the operator's own decision, taken in three specific places — and nowhere else. This is a deliberate departure from the usual "child object owned by its CR" pattern, because an owner reference does two jobs at once — it marks the owner, and it makes the object cascade-delete with that owner. The first is wanted, the second is a liability for a volume holding the only copy of a database. Every route that removes an `EtcdMember` would take the data with it: a stray `kubectl delete`, a GitOps prune, a CRD replacement during an upgrade, a chart rollback that drops the CRs. None of those say anything about whether the data still matters, and all of them are irreversible against a `Delete`-reclaim StorageClass. So identity and lifetime are separated: - **Identity** is the `etcd-operator.cozystack.io/member-uid` annotation. The member controller mounts a volume only when the annotation matches its own UID; anything else is refused rather than adopted (inheriting a foreign data dir crash-loops the pod once etcd finds a removed member ID in the WAL). Volumes from older operator versions, and those stamped by the migration tool, carry a controller owner reference instead — the first reconcile after upgrading migrates them: annotation stamped, owner reference stripped. -- **Lifetime** is the operator's explicit decision. Exactly two paths delete a volume, both places where the operator itself retired the member and the data is either replicated or unusable: +- **Lifetime** is the operator's explicit decision. Three paths delete a volume, each one a place where the operator itself decided the data is finished: - **scale-down** — the departing member's contents live on the remaining peers (its finalizer ran `MemberRemove` first). The shrink was asked for, so the storage goes back to the pool. - **crash-loop replacement** — the member cannot boot on its own data dir, and the quorum gate proved the rest of the cluster is healthy. The replacement starts clean and syncs from peers. + - **cluster deletion** — deleting the `EtcdCluster` is the user declaring the cluster finished, so its storage goes with it. This runs from the cluster's own finalizer (`etcd-operator.cozystack.io/cluster-cleanup`), which is held until the volumes are actually gone, not merely asked to go: a volume stays `Terminating` while a Pod still mounts it, and releasing early would let the `EtcdCluster` disappear with storage still allocated behind it — or let a namespace delete look complete while PVCs were still draining. -Everything else leaves the volume in place. A paused (`spec.replicas: 0`) cluster keeps its member CR and its volume; a member deleted by anything other than the two paths above leaves a volume behind that no future member will mount, because replacements get fresh apiserver-assigned names. +Everything else leaves the volume in place. A paused (`spec.replicas: 0`) cluster keeps its member CR and its volume; a member deleted by anything other than the paths above — a stray `kubectl delete`, a GitOps prune, a CRD replacement, a chart rollback — leaves a volume behind that no future member will mount, because replacements get fresh apiserver-assigned names. Leftover volumes are visible rather than silent. Each carries `etcd-operator.cozystack.io/pvc-status`, mirroring the marker CloudNativePG puts on its instance volumes: diff --git a/docs/installation.md b/docs/installation.md index 945cba4a..74bf401c 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -318,7 +318,7 @@ kubectl delete crd etcdclusters.etcd-operator.cozystack.io \ etcdsnapshots.etcd-operator.cozystack.io ``` -Deleting an `EtcdCluster` while it's running cascades through every owned resource: the operator's finalizer on each `EtcdMember` calls `MemberRemove` (when the cluster itself is also being deleted, the operator detects this and skips `MemberRemove` to avoid a deadlock — see `handleDeletion` in `controllers/etcdmember_controller.go`). Pods are then GC'd via owner-refs. **Data volumes are not** — they carry no owner reference by design ([data volume lifecycle](concepts.md#data-volume-lifecycle)), so deleting a cluster leaves its `data-` PVCs behind, marked `pvc-status: detached`. Reclaim them yourself once you are sure the data is no longer needed. +Deleting an `EtcdCluster` while it's running cascades through every owned resource: the operator's finalizer on each `EtcdMember` calls `MemberRemove` (when the cluster itself is also being deleted, the operator detects this and skips `MemberRemove` to avoid a deadlock — see `handleDeletion` in `controllers/etcdmember_controller.go`). Pods are then GC'd via owner-refs. Data volumes carry no owner reference by design ([data volume lifecycle](concepts.md#data-volume-lifecycle)), so they are reclaimed explicitly by the cluster's own finalizer, which is held until they are actually gone — a deleted `EtcdCluster` therefore implies its storage is gone too, and a namespace delete does not complete while volumes are still draining. If the operator is uninstalled while `EtcdCluster` resources still exist, they're stranded — the finalizers won't run because no controller is reading the queue. Recovery is to either re-install the operator, or `kubectl patch ... --type=merge -p '{"metadata":{"finalizers":null}}'` on each `EtcdMember` (manual, leaves PVCs and Pods in place — clean them up by label). diff --git a/docs/operations.md b/docs/operations.md index 55acfa7e..fc0da235 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -161,10 +161,10 @@ kubectl get etcdmember,pvc -l etcd-operator.cozystack.io/cluster= -n .yaml ``` -Volumes from the deleted cluster are **not** garbage-collected (see [data volume lifecycle](concepts.md#data-volume-lifecycle)) — they remain as `detached` PVCs. A recreated cluster never adopts them: its members get fresh names and fresh UIDs, and the member-UID check refuses anything that is not theirs. Delete the leftovers yourself once you have confirmed the data is not needed: +Deleting the cluster reclaims its data volumes too — the cluster finalizer removes them and is only released once they are gone (see [data volume lifecycle](concepts.md#data-volume-lifecycle)). So a recreated cluster starts clean; wait for the `EtcdCluster` to actually disappear before re-applying: ```sh -kubectl get pvc -l etcd-operator.cozystack.io/cluster= -n +kubectl get etcdcluster.etcd-operator.cozystack.io,pvc -l etcd-operator.cozystack.io/cluster= -n ``` ### `Available=False/DeadlineExceeded`