From 26d4b0a7f51e698f8e2b687f2f9ad46ae0f14e78 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Tue, 10 Mar 2026 15:24:17 +0200 Subject: [PATCH 01/14] fix(vm/validators): resolve compilation errors in USB devices validator - Remove call to undefined validateAvailableUSBIPPortsDefault - Replace getNodeTotalPorts with direct client.Get for Node - Use usb.CheckFreePortForRequest from common/usb package - Remove unused imports Signed-off-by: Daniil Antoshin --- .../v1alpha2/usbdevicecondition/condition.go | 2 + .../pkg/common/usb/speed.go | 112 +++++++++ .../pkg/common/usb/speed_test.go | 237 ++++++++++++++++++ .../usbdevice/internal/handler/lifecycle.go | 47 +++- .../vm/internal/usb_device_attach_handler.go | 38 ++- .../validators/usb_devices_validator.go | 112 ++------- 6 files changed, 445 insertions(+), 103 deletions(-) create mode 100644 images/virtualization-artifact/pkg/common/usb/speed.go create mode 100644 images/virtualization-artifact/pkg/common/usb/speed_test.go diff --git a/api/core/v1alpha2/usbdevicecondition/condition.go b/api/core/v1alpha2/usbdevicecondition/condition.go index c8a9a1c2cf..57686ca0d7 100644 --- a/api/core/v1alpha2/usbdevicecondition/condition.go +++ b/api/core/v1alpha2/usbdevicecondition/condition.go @@ -51,6 +51,8 @@ const ( Available AttachedReason = "Available" // DetachedForMigration signifies that device was detached for migration (e.g. live migration). DetachedForMigration AttachedReason = "DetachedForMigration" + // NoFreeUSBIPPort signifies that device cannot be attached because there are no free USBIP ports on the target node. + NoFreeUSBIPPort AttachedReason = "NoFreeUSBIPPort" ) func (r ReadyReason) String() string { diff --git a/images/virtualization-artifact/pkg/common/usb/speed.go b/images/virtualization-artifact/pkg/common/usb/speed.go new file mode 100644 index 0000000000..c4a640db2a --- /dev/null +++ b/images/virtualization-artifact/pkg/common/usb/speed.go @@ -0,0 +1,112 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package usb + +import ( + "fmt" + "strconv" + + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" +) + +// ResolveSpeed determines USB hub type from speed in Mbps. +// https://mjmwired.net/kernel/Documentation/ABI/testing/sysfs-bus-usb#502 +func ResolveSpeed(speed int) (isHS, isSS bool) { + return speed == 480, speed >= 5000 +} + +// GetTotalPortsPerHub returns the number of ports per hub (total / 2). +func GetTotalPortsPerHub(nodeAnnotations map[string]string) (int, error) { + totalPortsStr, exists := nodeAnnotations[annotations.AnnUSBIPTotalPorts] + if !exists { + return 0, fmt.Errorf("node does not have %s annotation", annotations.AnnUSBIPTotalPorts) + } + totalPorts, err := strconv.Atoi(totalPortsStr) + if err != nil { + return 0, fmt.Errorf("failed to parse %s annotation: %w", annotations.AnnUSBIPTotalPorts, err) + } + return totalPorts / 2, nil +} + +// GetUsedPorts returns the number of used ports for the given hub type. +func GetUsedPorts(nodeAnnotations map[string]string, hubAnnotation string) (int, error) { + usedPortsStr, exists := nodeAnnotations[hubAnnotation] + if !exists { + return 0, fmt.Errorf("node does not have %s annotation", hubAnnotation) + } + usedPorts, err := strconv.Atoi(usedPortsStr) + if err != nil { + return 0, fmt.Errorf("failed to parse %s annotation: %w", hubAnnotation, err) + } + return usedPorts, nil +} + +// CheckFreePort checks if a node has free USBIP ports for the given speed. +// Returns true if there is at least one free port, false otherwise. +func CheckFreePort(nodeAnnotations map[string]string, speed int) (bool, error) { + isHS, isSS := ResolveSpeed(speed) + + var hubAnnotation string + switch { + case isHS: + hubAnnotation = annotations.AnnUSBIPHighSpeedHubUsedPorts + case isSS: + hubAnnotation = annotations.AnnUSBIPSuperSpeedHubUsedPorts + default: + return false, fmt.Errorf("unsupported USB speed: %d", speed) + } + + totalPortsPerHub, err := GetTotalPortsPerHub(nodeAnnotations) + if err != nil { + return false, err + } + + usedPorts, err := GetUsedPorts(nodeAnnotations, hubAnnotation) + if err != nil { + return false, err + } + + return usedPorts < totalPortsPerHub, nil +} + +// CheckFreePortForRequest checks if there are enough free ports for a specific request. +// It adds the requested count to the currently used ports and compares with total. +func CheckFreePortForRequest(nodeAnnotations map[string]string, speed, requestedCount int) (bool, error) { + isHS, isSS := ResolveSpeed(speed) + + var hubAnnotation string + switch { + case isHS: + hubAnnotation = annotations.AnnUSBIPHighSpeedHubUsedPorts + case isSS: + hubAnnotation = annotations.AnnUSBIPSuperSpeedHubUsedPorts + default: + return false, fmt.Errorf("unsupported USB speed: %d", speed) + } + + totalPortsPerHub, err := GetTotalPortsPerHub(nodeAnnotations) + if err != nil { + return false, err + } + + usedPorts, err := GetUsedPorts(nodeAnnotations, hubAnnotation) + if err != nil { + return false, err + } + + return (usedPorts + requestedCount) <= totalPortsPerHub, nil +} diff --git a/images/virtualization-artifact/pkg/common/usb/speed_test.go b/images/virtualization-artifact/pkg/common/usb/speed_test.go new file mode 100644 index 0000000000..0632b0c64b --- /dev/null +++ b/images/virtualization-artifact/pkg/common/usb/speed_test.go @@ -0,0 +1,237 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package usb + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" +) + +func TestSpeed(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "USB speed package tests") +} + +var _ = Describe("ResolveSpeed", func() { + DescribeTable("returns correct hub type for speed", + func(speed int, expectedHS, expectedSS bool) { + isHS, isSS := ResolveSpeed(speed) + Expect(isHS).To(Equal(expectedHS)) + Expect(isSS).To(Equal(expectedSS)) + }, + Entry("low speed 1.0", 1, false, false), + Entry("full speed 1.1", 12, false, false), + Entry("high speed 2.0", 480, true, false), + Entry("wireless 2.5", 2500, false, false), + Entry("super speed 3.0", 5000, false, true), + Entry("super speed 3.1", 10000, false, true), + Entry("super speed 3.2", 20000, false, true), + Entry("super speed plus 3.1", 10000, false, true), + ) +}) + +var _ = Describe("CheckFreePort", func() { + DescribeTable("returns correct availability", + func(speed int, nodeAnnotations map[string]string, expectedResult, expectError bool) { + result, err := CheckFreePort(nodeAnnotations, speed) + if expectError { + Expect(err).To(HaveOccurred()) + } else { + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(expectedResult)) + } + }, + Entry("HS speed, enough ports", + 480, + map[string]string{ + annotations.AnnUSBIPTotalPorts: "8", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "3", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0", + }, + true, false, + ), + Entry("HS speed, no ports left", + 480, + map[string]string{ + annotations.AnnUSBIPTotalPorts: "8", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "4", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0", + }, + false, false, + ), + Entry("SS speed, enough ports", + 5000, + map[string]string{ + annotations.AnnUSBIPTotalPorts: "8", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "4", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "2", + }, + true, false, + ), + Entry("SS speed, no ports left", + 10000, + map[string]string{ + annotations.AnnUSBIPTotalPorts: "8", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "4", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "4", + }, + false, false, + ), + Entry("unsupported speed", + 12, + map[string]string{ + annotations.AnnUSBIPTotalPorts: "8", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "0", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0", + }, + false, true, + ), + Entry("missing total ports annotation", + 480, + map[string]string{ + annotations.AnnUSBIPHighSpeedHubUsedPorts: "0", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0", + }, + false, true, + ), + Entry("missing HS hub annotation", + 480, + map[string]string{ + annotations.AnnUSBIPTotalPorts: "8", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0", + }, + false, true, + ), + Entry("missing SS hub annotation", + 5000, + map[string]string{ + annotations.AnnUSBIPTotalPorts: "8", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "0", + }, + false, true, + ), + ) +}) + +var _ = Describe("CheckFreePortForRequest", func() { + DescribeTable("returns correct availability for request", + func(speed int, nodeAnnotations map[string]string, requestedCount int, expectedResult, expectError bool) { + result, err := CheckFreePortForRequest(nodeAnnotations, speed, requestedCount) + if expectError { + Expect(err).To(HaveOccurred()) + } else { + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(expectedResult)) + } + }, + Entry("HS speed, request 1, enough ports", + 480, + map[string]string{ + annotations.AnnUSBIPTotalPorts: "8", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "3", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0", + }, + 1, true, false, + ), + Entry("HS speed, request 2, no ports", + 480, + map[string]string{ + annotations.AnnUSBIPTotalPorts: "8", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "3", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0", + }, + 2, false, false, + ), + Entry("HS speed, request 2, exactly at limit", + 480, + map[string]string{ + annotations.AnnUSBIPTotalPorts: "8", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "2", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0", + }, + 2, true, false, + ), + ) +}) + +var _ = Describe("GetTotalPortsPerHub", func() { + DescribeTable("returns correct total ports per hub", + func(nodeAnnotations map[string]string, expectedResult int, expectError bool) { + result, err := GetTotalPortsPerHub(nodeAnnotations) + if expectError { + Expect(err).To(HaveOccurred()) + } else { + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(expectedResult)) + } + }, + Entry("total 8 ports", + map[string]string{annotations.AnnUSBIPTotalPorts: "8"}, + 4, false, + ), + Entry("total 4 ports", + map[string]string{annotations.AnnUSBIPTotalPorts: "4"}, + 2, false, + ), + Entry("missing annotation", + map[string]string{}, + 0, true, + ), + Entry("invalid value", + map[string]string{annotations.AnnUSBIPTotalPorts: "invalid"}, + 0, true, + ), + ) +}) + +var _ = Describe("GetUsedPorts", func() { + DescribeTable("returns correct used ports", + func(nodeAnnotations map[string]string, hubAnnotation string, expectedResult int, expectError bool) { + result, err := GetUsedPorts(nodeAnnotations, hubAnnotation) + if expectError { + Expect(err).To(HaveOccurred()) + } else { + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(expectedResult)) + } + }, + Entry("HS hub, 3 used", + map[string]string{annotations.AnnUSBIPHighSpeedHubUsedPorts: "3"}, + annotations.AnnUSBIPHighSpeedHubUsedPorts, + 3, false, + ), + Entry("SS hub, 0 used", + map[string]string{annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0"}, + annotations.AnnUSBIPSuperSpeedHubUsedPorts, + 0, false, + ), + Entry("missing annotation", + map[string]string{}, + annotations.AnnUSBIPHighSpeedHubUsedPorts, + 0, true, + ), + Entry("invalid value", + map[string]string{annotations.AnnUSBIPHighSpeedHubUsedPorts: "invalid"}, + annotations.AnnUSBIPHighSpeedHubUsedPorts, + 0, true, + ), + ) +}) diff --git a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go index 53c8f6efbd..53206c0ed5 100644 --- a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go +++ b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go @@ -22,6 +22,7 @@ import ( "reflect" "strconv" + corev1 "k8s.io/api/core/v1" resourcev1 "k8s.io/api/resource/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -32,6 +33,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + "github.com/deckhouse/virtualization-controller/pkg/common/usb" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/usbdevice/internal/state" "github.com/deckhouse/virtualization-controller/pkg/logger" @@ -192,9 +194,13 @@ func (h *LifecycleHandler) ensureResourceClaimTemplate(ctx context.Context, s st } func (h *LifecycleHandler) syncAttached(ctx context.Context, s state.USBDeviceState) error { + log := logger.FromContext(ctx).With(logger.SlogHandler(nameLifecycleHandler)) + current := s.USBDevice().Current() changed := s.USBDevice().Changed() + usbDevice := current + vms, err := s.VirtualMachinesUsingDevice(ctx) if err != nil { return fmt.Errorf("failed to find VirtualMachines using USBDevice: %w", err) @@ -212,11 +218,42 @@ func (h *LifecycleHandler) syncAttached(ctx context.Context, s state.USBDeviceSt return nil } - reason = usbdevicecondition.AttachedToVirtualMachine - status = metav1.ConditionTrue - message = fmt.Sprintf("Device is attached to %d VirtualMachines.", len(vms)) - if len(vms) == 1 { - message = fmt.Sprintf("Device is attached to VirtualMachine %s/%s.", vms[0].Namespace, vms[0].Name) + noFreePort := false + for _, vm := range vms { + if usbDevice.Status.NodeName != "" && usbDevice.Status.NodeName != vm.Status.Node { + node := &corev1.Node{} + if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get node %s: %w", vm.Status.Node, err) + } + continue + } + + hasFreePort, err := usb.CheckFreePort(node.Annotations, usbDevice.Status.Attributes.Speed) + if err != nil { + log.Error("failed to check free USBIP ports", "error", err, "device", usbDevice.Name, "node", vm.Status.Node) + continue + } + + if !hasFreePort { + noFreePort = true + message = fmt.Sprintf("Device is attached to VirtualMachine %s/%s, but no free USBIP ports available on node %s for speed %d.", + vm.Namespace, vm.Name, vm.Status.Node, usbDevice.Status.Attributes.Speed) + break + } + } + } + + if noFreePort { + reason = usbdevicecondition.NoFreeUSBIPPort + status = metav1.ConditionTrue + } else { + reason = usbdevicecondition.AttachedToVirtualMachine + status = metav1.ConditionTrue + message = fmt.Sprintf("Device is attached to %d VirtualMachines.", len(vms)) + if len(vms) == 1 { + message = fmt.Sprintf("Device is attached to VirtualMachine %s/%s.", vms[0].Namespace, vms[0].Name) + } } setAttachedCondition(current, &changed.Status.Conditions, status, reason, message) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go index 1075afa870..d6804c6972 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go @@ -22,11 +22,13 @@ import ( "strings" "time" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "github.com/deckhouse/virtualization-controller/pkg/common/usb" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" "github.com/deckhouse/virtualization-controller/pkg/logger" @@ -93,6 +95,10 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach var kvvmi *virtv1.VirtualMachineInstance var hostDeviceReadyByName map[string]bool + // Lazy-loaded node for USBIP port check + var nodeLoaded bool + var node *corev1.Node + var nextStatusRefs []v1alpha2.USBDeviceStatusRef for _, usbDeviceRef := range vm.Spec.USBDevices { deviceName := usbDeviceRef.Name @@ -130,7 +136,37 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach continue } - // 3) Runtime evidence from KVVMI and attach action. + // 3) Check free USBIP ports for devices from other nodes (USBIP scenario) + if usbDevice.Status.NodeName != "" && usbDevice.Status.NodeName != vm.Status.Node { + if !nodeLoaded && vm.Status.Node != "" { + node = &corev1.Node{} + if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { + if !apierrors.IsNotFound(err) { + return reconcile.Result{}, fmt.Errorf("failed to get node %s: %w", vm.Status.Node, err) + } + node = nil + } + nodeLoaded = true + } + + if node != nil { + hasFreePort, err := usb.CheckFreePort(node.Annotations, usbDevice.Status.Attributes.Speed) + if err != nil { + log.Error("failed to check free USBIP ports", "error", err, "device", deviceName, "node", vm.Status.Node) + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + continue + } + if !hasFreePort { + log.Info("no free USBIP ports available", "device", deviceName, "speed", usbDevice.Status.Attributes.Speed, "node", vm.Status.Node) + // Return requeue to retry after 5 seconds + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + changed.Status.USBDevices = nextStatusRefs + return reconcile.Result{RequeueAfter: 5 * time.Second}, nil + } + } + } + + // 4) Runtime evidence from KVVMI and attach action. if !kvvmiLoaded { fetchedKVVMI, err := s.KVVMI(ctx) if err != nil { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go index a161f3c7a4..2059dd0d29 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go @@ -19,8 +19,6 @@ package validators import ( "context" "fmt" - "strconv" - "strings" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" @@ -28,10 +26,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + "github.com/deckhouse/virtualization-controller/pkg/common/usb" "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" "github.com/deckhouse/virtualization-controller/pkg/featuregates" - "github.com/deckhouse/virtualization-controller/pkg/kubeapi" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -145,10 +142,7 @@ func getUSBDeviceNames(refs []v1alpha2.USBDeviceSpecRef) map[string]struct{} { } func (v *USBDevicesValidator) validateAvailableUSBIPPorts(ctx context.Context, vm *v1alpha2.VirtualMachine, oldUSBDevices map[string]struct{}) (admission.Warnings, error) { - if kubeapi.HasDRAPartitionableDevices() { - return v.validateAvailableUSBIPPortsWithPartitionableDevices(ctx, vm, oldUSBDevices) - } - return v.validateAvailableUSBIPPortsDefault(ctx, vm, oldUSBDevices) + return v.validateAvailableUSBIPPortsWithPartitionableDevices(ctx, vm, oldUSBDevices) } func (v *USBDevicesValidator) validateAvailableUSBIPPortsWithPartitionableDevices(ctx context.Context, vm *v1alpha2.VirtualMachine, oldUSBDevices map[string]struct{}) (admission.Warnings, error) { @@ -177,7 +171,7 @@ func (v *USBDevicesValidator) validateAvailableUSBIPPortsWithPartitionableDevice continue } - isHs, isSS := resolveSpeed(usbDevice.Status.Attributes.Speed) + isHs, isSS := usb.ResolveSpeed(usbDevice.Status.Attributes.Speed) switch { case isHs: hsUSBFromOtherNodes = append(hsUSBFromOtherNodes, ref.Name) @@ -192,107 +186,31 @@ func (v *USBDevicesValidator) validateAvailableUSBIPPortsWithPartitionableDevice return admission.Warnings{}, nil } - node, totalPorts, err := v.getNodeTotalPorts(ctx, vm.Status.Node) + node := &corev1.Node{} + err := v.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node) if err != nil { return admission.Warnings{}, err } - totalPortsPerHub := totalPorts / 2 - if len(hsUSBFromOtherNodes) > 0 { - if err = validateUsedPortsByAnnotation(node, annotations.AnnUSBIPHighSpeedHubUsedPorts, hsUSBFromOtherNodes, totalPortsPerHub); err != nil { + hasFree, err := usb.CheckFreePortForRequest(node.Annotations, 480, len(hsUSBFromOtherNodes)) + if err != nil { return admission.Warnings{}, err } - } - - if len(ssUSBFromOtherNodes) > 0 { - if err = validateUsedPortsByAnnotation(node, annotations.AnnUSBIPSuperSpeedHubUsedPorts, ssUSBFromOtherNodes, totalPortsPerHub); err != nil { - return admission.Warnings{}, err + if !hasFree { + return admission.Warnings{}, fmt.Errorf("node %s has no available ports for sharing USB devices %v", vm.Status.Node, hsUSBFromOtherNodes) } } - return admission.Warnings{}, nil -} - -func (v *USBDevicesValidator) getNodeTotalPorts(ctx context.Context, nodeName string) (*corev1.Node, int, error) { - node := &corev1.Node{} - err := v.client.Get(ctx, client.ObjectKey{Name: nodeName}, node) - if err != nil { - return nil, -1, fmt.Errorf("failed to get node %s: %w", nodeName, err) - } - - totalPorts, exists := node.Annotations[annotations.AnnUSBIPTotalPorts] - if !exists { - return nil, -1, fmt.Errorf("node %s does not have %s annotation", nodeName, annotations.AnnUSBIPTotalPorts) - } - totalPortsInt, err := strconv.Atoi(totalPorts) - if err != nil { - return nil, -1, fmt.Errorf("failed to parse %s annotation: %w", annotations.AnnUSBIPTotalPorts, err) - } - - return node, totalPortsInt, nil -} - -func validateUsedPortsByAnnotation(node *corev1.Node, anno string, usbFromOtherNodes []string, totalPortsPerHub int) error { - usedPorts, exists := node.Annotations[anno] - if !exists { - return fmt.Errorf("node %s does not have %s annotation", node.Name, anno) - } - usedPortsInt, err := strconv.Atoi(usedPorts) - if err != nil { - return fmt.Errorf("failed to parse %s annotation: %w", anno, err) - } - - wantedPorts := usedPortsInt + len(usbFromOtherNodes) - if wantedPorts > totalPortsPerHub { - return fmt.Errorf("node %s not available ports for sharing USB devices %s. total: %d, used: %d, wanted: %d", node.Name, strings.Join(usbFromOtherNodes, ", "), totalPortsPerHub, usedPortsInt, wantedPorts) - } - - return nil -} - -// https://mjmwired.net/kernel/Documentation/ABI/testing/sysfs-bus-usb#502 -func resolveSpeed(speed int) (isHs, isSS bool) { - return speed == 480, speed >= 5000 -} - -func (v *USBDevicesValidator) validateAvailableUSBIPPortsDefault(ctx context.Context, vm *v1alpha2.VirtualMachine, oldUSBDevices map[string]struct{}) (admission.Warnings, error) { - if vm.Status.Node == "" { - return admission.Warnings{}, nil - } - if vm.Spec.USBDevices == nil { - return admission.Warnings{}, nil - } - - var usbFromOtherNodes []string - - for _, ref := range vm.Spec.USBDevices { - if _, exists := oldUSBDevices[ref.Name]; exists { - continue - } - - usbDevice := &v1alpha2.USBDevice{} - err := v.client.Get(ctx, client.ObjectKey{Name: ref.Name, Namespace: vm.Namespace}, usbDevice) + if len(ssUSBFromOtherNodes) > 0 { + hasFree, err := usb.CheckFreePortForRequest(node.Annotations, 5000, len(ssUSBFromOtherNodes)) if err != nil { - return admission.Warnings{}, fmt.Errorf("failed to get USB device %s: %w", ref.Name, err) + return admission.Warnings{}, err } - - if usbDevice.Status.NodeName != vm.Status.Node { - usbFromOtherNodes = append(usbFromOtherNodes, ref.Name) + if !hasFree { + return admission.Warnings{}, fmt.Errorf("node %s has no available ports for sharing USB devices %v", vm.Status.Node, ssUSBFromOtherNodes) } } - if len(usbFromOtherNodes) == 0 { - return admission.Warnings{}, nil - } - - node, totalPorts, err := v.getNodeTotalPorts(ctx, vm.Status.Node) - if err != nil { - return admission.Warnings{}, err - } - - // total for 2 usb hubs (2.0 and 3.0) - totalPorts /= 2 - - return admission.Warnings{}, validateUsedPortsByAnnotation(node, annotations.AnnUSBIPUsedPorts, usbFromOtherNodes, totalPorts) + return admission.Warnings{}, nil } From 88f5ceda02e06371f0c8eedae5e8d912973c53d1 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Wed, 25 Mar 2026 12:25:12 +0200 Subject: [PATCH 02/14] fix(usb): skip port check for already attached devices Check free USBIP ports only for new attachments, not for devices already attached in KVVMI. This prevents resetting attached: true to false when ports are exhausted but device is already working. Fixes the issue where attached USB devices were marked as detached due to 'no free USBIP ports available' logs. Signed-off-by: Daniil Antoshin --- .../vm/internal/usb_device_attach_handler.go | 62 ++++++++----------- 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go index d6804c6972..77faa56c3f 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go @@ -95,10 +95,6 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach var kvvmi *virtv1.VirtualMachineInstance var hostDeviceReadyByName map[string]bool - // Lazy-loaded node for USBIP port check - var nodeLoaded bool - var node *corev1.Node - var nextStatusRefs []v1alpha2.USBDeviceStatusRef for _, usbDeviceRef := range vm.Spec.USBDevices { deviceName := usbDeviceRef.Name @@ -136,37 +132,7 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach continue } - // 3) Check free USBIP ports for devices from other nodes (USBIP scenario) - if usbDevice.Status.NodeName != "" && usbDevice.Status.NodeName != vm.Status.Node { - if !nodeLoaded && vm.Status.Node != "" { - node = &corev1.Node{} - if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { - if !apierrors.IsNotFound(err) { - return reconcile.Result{}, fmt.Errorf("failed to get node %s: %w", vm.Status.Node, err) - } - node = nil - } - nodeLoaded = true - } - - if node != nil { - hasFreePort, err := usb.CheckFreePort(node.Annotations, usbDevice.Status.Attributes.Speed) - if err != nil { - log.Error("failed to check free USBIP ports", "error", err, "device", deviceName, "node", vm.Status.Node) - nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) - continue - } - if !hasFreePort { - log.Info("no free USBIP ports available", "device", deviceName, "speed", usbDevice.Status.Attributes.Speed, "node", vm.Status.Node) - // Return requeue to retry after 5 seconds - nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) - changed.Status.USBDevices = nextStatusRefs - return reconcile.Result{RequeueAfter: 5 * time.Second}, nil - } - } - } - - // 4) Runtime evidence from KVVMI and attach action. + // 3) Runtime evidence from KVVMI and attach action. if !kvvmiLoaded { fetchedKVVMI, err := s.KVVMI(ctx) if err != nil { @@ -180,6 +146,7 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach hostDeviceReadyByName = h.hostDeviceReadyByName(kvvmi) } + // If device is already attached in KVVMI - preserve status, skip port check. if hostDeviceReadyByName[deviceName] { address := h.getUSBAddressFromKVVMI(deviceName, kvvmi) isHotplugged := vm.Status.Phase == v1alpha2.MachineRunning @@ -196,6 +163,31 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach continue } + // 4) Check free USBIP ports for new attachments from other nodes. + if usbDevice.Status.NodeName != "" && usbDevice.Status.NodeName != vm.Status.Node { + node := &corev1.Node{} + if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { + if !apierrors.IsNotFound(err) { + return reconcile.Result{}, fmt.Errorf("failed to get node %s: %w", vm.Status.Node, err) + } + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + continue + } + + hasFreePort, err := usb.CheckFreePort(node.Annotations, usbDevice.Status.Attributes.Speed) + if err != nil { + log.Error("failed to check free USBIP ports", "error", err, "device", deviceName, "node", vm.Status.Node) + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + continue + } + if !hasFreePort { + log.Info("no free USBIP ports available", "device", deviceName, "speed", usbDevice.Status.Attributes.Speed, "node", vm.Status.Node) + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + changed.Status.USBDevices = nextStatusRefs + return reconcile.Result{RequeueAfter: 5 * time.Second}, nil + } + } + requestName := h.getResourceClaimRequestName(deviceName) err := h.attachUSBDevice(ctx, vm, deviceName, templateName, requestName) if err != nil && !apierrors.IsAlreadyExists(err) && !strings.Contains(err.Error(), "already exists") { From 982923536465bc64bef406ee190bce5c51b7906a Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Wed, 25 Mar 2026 16:58:20 +0200 Subject: [PATCH 03/14] fix(usb): add vm.Status.Node check before USBIP port validation Signed-off-by: Daniil Antoshin --- .../pkg/controller/vm/internal/usb_device_attach_handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go index 77faa56c3f..9d04e40e02 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go @@ -164,7 +164,7 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach } // 4) Check free USBIP ports for new attachments from other nodes. - if usbDevice.Status.NodeName != "" && usbDevice.Status.NodeName != vm.Status.Node { + if usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { node := &corev1.Node{} if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { if !apierrors.IsNotFound(err) { From 1800729e5e4f117f0dff2c97b266ca6383aee9f7 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Wed, 25 Mar 2026 17:16:56 +0200 Subject: [PATCH 04/14] fix(usb): skip port check for devices already in progress When a USB device attach request is already in flight (existingStatus exists but not yet attached), skip the USBIP port availability check. The port was available when the request was made, and re-checking would cause devices to get stuck if ports become exhausted mid-flight while DRA/KubeVirt is still processing the request. Signed-off-by: Daniil Antoshin --- .../pkg/controller/vm/internal/usb_device_attach_handler.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go index 9d04e40e02..b969d91f91 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go @@ -164,7 +164,11 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach } // 4) Check free USBIP ports for new attachments from other nodes. - if usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { + // Skip this check if the device is already being processed (request already sent), + // as the port was available when the request was made. Re-checking would cause + // devices to get stuck when ports are exhausted mid-flight. + isAlreadyProcessing := existingStatus != nil && !existingStatus.Attached + if !isAlreadyProcessing && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { node := &corev1.Node{} if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { if !apierrors.IsNotFound(err) { From fbafacc22f12cbeafa0a04844edf6957f8216c4b Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Wed, 25 Mar 2026 20:13:47 +0200 Subject: [PATCH 05/14] fix(usb): improve in-progress detection for USB device attach Use Ready=true && Attached=false instead of just Attached=false to determine if a USB device attach request is already in flight. This is more reliable because: - existingStatus may exist from the start when VM is created with USB device - Ready=true means the USBDevice is ready and ResourceClaimTemplate exists - Attached=false means KVVMI has not yet reported the device as attached This ensures we only skip port checks when the request has actually been sent, not just when any status exists. Signed-off-by: Daniil Antoshin --- .../vm/internal/usb_device_attach_handler.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go index b969d91f91..73c77bd037 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go @@ -164,11 +164,11 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach } // 4) Check free USBIP ports for new attachments from other nodes. - // Skip this check if the device is already being processed (request already sent), - // as the port was available when the request was made. Re-checking would cause - // devices to get stuck when ports are exhausted mid-flight. - isAlreadyProcessing := existingStatus != nil && !existingStatus.Attached - if !isAlreadyProcessing && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { + // Skip this check if the device is already being processed (request already sent). + // A device is considered "in progress" when Ready=true but not yet Attached in KVVMI. + // Re-checking would cause devices to get stuck when ports are exhausted mid-flight. + isInProgress := existingStatus != nil && existingStatus.Ready && !existingStatus.Attached + if !isInProgress && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { node := &corev1.Node{} if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { if !apierrors.IsNotFound(err) { From 24d2eaef894ad93daec4e1ae68db95277ca9b953 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Wed, 25 Mar 2026 20:16:03 +0200 Subject: [PATCH 06/14] fix(usb): check ports only for new device attachments Check USBIP port availability only when device is added for the first time (existingStatus == nil). Once a device has a status, the attach request was already sent and ports were available at that time. This prevents devices from getting stuck if ports are exhausted mid-flight while DRA processes the request. Signed-off-by: Daniil Antoshin --- .../vm/internal/usb_device_attach_handler.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go index 73c77bd037..3bbc8659e7 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go @@ -163,12 +163,10 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach continue } - // 4) Check free USBIP ports for new attachments from other nodes. - // Skip this check if the device is already being processed (request already sent). - // A device is considered "in progress" when Ready=true but not yet Attached in KVVMI. - // Re-checking would cause devices to get stuck when ports are exhausted mid-flight. - isInProgress := existingStatus != nil && existingStatus.Ready && !existingStatus.Attached - if !isInProgress && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { + // 4) Check free USBIP ports only for NEW attachments from other nodes. + // If device already has a status (existingStatus != nil), the request was already sent. + // Skip re-checking to avoid stuck devices when ports are exhausted mid-flight. + if existingStatus == nil && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { node := &corev1.Node{} if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { if !apierrors.IsNotFound(err) { From 89174590f4015e43b4292ed4f256ef2290c58131 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Wed, 25 Mar 2026 20:18:31 +0200 Subject: [PATCH 07/14] fix(usb): add hostDeviceExistsByName helper function Add helper to check if a device exists in KVVMI regardless of its phase. Used by usb_device_attach_handler to skip port checks for devices that are already in flight (exist in KVVMI, even if not yet ready). Signed-off-by: Daniil Antoshin --- .../vm/internal/usb_device_handler.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_handler.go index c4f0000232..bddda99a9e 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_handler.go @@ -98,6 +98,23 @@ func (h *usbDeviceHandlerBase) hostDeviceReadyByName(kvvmi *virtv1.VirtualMachin return hostDeviceReadyByName } +func (h *usbDeviceHandlerBase) hostDeviceExistsByName(kvvmi *virtv1.VirtualMachineInstance) map[string]bool { + hostDeviceExistsByName := make(map[string]bool) + if kvvmi == nil || kvvmi.Status.DeviceStatus == nil { + return hostDeviceExistsByName + } + + for _, hostDeviceStatus := range kvvmi.Status.DeviceStatus.HostDeviceStatuses { + if hostDeviceStatus.Name == "" { + continue + } + + hostDeviceExistsByName[hostDeviceStatus.Name] = true + } + + return hostDeviceExistsByName +} + func (h *usbDeviceHandlerBase) attachUSBDevice( ctx context.Context, vm *v1alpha2.VirtualMachine, From 3b7852da1a3334669ef9669d48cfffb6ece68007 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Wed, 25 Mar 2026 20:24:19 +0200 Subject: [PATCH 08/14] fix(usb): use hostDeviceExistsByName for USBIP port check gate Skip free-port validation when the device already appears in KVVMI host device statuses (attach in progress), matching hostDeviceExistsByName helper added earlier. Signed-off-by: Daniil Antoshin --- .../controller/vm/internal/usb_device_attach_handler.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go index 3bbc8659e7..217870aece 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go @@ -94,6 +94,7 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach var kvvmiLoaded bool var kvvmi *virtv1.VirtualMachineInstance var hostDeviceReadyByName map[string]bool + var hostDeviceExistsByName map[string]bool var nextStatusRefs []v1alpha2.USBDeviceStatusRef for _, usbDeviceRef := range vm.Spec.USBDevices { @@ -163,10 +164,15 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach continue } + if hostDeviceExistsByName == nil { + hostDeviceExistsByName = h.hostDeviceExistsByName(kvvmi) + } + // 4) Check free USBIP ports only for NEW attachments from other nodes. // If device already has a status (existingStatus != nil), the request was already sent. + // If the host device is already listed in KVVMI, attach is in progress — skip port check here. // Skip re-checking to avoid stuck devices when ports are exhausted mid-flight. - if existingStatus == nil && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { + if existingStatus == nil && !hostDeviceExistsByName[deviceName] && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { node := &corev1.Node{} if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { if !apierrors.IsNotFound(err) { From 21a7089473166713fee7e0d94d21224a9a964fbc Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Wed, 25 Mar 2026 20:30:10 +0200 Subject: [PATCH 09/14] refactor(vm/usb): replace map[string]bool with map[string]struct{} for device sets Signed-off-by: Daniil Antoshin --- .../vm/internal/usb_device_attach_handler.go | 46 ++++++++++--------- .../vm/internal/usb_device_handler.go | 14 +++--- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go index 217870aece..a3a2ccdab2 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go @@ -93,8 +93,8 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach var kvvmiLoaded bool var kvvmi *virtv1.VirtualMachineInstance - var hostDeviceReadyByName map[string]bool - var hostDeviceExistsByName map[string]bool + var hostDeviceReadyByName map[string]struct{} + var hostDeviceExistsByName map[string]struct{} var nextStatusRefs []v1alpha2.USBDeviceStatusRef for _, usbDeviceRef := range vm.Spec.USBDevices { @@ -148,7 +148,7 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach } // If device is already attached in KVVMI - preserve status, skip port check. - if hostDeviceReadyByName[deviceName] { + if _, exists := hostDeviceReadyByName[deviceName]; exists { address := h.getUSBAddressFromKVVMI(deviceName, kvvmi) isHotplugged := vm.Status.Phase == v1alpha2.MachineRunning @@ -172,27 +172,29 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach // If device already has a status (existingStatus != nil), the request was already sent. // If the host device is already listed in KVVMI, attach is in progress — skip port check here. // Skip re-checking to avoid stuck devices when ports are exhausted mid-flight. - if existingStatus == nil && !hostDeviceExistsByName[deviceName] && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { - node := &corev1.Node{} - if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { - if !apierrors.IsNotFound(err) { - return reconcile.Result{}, fmt.Errorf("failed to get node %s: %w", vm.Status.Node, err) + if existingStatus == nil { + if _, exists := hostDeviceExistsByName[deviceName]; !exists && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { + node := &corev1.Node{} + if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { + if !apierrors.IsNotFound(err) { + return reconcile.Result{}, fmt.Errorf("failed to get node %s: %w", vm.Status.Node, err) + } + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + continue } - nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) - continue - } - hasFreePort, err := usb.CheckFreePort(node.Annotations, usbDevice.Status.Attributes.Speed) - if err != nil { - log.Error("failed to check free USBIP ports", "error", err, "device", deviceName, "node", vm.Status.Node) - nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) - continue - } - if !hasFreePort { - log.Info("no free USBIP ports available", "device", deviceName, "speed", usbDevice.Status.Attributes.Speed, "node", vm.Status.Node) - nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) - changed.Status.USBDevices = nextStatusRefs - return reconcile.Result{RequeueAfter: 5 * time.Second}, nil + hasFreePort, err := usb.CheckFreePort(node.Annotations, usbDevice.Status.Attributes.Speed) + if err != nil { + log.Error("failed to check free USBIP ports", "error", err, "device", deviceName, "node", vm.Status.Node) + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + continue + } + if !hasFreePort { + log.Info("no free USBIP ports available", "device", deviceName, "speed", usbDevice.Status.Attributes.Speed, "node", vm.Status.Node) + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + changed.Status.USBDevices = nextStatusRefs + return reconcile.Result{RequeueAfter: 5 * time.Second}, nil + } } } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_handler.go index bddda99a9e..1d4318f907 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_handler.go @@ -81,8 +81,8 @@ func (h *usbDeviceHandlerBase) isUSBDeviceReady(usbDevice *v1alpha2.USBDevice) b return found && readyCondition.Status == metav1.ConditionTrue } -func (h *usbDeviceHandlerBase) hostDeviceReadyByName(kvvmi *virtv1.VirtualMachineInstance) map[string]bool { - hostDeviceReadyByName := make(map[string]bool) +func (h *usbDeviceHandlerBase) hostDeviceReadyByName(kvvmi *virtv1.VirtualMachineInstance) map[string]struct{} { + hostDeviceReadyByName := make(map[string]struct{}) if kvvmi == nil || kvvmi.Status.DeviceStatus == nil { return hostDeviceReadyByName } @@ -92,14 +92,16 @@ func (h *usbDeviceHandlerBase) hostDeviceReadyByName(kvvmi *virtv1.VirtualMachin continue } - hostDeviceReadyByName[hostDeviceStatus.Name] = hostDeviceReadyByName[hostDeviceStatus.Name] || hostDeviceStatus.Phase == virtv1.DeviceReady + if hostDeviceStatus.Phase == virtv1.DeviceReady { + hostDeviceReadyByName[hostDeviceStatus.Name] = struct{}{} + } } return hostDeviceReadyByName } -func (h *usbDeviceHandlerBase) hostDeviceExistsByName(kvvmi *virtv1.VirtualMachineInstance) map[string]bool { - hostDeviceExistsByName := make(map[string]bool) +func (h *usbDeviceHandlerBase) hostDeviceExistsByName(kvvmi *virtv1.VirtualMachineInstance) map[string]struct{} { + hostDeviceExistsByName := make(map[string]struct{}) if kvvmi == nil || kvvmi.Status.DeviceStatus == nil { return hostDeviceExistsByName } @@ -109,7 +111,7 @@ func (h *usbDeviceHandlerBase) hostDeviceExistsByName(kvvmi *virtv1.VirtualMachi continue } - hostDeviceExistsByName[hostDeviceStatus.Name] = true + hostDeviceExistsByName[hostDeviceStatus.Name] = struct{}{} } return hostDeviceExistsByName From 5603543096f8d756e6c3cd59fc026dd9f1e09f86 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Wed, 25 Mar 2026 22:49:35 +0200 Subject: [PATCH 10/14] fix(core): keep usb attach fast-fail across reconciles Signed-off-by: Daniil Antoshin --- .../vm/internal/usb_device_attach_handler.go | 46 +++++----- .../usb_device_attach_handler_test.go | 83 ++++++++++++++++++- .../validators/usb_devices_validator.go | 4 +- 3 files changed, 105 insertions(+), 28 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go index a3a2ccdab2..fc7f7759be 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go @@ -168,33 +168,29 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach hostDeviceExistsByName = h.hostDeviceExistsByName(kvvmi) } - // 4) Check free USBIP ports only for NEW attachments from other nodes. - // If device already has a status (existingStatus != nil), the request was already sent. - // If the host device is already listed in KVVMI, attach is in progress — skip port check here. - // Skip re-checking to avoid stuck devices when ports are exhausted mid-flight. - if existingStatus == nil { - if _, exists := hostDeviceExistsByName[deviceName]; !exists && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { - node := &corev1.Node{} - if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { - if !apierrors.IsNotFound(err) { - return reconcile.Result{}, fmt.Errorf("failed to get node %s: %w", vm.Status.Node, err) - } - nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) - continue + // 4) Check free USBIP ports for cross-node attachments until KVVMI reflects the device. + // Once the host device is listed in KVVMI, attach is already in progress, so skip the check. + if _, exists := hostDeviceExistsByName[deviceName]; !exists && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { + node := &corev1.Node{} + if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { + if !apierrors.IsNotFound(err) { + return reconcile.Result{}, fmt.Errorf("failed to get node %s: %w", vm.Status.Node, err) } + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + continue + } - hasFreePort, err := usb.CheckFreePort(node.Annotations, usbDevice.Status.Attributes.Speed) - if err != nil { - log.Error("failed to check free USBIP ports", "error", err, "device", deviceName, "node", vm.Status.Node) - nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) - continue - } - if !hasFreePort { - log.Info("no free USBIP ports available", "device", deviceName, "speed", usbDevice.Status.Attributes.Speed, "node", vm.Status.Node) - nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) - changed.Status.USBDevices = nextStatusRefs - return reconcile.Result{RequeueAfter: 5 * time.Second}, nil - } + hasFreePort, err := usb.CheckFreePort(node.Annotations, usbDevice.Status.Attributes.Speed) + if err != nil { + log.Error("failed to check free USBIP ports", "error", err, "device", deviceName, "node", vm.Status.Node) + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + continue + } + if !hasFreePort { + log.Info("no free USBIP ports available", "device", deviceName, "speed", usbDevice.Status.Attributes.Speed, "node", vm.Status.Node) + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + changed.Status.USBDevices = nextStatusRefs + return reconcile.Result{RequeueAfter: 5 * time.Second}, nil } } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler_test.go index 6bf8831ba7..2e181ef626 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler_test.go @@ -20,10 +20,12 @@ import ( "context" "errors" "log/slog" + "strconv" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" resourcev1 "k8s.io/api/resource/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -33,6 +35,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/controller/reconciler" "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" "github.com/deckhouse/virtualization-controller/pkg/logger" @@ -152,7 +155,7 @@ var _ = Describe("USBDeviceAttachHandler", func() { return &v1alpha2.USBDevice{ ObjectMeta: usbMeta, Status: v1alpha2.USBDeviceStatus{ - Attributes: v1alpha2.NodeUSBDeviceAttributes{Name: attributeName, VendorID: vid, ProductID: productID}, + Attributes: v1alpha2.NodeUSBDeviceAttributes{Name: attributeName, VendorID: vid, ProductID: productID, Speed: 480}, NodeName: "node-1", Conditions: conds, }, @@ -172,6 +175,19 @@ var _ = Describe("USBDeviceAttachHandler", func() { } } + newNode := func(name string, usedHSPorts, usedSSPorts int) *corev1.Node { + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Annotations: map[string]string{ + annotations.AnnUSBIPTotalPorts: "8", + annotations.AnnUSBIPHighSpeedHubUsedPorts: strconv.Itoa(usedHSPorts), + annotations.AnnUSBIPSuperSpeedHubUsedPorts: strconv.Itoa(usedSSPorts), + }, + }, + } + } + runHandle := func(vm *v1alpha2.VirtualMachine, objs ...client.Object) (reconcile.Result, *reconciler.Resource[*v1alpha2.VirtualMachine, v1alpha2.VirtualMachineStatus], state.VirtualMachineState, error) { fakeClient, vmResource, vmState = setupEnvironment(vm, objs...) handler = NewUSBDeviceAttachHandler(fakeClient, mockVirtCl) @@ -481,6 +497,71 @@ var _ = Describe("USBDeviceAttachHandler", func() { Expect(status.Ready).To(BeTrue()) }) + It("requeues on every reconcile while cross-node USBIP ports remain exhausted", func() { + vm := newVM(v1alpha2.MachineRunning) + vm.Status.Node = "node-2" + + result, _, _, err := runHandle( + vm, + newUSBDevice(true, usbDeviceName, false), + newResourceClaimTemplate(), + newNode("node-2", 4, 0), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(BeNumerically(">", 0)) + + mockVM := mockVirtCl.vmClients[vmNamespace] + Expect(mockVM.addResourceClaimCalls).To(HaveLen(0)) + status := expectSingleUSBStatus() + Expect(status.Attached).To(BeFalse()) + Expect(status.Ready).To(BeTrue()) + + vm.Status.USBDevices = vmResource.Changed().Status.USBDevices + + result, _, _, err = runHandle( + vm, + newUSBDevice(true, usbDeviceName, false), + newResourceClaimTemplate(), + newNode("node-2", 4, 0), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(BeNumerically(">", 0)) + Expect(mockVM.addResourceClaimCalls).To(HaveLen(0)) + status = expectSingleUSBStatus() + Expect(status.Attached).To(BeFalse()) + Expect(status.Ready).To(BeTrue()) + }) + + It("skips port checks once KVVMI already reflects the host device", func() { + vm := newVM(v1alpha2.MachineRunning, v1alpha2.USBDeviceStatusRef{ + Name: usbDeviceName, + Attached: false, + Ready: true, + }) + vm.Status.Node = "node-2" + + hostDeviceStatus := virtv1.DeviceStatusInfo{ + Name: usbDeviceName, + Phase: virtv1.DevicePending, + } + + result, _, _, err := runHandle( + vm, + newUSBDevice(true, usbDeviceName, false), + newResourceClaimTemplate(), + newKVVMI(hostDeviceStatus), + newNode("node-2", 4, 0), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + + mockVM := mockVirtCl.vmClients[vmNamespace] + Expect(mockVM.addResourceClaimCalls).To(HaveLen(1)) + status := expectSingleUSBStatus() + Expect(status.Attached).To(BeFalse()) + Expect(status.Ready).To(BeTrue()) + }) + It("clears USB status on VM deletion", func() { now := metav1.NewTime(time.Now()) vm := &v1alpha2.VirtualMachine{ diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go index 2059dd0d29..3da0120fcc 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go @@ -171,9 +171,9 @@ func (v *USBDevicesValidator) validateAvailableUSBIPPortsWithPartitionableDevice continue } - isHs, isSS := usb.ResolveSpeed(usbDevice.Status.Attributes.Speed) + isHS, isSS := usb.ResolveSpeed(usbDevice.Status.Attributes.Speed) switch { - case isHs: + case isHS: hsUSBFromOtherNodes = append(hsUSBFromOtherNodes, ref.Name) case isSS: ssUSBFromOtherNodes = append(ssUSBFromOtherNodes, ref.Name) From 682aba910bec20533a835470a9b7c7474f02ac43 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Thu, 26 Mar 2026 13:22:13 +0200 Subject: [PATCH 11/14] fix(usb): exclude local USBs from USBIP port accounting Signed-off-by: Daniil Antoshin --- .../pkg/common/usb/availability.go | 125 ++++++++++++++++++ .../usbdevice/internal/handler/lifecycle.go | 38 +++--- .../internal/handler/lifecycle_test.go | 72 +++++++++- .../usbdevice/internal/state/state.go | 34 +++-- .../vm/internal/usb_device_attach_handler.go | 16 +-- .../validators/usb_devices_validator.go | 11 +- .../validators/usb_devices_validator_test.go | 32 +++++ 7 files changed, 278 insertions(+), 50 deletions(-) create mode 100644 images/virtualization-artifact/pkg/common/usb/availability.go diff --git a/images/virtualization-artifact/pkg/common/usb/availability.go b/images/virtualization-artifact/pkg/common/usb/availability.go new file mode 100644 index 0000000000..6af0fb408a --- /dev/null +++ b/images/virtualization-artifact/pkg/common/usb/availability.go @@ -0,0 +1,125 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package usb + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +func CheckFreePortOnNodeExcludingLocalUSBs(ctx context.Context, cl client.Client, nodeName string, speed int) (bool, error) { + return CheckFreePortForRequestOnNodeExcludingLocalUSBs(ctx, cl, nodeName, speed, 1) +} + +func CheckFreePortForRequestOnNodeExcludingLocalUSBs(ctx context.Context, cl client.Client, nodeName string, speed, requestedCount int) (bool, error) { + node := &corev1.Node{} + if err := cl.Get(ctx, client.ObjectKey{Name: nodeName}, node); err != nil { + return false, err + } + + isHS, isSS := ResolveSpeed(speed) + if !isHS && !isSS { + return false, fmt.Errorf("unsupported USB speed: %d", speed) + } + + totalPortsPerHub, err := GetTotalPortsPerHub(node.Annotations) + if err != nil { + return false, err + } + + usedPorts, err := getUsedPortsForSpeed(node.Annotations, speed) + if err != nil { + return false, err + } + + excludedLocalUSBs, err := countLocalAttachedUSBsOnNodeBySpeed(ctx, cl, nodeName, speed) + if err != nil { + return false, err + } + + effectiveUsedPorts := usedPorts - excludedLocalUSBs + if effectiveUsedPorts < 0 { + effectiveUsedPorts = 0 + } + + return (effectiveUsedPorts + requestedCount) <= totalPortsPerHub, nil +} + +func getUsedPortsForSpeed(nodeAnnotations map[string]string, speed int) (int, error) { + isHS, isSS := ResolveSpeed(speed) + + switch { + case isHS: + return GetUsedPorts(nodeAnnotations, annotations.AnnUSBIPHighSpeedHubUsedPorts) + case isSS: + return GetUsedPorts(nodeAnnotations, annotations.AnnUSBIPSuperSpeedHubUsedPorts) + default: + return 0, fmt.Errorf("unsupported USB speed: %d", speed) + } +} + +func countLocalAttachedUSBsOnNodeBySpeed(ctx context.Context, cl client.Client, nodeName string, speed int) (int, error) { + var vmList v1alpha2.VirtualMachineList + if err := cl.List(ctx, &vmList, client.MatchingFields{indexer.IndexFieldVMByNode: nodeName}); err != nil { + return 0, err + } + + count := 0 + usbCache := make(map[client.ObjectKey]*v1alpha2.USBDevice) + for i := range vmList.Items { + vm := &vmList.Items[i] + for _, usbStatus := range vm.Status.USBDevices { + if !usbStatus.Attached { + continue + } + + key := client.ObjectKey{Name: usbStatus.Name, Namespace: vm.Namespace} + usbDevice, ok := usbCache[key] + if !ok { + usbDevice = &v1alpha2.USBDevice{} + if err := cl.Get(ctx, key, usbDevice); err != nil { + return 0, err + } + usbCache[key] = usbDevice + } + + if usbDevice.Status.NodeName != nodeName { + continue + } + + if sameSpeedClass(usbDevice.Status.Attributes.Speed, speed) { + count++ + } + } + } + + return count, nil +} + +func sameSpeedClass(deviceSpeed, requestedSpeed int) bool { + deviceHS, deviceSS := ResolveSpeed(deviceSpeed) + requestedHS, requestedSS := ResolveSpeed(requestedSpeed) + + return (deviceHS && requestedHS) || (deviceSS && requestedSS) +} diff --git a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go index 53206c0ed5..e8dc22a6bd 100644 --- a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go +++ b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go @@ -22,7 +22,6 @@ import ( "reflect" "strconv" - corev1 "k8s.io/api/core/v1" resourcev1 "k8s.io/api/resource/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -199,18 +198,23 @@ func (h *LifecycleHandler) syncAttached(ctx context.Context, s state.USBDeviceSt current := s.USBDevice().Current() changed := s.USBDevice().Changed() - usbDevice := current + usbDevice := changed - vms, err := s.VirtualMachinesUsingDevice(ctx) + attachedVMs, err := s.VirtualMachinesUsingDevice(ctx) if err != nil { return fmt.Errorf("failed to find VirtualMachines using USBDevice: %w", err) } + referencingVMs, err := s.VirtualMachinesReferencingDevice(ctx) + if err != nil { + return fmt.Errorf("failed to find VirtualMachines referencing USBDevice: %w", err) + } + var reason usbdevicecondition.AttachedReason var status metav1.ConditionStatus var message string - if len(vms) == 0 { + if len(referencingVMs) == 0 { reason = usbdevicecondition.Available status = metav1.ConditionFalse message = "Device is available for attachment to a virtual machine." @@ -219,22 +223,16 @@ func (h *LifecycleHandler) syncAttached(ctx context.Context, s state.USBDeviceSt } noFreePort := false - for _, vm := range vms { + for _, vm := range referencingVMs { if usbDevice.Status.NodeName != "" && usbDevice.Status.NodeName != vm.Status.Node { - node := &corev1.Node{} - if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { + hasFreePort, err := usb.CheckFreePortOnNodeExcludingLocalUSBs(ctx, h.client, vm.Status.Node, usbDevice.Status.Attributes.Speed) + if err != nil { if !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to get node %s: %w", vm.Status.Node, err) + log.Error("failed to check free USBIP ports", "error", err, "device", usbDevice.Name, "node", vm.Status.Node) } continue } - hasFreePort, err := usb.CheckFreePort(node.Annotations, usbDevice.Status.Attributes.Speed) - if err != nil { - log.Error("failed to check free USBIP ports", "error", err, "device", usbDevice.Name, "node", vm.Status.Node) - continue - } - if !hasFreePort { noFreePort = true message = fmt.Sprintf("Device is attached to VirtualMachine %s/%s, but no free USBIP ports available on node %s for speed %d.", @@ -247,13 +245,17 @@ func (h *LifecycleHandler) syncAttached(ctx context.Context, s state.USBDeviceSt if noFreePort { reason = usbdevicecondition.NoFreeUSBIPPort status = metav1.ConditionTrue - } else { + } else if len(attachedVMs) > 0 { reason = usbdevicecondition.AttachedToVirtualMachine status = metav1.ConditionTrue - message = fmt.Sprintf("Device is attached to %d VirtualMachines.", len(vms)) - if len(vms) == 1 { - message = fmt.Sprintf("Device is attached to VirtualMachine %s/%s.", vms[0].Namespace, vms[0].Name) + message = fmt.Sprintf("Device is attached to %d VirtualMachines.", len(attachedVMs)) + if len(attachedVMs) == 1 { + message = fmt.Sprintf("Device is attached to VirtualMachine %s/%s.", attachedVMs[0].Namespace, attachedVMs[0].Name) } + } else { + reason = usbdevicecondition.Available + status = metav1.ConditionFalse + message = "Device is requested by a virtual machine but not attached yet." } setAttachedCondition(current, &changed.Status.Conditions, status, reason, message) diff --git a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go index 24c20273a4..20affbd206 100644 --- a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go +++ b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go @@ -22,6 +22,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" resourcev1 "k8s.io/api/resource/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -31,6 +32,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" "github.com/deckhouse/virtualization-controller/pkg/controller/reconciler" "github.com/deckhouse/virtualization-controller/pkg/controller/usbdevice/internal/state" @@ -48,6 +50,7 @@ var _ = Describe("LifecycleHandler", func() { ctx = logger.ToContext(context.TODO(), slog.Default()) scheme = apiruntime.NewScheme() Expect(v1alpha2.AddToScheme(scheme)).To(Succeed()) + Expect(corev1.AddToScheme(scheme)).To(Succeed()) Expect(resourcev1.AddToScheme(scheme)).To(Succeed()) }) @@ -88,7 +91,8 @@ var _ = Describe("LifecycleHandler", func() { } vmObj, vmField, vmExtractValue := indexer.IndexVMByUSBDevice() - cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).WithIndex(vmObj, vmField, vmExtractValue).Build() + vmNodeObj, vmNodeField, vmNodeExtractValue := indexer.IndexVMByNode() + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).WithIndex(vmObj, vmField, vmExtractValue).WithIndex(vmNodeObj, vmNodeField, vmNodeExtractValue).Build() res := reconciler.NewResource( types.NamespacedName{Name: usbDevice.Name, Namespace: usbDevice.Namespace}, @@ -124,6 +128,63 @@ var _ = Describe("LifecycleHandler", func() { Entry("node missing", false, "", false, false, metav1.ConditionFalse, string(usbdevicecondition.NotFound), string(usbdevicecondition.Available)), ) + It("should set NoFreeUSBIPPort when VM references device but attach cannot start", func() { + usbDevice := &v1alpha2.USBDevice{ + ObjectMeta: metav1.ObjectMeta{Name: "usb-device-1", Namespace: "default", UID: "usb-uid-1"}, + Status: v1alpha2.USBDeviceStatus{Attributes: v1alpha2.NodeUSBDeviceAttributes{ + Name: "usb-device-1", + VendorID: "1234", + ProductID: "5678", + Speed: 480, + }}, + } + + nodeUSBDevice := &v1alpha2.NodeUSBDevice{ + ObjectMeta: metav1.ObjectMeta{Name: "usb-device-1"}, + Status: v1alpha2.NodeUSBDeviceStatus{ + Attributes: v1alpha2.NodeUSBDeviceAttributes{Name: "usb-device-1", VendorID: "1234", ProductID: "5678", Speed: 480}, + NodeName: "node-1", + Conditions: []metav1.Condition{{Type: string(nodeusbdevicecondition.ReadyType), Status: metav1.ConditionTrue, Reason: string(nodeusbdevicecondition.Ready), Message: "Node status"}}, + }, + } + + vm := &v1alpha2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{Name: "vm-1", Namespace: "default"}, + Spec: v1alpha2.VirtualMachineSpec{USBDevices: []v1alpha2.USBDeviceSpecRef{{Name: "usb-device-1"}}}, + Status: v1alpha2.VirtualMachineStatus{ + Node: "node-2", + USBDevices: []v1alpha2.USBDeviceStatusRef{{Name: "usb-device-1", Attached: false}}, + }, + } + + node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-2", Annotations: map[string]string{ + annotations.AnnUSBIPTotalPorts: "2", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "1", + }}} + + vmObj, vmField, vmExtractValue := indexer.IndexVMByUSBDevice() + vmNodeObj, vmNodeField, vmNodeExtractValue := indexer.IndexVMByNode() + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(usbDevice, nodeUSBDevice, vm, node).WithIndex(vmObj, vmField, vmExtractValue).WithIndex(vmNodeObj, vmNodeField, vmNodeExtractValue).Build() + + res := reconciler.NewResource( + types.NamespacedName{Name: usbDevice.Name, Namespace: usbDevice.Namespace}, + cl, + func() *v1alpha2.USBDevice { return &v1alpha2.USBDevice{} }, + func(obj *v1alpha2.USBDevice) v1alpha2.USBDeviceStatus { return obj.Status }, + ) + Expect(res.Fetch(ctx)).To(Succeed()) + + st := state.New(cl, res) + h := NewLifecycleHandler(cl) + _, err := h.Handle(ctx, st) + Expect(err).NotTo(HaveOccurred()) + + attached := meta.FindStatusCondition(res.Changed().Status.Conditions, string(usbdevicecondition.AttachedType)) + Expect(attached).NotTo(BeNil()) + Expect(attached.Reason).To(Equal(string(usbdevicecondition.NoFreeUSBIPPort))) + Expect(attached.Status).To(Equal(metav1.ConditionTrue)) + }) + It("should skip ResourceClaimTemplate when attribute name is empty", func() { usbDevice := &v1alpha2.USBDevice{ ObjectMeta: metav1.ObjectMeta{Name: "usb-device-1", Namespace: "default", UID: "usb-uid-1"}, @@ -133,7 +194,8 @@ var _ = Describe("LifecycleHandler", func() { objects := []client.Object{usbDevice} vmObj, vmField, vmExtractValue := indexer.IndexVMByUSBDevice() - cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).WithIndex(vmObj, vmField, vmExtractValue).Build() + vmNodeObj, vmNodeField, vmNodeExtractValue := indexer.IndexVMByNode() + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).WithIndex(vmObj, vmField, vmExtractValue).WithIndex(vmNodeObj, vmNodeField, vmNodeExtractValue).Build() res := reconciler.NewResource( types.NamespacedName{Name: usbDevice.Name, Namespace: usbDevice.Namespace}, @@ -170,7 +232,8 @@ var _ = Describe("LifecycleHandler", func() { } vmObj, vmField, vmExtractValue := indexer.IndexVMByUSBDevice() - cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(usbDevice, nodeUSBDevice).WithIndex(vmObj, vmField, vmExtractValue).Build() + vmNodeObj, vmNodeField, vmNodeExtractValue := indexer.IndexVMByNode() + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(usbDevice, nodeUSBDevice).WithIndex(vmObj, vmField, vmExtractValue).WithIndex(vmNodeObj, vmNodeField, vmNodeExtractValue).Build() res := reconciler.NewResource( types.NamespacedName{Name: usbDevice.Name, Namespace: usbDevice.Namespace}, @@ -238,7 +301,8 @@ var _ = Describe("LifecycleHandler", func() { } vmObj, vmField, vmExtractValue := indexer.IndexVMByUSBDevice() - cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(usbDevice, nodeUSBDevice, template).WithIndex(vmObj, vmField, vmExtractValue).Build() + vmNodeObj, vmNodeField, vmNodeExtractValue := indexer.IndexVMByNode() + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(usbDevice, nodeUSBDevice, template).WithIndex(vmObj, vmField, vmExtractValue).WithIndex(vmNodeObj, vmNodeField, vmNodeExtractValue).Build() res := reconciler.NewResource( types.NamespacedName{Name: usbDevice.Name, Namespace: usbDevice.Namespace}, diff --git a/images/virtualization-artifact/pkg/controller/usbdevice/internal/state/state.go b/images/virtualization-artifact/pkg/controller/usbdevice/internal/state/state.go index bf1da15aa4..2e83fa4b07 100644 --- a/images/virtualization-artifact/pkg/controller/usbdevice/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/usbdevice/internal/state/state.go @@ -31,6 +31,7 @@ type USBDeviceState interface { USBDevice() *reconciler.Resource[*v1alpha2.USBDevice, v1alpha2.USBDeviceStatus] NodeUSBDevice(ctx context.Context) (*v1alpha2.NodeUSBDevice, error) VirtualMachinesUsingDevice(ctx context.Context) ([]*v1alpha2.VirtualMachine, error) + VirtualMachinesReferencingDevice(ctx context.Context) ([]*v1alpha2.VirtualMachine, error) } func New(client client.Client, usbDevice *reconciler.Resource[*v1alpha2.USBDevice, v1alpha2.USBDeviceStatus]) USBDeviceState { @@ -67,7 +68,7 @@ func (s *usbDeviceState) NodeUSBDevice(ctx context.Context) (*v1alpha2.NodeUSBDe return nodeUSBDevice, nil } -func (s *usbDeviceState) VirtualMachinesUsingDevice(ctx context.Context) ([]*v1alpha2.VirtualMachine, error) { +func (s *usbDeviceState) VirtualMachinesReferencingDevice(ctx context.Context) ([]*v1alpha2.VirtualMachine, error) { usbDevice := s.usbDevice.Current() if usbDevice == nil { return nil, nil @@ -83,14 +84,31 @@ func (s *usbDeviceState) VirtualMachinesUsingDevice(ctx context.Context) ([]*v1a var result []*v1alpha2.VirtualMachine for i := range vmList.Items { vm := &vmList.Items[i] - // Check if VM is in the same namespace as USBDevice if vm.Namespace == usbDevice.Namespace { - // Verify that device is actually attached in VM status - for _, usbStatus := range vm.Status.USBDevices { - if usbStatus.Name == usbDevice.Name && usbStatus.Attached { - result = append(result, vm) - break - } + result = append(result, vm) + } + } + + return result, nil +} + +func (s *usbDeviceState) VirtualMachinesUsingDevice(ctx context.Context) ([]*v1alpha2.VirtualMachine, error) { + vms, err := s.VirtualMachinesReferencingDevice(ctx) + if err != nil { + return nil, err + } + + usbDevice := s.usbDevice.Current() + if usbDevice == nil { + return nil, nil + } + + var result []*v1alpha2.VirtualMachine + for _, vm := range vms { + for _, usbStatus := range vm.Status.USBDevices { + if usbStatus.Name == usbDevice.Name && usbStatus.Attached { + result = append(result, vm) + break } } } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go index fc7f7759be..e3683e7d06 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go @@ -22,7 +22,6 @@ import ( "strings" "time" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -171,21 +170,16 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach // 4) Check free USBIP ports for cross-node attachments until KVVMI reflects the device. // Once the host device is listed in KVVMI, attach is already in progress, so skip the check. if _, exists := hostDeviceExistsByName[deviceName]; !exists && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { - node := &corev1.Node{} - if err := h.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node); err != nil { + hasFreePort, err := usb.CheckFreePortOnNodeExcludingLocalUSBs(ctx, h.client, vm.Status.Node, usbDevice.Status.Attributes.Speed) + if err != nil { if !apierrors.IsNotFound(err) { - return reconcile.Result{}, fmt.Errorf("failed to get node %s: %w", vm.Status.Node, err) + log.Error("failed to check free USBIP ports", "error", err, "device", deviceName, "node", vm.Status.Node) + } else { + log.Debug("node not found while checking free USBIP ports", "device", deviceName, "node", vm.Status.Node) } nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) continue } - - hasFreePort, err := usb.CheckFreePort(node.Annotations, usbDevice.Status.Attributes.Speed) - if err != nil { - log.Error("failed to check free USBIP ports", "error", err, "device", deviceName, "node", vm.Status.Node) - nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) - continue - } if !hasFreePort { log.Info("no free USBIP ports available", "device", deviceName, "speed", usbDevice.Status.Attributes.Speed, "node", vm.Status.Node) nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go index 3da0120fcc..790ec7f2ac 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator.go @@ -20,7 +20,6 @@ import ( "context" "fmt" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" "k8s.io/component-base/featuregate" "sigs.k8s.io/controller-runtime/pkg/client" @@ -186,14 +185,8 @@ func (v *USBDevicesValidator) validateAvailableUSBIPPortsWithPartitionableDevice return admission.Warnings{}, nil } - node := &corev1.Node{} - err := v.client.Get(ctx, client.ObjectKey{Name: vm.Status.Node}, node) - if err != nil { - return admission.Warnings{}, err - } - if len(hsUSBFromOtherNodes) > 0 { - hasFree, err := usb.CheckFreePortForRequest(node.Annotations, 480, len(hsUSBFromOtherNodes)) + hasFree, err := usb.CheckFreePortForRequestOnNodeExcludingLocalUSBs(ctx, v.client, vm.Status.Node, 480, len(hsUSBFromOtherNodes)) if err != nil { return admission.Warnings{}, err } @@ -203,7 +196,7 @@ func (v *USBDevicesValidator) validateAvailableUSBIPPortsWithPartitionableDevice } if len(ssUSBFromOtherNodes) > 0 { - hasFree, err := usb.CheckFreePortForRequest(node.Annotations, 5000, len(ssUSBFromOtherNodes)) + hasFree, err := usb.CheckFreePortForRequestOnNodeExcludingLocalUSBs(ctx, v.client, vm.Status.Node, 5000, len(ssUSBFromOtherNodes)) if err != nil { return admission.Warnings{}, err } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator_test.go index ad2ad3934e..d2e8007417 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator_test.go @@ -20,6 +20,7 @@ import ( "strings" "testing" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/component-base/featuregate" @@ -111,6 +112,32 @@ func TestUSBDevicesValidatorValidateCreateSucceedsWhenUSBFeatureEnabled(t *testi } } +func TestUSBDevicesValidatorValidateUpdateExcludesLocalUSBsFromPortAccounting(t *testing.T) { + oldVM := newVirtualMachine("vm-current", []v1alpha2.USBDeviceSpecRef{{Name: "usb-local"}}) + oldVM.Status.Node = "node-1" + oldVM.Status.USBDevices = []v1alpha2.USBDeviceStatusRef{{Name: "usb-local", Attached: true}} + + newVM := oldVM.DeepCopy() + newVM.Spec.USBDevices = []v1alpha2.USBDeviceSpecRef{{Name: "usb-local"}, {Name: "usb-remote"}} + + objects := []client.Object{ + oldVM, + &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-1", Annotations: map[string]string{ + "usb.virtualization.deckhouse.io/usbip-total-ports": "2", + "usb.virtualization.deckhouse.io/usbip-high-speed-hub-used-ports": "1", + "usb.virtualization.deckhouse.io/usbip-super-speed-hub-used-ports": "0", + }}}, + &v1alpha2.USBDevice{ObjectMeta: metav1.ObjectMeta{Name: "usb-local", Namespace: "default"}, Status: v1alpha2.USBDeviceStatus{NodeName: "node-1", Attributes: v1alpha2.NodeUSBDeviceAttributes{Speed: 480}}}, + &v1alpha2.USBDevice{ObjectMeta: metav1.ObjectMeta{Name: "usb-remote", Namespace: "default"}, Status: v1alpha2.USBDeviceStatus{NodeName: "node-2", Attributes: v1alpha2.NodeUSBDeviceAttributes{Speed: 480}}}, + } + + validator := NewUSBDevicesValidator(newFakeClientWithUSBVMIndexer(t, objects...), newUSBFeatureGate(t, true)) + _, err := validator.ValidateUpdate(t.Context(), oldVM, newVM) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } +} + func TestUSBDevicesValidatorValidateUpdateReturnsErrorWhenUSBFeatureDisabled(t *testing.T) { oldVM := newVirtualMachine("vm-current", nil) newVM := newVirtualMachine("vm-current", []v1alpha2.USBDeviceSpecRef{{Name: "usb-1"}}) @@ -133,12 +160,17 @@ func newFakeClientWithUSBVMIndexer(t *testing.T, objects ...client.Object) clien if err := v1alpha2.AddToScheme(scheme); err != nil { t.Fatalf("failed to add virtualization API scheme: %v", err) } + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add core API scheme: %v", err) + } vmObj, vmField, vmExtractValue := indexer.IndexVMByUSBDevice() + vmNodeObj, vmNodeField, vmNodeExtractValue := indexer.IndexVMByNode() return fake.NewClientBuilder(). WithScheme(scheme). WithObjects(objects...). WithIndex(vmObj, vmField, vmExtractValue). + WithIndex(vmNodeObj, vmNodeField, vmNodeExtractValue). Build() } From 999606d33d5ea9b82965e83a856843499149debd Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Thu, 26 Mar 2026 13:30:24 +0200 Subject: [PATCH 12/14] fix(usb): harden fast-fail port checks and conditions Signed-off-by: Daniil Antoshin --- docs/USER_GUIDE.md | 1 + docs/USER_GUIDE.ru.md | 1 + .../pkg/common/usb/availability.go | 4 + .../pkg/common/usb/availability_test.go | 131 ++++++++++++++++++ .../usbdevice/internal/handler/lifecycle.go | 12 +- .../internal/handler/lifecycle_test.go | 54 +++++++- .../vm/internal/usb_device_attach_handler.go | 10 +- .../usb_device_attach_handler_test.go | 26 ++++ 8 files changed, 228 insertions(+), 11 deletions(-) create mode 100644 images/virtualization-artifact/pkg/common/usb/availability_test.go diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 17edb3043c..f406cdce06 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -3734,6 +3734,7 @@ The [USBDevice](/modules/virtualization/cr.html#usbdevice) resource provides sta - **Attached**: Indicates whether the device is attached to a virtual machine. - `AttachedToVirtualMachine`: Device is attached to a VM. - `Available`: Device is available for attachment. + - `NoFreeUSBIPPort`: Device is requested by a VM but cannot be attached because there are no free USBIP ports on the target node. In this case, `Attached=False`. ### Attaching USB Device to VM diff --git a/docs/USER_GUIDE.ru.md b/docs/USER_GUIDE.ru.md index 013238a760..14ec735046 100644 --- a/docs/USER_GUIDE.ru.md +++ b/docs/USER_GUIDE.ru.md @@ -3770,6 +3770,7 @@ logitech-webcam node-2 Logitech Webcam C920 ABC123456 False - **Attached**: Указывает, подключено ли устройство к виртуальной машине. - `AttachedToVirtualMachine` — устройство подключено к ВМ; - `Available` — устройство доступно для подключения; + - `NoFreeUSBIPPort` — устройство запрошено ВМ, но не может быть подключено, потому что на целевой ноде нет свободных USBIP-портов. В этом случае `Attached=False`; ### Подключение USB-устройства к ВМ diff --git a/images/virtualization-artifact/pkg/common/usb/availability.go b/images/virtualization-artifact/pkg/common/usb/availability.go index 6af0fb408a..27f006ca3b 100644 --- a/images/virtualization-artifact/pkg/common/usb/availability.go +++ b/images/virtualization-artifact/pkg/common/usb/availability.go @@ -21,6 +21,7 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" @@ -99,6 +100,9 @@ func countLocalAttachedUSBsOnNodeBySpeed(ctx context.Context, cl client.Client, if !ok { usbDevice = &v1alpha2.USBDevice{} if err := cl.Get(ctx, key, usbDevice); err != nil { + if apierrors.IsNotFound(err) { + continue + } return 0, err } usbCache[key] = usbDevice diff --git a/images/virtualization-artifact/pkg/common/usb/availability_test.go b/images/virtualization-artifact/pkg/common/usb/availability_test.go new file mode 100644 index 0000000000..30ab0855fa --- /dev/null +++ b/images/virtualization-artifact/pkg/common/usb/availability_test.go @@ -0,0 +1,131 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package usb + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apiruntime "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +var _ = Describe("availability helpers", func() { + newNode := func(name string, totalPorts, usedHSPorts, usedSSPorts string) *corev1.Node { + return &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: name, Annotations: map[string]string{ + annotations.AnnUSBIPTotalPorts: totalPorts, + annotations.AnnUSBIPHighSpeedHubUsedPorts: usedHSPorts, + annotations.AnnUSBIPSuperSpeedHubUsedPorts: usedSSPorts, + }}} + } + + newVM := func(name, nodeName string, statuses ...v1alpha2.USBDeviceStatusRef) *v1alpha2.VirtualMachine { + return &v1alpha2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + Status: v1alpha2.VirtualMachineStatus{ + Node: nodeName, + USBDevices: statuses, + }, + } + } + + newUSBDevice := func(name, nodeName string, speed int) *v1alpha2.USBDevice { + return &v1alpha2.USBDevice{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + Status: v1alpha2.USBDeviceStatus{ + NodeName: nodeName, + Attributes: v1alpha2.NodeUSBDeviceAttributes{ + Speed: speed, + }, + }, + } + } + + newClient := func(objects ...client.Object) client.Client { + scheme := apiruntime.NewScheme() + Expect(v1alpha2.AddToScheme(scheme)).To(Succeed()) + Expect(corev1.AddToScheme(scheme)).To(Succeed()) + + vmNodeObj, vmNodeField, vmNodeExtractValue := indexer.IndexVMByNode() + return fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objects...). + WithIndex(vmNodeObj, vmNodeField, vmNodeExtractValue). + Build() + } + + It("excludes local attached USB devices of the same speed class from used port accounting", func() { + cl := newClient( + newNode("node-1", "2", "1", "0"), + newVM("vm-1", "node-1", v1alpha2.USBDeviceStatusRef{Name: "usb-local", Attached: true}), + newUSBDevice("usb-local", "node-1", 480), + ) + + hasFree, err := CheckFreePortForRequestOnNodeExcludingLocalUSBs(context.Background(), cl, "node-1", 480, 1) + Expect(err).NotTo(HaveOccurred()) + Expect(hasFree).To(BeTrue()) + }) + + It("does not exclude local attached USB devices from another speed class", func() { + cl := newClient( + newNode("node-1", "2", "1", "0"), + newVM("vm-1", "node-1", v1alpha2.USBDeviceStatusRef{Name: "usb-local-ss", Attached: true}), + newUSBDevice("usb-local-ss", "node-1", 5000), + ) + + hasFree, err := CheckFreePortForRequestOnNodeExcludingLocalUSBs(context.Background(), cl, "node-1", 480, 1) + Expect(err).NotTo(HaveOccurred()) + Expect(hasFree).To(BeFalse()) + }) + + It("ignores stale VM status entries when the referenced USBDevice is missing", func() { + cl := newClient( + newNode("node-1", "2", "1", "0"), + newVM("vm-1", "node-1", v1alpha2.USBDeviceStatusRef{Name: "missing-usb", Attached: true}), + ) + + hasFree, err := CheckFreePortForRequestOnNodeExcludingLocalUSBs(context.Background(), cl, "node-1", 480, 1) + Expect(err).NotTo(HaveOccurred()) + Expect(hasFree).To(BeFalse()) + }) + + It("clamps effective used ports to zero when excluded local devices exceed node annotations", func() { + cl := newClient( + newNode("node-1", "2", "0", "0"), + newVM( + "vm-1", + "node-1", + v1alpha2.USBDeviceStatusRef{Name: "usb-local-1", Attached: true}, + v1alpha2.USBDeviceStatusRef{Name: "usb-local-2", Attached: true}, + ), + newUSBDevice("usb-local-1", "node-1", 480), + newUSBDevice("usb-local-2", "node-1", 480), + ) + + hasFree, err := CheckFreePortForRequestOnNodeExcludingLocalUSBs(context.Background(), cl, "node-1", 480, 1) + Expect(err).NotTo(HaveOccurred()) + Expect(hasFree).To(BeTrue()) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go index e8dc22a6bd..f71efc5309 100644 --- a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go +++ b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go @@ -227,15 +227,17 @@ func (h *LifecycleHandler) syncAttached(ctx context.Context, s state.USBDeviceSt if usbDevice.Status.NodeName != "" && usbDevice.Status.NodeName != vm.Status.Node { hasFreePort, err := usb.CheckFreePortOnNodeExcludingLocalUSBs(ctx, h.client, vm.Status.Node, usbDevice.Status.Attributes.Speed) if err != nil { - if !apierrors.IsNotFound(err) { - log.Error("failed to check free USBIP ports", "error", err, "device", usbDevice.Name, "node", vm.Status.Node) + if apierrors.IsNotFound(err) { + log.Debug("node not found while checking free USBIP ports", "device", usbDevice.Name, "node", vm.Status.Node) + continue } - continue + + return fmt.Errorf("failed to check free USBIP ports for USBDevice %s on node %s: %w", usbDevice.Name, vm.Status.Node, err) } if !hasFreePort { noFreePort = true - message = fmt.Sprintf("Device is attached to VirtualMachine %s/%s, but no free USBIP ports available on node %s for speed %d.", + message = fmt.Sprintf("Device is requested by VirtualMachine %s/%s, but no free USBIP ports are available on node %s for speed %d.", vm.Namespace, vm.Name, vm.Status.Node, usbDevice.Status.Attributes.Speed) break } @@ -244,7 +246,7 @@ func (h *LifecycleHandler) syncAttached(ctx context.Context, s state.USBDeviceSt if noFreePort { reason = usbdevicecondition.NoFreeUSBIPPort - status = metav1.ConditionTrue + status = metav1.ConditionFalse } else if len(attachedVMs) > 0 { reason = usbdevicecondition.AttachedToVirtualMachine status = metav1.ConditionTrue diff --git a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go index 20affbd206..78820ebc5a 100644 --- a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go +++ b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go @@ -182,7 +182,59 @@ var _ = Describe("LifecycleHandler", func() { attached := meta.FindStatusCondition(res.Changed().Status.Conditions, string(usbdevicecondition.AttachedType)) Expect(attached).NotTo(BeNil()) Expect(attached.Reason).To(Equal(string(usbdevicecondition.NoFreeUSBIPPort))) - Expect(attached.Status).To(Equal(metav1.ConditionTrue)) + Expect(attached.Status).To(Equal(metav1.ConditionFalse)) + Expect(attached.Message).To(ContainSubstring("requested by VirtualMachine")) + }) + + It("should return error when USBIP port availability check fails unexpectedly", func() { + usbDevice := &v1alpha2.USBDevice{ + ObjectMeta: metav1.ObjectMeta{Name: "usb-device-1", Namespace: "default", UID: "usb-uid-1"}, + Status: v1alpha2.USBDeviceStatus{Attributes: v1alpha2.NodeUSBDeviceAttributes{ + Name: "usb-device-1", + VendorID: "1234", + ProductID: "5678", + Speed: 480, + }}, + } + + nodeUSBDevice := &v1alpha2.NodeUSBDevice{ + ObjectMeta: metav1.ObjectMeta{Name: "usb-device-1"}, + Status: v1alpha2.NodeUSBDeviceStatus{ + Attributes: v1alpha2.NodeUSBDeviceAttributes{Name: "usb-device-1", VendorID: "1234", ProductID: "5678", Speed: 480}, + NodeName: "node-1", + Conditions: []metav1.Condition{{Type: string(nodeusbdevicecondition.ReadyType), Status: metav1.ConditionTrue, Reason: string(nodeusbdevicecondition.Ready), Message: "Node status"}}, + }, + } + + vm := &v1alpha2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{Name: "vm-1", Namespace: "default"}, + Spec: v1alpha2.VirtualMachineSpec{USBDevices: []v1alpha2.USBDeviceSpecRef{{Name: "usb-device-1"}}}, + Status: v1alpha2.VirtualMachineStatus{Node: "node-2", USBDevices: []v1alpha2.USBDeviceStatusRef{{Name: "usb-device-1", Attached: false}}}, + } + + badNode := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-2", Annotations: map[string]string{ + annotations.AnnUSBIPTotalPorts: "invalid", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "0", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0", + }}} + + vmObj, vmField, vmExtractValue := indexer.IndexVMByUSBDevice() + vmNodeObj, vmNodeField, vmNodeExtractValue := indexer.IndexVMByNode() + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(usbDevice, nodeUSBDevice, vm, badNode).WithIndex(vmObj, vmField, vmExtractValue).WithIndex(vmNodeObj, vmNodeField, vmNodeExtractValue).Build() + + res := reconciler.NewResource( + types.NamespacedName{Name: usbDevice.Name, Namespace: usbDevice.Namespace}, + cl, + func() *v1alpha2.USBDevice { return &v1alpha2.USBDevice{} }, + func(obj *v1alpha2.USBDevice) v1alpha2.USBDeviceStatus { return obj.Status }, + ) + Expect(res.Fetch(ctx)).To(Succeed()) + + st := state.New(cl, res) + h := NewLifecycleHandler(cl) + _, err := h.Handle(ctx, st) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to check free USBIP ports for USBDevice")) }) It("should skip ResourceClaimTemplate when attribute name is empty", func() { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go index e3683e7d06..8980850ce7 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler.go @@ -172,13 +172,13 @@ func (h *USBDeviceAttachHandler) Handle(ctx context.Context, s state.VirtualMach if _, exists := hostDeviceExistsByName[deviceName]; !exists && usbDevice.Status.NodeName != "" && vm.Status.Node != "" && usbDevice.Status.NodeName != vm.Status.Node { hasFreePort, err := usb.CheckFreePortOnNodeExcludingLocalUSBs(ctx, h.client, vm.Status.Node, usbDevice.Status.Attributes.Speed) if err != nil { - if !apierrors.IsNotFound(err) { - log.Error("failed to check free USBIP ports", "error", err, "device", deviceName, "node", vm.Status.Node) - } else { + if apierrors.IsNotFound(err) { log.Debug("node not found while checking free USBIP ports", "device", deviceName, "node", vm.Status.Node) + nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) + continue } - nextStatusRefs = append(nextStatusRefs, h.buildDetachedStatus(existingStatus, deviceName, isReady)) - continue + + return reconcile.Result{RequeueAfter: 5 * time.Second}, fmt.Errorf("failed to check free USBIP ports for device %s on node %s: %w", deviceName, vm.Status.Node, err) } if !hasFreePort { log.Info("no free USBIP ports available", "device", deviceName, "speed", usbDevice.Status.Attributes.Speed, "node", vm.Status.Node) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler_test.go index 2e181ef626..14de18a5cc 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/usb_device_attach_handler_test.go @@ -532,6 +532,32 @@ var _ = Describe("USBDeviceAttachHandler", func() { Expect(status.Ready).To(BeTrue()) }) + It("returns error when USBIP port availability check fails unexpectedly", func() { + vm := newVM(v1alpha2.MachineRunning) + vm.Status.Node = "node-2" + + badNode := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node-2", + Annotations: map[string]string{ + annotations.AnnUSBIPTotalPorts: "invalid", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "0", + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0", + }, + }, + } + + result, _, _, err := runHandle( + vm, + newUSBDevice(true, usbDeviceName, false), + newResourceClaimTemplate(), + badNode, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to check free USBIP ports for device")) + Expect(result.RequeueAfter).To(BeNumerically(">", 0)) + }) + It("skips port checks once KVVMI already reflects the host device", func() { vm := newVM(v1alpha2.MachineRunning, v1alpha2.USBDeviceStatusRef{ Name: usbDeviceName, From a6d9aea8b78d4317bc41e2ea1265747676fa5d1e Mon Sep 17 00:00:00 2001 From: Vladislav Panfilov <97229646+prismagod@users.noreply.github.com> Date: Fri, 27 Mar 2026 12:28:00 +0400 Subject: [PATCH 13/14] Update docs/USER_GUIDE.ru.md Signed-off-by: Vladislav Panfilov <97229646+prismagod@users.noreply.github.com> --- docs/USER_GUIDE.ru.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/USER_GUIDE.ru.md b/docs/USER_GUIDE.ru.md index 14ec735046..32aac2a285 100644 --- a/docs/USER_GUIDE.ru.md +++ b/docs/USER_GUIDE.ru.md @@ -3770,7 +3770,7 @@ logitech-webcam node-2 Logitech Webcam C920 ABC123456 False - **Attached**: Указывает, подключено ли устройство к виртуальной машине. - `AttachedToVirtualMachine` — устройство подключено к ВМ; - `Available` — устройство доступно для подключения; - - `NoFreeUSBIPPort` — устройство запрошено ВМ, но не может быть подключено, потому что на целевой ноде нет свободных USBIP-портов. В этом случае `Attached=False`; + - `NoFreeUSBIPPort` — устройство запрошено ВМ, но не может быть подключено, так как на целевом узле нет свободных USBIP-портов. В этом случае `Attached=False`. ### Подключение USB-устройства к ВМ From 783fb94f7df64d7bfc057149746c4c41461ceabf Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Mon, 30 Mar 2026 12:34:32 +0200 Subject: [PATCH 14/14] fix(usb): preserve attached status when ports are exhausted Signed-off-by: Daniil Antoshin --- .../pkg/common/usb/availability_test.go | 42 ++++++------ .../usbdevice/internal/handler/lifecycle.go | 18 +++-- .../internal/handler/lifecycle_test.go | 67 +++++++++++++++++++ .../validators/usb_devices_validator_test.go | 2 +- 4 files changed, 99 insertions(+), 30 deletions(-) diff --git a/images/virtualization-artifact/pkg/common/usb/availability_test.go b/images/virtualization-artifact/pkg/common/usb/availability_test.go index 30ab0855fa..465b643e26 100644 --- a/images/virtualization-artifact/pkg/common/usb/availability_test.go +++ b/images/virtualization-artifact/pkg/common/usb/availability_test.go @@ -33,29 +33,29 @@ import ( ) var _ = Describe("availability helpers", func() { - newNode := func(name string, totalPorts, usedHSPorts, usedSSPorts string) *corev1.Node { - return &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: name, Annotations: map[string]string{ - annotations.AnnUSBIPTotalPorts: totalPorts, + newNode := func(usedHSPorts string) *corev1.Node { + return &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-1", Annotations: map[string]string{ + annotations.AnnUSBIPTotalPorts: "2", annotations.AnnUSBIPHighSpeedHubUsedPorts: usedHSPorts, - annotations.AnnUSBIPSuperSpeedHubUsedPorts: usedSSPorts, + annotations.AnnUSBIPSuperSpeedHubUsedPorts: "0", }}} } - newVM := func(name, nodeName string, statuses ...v1alpha2.USBDeviceStatusRef) *v1alpha2.VirtualMachine { + newVM := func(statuses ...v1alpha2.USBDeviceStatusRef) *v1alpha2.VirtualMachine { return &v1alpha2.VirtualMachine{ - ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + ObjectMeta: metav1.ObjectMeta{Name: "vm-1", Namespace: "default"}, Status: v1alpha2.VirtualMachineStatus{ - Node: nodeName, + Node: "node-1", USBDevices: statuses, }, } } - newUSBDevice := func(name, nodeName string, speed int) *v1alpha2.USBDevice { + newUSBDevice := func(name string, speed int) *v1alpha2.USBDevice { return &v1alpha2.USBDevice{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, Status: v1alpha2.USBDeviceStatus{ - NodeName: nodeName, + NodeName: "node-1", Attributes: v1alpha2.NodeUSBDeviceAttributes{ Speed: speed, }, @@ -78,9 +78,9 @@ var _ = Describe("availability helpers", func() { It("excludes local attached USB devices of the same speed class from used port accounting", func() { cl := newClient( - newNode("node-1", "2", "1", "0"), - newVM("vm-1", "node-1", v1alpha2.USBDeviceStatusRef{Name: "usb-local", Attached: true}), - newUSBDevice("usb-local", "node-1", 480), + newNode("1"), + newVM(v1alpha2.USBDeviceStatusRef{Name: "usb-local", Attached: true}), + newUSBDevice("usb-local", 480), ) hasFree, err := CheckFreePortForRequestOnNodeExcludingLocalUSBs(context.Background(), cl, "node-1", 480, 1) @@ -90,9 +90,9 @@ var _ = Describe("availability helpers", func() { It("does not exclude local attached USB devices from another speed class", func() { cl := newClient( - newNode("node-1", "2", "1", "0"), - newVM("vm-1", "node-1", v1alpha2.USBDeviceStatusRef{Name: "usb-local-ss", Attached: true}), - newUSBDevice("usb-local-ss", "node-1", 5000), + newNode("1"), + newVM(v1alpha2.USBDeviceStatusRef{Name: "usb-local-ss", Attached: true}), + newUSBDevice("usb-local-ss", 5000), ) hasFree, err := CheckFreePortForRequestOnNodeExcludingLocalUSBs(context.Background(), cl, "node-1", 480, 1) @@ -102,8 +102,8 @@ var _ = Describe("availability helpers", func() { It("ignores stale VM status entries when the referenced USBDevice is missing", func() { cl := newClient( - newNode("node-1", "2", "1", "0"), - newVM("vm-1", "node-1", v1alpha2.USBDeviceStatusRef{Name: "missing-usb", Attached: true}), + newNode("1"), + newVM(v1alpha2.USBDeviceStatusRef{Name: "missing-usb", Attached: true}), ) hasFree, err := CheckFreePortForRequestOnNodeExcludingLocalUSBs(context.Background(), cl, "node-1", 480, 1) @@ -113,15 +113,13 @@ var _ = Describe("availability helpers", func() { It("clamps effective used ports to zero when excluded local devices exceed node annotations", func() { cl := newClient( - newNode("node-1", "2", "0", "0"), + newNode("0"), newVM( - "vm-1", - "node-1", v1alpha2.USBDeviceStatusRef{Name: "usb-local-1", Attached: true}, v1alpha2.USBDeviceStatusRef{Name: "usb-local-2", Attached: true}, ), - newUSBDevice("usb-local-1", "node-1", 480), - newUSBDevice("usb-local-2", "node-1", 480), + newUSBDevice("usb-local-1", 480), + newUSBDevice("usb-local-2", 480), ) hasFree, err := CheckFreePortForRequestOnNodeExcludingLocalUSBs(context.Background(), cl, "node-1", 480, 1) diff --git a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go index f71efc5309..b3c3c3d23e 100644 --- a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go +++ b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go @@ -222,6 +222,17 @@ func (h *LifecycleHandler) syncAttached(ctx context.Context, s state.USBDeviceSt return nil } + if len(attachedVMs) > 0 { + reason = usbdevicecondition.AttachedToVirtualMachine + status = metav1.ConditionTrue + message = fmt.Sprintf("Device is attached to %d VirtualMachines.", len(attachedVMs)) + if len(attachedVMs) == 1 { + message = fmt.Sprintf("Device is attached to VirtualMachine %s/%s.", attachedVMs[0].Namespace, attachedVMs[0].Name) + } + setAttachedCondition(current, &changed.Status.Conditions, status, reason, message) + return nil + } + noFreePort := false for _, vm := range referencingVMs { if usbDevice.Status.NodeName != "" && usbDevice.Status.NodeName != vm.Status.Node { @@ -247,13 +258,6 @@ func (h *LifecycleHandler) syncAttached(ctx context.Context, s state.USBDeviceSt if noFreePort { reason = usbdevicecondition.NoFreeUSBIPPort status = metav1.ConditionFalse - } else if len(attachedVMs) > 0 { - reason = usbdevicecondition.AttachedToVirtualMachine - status = metav1.ConditionTrue - message = fmt.Sprintf("Device is attached to %d VirtualMachines.", len(attachedVMs)) - if len(attachedVMs) == 1 { - message = fmt.Sprintf("Device is attached to VirtualMachine %s/%s.", attachedVMs[0].Namespace, attachedVMs[0].Name) - } } else { reason = usbdevicecondition.Available status = metav1.ConditionFalse diff --git a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go index 78820ebc5a..0bc5ff960c 100644 --- a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go +++ b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle_test.go @@ -186,6 +186,73 @@ var _ = Describe("LifecycleHandler", func() { Expect(attached.Message).To(ContainSubstring("requested by VirtualMachine")) }) + It("should keep AttachedToVirtualMachine when at least one VM already has device attached", func() { + usbDevice := &v1alpha2.USBDevice{ + ObjectMeta: metav1.ObjectMeta{Name: "usb-device-1", Namespace: "default", UID: "usb-uid-1"}, + Status: v1alpha2.USBDeviceStatus{Attributes: v1alpha2.NodeUSBDeviceAttributes{ + Name: "usb-device-1", + VendorID: "1234", + ProductID: "5678", + Speed: 480, + }}, + } + + nodeUSBDevice := &v1alpha2.NodeUSBDevice{ + ObjectMeta: metav1.ObjectMeta{Name: "usb-device-1"}, + Status: v1alpha2.NodeUSBDeviceStatus{ + Attributes: v1alpha2.NodeUSBDeviceAttributes{Name: "usb-device-1", VendorID: "1234", ProductID: "5678", Speed: 480}, + NodeName: "node-1", + Conditions: []metav1.Condition{{Type: string(nodeusbdevicecondition.ReadyType), Status: metav1.ConditionTrue, Reason: string(nodeusbdevicecondition.Ready), Message: "Node status"}}, + }, + } + + vmAttached := &v1alpha2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{Name: "vm-attached", Namespace: "default"}, + Spec: v1alpha2.VirtualMachineSpec{USBDevices: []v1alpha2.USBDeviceSpecRef{{Name: "usb-device-1"}}}, + Status: v1alpha2.VirtualMachineStatus{ + Node: "node-2", + USBDevices: []v1alpha2.USBDeviceStatusRef{{Name: "usb-device-1", Attached: true}}, + }, + } + + vmPending := &v1alpha2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{Name: "vm-pending", Namespace: "default"}, + Spec: v1alpha2.VirtualMachineSpec{USBDevices: []v1alpha2.USBDeviceSpecRef{{Name: "usb-device-1"}}}, + Status: v1alpha2.VirtualMachineStatus{ + Node: "node-2", + USBDevices: []v1alpha2.USBDeviceStatusRef{{Name: "usb-device-1", Attached: false}}, + }, + } + + node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-2", Annotations: map[string]string{ + annotations.AnnUSBIPTotalPorts: "2", + annotations.AnnUSBIPHighSpeedHubUsedPorts: "1", + }}} + + vmObj, vmField, vmExtractValue := indexer.IndexVMByUSBDevice() + vmNodeObj, vmNodeField, vmNodeExtractValue := indexer.IndexVMByNode() + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(usbDevice, nodeUSBDevice, vmAttached, vmPending, node).WithIndex(vmObj, vmField, vmExtractValue).WithIndex(vmNodeObj, vmNodeField, vmNodeExtractValue).Build() + + res := reconciler.NewResource( + types.NamespacedName{Name: usbDevice.Name, Namespace: usbDevice.Namespace}, + cl, + func() *v1alpha2.USBDevice { return &v1alpha2.USBDevice{} }, + func(obj *v1alpha2.USBDevice) v1alpha2.USBDeviceStatus { return obj.Status }, + ) + Expect(res.Fetch(ctx)).To(Succeed()) + + st := state.New(cl, res) + h := NewLifecycleHandler(cl) + _, err := h.Handle(ctx, st) + Expect(err).NotTo(HaveOccurred()) + + attached := meta.FindStatusCondition(res.Changed().Status.Conditions, string(usbdevicecondition.AttachedType)) + Expect(attached).NotTo(BeNil()) + Expect(attached.Reason).To(Equal(string(usbdevicecondition.AttachedToVirtualMachine))) + Expect(attached.Status).To(Equal(metav1.ConditionTrue)) + Expect(attached.Message).To(ContainSubstring("attached to")) + }) + It("should return error when USBIP port availability check fails unexpectedly", func() { usbDevice := &v1alpha2.USBDevice{ ObjectMeta: metav1.ObjectMeta{Name: "usb-device-1", Namespace: "default", UID: "usb-uid-1"}, diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator_test.go index d2e8007417..1b2f4c8a0d 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/usb_devices_validator_test.go @@ -123,7 +123,7 @@ func TestUSBDevicesValidatorValidateUpdateExcludesLocalUSBsFromPortAccounting(t objects := []client.Object{ oldVM, &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-1", Annotations: map[string]string{ - "usb.virtualization.deckhouse.io/usbip-total-ports": "2", + "usb.virtualization.deckhouse.io/usbip-total-ports": "2", "usb.virtualization.deckhouse.io/usbip-high-speed-hub-used-ports": "1", "usb.virtualization.deckhouse.io/usbip-super-speed-hub-used-ports": "0", }}},