fix(vd): improve virtual disk protection logic during deletion (#1285)

fix(vd): improve virtual disk protection logic during deletion

Refactor the deletion and protection handlers to enhance the logic for managing virtual disk deletion. The deletion handler now checks for the presence of a finalizer before proceeding with cleanup. The protection handler now properly adds the vd-protection finalizer when a VirtualDisk is attached to any VM (including via VMBDA). This fixes protection logic for VMBDA attached disks and PVC deletion spam when deleting attached VDs.

---------

Signed-off-by: Daniil Loktev <[email protected]>

Author: loktev-d

images/virtualization-artifact/pkg/controller/vd/internal/deletion.go

@@ -44,6 +44,10 @@ func (h DeletionHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (re
 	log := logger.FromContext(ctx).With(logger.SlogHandler(deletionHandlerName))
 
 	if vd.DeletionTimestamp != nil {
+		if controllerutil.ContainsFinalizer(vd, virtv2.FinalizerVDProtection) {
+			return reconcile.Result{}, nil
+		}
+
 		requeue, err := h.sources.CleanUp(ctx, vd)
 		if err != nil {
 			return reconcile.Result{}, err

images/virtualization-artifact/pkg/controller/vd/internal/protection.go

@@ -39,27 +39,23 @@ func (h ProtectionHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (
 		log.Debug("virtual disk connected to multiple virtual machines", "vms", len(vd.Status.AttachedToVirtualMachines))
 	}
 
-	switch {
-	case len(vd.Status.AttachedToVirtualMachines) == 0:
+	unmounted := true
+	for _, vm := range vd.Status.AttachedToVirtualMachines {
+		if vm.Mounted {
+			unmounted = false
+			break
+		}
+	}
+
+	if unmounted {
 		log.Debug("Allow virtual disk deletion")
 		controllerutil.RemoveFinalizer(vd, virtv2.FinalizerVDProtection)
-	case len(vd.Status.AttachedToVirtualMachines) != 0:
-		unmounted := true
-		for _, vm := range vd.Status.AttachedToVirtualMachines {
-			if vm.Mounted {
-				unmounted = false
-			}
-		}
-		if unmounted {
-			log.Debug("Allow virtual disk deletion")
-			controllerutil.RemoveFinalizer(vd, virtv2.FinalizerVDProtection)
-		}
-	case vd.DeletionTimestamp == nil:
+		return reconcile.Result{}, nil
+	}
+
+	if vd.DeletionTimestamp == nil {
 		log.Debug("Protect virtual disk from deletion")
 		controllerutil.AddFinalizer(vd, virtv2.FinalizerVDProtection)
-	default:
-		log.Debug("Virtual disk deletion is delayed: it's protected by virtual machines")
 	}
-
 	return reconcile.Result{}, nil
 }

images/virtualization-artifact/pkg/controller/vd/vd_reconciler.go

@@ -19,6 +19,7 @@ package vd
 import (
 	"context"
 	"fmt"
+	"reflect"
 
 	"k8s.io/apimachinery/pkg/types"
 	cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
@@ -91,7 +92,7 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr
 			CreateFunc: func(e event.CreateEvent) bool { return true },
 			DeleteFunc: func(e event.DeleteEvent) bool { return true },
 			UpdateFunc: func(e event.UpdateEvent) bool {
-				return e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration()
+				return !reflect.DeepEqual(e.ObjectOld.GetFinalizers(), e.ObjectNew.GetFinalizers()) || e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration()
 			},
 		},
 	); err != nil {