From db297fdfe0cef5a211c0fa3b8744a852a2f33a79 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Thu, 9 Jul 2026 15:41:41 +0200 Subject: [PATCH 1/2] fix(vm): track USB ResourceClaimTemplate spec via hash annotation Comparing the stored spec with the rendered one via DeepEqual breaks as soon as the API server defaults any field absent from the rendered spec: the comparison never converges and the handler deletes and recreates the template on every reconcile. The USB template spec is especially exposed because it embeds ObjectMeta with annotations. Stamp the rendered spec hash into an annotation and compare hashes instead, following the tolerations-hash pattern. A template without the annotation counts as outdated and is recreated once. Signed-off-by: Daniil Antoshin --- .../pkg/common/annotations/annotations.go | 3 ++ .../usbdevice/internal/handler/lifecycle.go | 46 +++++++++++-------- .../internal/handler/lifecycle_test.go | 42 +++++++++++++++++ 3 files changed, 73 insertions(+), 18 deletions(-) diff --git a/images/virtualization-artifact/pkg/common/annotations/annotations.go b/images/virtualization-artifact/pkg/common/annotations/annotations.go index 8e5b7f9d51..ec8ed305bb 100644 --- a/images/virtualization-artifact/pkg/common/annotations/annotations.go +++ b/images/virtualization-artifact/pkg/common/annotations/annotations.go @@ -248,6 +248,9 @@ const ( // AnnDVCRGarbageCollectionResult is an annotation on deployment dvcr with last garbage collection result JSON. AnnDVCRGarbageCollectionResult = AnnAPIGroupV + "/dvcr-garbage-collection-result" + // AnnUSBClaimSpecHash provides a const for annotation with hash of the rendered USB ResourceClaimTemplate spec. + AnnUSBClaimSpecHash = AnnAPIGroup + "/usb-claim-spec-hash" + // AnnUSBDeviceGroup is the annotation for device group in ResourceClaimTemplate. AnnUSBDeviceGroup = "usb.virtualization.deckhouse.io/device-group" // AnnUSBDeviceUser is the annotation for device user (owner) in ResourceClaimTemplate. 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 47d0431377..cd831254bb 100644 --- a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go +++ b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go @@ -18,8 +18,10 @@ package handler import ( "context" + "crypto/md5" + "encoding/hex" + "encoding/json" "fmt" - "reflect" "strconv" "strings" @@ -153,19 +155,14 @@ func (h *LifecycleHandler) ensureResourceClaimTemplate(ctx context.Context, s st } if !apierrors.IsNotFound(err) { - if !reflect.DeepEqual(template.Spec, desiredSpec) { + // The stored spec is not compared directly: API-server defaulting could make + // it permanently differ from the rendered spec and loop delete/recreate. + if template.Annotations[annotations.AnnUSBClaimSpecHash] != claimSpecHash(desiredSpec) { if err := h.client.Delete(ctx, template); err != nil && !apierrors.IsNotFound(err) { return fmt.Errorf("failed to delete outdated ResourceClaimTemplate: %w", err) } - template = &resourcev1.ResourceClaimTemplate{ - ObjectMeta: metav1.ObjectMeta{ - Name: templateName, - Namespace: usbDevice.Namespace, - OwnerReferences: []metav1.OwnerReference{service.MakeControllerOwnerReference(usbDevice)}, - }, - Spec: desiredSpec, - } + template = buildResourceClaimTemplate(usbDevice, templateName, desiredSpec) if err := h.client.Create(ctx, template); err != nil { if isAlreadyExistsResourceClaimTemplateError(err, templateName) { @@ -180,14 +177,7 @@ func (h *LifecycleHandler) ensureResourceClaimTemplate(ctx context.Context, s st return nil } - template = &resourcev1.ResourceClaimTemplate{ - ObjectMeta: metav1.ObjectMeta{ - Name: templateName, - Namespace: usbDevice.Namespace, - OwnerReferences: []metav1.OwnerReference{service.MakeControllerOwnerReference(usbDevice)}, - }, - Spec: desiredSpec, - } + template = buildResourceClaimTemplate(usbDevice, templateName, desiredSpec) if err := h.client.Create(ctx, template); err != nil { if isAlreadyExistsResourceClaimTemplateError(err, templateName) { @@ -267,6 +257,26 @@ func isAlreadyExistsResourceClaimTemplateError(err error, templateName string) b return strings.Contains(errText, "resourceclaimtemplates.resource.k8s.io") && strings.Contains(errText, templateName) && strings.Contains(errText, "already exists") } +func buildResourceClaimTemplate(usbDevice *v1alpha2.USBDevice, name string, spec resourcev1.ResourceClaimTemplateSpec) *resourcev1.ResourceClaimTemplate { + return &resourcev1.ResourceClaimTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: usbDevice.Namespace, + Annotations: map[string]string{annotations.AnnUSBClaimSpecHash: claimSpecHash(spec)}, + OwnerReferences: []metav1.OwnerReference{service.MakeControllerOwnerReference(usbDevice)}, + }, + Spec: spec, + } +} + +func claimSpecHash(spec resourcev1.ResourceClaimTemplateSpec) string { + // Marshalling a plain API struct cannot fail; on the impossible failure both + // sides hash the same empty payload, so the comparison still converges. + raw, _ := json.Marshal(&spec) + hash := md5.Sum(raw) + return hex.EncodeToString(hash[:]) +} + func buildResourceClaimTemplateSpec(usbDevice *v1alpha2.USBDevice) resourcev1.ResourceClaimTemplateSpec { attributes := usbDevice.Status.Attributes selectorDeviceName := attributes.Name 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 f096666bb4..9cc4cffee8 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 @@ -483,5 +483,47 @@ var _ = Describe("LifecycleHandler", func() { Expect(err).NotTo(HaveOccurred()) expr := updated.Spec.Spec.Devices.Requests[0].Exactly.Selectors[0].CEL.Expression Expect(expr).To(ContainSubstring(`device.attributes["virtualization-usb"].name == "usb-new-name"`)) + Expect(updated.Annotations).To(HaveKeyWithValue(annotations.AnnUSBClaimSpecHash, claimSpecHash(updated.Spec))) + }) + + It("should not recreate ResourceClaimTemplate when spec hash matches", func() { + usbDevice := &v1alpha2.USBDevice{ + ObjectMeta: metav1.ObjectMeta{Name: "usb-device-cr", Namespace: "default", UID: "usb-uid-1"}, + Status: v1alpha2.USBDeviceStatus{Attributes: v1alpha2.NodeUSBDeviceAttributes{Name: "usb-name", VendorID: "1234", ProductID: "5678"}}, + } + + nodeUSBDevice := &v1alpha2.NodeUSBDevice{ + ObjectMeta: metav1.ObjectMeta{Name: "usb-device-cr"}, + Status: v1alpha2.NodeUSBDeviceStatus{ + Attributes: v1alpha2.NodeUSBDeviceAttributes{Name: "usb-name", VendorID: "1234", ProductID: "5678"}, + NodeName: "node-1", + Conditions: []metav1.Condition{{Type: string(nodeusbdevicecondition.ReadyType), Status: metav1.ConditionTrue, Reason: string(nodeusbdevicecondition.Ready), Message: "Node status"}}, + }, + } + + template := buildResourceClaimTemplate(usbDevice, ResourceClaimTemplateName("usb-device-cr"), buildResourceClaimTemplateSpec(usbDevice)) + template.Labels = map[string]string{"keep": "me"} + + vmObj, vmField, vmExtractValue := indexer.IndexVMByUSBDevice() + 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}, + 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()) + + stored := &resourcev1.ResourceClaimTemplate{} + err = cl.Get(ctx, types.NamespacedName{Name: ResourceClaimTemplateName("usb-device-cr"), Namespace: "default"}, stored) + Expect(err).NotTo(HaveOccurred()) + Expect(stored.Labels).To(HaveKeyWithValue("keep", "me")) }) }) From 9a92dc622d840ebed15cdb08f46de2fb3cb7378f Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Thu, 9 Jul 2026 15:54:54 +0200 Subject: [PATCH 2/2] fix(vm): fall back to spec comparison for USB claim templates without hash Templates created before the hash annotation existed would otherwise be recreated once on controller upgrade. Compare their stored spec directly instead; they migrate to the hash on their next legitimate recreation. Signed-off-by: Daniil Antoshin --- .../usbdevice/internal/handler/lifecycle.go | 18 +++++-- .../internal/handler/lifecycle_test.go | 48 +++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) 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 cd831254bb..6d51681fc5 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 ( "encoding/hex" "encoding/json" "fmt" + "reflect" "strconv" "strings" @@ -155,9 +156,7 @@ func (h *LifecycleHandler) ensureResourceClaimTemplate(ctx context.Context, s st } if !apierrors.IsNotFound(err) { - // The stored spec is not compared directly: API-server defaulting could make - // it permanently differ from the rendered spec and loop delete/recreate. - if template.Annotations[annotations.AnnUSBClaimSpecHash] != claimSpecHash(desiredSpec) { + if !claimTemplateUpToDate(template, desiredSpec) { if err := h.client.Delete(ctx, template); err != nil && !apierrors.IsNotFound(err) { return fmt.Errorf("failed to delete outdated ResourceClaimTemplate: %w", err) } @@ -269,6 +268,19 @@ func buildResourceClaimTemplate(usbDevice *v1alpha2.USBDevice, name string, spec } } +// claimTemplateUpToDate prefers the spec-hash annotation over comparing the +// stored spec directly: API-server defaulting could make the stored spec +// permanently differ from the rendered one and loop delete/recreate. +// Templates created before the annotation existed fall back to DeepEqual and +// migrate to the hash on their next legitimate recreation. +func claimTemplateUpToDate(template *resourcev1.ResourceClaimTemplate, desiredSpec resourcev1.ResourceClaimTemplateSpec) bool { + storedHash, ok := template.Annotations[annotations.AnnUSBClaimSpecHash] + if !ok { + return reflect.DeepEqual(template.Spec, desiredSpec) + } + return storedHash == claimSpecHash(desiredSpec) +} + func claimSpecHash(spec resourcev1.ResourceClaimTemplateSpec) string { // Marshalling a plain API struct cannot fail; on the impossible failure both // sides hash the same empty payload, so the comparison still converges. 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 9cc4cffee8..32078122ae 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 @@ -486,6 +486,54 @@ var _ = Describe("LifecycleHandler", func() { Expect(updated.Annotations).To(HaveKeyWithValue(annotations.AnnUSBClaimSpecHash, claimSpecHash(updated.Spec))) }) + It("should not recreate ResourceClaimTemplate without hash annotation when spec matches", func() { + usbDevice := &v1alpha2.USBDevice{ + ObjectMeta: metav1.ObjectMeta{Name: "usb-device-cr", Namespace: "default", UID: "usb-uid-1"}, + Status: v1alpha2.USBDeviceStatus{Attributes: v1alpha2.NodeUSBDeviceAttributes{Name: "usb-name", VendorID: "1234", ProductID: "5678"}}, + } + + nodeUSBDevice := &v1alpha2.NodeUSBDevice{ + ObjectMeta: metav1.ObjectMeta{Name: "usb-device-cr"}, + Status: v1alpha2.NodeUSBDeviceStatus{ + Attributes: v1alpha2.NodeUSBDeviceAttributes{Name: "usb-name", VendorID: "1234", ProductID: "5678"}, + NodeName: "node-1", + Conditions: []metav1.Condition{{Type: string(nodeusbdevicecondition.ReadyType), Status: metav1.ConditionTrue, Reason: string(nodeusbdevicecondition.Ready), Message: "Node status"}}, + }, + } + + template := &resourcev1.ResourceClaimTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: ResourceClaimTemplateName("usb-device-cr"), + Namespace: "default", + Labels: map[string]string{"keep": "me"}, + }, + Spec: buildResourceClaimTemplateSpec(usbDevice), + } + + vmObj, vmField, vmExtractValue := indexer.IndexVMByUSBDevice() + 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}, + 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()) + + stored := &resourcev1.ResourceClaimTemplate{} + err = cl.Get(ctx, types.NamespacedName{Name: ResourceClaimTemplateName("usb-device-cr"), Namespace: "default"}, stored) + Expect(err).NotTo(HaveOccurred()) + Expect(stored.Labels).To(HaveKeyWithValue("keep", "me")) + Expect(stored.Annotations).NotTo(HaveKey(annotations.AnnUSBClaimSpecHash)) + }) + It("should not recreate ResourceClaimTemplate when spec hash matches", func() { usbDevice := &v1alpha2.USBDevice{ ObjectMeta: metav1.ObjectMeta{Name: "usb-device-cr", Namespace: "default", UID: "usb-uid-1"},