From 9f3122431952876ea56d32e690a5cb9c297ca97c Mon Sep 17 00:00:00 2001 From: juanjo Date: Wed, 28 Feb 2024 00:13:55 +0100 Subject: [PATCH 1/2] Fixing issue about pv with policyClaim delete not being removed --- .../actions.summerwind.net/sync_volumes.go | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/controllers/actions.summerwind.net/sync_volumes.go b/controllers/actions.summerwind.net/sync_volumes.go index a8cbae0f..3c71bb1e 100644 --- a/controllers/actions.summerwind.net/sync_volumes.go +++ b/controllers/actions.summerwind.net/sync_volumes.go @@ -150,7 +150,7 @@ func syncPV(ctx context.Context, c client.Client, log logr.Logger, ns string, pv log.V(2).Info("Reconciling PV") if pv.Labels[labelKeyCleanup] == "" { - // We assume that the pvc is shortly terminated, hence retry forever until it gets removed. + // We assume that the PVC is shortly terminated, hence retry forever until it gets removed. retry := 10 * time.Second log.V(2).Info("Retrying sync to see if this PV needs to be managed by ARC", "requeueAfter", retry) return &ctrl.Result{RequeueAfter: retry}, nil @@ -159,27 +159,31 @@ func syncPV(ctx context.Context, c client.Client, log logr.Logger, ns string, pv log.V(2).Info("checking pv phase", "phase", pv.Status.Phase) if pv.Status.Phase != corev1.VolumeReleased { - // We assume that the pvc is shortly terminated, hence retry forever until it gets removed. + // We assume that the PVC is shortly terminated, hence retry forever until it gets removed. retry := 10 * time.Second - log.V(1).Info("Retrying sync until pvc gets released", "requeueAfter", retry) + log.V(1).Info("Retrying sync until PVC gets released", "requeueAfter", retry) return &ctrl.Result{RequeueAfter: retry}, nil } - // At this point, the PV is still Released - - pvCopy := pv.DeepCopy() - delete(pvCopy.Labels, labelKeyCleanup) - pvCopy.Spec.ClaimRef = nil - log.V(2).Info("Unsetting PV's claimRef", "pv", pv.Name) - if err := c.Update(ctx, pvCopy); err != nil { - return nil, err + // Check if the PV has ReclaimPolicy "Delete". + if pv.Spec.PersistentVolumeReclaimPolicy == corev1.PersistentVolumeReclaimDelete { + log.Info("Skipping manipulation for PV with 'Delete' reclaim policy", "pv", pv.Name) + // For PVs with ReclaimPolicy "Delete", we don't need to do anything. + return nil, nil } - log.Info("PV should be Available now") + // If ReclaimPolicy is not "Delete", we proceed to clean up the ClaimRef. + if pv.Status.Phase == corev1.VolumeReleased { + pvCopy := pv.DeepCopy() + delete(pvCopy.Labels, labelKeyCleanup) + pvCopy.Spec.ClaimRef = nil + log.V(2).Info("Unsetting PV's claimRef", "pv", pv.Name) + if err := c.Update(ctx, pvCopy); err != nil { + return nil, err + } - // At this point, the PV becomes Available, if it's reclaim policy is "Retain". - // I have not yet tested it with "Delete" but perhaps it's deleted automatically after the update? - // https://kubernetes.io/docs/concepts/storage/persistent-volumes/#retain + log.Info("PV should be Available now") + } return nil, nil -} +} \ No newline at end of file From 6bc9e97e2548125cce54c172f08c3ae84560d066 Mon Sep 17 00:00:00 2001 From: juanjo Date: Thu, 28 Mar 2024 12:10:17 +0100 Subject: [PATCH 2/2] Removing unneeded if --- .../actions.summerwind.net/sync_volumes.go | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/controllers/actions.summerwind.net/sync_volumes.go b/controllers/actions.summerwind.net/sync_volumes.go index 3c71bb1e..0b458765 100644 --- a/controllers/actions.summerwind.net/sync_volumes.go +++ b/controllers/actions.summerwind.net/sync_volumes.go @@ -173,17 +173,15 @@ func syncPV(ctx context.Context, c client.Client, log logr.Logger, ns string, pv } // If ReclaimPolicy is not "Delete", we proceed to clean up the ClaimRef. - if pv.Status.Phase == corev1.VolumeReleased { - pvCopy := pv.DeepCopy() - delete(pvCopy.Labels, labelKeyCleanup) - pvCopy.Spec.ClaimRef = nil - log.V(2).Info("Unsetting PV's claimRef", "pv", pv.Name) - if err := c.Update(ctx, pvCopy); err != nil { - return nil, err - } - - log.Info("PV should be Available now") + pvCopy := pv.DeepCopy() + delete(pvCopy.Labels, labelKeyCleanup) + pvCopy.Spec.ClaimRef = nil + log.V(2).Info("Unsetting PV's claimRef", "pv", pv.Name) + if err := c.Update(ctx, pvCopy); err != nil { + return nil, err } + log.Info("PV should be Available now") + return nil, nil -} \ No newline at end of file +}