Skip to content

Commit 30c8209

Browse files
authored
fix: PVCs being garbage-collected despite retention policy (#10043)
(cherry picked from commit 7a1c7f6)
1 parent ef130c9 commit 30c8209

2 files changed

Lines changed: 35 additions & 4 deletions

File tree

controllers/workloads/instanceset_controller_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,11 @@ var _ = Describe("InstanceSet Controller", func() {
344344
Consistently(testapps.CheckObj(&testCtx, pvcKey, func(g Gomega, pvc *corev1.PersistentVolumeClaim) {
345345
g.Expect(pvc.DeletionTimestamp).Should(BeNil())
346346
})).Should(Succeed())
347+
Eventually(testapps.CheckObj(&testCtx, pvcKey, func(g Gomega, pvc *corev1.PersistentVolumeClaim) {
348+
// verify owner references are cleared to prevent garbage collection
349+
ownerRefs := pvc.GetOwnerReferences()
350+
g.Expect(ownerRefs).Should(HaveLen(0), "Owner references should be cleared when retention policy is Retain")
351+
})).Should(Succeed())
347352
})
348353

349354
It("when scaled - delete", func() {
@@ -405,6 +410,10 @@ var _ = Describe("InstanceSet Controller", func() {
405410
}
406411
Consistently(testapps.CheckObj(&testCtx, pvcKey, func(g Gomega, pvc *corev1.PersistentVolumeClaim) {
407412
g.Expect(pvc.DeletionTimestamp).Should(BeNil())
413+
// verify owner references still exist since InstanceSet is not deleted
414+
ownerRefs := pvc.GetOwnerReferences()
415+
g.Expect(ownerRefs).Should(HaveLen(1), "Owner references should still exist when scaling down and retention policy is Retain")
416+
g.Expect(ownerRefs[0].Kind).Should(Equal("InstanceSet"))
408417
})).Should(Succeed())
409418
})
410419
})

pkg/controller/instanceset/reconciler_deletion.go

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"maps"
2424

2525
corev1 "k8s.io/api/core/v1"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2627

2728
kbappsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
2829
workloads "github.com/apecloud/kubeblocks/apis/workloads/v1"
@@ -49,7 +50,7 @@ func (r *deletionReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuild
4950
retainPVC := pvcRetentionPolicy != nil && pvcRetentionPolicy.WhenDeleted == kbappsv1.RetainPersistentVolumeClaimRetentionPolicyType
5051

5152
// delete secondary objects first
52-
if has, err := r.deleteSecondaryObjects(tree, retainPVC); has {
53+
if has, err := r.deleteSecondaryObjects(tree, its, retainPVC); has {
5354
return kubebuilderx.Continue, err
5455
}
5556

@@ -58,13 +59,34 @@ func (r *deletionReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuild
5859
return kubebuilderx.Continue, nil
5960
}
6061

61-
func (r *deletionReconciler) deleteSecondaryObjects(tree *kubebuilderx.ObjectTree, retainPVC bool) (bool, error) {
62+
func (r *deletionReconciler) deleteSecondaryObjects(tree *kubebuilderx.ObjectTree, its *workloads.InstanceSet, retainPVC bool) (bool, error) {
6263
// secondary objects to be deleted
6364
secondaryObjects := maps.Clone(tree.GetSecondaryObjects())
6465
if retainPVC {
65-
// exclude PVCs from them
66+
// exclude PVCs from them and remove InstanceSet's owner references
6667
pvcList := tree.List(&corev1.PersistentVolumeClaim{})
67-
for _, pvc := range pvcList {
68+
for _, pvcObj := range pvcList {
69+
pvc, ok := pvcObj.(*corev1.PersistentVolumeClaim)
70+
if !ok {
71+
continue
72+
}
73+
// Remove InstanceSet's owner references to prevent garbage collection
74+
ownerRefs := pvc.GetOwnerReferences()
75+
if len(ownerRefs) > 0 {
76+
// Filter out owner references that belong to this InstanceSet
77+
filteredRefs := make([]metav1.OwnerReference, 0, len(ownerRefs))
78+
for _, ref := range ownerRefs {
79+
if ref.UID != its.UID {
80+
filteredRefs = append(filteredRefs, ref)
81+
}
82+
}
83+
if len(filteredRefs) != len(ownerRefs) {
84+
pvc.SetOwnerReferences(filteredRefs)
85+
if err := tree.Update(pvc); err != nil {
86+
return true, err
87+
}
88+
}
89+
}
6890
name, err := model.GetGVKName(pvc)
6991
if err != nil {
7092
return true, err

0 commit comments

Comments
 (0)