fix(usb): autodelete nodeusbdevice (#2220)

Implement automatic cleanup of stale NodeUSBDevice objects when the USB device is no longer present on the host and the object is not assigned to any namespace.

The deletion flow was moved to the lifecycle handler logic:

add and persist FinalizerNodeUSBDeviceCleanup first;
only after that, mark orphaned NodeUSBDevice for deletion;
perform USBDevice cleanup only in the standard deletion path after deletionTimestamp is set.

---------

Signed-off-by: Daniil Antoshin <daniil.antoshin@flant.com>

Author: danilrwx

images/virtualization-artifact/pkg/controller/nodeusbdevice/internal/handler/deletion.go

@@ -21,13 +21,16 @@ import (
 	"fmt"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/deckhouse/virtualization-controller/pkg/controller/nodeusbdevice/internal/state"
+	"github.com/deckhouse/virtualization-controller/pkg/controller/reconciler"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2"
+	"github.com/deckhouse/virtualization/api/core/v1alpha2/nodeusbdevicecondition"
 )
 
 const (
@@ -56,7 +59,18 @@ func (h *DeletionHandler) Handle(ctx context.Context, s state.NodeUSBDeviceState
 
 	switch {
 	case current.GetDeletionTimestamp().IsZero():
-		controllerutil.AddFinalizer(changed, v1alpha2.FinalizerNodeUSBDeviceCleanup)
+		if !controllerutil.ContainsFinalizer(current, v1alpha2.FinalizerNodeUSBDeviceCleanup) {
+			controllerutil.AddFinalizer(changed, v1alpha2.FinalizerNodeUSBDeviceCleanup)
+			return reconcile.Result{}, nil
+		}
+
+		if shouldAutoDeleteNodeUSBDevice(current) {
+			if err := h.client.Delete(ctx, current); err != nil && !apierrors.IsNotFound(err) {
+				return reconcile.Result{}, fmt.Errorf("failed to delete NodeUSBDevice: %w", err)
+			}
+			return reconcile.Result{}, reconciler.ErrStopHandlerChain
+		}
+
 		return reconcile.Result{}, nil
 
 	default:
@@ -92,3 +106,20 @@ func (h *DeletionHandler) cleanupOwnedUSBDevices(ctx context.Context, owner *v1a
 func (h *DeletionHandler) Name() string {
 	return nameDeletionHandler
 }
+
+func shouldAutoDeleteNodeUSBDevice(nodeUSBDevice *v1alpha2.NodeUSBDevice) bool {
+	if nodeUSBDevice == nil || nodeUSBDevice.GetDeletionTimestamp() != nil {
+		return false
+	}
+
+	if nodeUSBDevice.Spec.AssignedNamespace != "" {
+		return false
+	}
+
+	readyCondition := meta.FindStatusCondition(nodeUSBDevice.Status.Conditions, string(nodeusbdevicecondition.ReadyType))
+	if readyCondition == nil {
+		return false
+	}
+
+	return readyCondition.Status == metav1.ConditionFalse && readyCondition.Reason == string(nodeusbdevicecondition.NotFound)
+}

images/virtualization-artifact/pkg/controller/nodeusbdevice/internal/handler/deletion_test.go

@@ -33,6 +33,7 @@ import (
 	"github.com/deckhouse/virtualization-controller/pkg/controller/reconciler"
 	"github.com/deckhouse/virtualization-controller/pkg/logger"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2"
+	"github.com/deckhouse/virtualization/api/core/v1alpha2/nodeusbdevicecondition"
 )
 
 var _ = Describe("DeletionHandler", func() {
@@ -43,15 +44,27 @@ var _ = Describe("DeletionHandler", func() {
 	})
 
 	DescribeTable("Handle",
-		func(deleting, withOwnedUSB bool, usbNamespace string, expectFinalizerPresent, expectOwnedUSBDeleted bool) {
+		func(deleting, autoDelete, withOwnedUSB, withFinalizer bool, assignedNamespace, usbNamespace string, expectFinalizerPresent, expectOwnedUSBDeleted, expectNodeDeleted, expectStopChain bool) {
 			scheme := apiruntime.NewScheme()
 			Expect(v1alpha2.AddToScheme(scheme)).To(Succeed())
 
-			node := &v1alpha2.NodeUSBDevice{ObjectMeta: metav1.ObjectMeta{Name: "usb-device-1", UID: "node-usb-uid"}}
+			node := &v1alpha2.NodeUSBDevice{
+				ObjectMeta: metav1.ObjectMeta{Name: "usb-device-1", UID: "node-usb-uid"},
+				Spec:       v1alpha2.NodeUSBDeviceSpec{AssignedNamespace: assignedNamespace},
+			}
+			if autoDelete {
+				node.Status.Conditions = []metav1.Condition{{
+					Type:   string(nodeusbdevicecondition.ReadyType),
+					Status: metav1.ConditionFalse,
+					Reason: string(nodeusbdevicecondition.NotFound),
+				}}
+			}
+			if withFinalizer {
+				node.Finalizers = []string{v1alpha2.FinalizerNodeUSBDeviceCleanup}
+			}
 			if deleting {
 				now := metav1.Now()
 				node.DeletionTimestamp = &now
-				node.Finalizers = []string{v1alpha2.FinalizerNodeUSBDeviceCleanup}
 			}
 
 			objects := []client.Object{node}
@@ -83,7 +96,11 @@ var _ = Describe("DeletionHandler", func() {
 			h := NewDeletionHandler(cl)
 			st := state.New(cl, res)
 			_, err := h.Handle(ctx, st)
-			Expect(err).NotTo(HaveOccurred())
+			if expectStopChain {
+				Expect(err).To(MatchError(reconciler.ErrStopHandlerChain))
+			} else {
+				Expect(err).NotTo(HaveOccurred())
+			}
 
 			if expectFinalizerPresent {
 				Expect(res.Changed().GetFinalizers()).To(ContainElement(v1alpha2.FinalizerNodeUSBDeviceCleanup))
@@ -100,10 +117,21 @@ var _ = Describe("DeletionHandler", func() {
 					Expect(err).NotTo(HaveOccurred())
 				}
 			}
+
+			deletedNode := &v1alpha2.NodeUSBDevice{}
+			err = cl.Get(ctx, types.NamespacedName{Name: node.Name}, deletedNode)
+			if expectNodeDeleted {
+				Expect(err).To(HaveOccurred())
+			} else {
+				Expect(err).NotTo(HaveOccurred())
+			}
 		},
-		Entry("not deleting adds finalizer", false, false, "", true, false),
-		Entry("deleting removes finalizer and owned USB", true, true, "test-namespace", false, true),
-		Entry("deleting removes finalizer even without owned USB", true, false, "", false, false),
-		Entry("deleting removes owned USB in different namespace", true, true, "previous-namespace", false, true),
+		Entry("not deleting adds finalizer", false, false, false, false, "", "", true, false, false, false),
+		Entry("auto-delete first adds finalizer without deleting node object", false, true, true, false, "", "test-namespace", true, false, false, false),
+		Entry("auto-delete marks node object for deletion when finalizer is already present", false, true, true, true, "", "test-namespace", true, false, false, true),
+		Entry("assigned not found device is not auto-deleted", false, true, false, true, "test-namespace", "", true, false, false, false),
+		Entry("deleting removes finalizer and owned USB", true, false, true, true, "", "test-namespace", false, true, false, false),
+		Entry("deleting removes finalizer even without owned USB", true, false, false, true, "", "", false, false, false, false),
+		Entry("deleting removes owned USB in different namespace", true, false, true, true, "", "previous-namespace", false, true, false, false),
 	)
 })