Inspect PVCs again quickly when finalizers cannot (yet) be removed

Author: ewoutp

pkg/deployment/deployment_inspector.go

@@ -94,9 +94,11 @@ func (d *Deployment) inspectDeployment(lastInterval time.Duration) time.Duration
 		} else {
 			nextInterval = util.MinDuration(nextInterval, x)
 		}
-		if err := d.resources.InspectPVCs(ctx); err != nil {
+		if x, err := d.resources.InspectPVCs(ctx); err != nil {
 			hasError = true
 			d.CreateEvent(k8sutil.NewErrorEvent("PVC inspection failed", err, d.apiObject))
+		} else {
+			nextInterval = util.MinDuration(nextInterval, x)
 		}
 
 		// Check members for resilience

pkg/deployment/resources/pod_finalizers.go

@@ -36,7 +36,7 @@ import (
 )
 
 const (
-	recheckPodFinalizerInterval = time.Second * 10
+	recheckPodFinalizerInterval = time.Second * 10 // Interval used when Pod finalizers need to be rechecked soon
 )
 
 // runPodFinalizers goes through the list of pod finalizers to see if they can be removed.
@@ -69,9 +69,8 @@ func (r *Resources) runPodFinalizers(ctx context.Context, p *v1.Pod, memberStatu
 		if err := k8sutil.RemovePodFinalizers(log, kubecli, p, removalList, ignoreNotFound); err != nil {
 			log.Debug().Err(err).Msg("Failed to update pod (to remove finalizers)")
 			return 0, maskAny(err)
-		} else {
-			log.Debug().Strs("finalizers", removalList).Msg("Removed finalizer(s) from Pod")
 		}
+		log.Debug().Strs("finalizers", removalList).Msg("Removed finalizer(s) from Pod")
 	} else {
 		// Check again at given interval
 		return recheckPodFinalizerInterval, nil

pkg/deployment/resources/pod_inspector.go

@@ -41,7 +41,7 @@ var (
 
 const (
 	podScheduleTimeout      = time.Minute // How long we allow the schedule to take scheduling a pod.
-	maxPodInspectorInterval = time.Hour
+	maxPodInspectorInterval = time.Hour   // Maximum time between Pod inspection (if nothing else happens)
 )
 
 // InspectPods lists all pods that belong to the given deployment and updates

pkg/deployment/resources/pvc_finalizers.go

@@ -25,6 +25,7 @@ package resources
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/rs/zerolog"
 	"k8s.io/api/core/v1"
@@ -35,8 +36,13 @@ import (
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
 )
 
+const (
+	recheckPVCFinalizerInterval = time.Second * 10 // Interval used when PVC finalizers need to be rechecked soon
+)
+
 // runPVCFinalizers goes through the list of PVC finalizers to see if they can be removed.
-func (r *Resources) runPVCFinalizers(ctx context.Context, p *v1.PersistentVolumeClaim, group api.ServerGroup, memberStatus api.MemberStatus, updateMember func(api.MemberStatus) error) error {
+// Returns: Interval_till_next_inspection, error
+func (r *Resources) runPVCFinalizers(ctx context.Context, p *v1.PersistentVolumeClaim, group api.ServerGroup, memberStatus api.MemberStatus, updateMember func(api.MemberStatus) error) (time.Duration, error) {
 	log := r.log.With().Str("pvc-name", p.GetName()).Logger()
 	var removalList []string
 	for _, f := range p.ObjectMeta.GetFinalizers() {
@@ -56,12 +62,15 @@ func (r *Resources) runPVCFinalizers(ctx context.Context, p *v1.PersistentVolume
 		ignoreNotFound := false
 		if err := k8sutil.RemovePVCFinalizers(log, kubecli, p, removalList, ignoreNotFound); err != nil {
 			log.Debug().Err(err).Msg("Failed to update PVC (to remove finalizers)")
-			return maskAny(err)
+			return 0, maskAny(err)
 		} else {
 			log.Debug().Strs("finalizers", removalList).Msg("Removed finalizer(s) from PVC")
 		}
+	} else {
+		// Check again at given interval
+		return recheckPVCFinalizerInterval, nil
 	}
-	return nil
+	return maxPVCInspectorInterval, nil
 }
 
 // inspectFinalizerPVCMemberExists checks the finalizer condition for member-exists.

pkg/deployment/resources/pvc_inspector.go

@@ -24,25 +24,29 @@ package resources
 
 import (
 	"context"
+	"time"
 
 	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha"
 	"github.com/arangodb/kube-arangodb/pkg/metrics"
+	"github.com/arangodb/kube-arangodb/pkg/util"
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
 )
 
 var (
-	inspectedPVCCounter = metrics.MustRegisterCounter("deployment", "inspected_ppvcs", "Number of PVCs inspections")
+	inspectedPVCCounter     = metrics.MustRegisterCounter("deployment", "inspected_ppvcs", "Number of PVCs inspections")
+	maxPVCInspectorInterval = time.Hour // Maximum time between PVC inspection (if nothing else happens)
 )
 
 // InspectPVCs lists all PVCs that belong to the given deployment and updates
 // the member status of the deployment accordingly.
-func (r *Resources) InspectPVCs(ctx context.Context) error {
+func (r *Resources) InspectPVCs(ctx context.Context) (time.Duration, error) {
 	log := r.log
+	nextInterval := maxPVCInspectorInterval
 
 	pvcs, err := r.context.GetOwnedPVCs()
 	if err != nil {
 		log.Debug().Err(err).Msg("Failed to get owned PVCs")
-		return maskAny(err)
+		return 0, maskAny(err)
 	}
 
 	// Update member status from all pods found
@@ -63,7 +67,7 @@ func (r *Resources) InspectPVCs(ctx context.Context) error {
 				ignoreNotFound := false
 				if err := k8sutil.RemovePVCFinalizers(log, kubecli, &p, p.GetFinalizers(), ignoreNotFound); err != nil {
 					log.Debug().Err(err).Msg("Failed to update PVC (to remove all finalizers)")
-					return maskAny(err)
+					return 0, maskAny(err)
 				}
 			}
 			continue
@@ -72,21 +76,23 @@ func (r *Resources) InspectPVCs(ctx context.Context) error {
 		updateMemberStatusNeeded := false
 		if k8sutil.IsPersistentVolumeClaimMarkedForDeletion(&p) {
 			// Process finalizers
-			if err := r.runPVCFinalizers(ctx, &p, group, memberStatus, func(m api.MemberStatus) error {
+			if x, err := r.runPVCFinalizers(ctx, &p, group, memberStatus, func(m api.MemberStatus) error {
 				updateMemberStatusNeeded = true
 				memberStatus = m
 				return nil
 			}); err != nil {
 				// Only log here, since we'll be called to try again.
 				log.Warn().Err(err).Msg("Failed to run PVC finalizers")
+			} else {
+				nextInterval = util.MinDuration(nextInterval, x)
 			}
 		}
 		if updateMemberStatusNeeded {
 			if err := status.Members.Update(memberStatus, group); err != nil {
-				return maskAny(err)
+				return 0, maskAny(err)
 			}
 		}
 	}
 
-	return nil
+	return nextInterval, nil
 }