diff --git a/images/virtualization-artifact/pkg/controller/nodeusbdevice/internal/handler/deletion.go b/images/virtualization-artifact/pkg/controller/nodeusbdevice/internal/handler/deletion.go index e8138f422a..89808a41ed 100644 --- a/images/virtualization-artifact/pkg/controller/nodeusbdevice/internal/handler/deletion.go +++ b/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) +} diff --git a/images/virtualization-artifact/pkg/controller/nodeusbdevice/internal/handler/deletion_test.go b/images/virtualization-artifact/pkg/controller/nodeusbdevice/internal/handler/deletion_test.go index ec39f47379..5c83fb0009 100644 --- a/images/virtualization-artifact/pkg/controller/nodeusbdevice/internal/handler/deletion_test.go +++ b/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), ) })