From 91934247797a8bda1672528cc64eaff51e0749e2 Mon Sep 17 00:00:00 2001 From: jwierzbo Date: Wed, 27 Apr 2022 12:32:23 +0200 Subject: [PATCH] [Fix] GT-11 Orphan PVC are not removed (#962) --- CHANGELOG.md | 1 + pkg/deployment/deployment_inspector.go | 2 ++ pkg/deployment/resources/labels.go | 11 ++++++--- pkg/deployment/resources/pvc_inspector.go | 29 ++++++++++++++++++++--- pkg/util/k8sutil/util.go | 15 ++++++++++++ pkg/util/k8sutil/util_test.go | 16 +++++++++++++ 6 files changed, 68 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89b1be77b..05469ba79 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/pkg/deployment/deployment_inspector.go b/pkg/deployment/deployment_inspector.go index e7f7feff3..90d7a629f 100644 --- a/pkg/deployment/deployment_inspector.go +++ b/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 { diff --git a/pkg/deployment/resources/labels.go b/pkg/deployment/resources/labels.go index cc65f94a9..163520be1 100644 --- a/pkg/deployment/resources/labels.go +++ b/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 } diff --git a/pkg/deployment/resources/pvc_inspector.go b/pkg/deployment/resources/pvc_inspector.go index 8d89d8625..a1238b891 100644 --- a/pkg/deployment/resources/pvc_inspector.go +++ b/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 { diff --git a/pkg/util/k8sutil/util.go b/pkg/util/k8sutil/util.go index cc9f9e10e..13ce432e3 100644 --- a/pkg/util/k8sutil/util.go +++ b/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{ diff --git a/pkg/util/k8sutil/util_test.go b/pkg/util/k8sutil/util_test.go index c3961e282..976fe922b 100644 --- a/pkg/util/k8sutil/util_test.go +++ b/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")