[Bug]: Detached PVCs may be reused after destroy --keep-pvc #11632

Activity

1. devantler commented on Oct 10, 2026

   

   devantler

   on Oct 10, 2026

   Author

   More actions

   I have now run this against the v1.30.1 code, so the report no longer rests on source reading alone. It reproduces with the code of the official command.

   What I ran. A test added to internal/cmd/plugin/destroy at tag v1.30.1 (2a35abb), using the same fake client the existing tests use. It creates a three-instance cluster (pg-1, pg-2, pg-3) with owned, bound claims and ready Pods, calls Destroy(ctx, "pg", "pg-2", true), and then calls reconcileMissingInstance three times, marking the join Job as succeeded after the first.

   Result.

   	Observed
   After Destroy(..., keepPVC=true)	claim pg-2 keeps its UID, has no owner reference, cnpg.io/pvcStatus=detached
   Every reconcile	Status.InstanceNames is [pg-1 pg-3], generateNodeSerial returns 2, Job pg-2-join exists and its Pod template mounts claim pg-2
   After the Job has succeeded	the same again: serial 2, the Job is adopted, the claim is still unowned and detached, no pg-2 Pod is created
   Control, keepPVC=false	a new claim owned by the cluster is created and InstanceNames returns to [pg-1 pg-2 pg-3]

   So with contiguous serials the replacement is always given the name of the instance that was just destroyed. createIfNotExists accepts the retained claim because it exists and is not terminating. That claim is never owned again, so it never shows up in InstanceNames, and the reconciler picks the same serial on every loop. This matches what I saw on a v1.30.1 cluster: "Creating new Job" followed by "Job already exists, adopting it", repeating, with the instance never coming back.

   Limits. This is the fake client, not a cluster: no kubelet ran, nothing was mounted and no base backup was taken. I called reconcileMissingInstance directly rather than the whole Reconcile. What EnsureTargetDirectoriesDoNotExist then does with the retained data directory is from reading the code, not from this test. I have not run this on main, but createIfNotExists there still has no owner or detached check.

   Patch against v1.30.1

   The forwarder file only exposes unexported reconciler functions to the test; it adds no logic.

   go test ./internal/cmd/plugin/destroy/ -v -ginkgo.focus REPRO -ginkgo.v

   diff --git a/internal/cmd/plugin/destroy/zz_repro_test.go b/internal/cmd/plugin/destroy/zz_repro_test.go
   new file mode 100644
   index 0000000..053be84
   --- /dev/null
   +++ b/internal/cmd/plugin/destroy/zz_repro_test.go
   @@ -0,0 +1,244 @@
   +package destroy
   +
   +import (
   +	"context"
   +	"fmt"
   +
   +	batchv1 "k8s.io/api/batch/v1"
   +	corev1 "k8s.io/api/core/v1"
   +	apierrs "k8s.io/apimachinery/pkg/api/errors"
   +	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
   +	"k8s.io/apimachinery/pkg/types"
   +	"k8s.io/client-go/tools/record"
   +	"sigs.k8s.io/controller-runtime/pkg/client"
   +	"sigs.k8s.io/controller-runtime/pkg/client/fake"
   +
   +	apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
   +	"github.com/cloudnative-pg/cloudnative-pg/internal/cmd/plugin"
   +	"github.com/cloudnative-pg/cloudnative-pg/internal/controller"
   +	"github.com/cloudnative-pg/cloudnative-pg/internal/scheme"
   +	"github.com/cloudnative-pg/cloudnative-pg/pkg/postgres"
   +	"github.com/cloudnative-pg/cloudnative-pg/pkg/reconciler/persistentvolumeclaim"
   +	"github.com/cloudnative-pg/cloudnative-pg/pkg/utils"
   +
   +	. "github.com/onsi/ginkgo/v2"
   +	. "github.com/onsi/gomega"
   +)
   +
   +// Local reproduction, not part of upstream. It runs the real destroy command
   +// code and the reconciler's real replica-creation step against a fake API
   +// server holding a healthy three-instance cluster.
   +var _ = Describe("REPRO detached claim after destroy --keep-pvc", func() {
   +	const (
   +		ns   = "repro"
   +		name = "pg"
   +	)
   +
   +	type world struct {
   +		cli     client.Client
   +		r       *controller.ClusterReconciler
   +		cluster *apiv1.Cluster
   +	}
   +
   +	originalUID := func(serial int) types.UID { return types.UID(fmt.Sprintf("original-claim-%d", serial)) }
   +
   +	newWorld := func(ctx context.Context) *world {
   +		sch := scheme.BuildWithAllKnownScheme()
   +		cli := fake.NewClientBuilder().WithScheme(sch).
   +			WithStatusSubresource(&apiv1.Cluster{}).
   +			WithIndex(&corev1.PersistentVolumeClaim{}, controller.ReproOwnerKey, func(o client.Object) []string {
   +				if owner, ok := controller.IsOwnedByCluster(o); ok {
   +					return []string{owner}
   +				}
   +				return nil
   +			}).
   +			WithIndex(&batchv1.Job{}, controller.ReproOwnerKey, controller.ReproJobOwnerIndex).
   +			WithIndex(&apiv1.Backup{}, ".spec.cluster.name", func(o client.Object) []string {
   +				return []string{o.(*apiv1.Backup).Spec.Cluster.Name}
   +			}).
   +			Build()
   +
   +		cluster := &apiv1.Cluster{
   +			ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns},
   +			Spec: apiv1.ClusterSpec{
   +				Instances:            3,
   +				StorageConfiguration: apiv1.StorageConfiguration{Size: "1Gi"},
   +			},
   +		}
   +		cluster.SetDefaults()
   +		Expect(cli.Create(ctx, cluster)).To(Succeed())
   +		cluster.TypeMeta = metav1.TypeMeta{Kind: apiv1.ClusterKind, APIVersion: apiv1.SchemeGroupVersion.String()}
   +
   +		for serial := 1; serial <= 3; serial++ {
   +			pvc, err := persistentvolumeclaim.Build(cluster, &persistentvolumeclaim.CreateConfiguration{
   +				Status:     persistentvolumeclaim.StatusReady,
   +				NodeSerial: serial,
   +				Calculator: persistentvolumeclaim.NewPgDataCalculator(),
   +				Storage:    cluster.Spec.StorageConfiguration,
   +			})
   +			Expect(err).ToNot(HaveOccurred())
   +			cluster.SetInheritedDataAndOwnership(&pvc.ObjectMeta)
   +			pvc.UID = originalUID(serial)
   +			pvc.Status.Phase = corev1.ClaimBound
   +			Expect(cli.Create(ctx, pvc)).To(Succeed())
   +
   +			pod := &corev1.Pod{
   +				ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("%s-%d", name, serial), Namespace: ns},
   +				Status: corev1.PodStatus{
   +					Phase:      corev1.PodRunning,
   +					Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}},
   +				},
   +			}
   +			cluster.SetInheritedDataAndOwnership(&pod.ObjectMeta)
   +			Expect(cli.Create(ctx, pod)).To(Succeed())
   +		}
   +
   +		plugin.Client = cli
   +		plugin.Namespace = ns
   +
   +		return &world{
   +			cli: cli,
   +			r: &controller.ClusterReconciler{
   +				Client:   cli,
   +				Scheme:   sch,
   +				Recorder: record.NewFakeRecorder(120),
   +			},
   +			cluster: cluster,
   +		}
   +	}
   +
   +	// observe does what the reconciler does at the start of a loop: list the
   +	// objects it owns and derive the instance list from the owned claims.
   +	observe := func(ctx context.Context, w *world) (
   +		corev1.PodList, corev1.PersistentVolumeClaimList, batchv1.JobList, postgres.PostgresqlStatusList,
   +	) {
   +		Expect(w.cli.Get(ctx, client.ObjectKeyFromObject(w.cluster), w.cluster)).To(Succeed())
   +		w.cluster.TypeMeta = metav1.TypeMeta{Kind: apiv1.ClusterKind, APIVersion: apiv1.SchemeGroupVersion.String()}
   +
   +		pvcs, err := w.r.ReproGetManagedPVCs(ctx, w.cluster)
   +		Expect(err).ToNot(HaveOccurred())
   +		jobs, err := w.r.ReproGetManagedJobs(ctx, w.cluster)
   +		Expect(err).ToNot(HaveOccurred())
   +		var pods corev1.PodList
   +		Expect(w.cli.List(ctx, &pods, client.InNamespace(ns))).To(Succeed())
   +
   +		persistentvolumeclaim.EnrichStatus(ctx, w.cluster, pods.Items, jobs.Items, pvcs.Items)
   +
   +		var st postgres.PostgresqlStatusList
   +		for i := range pods.Items {
   +			st.Items = append(st.Items, postgres.PostgresqlStatus{Pod: &pods.Items[i]})
   +		}
   +		return pods, pvcs, jobs, st
   +	}
   +
   +	claim := func(ctx context.Context, w *world, serial int) corev1.PersistentVolumeClaim {
   +		var pvc corev1.PersistentVolumeClaim
   +		Expect(w.cli.Get(ctx, client.ObjectKey{Namespace: ns, Name: fmt.Sprintf("%s-%d", name, serial)}, &pvc)).
   +			To(Succeed())
   +		return pvc
   +	}
   +
   +	joinJob := func(ctx context.Context, w *world, serial int) batchv1.Job {
   +		var job batchv1.Job
   +		Expect(w.cli.Get(ctx, client.ObjectKey{Namespace: ns, Name: fmt.Sprintf("%s-%d-join", name, serial)}, &job)).
   +			To(Succeed())
   +		return job
   +	}
   +
   +	claimsMountedBy := func(job batchv1.Job) []string {
   +		var names []string
   +		for _, v := range job.Spec.Template.Spec.Volumes {
   +			if v.PersistentVolumeClaim != nil {
   +				names = append(names, v.PersistentVolumeClaim.ClaimName)
   +			}
   +		}
   +		return names
   +	}
   +
   +	It("reuses the detached claim and never restores the instance", func(ctx SpecContext) {
   +		redirectStdoutToGinkgoWriter()
   +		w := newWorld(ctx)
   +
   +		observe(ctx, w)
   +		GinkgoWriter.Printf("before destroy: instances=%d names=%v\n", w.cluster.Status.Instances, w.cluster.Status.InstanceNames)
   +		Expect(w.cluster.Status.InstanceNames).To(Equal([]string{"pg-1", "pg-2", "pg-3"}))
   +
   +		// The official command's own code path: kubectl cnpg destroy pg 2 --keep-pvc
   +		Expect(Destroy(ctx, name, "pg-2", true)).To(Succeed())
   +
   +		kept := claim(ctx, w, 2)
   +		GinkgoWriter.Printf("after destroy --keep-pvc: claim pg-2 uid=%s owners=%d %s=%s\n",
   +			kept.UID, len(kept.OwnerReferences), utils.PVCStatusAnnotationName, kept.Annotations[utils.PVCStatusAnnotationName])
   +		Expect(kept.UID).To(Equal(originalUID(2)))
   +		Expect(kept.OwnerReferences).To(BeEmpty())
   +		Expect(kept.Annotations[utils.PVCStatusAnnotationName]).To(Equal(persistentvolumeclaim.StatusDetached))
   +
   +		for round := 1; round <= 3; round++ {
   +			pods, pvcs, jobs, st := observe(ctx, w)
   +			Expect(w.cluster.Status.InstanceNames).To(Equal([]string{"pg-1", "pg-3"}))
   +			Expect(w.cluster.Status.Instances).To(Equal(2))
   +
   +			seenInstances, seenNames := w.cluster.Status.Instances, w.cluster.Status.InstanceNames
   +			serial := w.r.ReproGenerateNodeSerial(w.cluster)
   +			Expect(serial).To(Equal(2))
   +
   +			_, err := w.r.ReproReconcileMissingInstance(ctx, w.cluster, pods, pvcs, jobs, st)
   +			Expect(err).To(MatchError(controller.ErrNextLoop))
   +
   +			job := joinJob(ctx, w, 2)
   +			Expect(claimsMountedBy(job)).To(ContainElement("pg-2"))
   +
   +			// The claim the join Job mounts is the retained one: same object,
   +			// still not owned by the cluster, still marked detached.
   +			kept = claim(ctx, w, 2)
   +			Expect(kept.UID).To(Equal(originalUID(2)))
   +			Expect(kept.OwnerReferences).To(BeEmpty())
   +			Expect(kept.Annotations[utils.PVCStatusAnnotationName]).To(Equal(persistentvolumeclaim.StatusDetached))
   +
   +			// No instance Pod is ever created for it.
   +			var pod corev1.Pod
   +			err = w.cli.Get(ctx, client.ObjectKey{Namespace: ns, Name: "pg-2"}, &pod)
   +			Expect(apierrs.IsNotFound(err)).To(BeTrue())
   +
   +			GinkgoWriter.Printf(
   +				"round %d: instances=%d names=%v next serial=%d job=%s mounts=%v succeeded=%d claim uid=%s owners=%d status=%s pod pg-2 exists=false\n",
   +				round, seenInstances, seenNames, serial, job.Name,
   +				claimsMountedBy(job), job.Status.Succeeded, kept.UID, len(kept.OwnerReferences),
   +				kept.Annotations[utils.PVCStatusAnnotationName])
   +
   +			if round == 1 {
   +				// What the kubelet reports once the join has finished.
   +				job.Status.Succeeded = 1
   +				Expect(w.cli.Status().Update(ctx, &job)).To(Succeed())
   +			}
   +		}
   +	})
   +
   +	It("control: without --keep-pvc the replacement gets a new claim and is counted again", func(ctx SpecContext) {
   +		redirectStdoutToGinkgoWriter()
   +		w := newWorld(ctx)
   +
   +		Expect(Destroy(ctx, name, "pg-2", false)).To(Succeed())
   +
   +		pods, pvcs, jobs, st := observe(ctx, w)
   +		Expect(w.cluster.Status.InstanceNames).To(Equal([]string{"pg-1", "pg-3"}))
   +		Expect(w.r.ReproGenerateNodeSerial(w.cluster)).To(Equal(2))
   +
   +		_, err := w.r.ReproReconcileMissingInstance(ctx, w.cluster, pods, pvcs, jobs, st)
   +		Expect(err).To(MatchError(controller.ErrNextLoop))
   +
   +		fresh := claim(ctx, w, 2)
   +		Expect(fresh.UID).ToNot(Equal(originalUID(2)))
   +		owner, owned := controller.IsOwnedByCluster(&fresh)
   +		Expect(owned).To(BeTrue())
   +		Expect(owner).To(Equal(name))
   +
   +		// Once the storage layer binds the new claim, it is an instance again.
   +		fresh.Status.Phase = corev1.ClaimBound
   +		Expect(w.cli.Status().Update(ctx, &fresh)).To(Succeed())
   +		observe(ctx, w)
   +		GinkgoWriter.Printf("control: new claim owner=%s uid=%q instances=%d names=%v\n",
   +			owner, fresh.UID, w.cluster.Status.Instances, w.cluster.Status.InstanceNames)
   +		Expect(w.cluster.Status.InstanceNames).To(Equal([]string{"pg-1", "pg-2", "pg-3"}))
   +	})
   +})
   diff --git a/internal/controller/zz_repro_forwarders.go b/internal/controller/zz_repro_forwarders.go
   new file mode 100644
   index 0000000..78d2812
   --- /dev/null
   +++ b/internal/controller/zz_repro_forwarders.go
   @@ -0,0 +1,57 @@
   +package controller
   +
   +import (
   +	"context"
   +
   +	batchv1 "k8s.io/api/batch/v1"
   +	corev1 "k8s.io/api/core/v1"
   +	ctrl "sigs.k8s.io/controller-runtime"
   +	"sigs.k8s.io/controller-runtime/pkg/client"
   +
   +	apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
   +	"github.com/cloudnative-pg/cloudnative-pg/pkg/postgres"
   +)
   +
   +// Forwarders for a local reproduction, not part of upstream. They add no logic
   +// of their own: each one only calls the unexported function, or exposes the
   +// unexported value, that it names.
   +
   +// ReproOwnerKey is the field index key the reconciler lists owned objects by.
   +const ReproOwnerKey = pvcOwnerKey
   +
   +// ReproJobOwnerIndex is the reconciler's own Job owner index function.
   +func ReproJobOwnerIndex(o client.Object) []string { return jobOwnerIndexFunc(o) }
   +
   +// ReproGetManagedPVCs calls getManagedPVCs.
   +func (r *ClusterReconciler) ReproGetManagedPVCs(
   +	ctx context.Context, cluster *apiv1.Cluster,
   +) (corev1.PersistentVolumeClaimList, error) {
   +	return r.getManagedPVCs(ctx, cluster)
   +}
   +
   +// ReproGetManagedJobs calls getManagedJobs.
   +func (r *ClusterReconciler) ReproGetManagedJobs(
   +	ctx context.Context, cluster *apiv1.Cluster,
   +) (batchv1.JobList, error) {
   +	return r.getManagedJobs(ctx, cluster)
   +}
   +
   +// ReproGenerateNodeSerial calls generateNodeSerial.
   +func (r *ClusterReconciler) ReproGenerateNodeSerial(cluster *apiv1.Cluster) int {
   +	return r.generateNodeSerial(cluster)
   +}
   +
   +// ReproReconcileMissingInstance calls reconcileMissingInstance with the
   +// resources the reconciler would have gathered.
   +func (r *ClusterReconciler) ReproReconcileMissingInstance(
   +	ctx context.Context,
   +	cluster *apiv1.Cluster,
   +	pods corev1.PodList,
   +	pvcs corev1.PersistentVolumeClaimList,
   +	jobs batchv1.JobList,
   +	instancesStatus postgres.PostgresqlStatusList,
   +) (ctrl.Result, error) {
   +	return r.reconcileMissingInstance(
   +		ctx, cluster, &managedResources{instances: pods, pvcs: pvcs, jobs: jobs}, instancesStatus,
   +	)
   +}

   One possible direction, if you agree this should not happen: ensureJobAdoptable already refuses an existing Job the cluster does not own, and createIfNotExists could do the same for an existing claim. I am happy to turn the test into a proper regression test or try that fix once the intended behaviour is agreed.

   AI assistance: Claude Opus 5.5 was used to write and run this test.
2. robertobandini commented on Nov 2, 2026

   

   robertobandini

   on Nov 2, 2026

   More actions

   This reproduces with the official command on a real cluster: CloudNativePG 1.30.1 from releases/cnpg-1.30.1.yaml on kind (kindest/node:v1.36.4), plugin kubectl-cnpg_1.30.1_linux_x86_64.tar.gz of the release (Build: {Version:1.30.1 Commit:2a35abb46 Date:2026-09-23}), on the replica of a two-instance cluster:

   kubectl cnpg destroy e2e-actions 1 --keep-pvc -n cnpg-e2e-actions
   # Instance e2e-actions-1 of cluster e2e-actions has been destroyed and the PVC was kept

   • The operator picks serial 1 again, and the join Job e2e-actions-1-join mounts the detached claim e2e-actions-1: the bootstrap finds the old data directory (pg_controldata check on existing directory succeeded, renaming the folders), runs pg_basebackup and completes in 5 s.
   • The Pod e2e-actions-1 is never created; the claim stays detached with no owner; the cluster stays in "Creating a new replica", 1 ready of 2.
   • The operator logs "Creating new Job" then "Job already exists, adopting it" for that Job about 8 times a second: 1,743 times in 3.5 minutes, nothing else for the cluster.
   • It recovers once the detached claim is deleted, then the completed Job (the Job's Pod holds the claim; deleting the Job first makes the operator create it again on the same claim).

   I filed it as #11829 before finding this issue, and closed it as a duplicate; it has the Cluster resource and the operator log lines.

   👍React with 👍1Reacted by Nikolai Emil Damm
3. robertobandini

   marked [Bug]: kubectl cnpg destroy --keep-pvc on a replica leaves the cluster in "Creating a new replica" #11829 as a duplicate of this issue on Nov 2, 2026