[Fix] GT-11 Orphan PVC are not removed (#962)

Author: jwierzbo

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Change Log
 
 ## [master](https://github.com/arangodb/kube-arangodb/tree/master) (N/A)
+- (Bugfix) Orphan PVC are not removed
 
 ## [1.2.10](https://github.com/arangodb/kube-arangodb/tree/1.2.10) (2022-04-27)
 - (Feature) Allow configuration for securityContext.runAsUser value

pkg/deployment/deployment_inspector.go

@@ -155,6 +155,7 @@ func (d *Deployment) inspectDeployment(lastInterval util.Interval) util.Interval
 	return nextInterval.ReduceTo(maxInspectionInterval)
 }
 
+// inspectDeploymentWithError ensures that the deployment is in a valid state
 func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterval util.Interval) (nextInterval util.Interval, inspectError error) {
 	t := time.Now()
 
@@ -434,6 +435,7 @@ func (d *Deployment) refreshMaintenanceTTL(ctx context.Context) {
 	}
 }
 
+// ensureResources creates all required resources for the deployment
 func (d *Deployment) ensureResources(ctx context.Context, lastInterval util.Interval, cachedStatus inspectorInterface.Inspector) (util.Interval, error) {
 	// Ensure all resources are created
 	if d.haveServiceMonitorCRD {

pkg/deployment/resources/labels.go

@@ -209,7 +209,8 @@ func (r *Resources) EnsurePodsLabels(ctx context.Context, cachedStatus inspector
 
 func (r *Resources) EnsurePersistentVolumeClaimsLabels(ctx context.Context, cachedStatus inspectorInterface.Inspector) error {
 	changed := false
-	if err := cachedStatus.PersistentVolumeClaim().V1().Iterate(func(persistentVolumeClaim *core.PersistentVolumeClaim) error {
+
+	actionFn := func(persistentVolumeClaim *core.PersistentVolumeClaim) error {
 		if ensureGroupLabelsMap(persistentVolumeClaim.Kind, persistentVolumeClaim, r.context.GetSpec(), func(name string, d []byte) error {
 			return globals.GetGlobalTimeouts().Kubernetes().RunWithTimeout(ctx, func(ctxChild context.Context) error {
 				_, err := r.context.PersistentVolumeClaimsModInterface().Patch(ctxChild, name, types.JSONPatchType, d, meta.PatchOptions{})
@@ -220,9 +221,13 @@ func (r *Resources) EnsurePersistentVolumeClaimsLabels(ctx context.Context, cach
 		}
 
 		return nil
-	}, func(persistentVolumeClaim *core.PersistentVolumeClaim) bool {
+	}
+
+	ownerFilterFn := func(persistentVolumeClaim *core.PersistentVolumeClaim) bool {
 		return r.isChildResource(persistentVolumeClaim)
-	}); err != nil {
+	}
+
+	if err := cachedStatus.PersistentVolumeClaim().V1().Iterate(actionFn, ownerFilterFn); err != nil {
 		return err
 	}
 

pkg/deployment/resources/pvc_inspector.go

@@ -24,14 +24,17 @@ import (
 	"context"
 	"time"
 
-	"github.com/arangodb/kube-arangodb/pkg/util/errors"
-	inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector"
-
 	v1 "k8s.io/api/core/v1"
+	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 
+	"github.com/arangodb/kube-arangodb/pkg/deployment/patch"
 	"github.com/arangodb/kube-arangodb/pkg/metrics"
 	"github.com/arangodb/kube-arangodb/pkg/util"
+	"github.com/arangodb/kube-arangodb/pkg/util/errors"
+	"github.com/arangodb/kube-arangodb/pkg/util/globals"
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
+	inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector"
 	pvcv1 "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector/persistentvolumeclaim/v1"
 )
 
@@ -76,6 +79,26 @@ func (r *Resources) InspectPVCs(ctx context.Context, cachedStatus inspectorInter
 			return nil
 		}
 
+		owner := r.context.GetAPIObject().AsOwner()
+		if k8sutil.UpdateOwnerRefToObjectIfNeeded(pvc.GetObjectMeta(), &owner) {
+			q := patch.NewPatch(patch.ItemReplace(patch.NewPath("metadata", "ownerReferences"), pvc.ObjectMeta.OwnerReferences))
+			d, err := q.Marshal()
+			if err != nil {
+				log.Debug().Err(err).Msg("Failed to prepare PVC patch (ownerReferences)")
+				return errors.WithStack(err)
+			}
+
+			err = globals.GetGlobalTimeouts().Kubernetes().RunWithTimeout(ctx, func(ctxChild context.Context) error {
+				_, err := r.context.PersistentVolumeClaimsModInterface().Patch(ctxChild, pvc.GetName(), types.JSONPatchType, d, meta.PatchOptions{})
+				return err
+			})
+
+			if err != nil {
+				log.Debug().Err(err).Msg("Failed to update PVC (ownerReferences)")
+				return errors.WithStack(err)
+			}
+		}
+
 		if k8sutil.IsPersistentVolumeClaimMarkedForDeletion(pvc) {
 			// Process finalizers
 			if x, err := r.runPVCFinalizers(ctx, pvc, group, memberStatus); err != nil {

pkg/util/k8sutil/util.go

@@ -56,6 +56,21 @@ func AddOwnerRefToObject(obj metav1.Object, ownerRef *metav1.OwnerReference) {
 	}
 }
 
+// UpdateOwnerRefToObjectIfNeeded add given owner reference to given object if it does not exist yet
+func UpdateOwnerRefToObjectIfNeeded(obj metav1.Object, ownerRef *metav1.OwnerReference) bool {
+	if ownerRef != nil {
+		for _, existingOwnerRef := range obj.GetOwnerReferences() {
+			if existingOwnerRef.UID == ownerRef.UID {
+				return false
+			}
+		}
+
+		AddOwnerRefToObject(obj, ownerRef)
+		return true
+	}
+	return false
+}
+
 // LabelsForExporterServiceSelector returns a map of labels, used to select the all arangodb-exporter containers
 func LabelsForExporterServiceSelector(deploymentName string) map[string]string {
 	return map[string]string{

pkg/util/k8sutil/util_test.go

@@ -38,6 +38,22 @@ func TestAddOwnerRefToObject(t *testing.T) {
 	assert.Len(t, p.GetOwnerReferences(), 1)
 }
 
+// UpdateOwnerRefToObjectIfNeeded tests UpdateOwnerRefToObjectIfNeeded.
+func TestUpdateOwnerRefToObjectIfNeeded(t *testing.T) {
+	p := &v1.Pod{}
+	result := UpdateOwnerRefToObjectIfNeeded(p, nil)
+	assert.Len(t, p.GetOwnerReferences(), 0)
+	assert.False(t, result)
+
+	result = UpdateOwnerRefToObjectIfNeeded(p, &metav1.OwnerReference{})
+	assert.Len(t, p.GetOwnerReferences(), 1)
+	assert.True(t, result)
+
+	result = UpdateOwnerRefToObjectIfNeeded(p, &metav1.OwnerReference{})
+	assert.Len(t, p.GetOwnerReferences(), 1)
+	assert.False(t, result)
+}
+
 // TestLabelsForDeployment tests LabelsForDeployment.
 func TestLabelsForDeployment(t *testing.T) {
 	l := LabelsForDeployment("test", "role")