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..6d51681fc5 100644 --- a/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go +++ b/images/virtualization-artifact/pkg/controller/usbdevice/internal/handler/lifecycle.go @@ -18,6 +18,9 @@ package handler import ( "context" + "crypto/md5" + "encoding/hex" + "encoding/json" "fmt" "reflect" "strconv" @@ -153,19 +156,12 @@ func (h *LifecycleHandler) ensureResourceClaimTemplate(ctx context.Context, s st } if !apierrors.IsNotFound(err) { - if !reflect.DeepEqual(template.Spec, 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) } - 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 +176,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 +256,39 @@ 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, + } +} + +// 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. + 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..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 @@ -483,5 +483,95 @@ 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 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"}, + 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")) }) })