From 9af308537966b5c8f197dd6efb671d3d89d4520d Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Fri, 6 Mar 2026 02:00:14 +0200 Subject: [PATCH 1/2] fix(vmrestore): drop usb devices in best effort clone Signed-off-by: Daniil Antoshin --- .../service/restorer/restorers/vm_restorer.go | 13 +++ .../restorer/restorers/vm_restorer_test.go | 93 ++++++++++++++++--- 2 files changed, 93 insertions(+), 13 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer.go b/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer.go index 8455eba409..0429c9bd3c 100644 --- a/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer.go +++ b/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer.go @@ -160,6 +160,10 @@ func (v *VirtualMachineHandler) ValidateClone(ctx context.Context) error { return err } + if err := v.filterUSBDependencies(ctx); err != nil { + return err + } + return nil } @@ -316,6 +320,15 @@ func (v *VirtualMachineHandler) imageExists(ctx context.Context, ref v1alpha2.Bl return true, nil } +func (v *VirtualMachineHandler) filterUSBDependencies(_ context.Context) error { + if v.mode != v1alpha2.SnapshotOperationModeBestEffort || len(v.vm.Spec.USBDevices) == 0 { + return nil + } + + v.vm.Spec.USBDevices = nil + return nil +} + func (v *VirtualMachineHandler) Object() client.Object { return v.vm } diff --git a/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer_test.go b/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer_test.go index 35267c7eb7..ecd46f593e 100644 --- a/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer_test.go +++ b/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package restorer +package restorer_test import ( "context" @@ -27,6 +27,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/common/testutil" + restorer "github.com/deckhouse/virtualization-controller/pkg/controller/service/restorer/restorers" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -63,10 +64,16 @@ var _ = Describe("VirtualMachineRestorer", func() { vm v1alpha2.VirtualMachine vmbda1 v1alpha2.VirtualMachineBlockDeviceAttachment vmbda2 v1alpha2.VirtualMachineBlockDeviceAttachment - handler *VirtualMachineHandler + handler *restorer.VirtualMachineHandler fakeClient client.WithWatch ) + currentHandlerVM := func() *v1alpha2.VirtualMachine { + vmObj, ok := handler.Object().(*v1alpha2.VirtualMachine) + Expect(ok).To(BeTrue()) + return vmObj + } + BeforeEach(func() { ctx = context.Background() restoreUID = "restore-uid-1234" @@ -214,11 +221,11 @@ var _ = Describe("VirtualMachineRestorer", func() { Expect(err).ToNot(HaveOccurred()) Expect(fakeClient).ToNot(BeNil()) - handler = NewVirtualMachineHandler(fakeClient, vm, restoreUID, v1alpha2.SnapshotOperationModeStrict) + handler = restorer.NewVirtualMachineHandler(fakeClient, vm, restoreUID, v1alpha2.SnapshotOperationModeStrict) Expect(handler).ToNot(BeNil()) // Verify that restore annotation was added - Expect(handler.vm.Annotations[annotations.AnnVMOPRestore]).To(Equal(restoreUID)) + Expect(currentHandlerVM().Annotations[annotations.AnnVMOPRestore]).To(Equal(restoreUID)) err = handler.ValidateRestore(ctx) if args.failValidation { @@ -375,34 +382,34 @@ var _ = Describe("VirtualMachineRestorer", func() { fakeClient, err = testutil.NewFakeClientWithInterceptorWithObjects(intercept) Expect(err).ToNot(HaveOccurred()) - handler = NewVirtualMachineHandler(fakeClient, vm, restoreUID, v1alpha2.SnapshotOperationModeStrict) + handler = restorer.NewVirtualMachineHandler(fakeClient, vm, restoreUID, v1alpha2.SnapshotOperationModeStrict) }) It("should override VM name", func() { handler.Override(rules) - Expect(handler.vm.Name).To(Equal("new-vm-name")) + Expect(currentHandlerVM().Name).To(Equal("new-vm-name")) }) It("should override VirtualMachineIPAddress", func() { handler.Override(rules) - Expect(handler.vm.Spec.VirtualMachineIPAddress).To(Equal("new-test-ip")) + Expect(currentHandlerVM().Spec.VirtualMachineIPAddress).To(Equal("new-test-ip")) }) It("should override disk names in BlockDeviceRefs", func() { handler.Override(rules) - Expect(handler.vm.Spec.BlockDeviceRefs[0].Name).To(Equal("new-test-disk-1")) - Expect(handler.vm.Spec.BlockDeviceRefs[1].Name).To(Equal("test-disk-2")) // unchanged + Expect(currentHandlerVM().Spec.BlockDeviceRefs[0].Name).To(Equal("new-test-disk-1")) + Expect(currentHandlerVM().Spec.BlockDeviceRefs[1].Name).To(Equal("test-disk-2")) // unchanged }) It("should override Secret name in UserDataRef", func() { - handler.vm.Spec.Provisioning = &v1alpha2.Provisioning{ + currentHandlerVM().Spec.Provisioning = &v1alpha2.Provisioning{ UserDataRef: &v1alpha2.UserDataRef{ Kind: v1alpha2.UserDataRefKindSecret, Name: "test-secret", }, } handler.Override(rules) - Expect(handler.vm.Spec.Provisioning.UserDataRef.Name).To(Equal("new-test-secret")) + Expect(currentHandlerVM().Spec.Provisioning.UserDataRef.Name).To(Equal("new-test-secret")) }) It("should not override non-matching names", func() { @@ -416,9 +423,69 @@ var _ = Describe("VirtualMachineRestorer", func() { }, } - originalName := handler.vm.Name + originalName := currentHandlerVM().Name handler.Override(nonMatchingRules) - Expect(handler.vm.Name).To(Equal(originalName)) + Expect(currentHandlerVM().Name).To(Equal(originalName)) + }) + }) + + Describe("Clone BestEffort USB filtering", func() { + It("should remove all usb devices from cloned vm in best effort mode", func() { + cloneVM := vm.DeepCopy() + cloneVM.Name = "clone-vm" + cloneVM.Spec.USBDevices = []v1alpha2.USBDeviceSpecRef{{Name: "usb-busy"}, {Name: "usb-free"}} + + otherVM := vm.DeepCopy() + otherVM.Name = "other-vm" + otherVM.Spec.USBDevices = []v1alpha2.USBDeviceSpecRef{{Name: "usb-busy"}} + + var createdVM *v1alpha2.VirtualMachine + interceptClone := interceptor.Funcs{ + Create: func(_ context.Context, _ client.WithWatch, obj client.Object, _ ...client.CreateOption) error { + if vmObj, ok := obj.(*v1alpha2.VirtualMachine); ok && vmObj.Name == cloneVM.Name { + createdVM = vmObj.DeepCopy() + } + return nil + }, + } + + fakeClient, err = testutil.NewFakeClientWithInterceptorWithObjects(interceptClone, otherVM) + Expect(err).ToNot(HaveOccurred()) + + handler = restorer.NewVirtualMachineHandler(fakeClient, *cloneVM, restoreUID, v1alpha2.SnapshotOperationModeBestEffort) + err = handler.ProcessClone(ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(createdVM).ToNot(BeNil()) + Expect(createdVM.Spec.USBDevices).To(BeNil()) + }) + + It("should keep usb devices in strict mode", func() { + cloneVM := vm.DeepCopy() + cloneVM.Name = "clone-vm" + cloneVM.Spec.USBDevices = []v1alpha2.USBDeviceSpecRef{{Name: "usb-busy"}, {Name: "usb-free"}} + + otherVM := vm.DeepCopy() + otherVM.Name = "other-vm" + otherVM.Spec.USBDevices = []v1alpha2.USBDeviceSpecRef{{Name: "usb-busy"}} + + var createdVM *v1alpha2.VirtualMachine + interceptClone := interceptor.Funcs{ + Create: func(_ context.Context, _ client.WithWatch, obj client.Object, _ ...client.CreateOption) error { + if vmObj, ok := obj.(*v1alpha2.VirtualMachine); ok && vmObj.Name == cloneVM.Name { + createdVM = vmObj.DeepCopy() + } + return nil + }, + } + + fakeClient, err = testutil.NewFakeClientWithInterceptorWithObjects(interceptClone, otherVM) + Expect(err).ToNot(HaveOccurred()) + + handler = restorer.NewVirtualMachineHandler(fakeClient, *cloneVM, restoreUID, v1alpha2.SnapshotOperationModeStrict) + err = handler.ProcessClone(ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(createdVM).ToNot(BeNil()) + Expect(createdVM.Spec.USBDevices).To(Equal([]v1alpha2.USBDeviceSpecRef{{Name: "usb-busy"}, {Name: "usb-free"}})) }) }) }) From 7e7bb96c79430139a0330cbd9063870e51db5f2f Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Tue, 10 Mar 2026 11:44:51 +0200 Subject: [PATCH 2/2] test(api, vmrestore): use restorer package without alias Signed-off-by: Daniil Antoshin --- .../restorer/restorers/vm_restorer_test.go | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer_test.go b/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer_test.go index ecd46f593e..8e54f3076e 100644 --- a/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer_test.go +++ b/images/virtualization-artifact/pkg/controller/service/restorer/restorers/vm_restorer_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package restorer_test +package restorer import ( "context" @@ -27,7 +27,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/common/testutil" - restorer "github.com/deckhouse/virtualization-controller/pkg/controller/service/restorer/restorers" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -64,16 +63,10 @@ var _ = Describe("VirtualMachineRestorer", func() { vm v1alpha2.VirtualMachine vmbda1 v1alpha2.VirtualMachineBlockDeviceAttachment vmbda2 v1alpha2.VirtualMachineBlockDeviceAttachment - handler *restorer.VirtualMachineHandler + handler *VirtualMachineHandler fakeClient client.WithWatch ) - currentHandlerVM := func() *v1alpha2.VirtualMachine { - vmObj, ok := handler.Object().(*v1alpha2.VirtualMachine) - Expect(ok).To(BeTrue()) - return vmObj - } - BeforeEach(func() { ctx = context.Background() restoreUID = "restore-uid-1234" @@ -221,11 +214,11 @@ var _ = Describe("VirtualMachineRestorer", func() { Expect(err).ToNot(HaveOccurred()) Expect(fakeClient).ToNot(BeNil()) - handler = restorer.NewVirtualMachineHandler(fakeClient, vm, restoreUID, v1alpha2.SnapshotOperationModeStrict) + handler = NewVirtualMachineHandler(fakeClient, vm, restoreUID, v1alpha2.SnapshotOperationModeStrict) Expect(handler).ToNot(BeNil()) // Verify that restore annotation was added - Expect(currentHandlerVM().Annotations[annotations.AnnVMOPRestore]).To(Equal(restoreUID)) + Expect(handler.vm.Annotations[annotations.AnnVMOPRestore]).To(Equal(restoreUID)) err = handler.ValidateRestore(ctx) if args.failValidation { @@ -382,34 +375,34 @@ var _ = Describe("VirtualMachineRestorer", func() { fakeClient, err = testutil.NewFakeClientWithInterceptorWithObjects(intercept) Expect(err).ToNot(HaveOccurred()) - handler = restorer.NewVirtualMachineHandler(fakeClient, vm, restoreUID, v1alpha2.SnapshotOperationModeStrict) + handler = NewVirtualMachineHandler(fakeClient, vm, restoreUID, v1alpha2.SnapshotOperationModeStrict) }) It("should override VM name", func() { handler.Override(rules) - Expect(currentHandlerVM().Name).To(Equal("new-vm-name")) + Expect(handler.vm.Name).To(Equal("new-vm-name")) }) It("should override VirtualMachineIPAddress", func() { handler.Override(rules) - Expect(currentHandlerVM().Spec.VirtualMachineIPAddress).To(Equal("new-test-ip")) + Expect(handler.vm.Spec.VirtualMachineIPAddress).To(Equal("new-test-ip")) }) It("should override disk names in BlockDeviceRefs", func() { handler.Override(rules) - Expect(currentHandlerVM().Spec.BlockDeviceRefs[0].Name).To(Equal("new-test-disk-1")) - Expect(currentHandlerVM().Spec.BlockDeviceRefs[1].Name).To(Equal("test-disk-2")) // unchanged + Expect(handler.vm.Spec.BlockDeviceRefs[0].Name).To(Equal("new-test-disk-1")) + Expect(handler.vm.Spec.BlockDeviceRefs[1].Name).To(Equal("test-disk-2")) // unchanged }) It("should override Secret name in UserDataRef", func() { - currentHandlerVM().Spec.Provisioning = &v1alpha2.Provisioning{ + handler.vm.Spec.Provisioning = &v1alpha2.Provisioning{ UserDataRef: &v1alpha2.UserDataRef{ Kind: v1alpha2.UserDataRefKindSecret, Name: "test-secret", }, } handler.Override(rules) - Expect(currentHandlerVM().Spec.Provisioning.UserDataRef.Name).To(Equal("new-test-secret")) + Expect(handler.vm.Spec.Provisioning.UserDataRef.Name).To(Equal("new-test-secret")) }) It("should not override non-matching names", func() { @@ -423,9 +416,9 @@ var _ = Describe("VirtualMachineRestorer", func() { }, } - originalName := currentHandlerVM().Name + originalName := handler.vm.Name handler.Override(nonMatchingRules) - Expect(currentHandlerVM().Name).To(Equal(originalName)) + Expect(handler.vm.Name).To(Equal(originalName)) }) }) @@ -452,7 +445,7 @@ var _ = Describe("VirtualMachineRestorer", func() { fakeClient, err = testutil.NewFakeClientWithInterceptorWithObjects(interceptClone, otherVM) Expect(err).ToNot(HaveOccurred()) - handler = restorer.NewVirtualMachineHandler(fakeClient, *cloneVM, restoreUID, v1alpha2.SnapshotOperationModeBestEffort) + handler = NewVirtualMachineHandler(fakeClient, *cloneVM, restoreUID, v1alpha2.SnapshotOperationModeBestEffort) err = handler.ProcessClone(ctx) Expect(err).ToNot(HaveOccurred()) Expect(createdVM).ToNot(BeNil()) @@ -481,7 +474,7 @@ var _ = Describe("VirtualMachineRestorer", func() { fakeClient, err = testutil.NewFakeClientWithInterceptorWithObjects(interceptClone, otherVM) Expect(err).ToNot(HaveOccurred()) - handler = restorer.NewVirtualMachineHandler(fakeClient, *cloneVM, restoreUID, v1alpha2.SnapshotOperationModeStrict) + handler = NewVirtualMachineHandler(fakeClient, *cloneVM, restoreUID, v1alpha2.SnapshotOperationModeStrict) err = handler.ProcessClone(ctx) Expect(err).ToNot(HaveOccurred()) Expect(createdVM).ToNot(BeNil())