fix: PVCs being garbage-collected despite retention policy (#10042)

(cherry picked from commit 7a1c7f6)

Author: leon-ape

controllers/workloads/instance_controller_test.go

@@ -248,6 +248,11 @@ var _ = Describe("Instance Controller", func() {
 			Consistently(testapps.CheckObj(&testCtx, pvcKey, func(g Gomega, pvc *corev1.PersistentVolumeClaim) {
 				g.Expect(pvc.DeletionTimestamp).Should(BeNil())
 			})).Should(Succeed())
+			Eventually(testapps.CheckObj(&testCtx, pvcKey, func(g Gomega, pvc *corev1.PersistentVolumeClaim) {
+				// verify owner references are cleared to prevent garbage collection
+				ownerRefs := pvc.GetOwnerReferences()
+				g.Expect(ownerRefs).Should(HaveLen(0), "Owner references should be cleared when retention policy is Retain")
+			})).Should(Succeed())
 		})
 	})
 

controllers/workloads/instanceset_controller_test.go

@@ -345,6 +345,11 @@ var _ = Describe("InstanceSet Controller", func() {
 			Consistently(testapps.CheckObj(&testCtx, pvcKey, func(g Gomega, pvc *corev1.PersistentVolumeClaim) {
 				g.Expect(pvc.DeletionTimestamp).Should(BeNil())
 			})).Should(Succeed())
+			Eventually(testapps.CheckObj(&testCtx, pvcKey, func(g Gomega, pvc *corev1.PersistentVolumeClaim) {
+				// verify owner references are cleared to prevent garbage collection
+				ownerRefs := pvc.GetOwnerReferences()
+				g.Expect(ownerRefs).Should(HaveLen(0), "Owner references should be cleared when retention policy is Retain")
+			})).Should(Succeed())
 		})
 
 		It("when scaled - delete", func() {
@@ -406,6 +411,10 @@ var _ = Describe("InstanceSet Controller", func() {
 			}
 			Consistently(testapps.CheckObj(&testCtx, pvcKey, func(g Gomega, pvc *corev1.PersistentVolumeClaim) {
 				g.Expect(pvc.DeletionTimestamp).Should(BeNil())
+				// verify owner references still exist since InstanceSet is not deleted
+				ownerRefs := pvc.GetOwnerReferences()
+				g.Expect(ownerRefs).Should(HaveLen(1), "Owner references should still exist when scaling down and retention policy is Retain")
+				g.Expect(ownerRefs[0].Kind).Should(Equal("InstanceSet"))
 			})).Should(Succeed())
 		})
 	})

pkg/controller/instance/reconciler_deletion.go

@@ -23,6 +23,7 @@ import (
 	"maps"
 
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
 
 	appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
@@ -59,7 +60,7 @@ func (r *deletionReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuild
 	}
 
 	// delete secondary objects first
-	if has, err := r.deleteSecondaryObjects(tree, retainPVC); has {
+	if has, err := r.deleteSecondaryObjects(tree, inst, retainPVC); has {
 		return kubebuilderx.Continue, err
 	}
 
@@ -68,13 +69,34 @@ func (r *deletionReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuild
 	return kubebuilderx.Continue, nil
 }
 
-func (r *deletionReconciler) deleteSecondaryObjects(tree *kubebuilderx.ObjectTree, retainPVC bool) (bool, error) {
+func (r *deletionReconciler) deleteSecondaryObjects(tree *kubebuilderx.ObjectTree, inst *workloads.Instance, retainPVC bool) (bool, error) {
 	// secondary objects to be deleted
 	secondaryObjects := maps.Clone(tree.GetSecondaryObjects())
 	if retainPVC {
-		// exclude PVCs from them
+		// exclude PVCs from them and clear their owner references
 		pvcList := tree.List(&corev1.PersistentVolumeClaim{})
-		for _, pvc := range pvcList {
+		for _, pvcObj := range pvcList {
+			pvc, ok := pvcObj.(*corev1.PersistentVolumeClaim)
+			if !ok {
+				continue
+			}
+			// Clear owner references to prevent garbage collection when Instance is deleted
+			ownerRefs := pvc.GetOwnerReferences()
+			if len(ownerRefs) > 0 {
+				// Filter out owner references that belong to this Instance
+				filteredRefs := make([]metav1.OwnerReference, 0, len(ownerRefs))
+				for _, ref := range ownerRefs {
+					if ref.UID != inst.UID {
+						filteredRefs = append(filteredRefs, ref)
+					}
+				}
+				if len(filteredRefs) != len(ownerRefs) {
+					pvc.SetOwnerReferences(filteredRefs)
+					if err := tree.Update(pvc); err != nil {
+						return true, err
+					}
+				}
+			}
 			name, err := model.GetGVKName(pvc)
 			if err != nil {
 				return true, err

pkg/controller/instanceset/reconciler_deletion.go

@@ -23,6 +23,7 @@ import (
 	"maps"
 
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	kbappsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
 	workloads "github.com/apecloud/kubeblocks/apis/workloads/v1"
@@ -49,7 +50,7 @@ func (r *deletionReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuild
 	retainPVC := pvcRetentionPolicy != nil && pvcRetentionPolicy.WhenDeleted == kbappsv1.RetainPersistentVolumeClaimRetentionPolicyType
 
 	// delete secondary objects first
-	if has, err := r.deleteSecondaryObjects(tree, retainPVC); has {
+	if has, err := r.deleteSecondaryObjects(tree, its, retainPVC); has {
 		return kubebuilderx.Continue, err
 	}
 
@@ -58,13 +59,34 @@ func (r *deletionReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuild
 	return kubebuilderx.Continue, nil
 }
 
-func (r *deletionReconciler) deleteSecondaryObjects(tree *kubebuilderx.ObjectTree, retainPVC bool) (bool, error) {
+func (r *deletionReconciler) deleteSecondaryObjects(tree *kubebuilderx.ObjectTree, its *workloads.InstanceSet, retainPVC bool) (bool, error) {
 	// secondary objects to be deleted
 	secondaryObjects := maps.Clone(tree.GetSecondaryObjects())
 	if retainPVC {
-		// exclude PVCs from them
+		// exclude PVCs from them and remove InstanceSet's owner references
 		pvcList := tree.List(&corev1.PersistentVolumeClaim{})
-		for _, pvc := range pvcList {
+		for _, pvcObj := range pvcList {
+			pvc, ok := pvcObj.(*corev1.PersistentVolumeClaim)
+			if !ok {
+				continue
+			}
+			// Remove InstanceSet's owner references to prevent garbage collection
+			ownerRefs := pvc.GetOwnerReferences()
+			if len(ownerRefs) > 0 {
+				// Filter out owner references that belong to this InstanceSet
+				filteredRefs := make([]metav1.OwnerReference, 0, len(ownerRefs))
+				for _, ref := range ownerRefs {
+					if ref.UID != its.UID {
+						filteredRefs = append(filteredRefs, ref)
+					}
+				}
+				if len(filteredRefs) != len(ownerRefs) {
+					pvc.SetOwnerReferences(filteredRefs)
+					if err := tree.Update(pvc); err != nil {
+						return true, err
+					}
+				}
+			}
 			name, err := model.GetGVKName(pvc)
 			if err != nil {
 				return true, err