From 379000b19233608a4429cb1157242b075cea0634 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Thu, 26 Mar 2026 17:06:44 +0200 Subject: [PATCH 1/7] fix(core): run firmware upgrade migration only for running vmi Signed-off-by: Daniil Antoshin --- .../internal/handler/lifecycle_test.go | 1 + .../internal/handler/firmware.go | 11 +++++++ .../internal/handler/firmware_test.go | 31 +++++++++++++++---- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle_test.go b/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle_test.go index 4b0495882e..e1da1ec8da 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle_test.go @@ -246,4 +246,5 @@ var _ = Describe("LifecycleHandler", func() { false, // targetMigrationEnabled ), ) + }) diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go index 8bbbb24156..174b3e3c3a 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go @@ -24,9 +24,12 @@ import ( appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + 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/object" + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/logger" @@ -66,6 +69,14 @@ func (h *FirmwareHandler) Handle(ctx context.Context, vm *v1alpha2.VirtualMachin log := logger.FromContext(ctx).With(logger.SlogHandler(firmwareHandler)) ctx = logger.ToContext(ctx, log) + kvvmi := &virtv1.VirtualMachineInstance{} + if err := h.client.Get(ctx, object.NamespacedName(vm), kvvmi); err != nil { + return reconcile.Result{}, client.IgnoreNotFound(err) + } + if kvvmi.Status.Phase != virtv1.Running { + return reconcile.Result{}, nil + } + if ready, err := h.isVirtControllerUpToDate(ctx); err != nil { return reconcile.Result{}, err } else if !ready { diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go index fa9ec5741a..69c50ea6a8 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go @@ -25,6 +25,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" @@ -67,6 +68,22 @@ var _ = Describe("TestFirmwareHandler", func() { return vm } + newKVVMI := func(phase virtv1.VirtualMachineInstancePhase) *virtv1.VirtualMachineInstance { + return &virtv1.VirtualMachineInstance{ + TypeMeta: metav1.TypeMeta{ + APIVersion: virtv1.GroupVersion.String(), + Kind: "VirtualMachineInstance", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Status: virtv1.VirtualMachineInstanceStatus{ + Phase: phase, + }, + } + } + newVirtController := func(ready bool, launcherImage string) *appsv1.Deployment { deploy := &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ @@ -99,8 +116,8 @@ var _ = Describe("TestFirmwareHandler", func() { } DescribeTable("FirmwareHandler should return serviceCompleteErr if migration executed", - func(vm *v1alpha2.VirtualMachine, deploy *appsv1.Deployment, needMigrate bool) { - fakeClient = setupEnvironment(vm, deploy) + func(vm *v1alpha2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, deploy *appsv1.Deployment, needMigrate bool) { + fakeClient = setupEnvironment(vm, kvvmi, deploy) mockMigration := &OneShotMigrationMock{ OnceMigrateFunc: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { @@ -118,9 +135,11 @@ var _ = Describe("TestFirmwareHandler", func() { Expect(err).NotTo(HaveOccurred()) } }, - Entry("Migration should be executed", newVM(true), newVirtController(true, firmwareImage), true), - Entry("Migration not should be executed", newVM(false), newVirtController(true, firmwareImage), false), - Entry("Migration not should be executed because virt-controller not ready", newVM(false), newVirtController(false, firmwareImage), false), - Entry("Migration not should be executed because virt-controller ready but has wrong image", newVM(false), newVirtController(true, "wrong-image"), false), + Entry("Migration should be executed", newVM(true), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), true), + Entry("Migration not should be executed", newVM(false), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), false), + Entry("Migration not should be executed because kvvmi is stopped", newVM(true), newKVVMI(virtv1.Succeeded), newVirtController(true, firmwareImage), false), + Entry("Migration not should be executed because kvvmi is pending", newVM(true), newKVVMI(virtv1.Pending), newVirtController(true, firmwareImage), false), + Entry("Migration not should be executed because virt-controller not ready", newVM(false), newKVVMI(virtv1.Running), newVirtController(false, firmwareImage), false), + Entry("Migration not should be executed because virt-controller ready but has wrong image", newVM(false), newKVVMI(virtv1.Running), newVirtController(true, "wrong-image"), false), ) }) From 414af6703a03750cc7283dad1219e558839091e1 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Thu, 26 Mar 2026 17:45:36 +0200 Subject: [PATCH 2/7] fix(workload-updater): handle missing KVVMI in firmware handler Signed-off-by: Daniil Antoshin --- .../internal/handler/firmware.go | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go index 174b3e3c3a..101a89ef36 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go @@ -23,14 +23,14 @@ import ( "time" appsv1 "k8s.io/api/apps/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" 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/object" - "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -39,7 +39,11 @@ import ( const firmwareHandler = "FirmwareHandler" -func NewFirmwareHandler(client client.Client, migration OneShotMigration, firmwareImage, namespace, virtControllerName string) *FirmwareHandler { +type firmwareMigration interface { + OnceMigrate(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) +} + +func NewFirmwareHandler(client client.Client, migration firmwareMigration, firmwareImage, namespace, virtControllerName string) *FirmwareHandler { return &FirmwareHandler{ client: client, oneShotMigration: migration, @@ -51,7 +55,7 @@ func NewFirmwareHandler(client client.Client, migration OneShotMigration, firmwa type FirmwareHandler struct { client client.Client - oneShotMigration OneShotMigration + oneShotMigration firmwareMigration firmwareImage string namespace string virtControllerName string @@ -70,10 +74,12 @@ func (h *FirmwareHandler) Handle(ctx context.Context, vm *v1alpha2.VirtualMachin ctx = logger.ToContext(ctx, log) kvvmi := &virtv1.VirtualMachineInstance{} - if err := h.client.Get(ctx, object.NamespacedName(vm), kvvmi); err != nil { - return reconcile.Result{}, client.IgnoreNotFound(err) + err := h.client.Get(ctx, object.NamespacedName(vm), kvvmi) + if err != nil && !k8serrors.IsNotFound(err) { + return reconcile.Result{}, err } - if kvvmi.Status.Phase != virtv1.Running { + + if !k8serrors.IsNotFound(err) && kvvmi.Status.Phase != virtv1.Running { return reconcile.Result{}, nil } From 400c809c3c1de8920d987ee56361a8af8bcbf0fd Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Thu, 26 Mar 2026 17:45:45 +0200 Subject: [PATCH 3/7] test(workload-updater): extend firmware handler branch coverage Signed-off-by: Daniil Antoshin --- .../internal/handler/firmware_test.go | 236 +++++++++++++++++- 1 file changed, 230 insertions(+), 6 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go index 69c50ea6a8..7ab3ffd6e7 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" "github.com/deckhouse/virtualization-controller/pkg/common/testutil" @@ -34,6 +35,14 @@ import ( "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" ) +type firmwareMigrationStub struct { + onceMigrate func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) +} + +func (s *firmwareMigrationStub) OnceMigrate(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { + return s.onceMigrate(ctx, vm, annotationKey, annotationExpectedValue) +} + var _ = Describe("TestFirmwareHandler", func() { const ( name = "vm-firmware" @@ -84,6 +93,14 @@ var _ = Describe("TestFirmwareHandler", func() { } } + setupFirmwareEnvironment := func(vm *v1alpha2.VirtualMachine, objs ...client.Object) client.Client { + allObjects := []client.Object{vm} + allObjects = append(allObjects, objs...) + fakeClient, err := testutil.NewFakeClientWithObjects(allObjects...) + Expect(err).NotTo(HaveOccurred()) + return fakeClient + } + newVirtController := func(ready bool, launcherImage string) *appsv1.Deployment { deploy := &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ @@ -107,9 +124,9 @@ var _ = Describe("TestFirmwareHandler", func() { }, }, } + deploy.Status.Replicas = 1 if ready { deploy.Status.ReadyReplicas = 1 - deploy.Status.Replicas = 1 } return deploy @@ -117,10 +134,14 @@ var _ = Describe("TestFirmwareHandler", func() { DescribeTable("FirmwareHandler should return serviceCompleteErr if migration executed", func(vm *v1alpha2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, deploy *appsv1.Deployment, needMigrate bool) { - fakeClient = setupEnvironment(vm, kvvmi, deploy) + objs := []client.Object{deploy} + if kvvmi != nil { + objs = append(objs, kvvmi) + } + fakeClient = setupFirmwareEnvironment(vm, objs...) - mockMigration := &OneShotMigrationMock{ - OnceMigrateFunc: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { + mockMigration := &firmwareMigrationStub{ + onceMigrate: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { return true, serviceCompleteErr }, } @@ -136,10 +157,213 @@ var _ = Describe("TestFirmwareHandler", func() { } }, Entry("Migration should be executed", newVM(true), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), true), + Entry("Migration should be executed when kvvmi is not found", newVM(true), nil, newVirtController(true, firmwareImage), true), Entry("Migration not should be executed", newVM(false), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), false), Entry("Migration not should be executed because kvvmi is stopped", newVM(true), newKVVMI(virtv1.Succeeded), newVirtController(true, firmwareImage), false), Entry("Migration not should be executed because kvvmi is pending", newVM(true), newKVVMI(virtv1.Pending), newVirtController(true, firmwareImage), false), - Entry("Migration not should be executed because virt-controller not ready", newVM(false), newKVVMI(virtv1.Running), newVirtController(false, firmwareImage), false), - Entry("Migration not should be executed because virt-controller ready but has wrong image", newVM(false), newKVVMI(virtv1.Running), newVirtController(true, "wrong-image"), false), + Entry("Migration not should be executed because virt-controller not ready", newVM(true), newKVVMI(virtv1.Running), newVirtController(false, firmwareImage), false), + Entry("Migration not should be executed because virt-controller ready but has wrong image", newVM(true), newKVVMI(virtv1.Running), newVirtController(true, "wrong-image"), false), ) + + It("should return error when kvvmi get returns non not-found error", func() { + vm := newVM(true) + deploy := newVirtController(true, firmwareImage) + kvvmiGetErr := errors.New("get kvvmi failed") + + interceptClient, err := testutil.NewFakeClientWithInterceptorWithObjects(interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*virtv1.VirtualMachineInstance); ok { + return kvvmiGetErr + } + return client.Get(ctx, key, obj, opts...) + }, + }, vm, deploy) + Expect(err).NotTo(HaveOccurred()) + + migrationCalled := false + h := NewFirmwareHandler(interceptClient, &firmwareMigrationStub{ + onceMigrate: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { + migrationCalled = true + return true, nil + }, + }, firmwareImage, virtControllerNamespace, virtControllerName) + + _, err = h.Handle(ctx, vm) + Expect(err).To(MatchError(kvvmiGetErr)) + Expect(migrationCalled).To(BeFalse()) + }) + + It("should not call migration when kvvmi is not running", func() { + vm := newVM(true) + kvvmi := newKVVMI(virtv1.Failed) + deploy := newVirtController(true, firmwareImage) + fakeClient = setupFirmwareEnvironment(vm, kvvmi, deploy) + + migrationCalled := false + h := NewFirmwareHandler(fakeClient, &firmwareMigrationStub{ + onceMigrate: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { + migrationCalled = true + return true, nil + }, + }, firmwareImage, virtControllerNamespace, virtControllerName) + + _, err := h.Handle(ctx, vm) + Expect(err).NotTo(HaveOccurred()) + Expect(migrationCalled).To(BeFalse()) + }) + + It("should continue processing when kvvmi is not found", func() { + vm := newVM(true) + deploy := newVirtController(true, firmwareImage) + fakeClient = setupFirmwareEnvironment(vm, deploy) + + migrationCalled := false + h := NewFirmwareHandler(fakeClient, &firmwareMigrationStub{ + onceMigrate: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { + migrationCalled = true + return false, nil + }, + }, firmwareImage, virtControllerNamespace, virtControllerName) + + _, err := h.Handle(ctx, vm) + Expect(err).NotTo(HaveOccurred()) + Expect(migrationCalled).To(BeTrue()) + }) + + It("should return nil when vm is nil", func() { + h := NewFirmwareHandler(nil, nil, firmwareImage, virtControllerNamespace, virtControllerName) + + _, err := h.Handle(ctx, nil) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should return nil when vm has deletion timestamp", func() { + vm := newVM(true) + now := metav1.Now() + vm.DeletionTimestamp = &now + + h := NewFirmwareHandler(nil, nil, firmwareImage, virtControllerNamespace, virtControllerName) + _, err := h.Handle(ctx, vm) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should return error when virt-controller deployment get fails", func() { + vm := newVM(true) + kvvmi := newKVVMI(virtv1.Running) + fakeClient = setupFirmwareEnvironment(vm, kvvmi) + + h := NewFirmwareHandler(fakeClient, &firmwareMigrationStub{ + onceMigrate: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { + return false, nil + }, + }, firmwareImage, virtControllerNamespace, virtControllerName) + + _, err := h.Handle(ctx, vm) + Expect(err).To(HaveOccurred()) + }) + + Describe("needUpdate", func() { + buildVMWithConditions := func(conditions []metav1.Condition) *v1alpha2.VirtualMachine { + vm := vmbuilder.NewEmpty(name, namespace) + vm.Status.Conditions = conditions + return vm + } + + DescribeTable("should evaluate FirmwareUpToDate condition", + func(vm *v1alpha2.VirtualMachine, expected bool) { + h := NewFirmwareHandler(nil, nil, firmwareImage, virtControllerNamespace, virtControllerName) + Expect(h.needUpdate(vm)).To(Equal(expected)) + }, + Entry("FirmwareUpToDate is False", buildVMWithConditions([]metav1.Condition{{ + Type: vmcondition.TypeFirmwareUpToDate.String(), + Status: metav1.ConditionFalse, + }}), true), + Entry("FirmwareUpToDate is True", buildVMWithConditions([]metav1.Condition{{ + Type: vmcondition.TypeFirmwareUpToDate.String(), + Status: metav1.ConditionTrue, + }}), false), + Entry("FirmwareUpToDate is absent", buildVMWithConditions([]metav1.Condition{{ + Type: "SomeOtherCondition", + Status: metav1.ConditionTrue, + }}), false), + ) + }) + + Describe("isVirtControllerUpToDate", func() { + It("should return error when deployment is not found", func() { + h := NewFirmwareHandler(setupFirmwareEnvironment(newVM(true)), nil, firmwareImage, virtControllerNamespace, virtControllerName) + + ready, err := h.isVirtControllerUpToDate(ctx) + Expect(err).To(HaveOccurred()) + Expect(ready).To(BeFalse()) + }) + + DescribeTable("should evaluate deployment state and launcher image", + func(deploy *appsv1.Deployment, expectedReady bool) { + h := NewFirmwareHandler(setupFirmwareEnvironment(newVM(true), deploy), nil, firmwareImage, virtControllerNamespace, virtControllerName) + + ready, err := h.isVirtControllerUpToDate(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(ready).To(Equal(expectedReady)) + }, + Entry("deployment is not ready", newVirtController(false, firmwareImage), false), + Entry("deployment is ready but image differs", newVirtController(true, "other-image"), false), + Entry("deployment is ready and image matches", newVirtController(true, firmwareImage), true), + ) + }) + + Describe("getVirtLauncherImage", func() { + It("should return empty when virt-controller container is absent", func() { + deploy := &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "sidecar", + Args: []string{"--launcher-image", firmwareImage}, + }}, + }, + }, + }, + } + Expect(getVirtLauncherImage(deploy)).To(BeEmpty()) + }) + + It("should return empty when launcher image argument is absent", func() { + deploy := &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "virt-controller", + Args: []string{"--some-flag", "value"}, + }}, + }, + }, + }, + } + Expect(getVirtLauncherImage(deploy)).To(BeEmpty()) + }) + + It("should return launcher image from --launcher-image=", func() { + deploy := &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "virt-controller", + Args: []string{"--launcher-image=" + firmwareImage}, + }}, + }, + }, + }, + } + Expect(getVirtLauncherImage(deploy)).To(Equal(firmwareImage)) + }) + }) + + It("should return firmware handler name", func() { + h := NewFirmwareHandler(nil, nil, firmwareImage, virtControllerNamespace, virtControllerName) + Expect(h.Name()).To(Equal(firmwareHandler)) + }) }) From dad8d79f4a4e84fa969777cf29888361cb82ce0a Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Thu, 26 Mar 2026 17:45:50 +0200 Subject: [PATCH 4/7] test(workload-updater): cover nodeplacement edge branches Signed-off-by: Daniil Antoshin --- .../internal/handler/lifecycle_test.go | 1 - .../internal/handler/nodeplacement_test.go | 88 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle_test.go b/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle_test.go index e1da1ec8da..4b0495882e 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle_test.go @@ -246,5 +246,4 @@ var _ = Describe("LifecycleHandler", func() { false, // targetMigrationEnabled ), ) - }) diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/nodeplacement_test.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/nodeplacement_test.go index c07dd7e2fd..ea1b41d3cd 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/nodeplacement_test.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/nodeplacement_test.go @@ -23,8 +23,12 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" "github.com/deckhouse/virtualization-controller/pkg/common/testutil" @@ -88,4 +92,88 @@ var _ = Describe("TestNodePlacementHandler", func() { Entry("Migration should be executed", true), Entry("Migration not should be executed", false), ) + + It("should return nil when vm is nil", func() { + h := NewNodePlacementHandler(nil, &OneShotMigrationMock{ + OnceMigrateFunc: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { + return false, nil + }, + }) + + _, err := h.Handle(ctx, nil) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should return nil when vm has deletion timestamp", func() { + vm := vmbuilder.NewEmpty(name, namespace) + now := metav1.Now() + vm.DeletionTimestamp = &now + + h := NewNodePlacementHandler(nil, &OneShotMigrationMock{ + OnceMigrateFunc: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { + return false, nil + }, + }) + + _, err := h.Handle(ctx, vm) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should return error when kvvmi get fails", func() { + vm := vmbuilder.NewEmpty(name, namespace) + getErr := errors.New("get kvvmi failed") + interceptClient, err := testutil.NewFakeClientWithInterceptorWithObjects(interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*virtv1.VirtualMachineInstance); ok { + return getErr + } + return client.Get(ctx, key, obj, opts...) + }, + }, vm) + Expect(err).NotTo(HaveOccurred()) + + h := NewNodePlacementHandler(interceptClient, &OneShotMigrationMock{ + OnceMigrateFunc: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { + return false, nil + }, + }) + + _, err = h.Handle(ctx, vm) + Expect(err).To(MatchError(getErr)) + }) + + It("should ignore not found error when kvvmi get returns not found", func() { + vm := vmbuilder.NewEmpty(name, namespace) + notFoundErr := k8serrors.NewNotFound(schema.GroupResource{Group: virtv1.GroupVersion.Group, Resource: "virtualmachineinstances"}, name) + interceptClient, err := testutil.NewFakeClientWithInterceptorWithObjects(interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*virtv1.VirtualMachineInstance); ok { + return notFoundErr + } + return client.Get(ctx, key, obj, opts...) + }, + }, vm) + Expect(err).NotTo(HaveOccurred()) + + h := NewNodePlacementHandler(interceptClient, &OneShotMigrationMock{ + OnceMigrateFunc: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { + return false, nil + }, + }) + + _, err = h.Handle(ctx, vm) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should return node placement handler name", func() { + h := NewNodePlacementHandler(nil, nil) + Expect(h.Name()).To(Equal(nodePlacementHandler)) + }) + + It("should return error for nil kvvmi in genNodePlacementSum", func() { + sum, err := genNodePlacementSum(nil) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("kvvmi is nil")) + Expect(sum).To(BeEmpty()) + }) }) From ff45f17276e7e42d9f52b0bb22b544413b224e80 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Fri, 27 Mar 2026 10:23:00 +0200 Subject: [PATCH 5/7] test(workload-updater): remove unparam warning in firmware tests Signed-off-by: Daniil Antoshin --- .../internal/handler/firmware_test.go | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go index 7ab3ffd6e7..9e42bcf699 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go @@ -64,19 +64,23 @@ var _ = Describe("TestFirmwareHandler", func() { fakeClient = nil }) - newVM := func(needMigrate bool) *v1alpha2.VirtualMachine { + newVM := func(firmwareUpToDateStatus metav1.ConditionStatus) *v1alpha2.VirtualMachine { vm := vmbuilder.NewEmpty(name, namespace) - status := metav1.ConditionTrue - if needMigrate { - status = metav1.ConditionFalse - } vm.Status.Conditions = append(vm.Status.Conditions, metav1.Condition{ Type: vmcondition.TypeFirmwareUpToDate.String(), - Status: status, + Status: firmwareUpToDateStatus, }) return vm } + newVMNeedMigrate := func() *v1alpha2.VirtualMachine { + return newVM(metav1.ConditionFalse) + } + + newVMUpToDate := func() *v1alpha2.VirtualMachine { + return newVM(metav1.ConditionTrue) + } + newKVVMI := func(phase virtv1.VirtualMachineInstancePhase) *virtv1.VirtualMachineInstance { return &virtv1.VirtualMachineInstance{ TypeMeta: metav1.TypeMeta{ @@ -156,17 +160,17 @@ var _ = Describe("TestFirmwareHandler", func() { Expect(err).NotTo(HaveOccurred()) } }, - Entry("Migration should be executed", newVM(true), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), true), - Entry("Migration should be executed when kvvmi is not found", newVM(true), nil, newVirtController(true, firmwareImage), true), - Entry("Migration not should be executed", newVM(false), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), false), - Entry("Migration not should be executed because kvvmi is stopped", newVM(true), newKVVMI(virtv1.Succeeded), newVirtController(true, firmwareImage), false), - Entry("Migration not should be executed because kvvmi is pending", newVM(true), newKVVMI(virtv1.Pending), newVirtController(true, firmwareImage), false), - Entry("Migration not should be executed because virt-controller not ready", newVM(true), newKVVMI(virtv1.Running), newVirtController(false, firmwareImage), false), - Entry("Migration not should be executed because virt-controller ready but has wrong image", newVM(true), newKVVMI(virtv1.Running), newVirtController(true, "wrong-image"), false), + Entry("Migration should be executed", newVMNeedMigrate(), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), true), + Entry("Migration should be executed when kvvmi is not found", newVMNeedMigrate(), nil, newVirtController(true, firmwareImage), true), + Entry("Migration not should be executed", newVMUpToDate(), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), false), + Entry("Migration not should be executed because kvvmi is stopped", newVMNeedMigrate(), newKVVMI(virtv1.Succeeded), newVirtController(true, firmwareImage), false), + Entry("Migration not should be executed because kvvmi is pending", newVMNeedMigrate(), newKVVMI(virtv1.Pending), newVirtController(true, firmwareImage), false), + Entry("Migration not should be executed because virt-controller not ready", newVMNeedMigrate(), newKVVMI(virtv1.Running), newVirtController(false, firmwareImage), false), + Entry("Migration not should be executed because virt-controller ready but has wrong image", newVMNeedMigrate(), newKVVMI(virtv1.Running), newVirtController(true, "wrong-image"), false), ) It("should return error when kvvmi get returns non not-found error", func() { - vm := newVM(true) + vm := newVMNeedMigrate() deploy := newVirtController(true, firmwareImage) kvvmiGetErr := errors.New("get kvvmi failed") @@ -194,7 +198,7 @@ var _ = Describe("TestFirmwareHandler", func() { }) It("should not call migration when kvvmi is not running", func() { - vm := newVM(true) + vm := newVMNeedMigrate() kvvmi := newKVVMI(virtv1.Failed) deploy := newVirtController(true, firmwareImage) fakeClient = setupFirmwareEnvironment(vm, kvvmi, deploy) @@ -213,7 +217,7 @@ var _ = Describe("TestFirmwareHandler", func() { }) It("should continue processing when kvvmi is not found", func() { - vm := newVM(true) + vm := newVMNeedMigrate() deploy := newVirtController(true, firmwareImage) fakeClient = setupFirmwareEnvironment(vm, deploy) @@ -238,7 +242,7 @@ var _ = Describe("TestFirmwareHandler", func() { }) It("should return nil when vm has deletion timestamp", func() { - vm := newVM(true) + vm := newVMNeedMigrate() now := metav1.Now() vm.DeletionTimestamp = &now @@ -248,7 +252,7 @@ var _ = Describe("TestFirmwareHandler", func() { }) It("should return error when virt-controller deployment get fails", func() { - vm := newVM(true) + vm := newVMNeedMigrate() kvvmi := newKVVMI(virtv1.Running) fakeClient = setupFirmwareEnvironment(vm, kvvmi) @@ -291,7 +295,7 @@ var _ = Describe("TestFirmwareHandler", func() { Describe("isVirtControllerUpToDate", func() { It("should return error when deployment is not found", func() { - h := NewFirmwareHandler(setupFirmwareEnvironment(newVM(true)), nil, firmwareImage, virtControllerNamespace, virtControllerName) + h := NewFirmwareHandler(setupFirmwareEnvironment(newVMNeedMigrate()), nil, firmwareImage, virtControllerNamespace, virtControllerName) ready, err := h.isVirtControllerUpToDate(ctx) Expect(err).To(HaveOccurred()) @@ -300,7 +304,7 @@ var _ = Describe("TestFirmwareHandler", func() { DescribeTable("should evaluate deployment state and launcher image", func(deploy *appsv1.Deployment, expectedReady bool) { - h := NewFirmwareHandler(setupFirmwareEnvironment(newVM(true), deploy), nil, firmwareImage, virtControllerNamespace, virtControllerName) + h := NewFirmwareHandler(setupFirmwareEnvironment(newVMNeedMigrate(), deploy), nil, firmwareImage, virtControllerNamespace, virtControllerName) ready, err := h.isVirtControllerUpToDate(ctx) Expect(err).NotTo(HaveOccurred()) From 246cd65fa5306aa8c37877e4d11ceff9ef942cef Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Fri, 27 Mar 2026 16:26:11 +0200 Subject: [PATCH 6/7] fix(core): gate firmware outdated state by running kvvmi Signed-off-by: Daniil Antoshin --- .../pkg/controller/vm/internal/firmware.go | 2 +- .../controller/vm/internal/firmware_test.go | 43 +++++++------- .../internal/handler/firmware.go | 13 ----- .../internal/handler/firmware_test.go | 56 +++++++------------ 4 files changed, 46 insertions(+), 68 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/firmware.go b/images/virtualization-artifact/pkg/controller/vm/internal/firmware.go index acba3d2dd4..df59a73637 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/firmware.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/firmware.go @@ -65,7 +65,7 @@ func (h *FirmwareHandler) syncFirmwareUpToDate(vm *v1alpha2.VirtualMachine, kvvm return } - upToDate := kvvmi == nil || kvvmi.Status.LauncherContainerImageVersion == "" || kvvmi.Status.LauncherContainerImageVersion == h.image + upToDate := kvvmi == nil || kvvmi.Status.Phase == virtv1.Succeeded || kvvmi.Status.Phase == virtv1.Failed || kvvmi.Status.LauncherContainerImageVersion == "" || kvvmi.Status.LauncherContainerImageVersion == h.image cb := conditions.NewConditionBuilder(vmcondition.TypeFirmwareUpToDate).Generation(vm.GetGeneration()) defer func() { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/firmware_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/firmware_test.go index f2da792651..e5ca86756f 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/firmware_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/firmware_test.go @@ -58,9 +58,10 @@ var _ = Describe("TestFirmwareHandler", func() { return vmbuilder.NewEmpty(name, namespace) } - newKVVMI := func(image string) *virtv1.VirtualMachineInstance { + newKVVMI := func(image string, phase virtv1.VirtualMachineInstancePhase) *virtv1.VirtualMachineInstance { kvvmi := newEmptyKVVMI(name, namespace) kvvmi.Status.LauncherContainerImageVersion = image + kvvmi.Status.Phase = phase return kvvmi } @@ -88,13 +89,15 @@ var _ = Describe("TestFirmwareHandler", func() { Expect(upToDate.Reason).To(Equal(expectedReason.String())) } }, - Entry("Should be up to date", newVM(), newKVVMI(expectedImage), metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate, false), + Entry("Should be up to date", newVM(), newKVVMI(expectedImage, virtv1.Running), metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate, false), Entry("Should be up to date because kvvmi is not exists", newVM(), nil, metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate, false), - Entry("Should be out of date 1", newVM(), newKVVMI("other-image-1"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), - Entry("Should be out of date 2", newVM(), newKVVMI("other-image-2"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), - Entry("Should be out of date 3", newVM(), newKVVMI("other-image-3"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), - Entry("Should be out of date 4", newVM(), newKVVMI("other-image-4"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), - Entry("Should be out of date 5", newVM(), newKVVMI("other-image-5"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), + Entry("Should be up to date because kvvmi is succeeded", newVM(), newKVVMI("other-image", virtv1.Succeeded), metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate, false), + Entry("Should be up to date because kvvmi is failed", newVM(), newKVVMI("other-image", virtv1.Failed), metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate, false), + Entry("Should be out of date 1", newVM(), newKVVMI("other-image-1", virtv1.Running), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), + Entry("Should be out of date 2", newVM(), newKVVMI("other-image-2", virtv1.Running), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), + Entry("Should be out of date 3", newVM(), newKVVMI("other-image-3", virtv1.Running), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), + Entry("Should be out of date 4", newVM(), newKVVMI("other-image-4", virtv1.Running), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), + Entry("Should be out of date 5", newVM(), newKVVMI("other-image-5", virtv1.Running), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), ) DescribeTable("Condition TypeFirmwareUpToDate should be in the expected state considering the VM phase", @@ -111,22 +114,24 @@ var _ = Describe("TestFirmwareHandler", func() { Expect(upToDate.Status).To(Equal(expectedStatus)) } }, - Entry("Running phase, condition should not be set", newVM(), v1alpha2.MachineRunning, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Running phase, condition should be set", newVM(), v1alpha2.MachineRunning, newKVVMI("other-image-1"), metav1.ConditionFalse, true), + Entry("Running phase, condition should not be set", newVM(), v1alpha2.MachineRunning, newKVVMI(expectedImage, virtv1.Running), metav1.ConditionUnknown, false), + Entry("Running phase, condition should be set", newVM(), v1alpha2.MachineRunning, newKVVMI("other-image-1", virtv1.Running), metav1.ConditionFalse, true), + Entry("Running phase, condition should not be set for succeeded kvvmi", newVM(), v1alpha2.MachineRunning, newKVVMI("other-image-1", virtv1.Succeeded), metav1.ConditionUnknown, false), + Entry("Running phase, condition should not be set for failed kvvmi", newVM(), v1alpha2.MachineRunning, newKVVMI("other-image-1", virtv1.Failed), metav1.ConditionUnknown, false), - Entry("Migrating phase, condition should not be set", newVM(), v1alpha2.MachineMigrating, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Migrating phase, condition should be set", newVM(), v1alpha2.MachineMigrating, newKVVMI("other-image-1"), metav1.ConditionFalse, true), + Entry("Migrating phase, condition should not be set", newVM(), v1alpha2.MachineMigrating, newKVVMI(expectedImage, virtv1.Running), metav1.ConditionUnknown, false), + Entry("Migrating phase, condition should be set", newVM(), v1alpha2.MachineMigrating, newKVVMI("other-image-1", virtv1.Running), metav1.ConditionFalse, true), - Entry("Stopping phase, condition should not be set", newVM(), v1alpha2.MachineStopping, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Stopping phase, condition should be set", newVM(), v1alpha2.MachineStopping, newKVVMI("other-image-1"), metav1.ConditionFalse, true), + Entry("Stopping phase, condition should not be set", newVM(), v1alpha2.MachineStopping, newKVVMI(expectedImage, virtv1.Running), metav1.ConditionUnknown, false), + Entry("Stopping phase, condition should be set", newVM(), v1alpha2.MachineStopping, newKVVMI("other-image-1", virtv1.Running), metav1.ConditionFalse, true), - Entry("Pending phase, condition should not be set", newVM(), v1alpha2.MachinePending, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Pending phase, condition should not be set", newVM(), v1alpha2.MachinePending, newKVVMI("other-image-1"), metav1.ConditionUnknown, false), + Entry("Pending phase, condition should not be set", newVM(), v1alpha2.MachinePending, newKVVMI(expectedImage, virtv1.Running), metav1.ConditionUnknown, false), + Entry("Pending phase, condition should not be set", newVM(), v1alpha2.MachinePending, newKVVMI("other-image-1", virtv1.Running), metav1.ConditionUnknown, false), - Entry("Starting phase, condition should not be set", newVM(), v1alpha2.MachineStarting, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Starting phase, condition should not be set", newVM(), v1alpha2.MachineStarting, newKVVMI("other-image-1"), metav1.ConditionUnknown, false), + Entry("Starting phase, condition should not be set", newVM(), v1alpha2.MachineStarting, newKVVMI(expectedImage, virtv1.Running), metav1.ConditionUnknown, false), + Entry("Starting phase, condition should not be set", newVM(), v1alpha2.MachineStarting, newKVVMI("other-image-1", virtv1.Running), metav1.ConditionUnknown, false), - Entry("Stopped phase, condition should not be set", newVM(), v1alpha2.MachineStopped, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Stopped phase, condition should not be set", newVM(), v1alpha2.MachineStopped, newKVVMI("other-image-1"), metav1.ConditionUnknown, false), + Entry("Stopped phase, condition should not be set", newVM(), v1alpha2.MachineStopped, newKVVMI(expectedImage, virtv1.Running), metav1.ConditionUnknown, false), + Entry("Stopped phase, condition should not be set", newVM(), v1alpha2.MachineStopped, newKVVMI("other-image-1", virtv1.Running), metav1.ConditionUnknown, false), ) }) diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go index 101a89ef36..a7654132c9 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware.go @@ -23,14 +23,11 @@ import ( "time" appsv1 "k8s.io/api/apps/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - 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/annotations" - "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -73,16 +70,6 @@ func (h *FirmwareHandler) Handle(ctx context.Context, vm *v1alpha2.VirtualMachin log := logger.FromContext(ctx).With(logger.SlogHandler(firmwareHandler)) ctx = logger.ToContext(ctx, log) - kvvmi := &virtv1.VirtualMachineInstance{} - err := h.client.Get(ctx, object.NamespacedName(vm), kvvmi) - if err != nil && !k8serrors.IsNotFound(err) { - return reconcile.Result{}, err - } - - if !k8serrors.IsNotFound(err) && kvvmi.Status.Phase != virtv1.Running { - return reconcile.Result{}, nil - } - if ready, err := h.isVirtControllerUpToDate(ctx); err != nil { return reconcile.Result{}, err } else if !ready { diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go index 9e42bcf699..1a78b74a97 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go @@ -27,7 +27,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" "github.com/deckhouse/virtualization-controller/pkg/common/testutil" @@ -163,41 +162,13 @@ var _ = Describe("TestFirmwareHandler", func() { Entry("Migration should be executed", newVMNeedMigrate(), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), true), Entry("Migration should be executed when kvvmi is not found", newVMNeedMigrate(), nil, newVirtController(true, firmwareImage), true), Entry("Migration not should be executed", newVMUpToDate(), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), false), - Entry("Migration not should be executed because kvvmi is stopped", newVMNeedMigrate(), newKVVMI(virtv1.Succeeded), newVirtController(true, firmwareImage), false), - Entry("Migration not should be executed because kvvmi is pending", newVMNeedMigrate(), newKVVMI(virtv1.Pending), newVirtController(true, firmwareImage), false), + Entry("Migration should be executed even when kvvmi is stopped", newVMNeedMigrate(), newKVVMI(virtv1.Succeeded), newVirtController(true, firmwareImage), true), + Entry("Migration should be executed even when kvvmi is pending", newVMNeedMigrate(), newKVVMI(virtv1.Pending), newVirtController(true, firmwareImage), true), Entry("Migration not should be executed because virt-controller not ready", newVMNeedMigrate(), newKVVMI(virtv1.Running), newVirtController(false, firmwareImage), false), Entry("Migration not should be executed because virt-controller ready but has wrong image", newVMNeedMigrate(), newKVVMI(virtv1.Running), newVirtController(true, "wrong-image"), false), ) - It("should return error when kvvmi get returns non not-found error", func() { - vm := newVMNeedMigrate() - deploy := newVirtController(true, firmwareImage) - kvvmiGetErr := errors.New("get kvvmi failed") - - interceptClient, err := testutil.NewFakeClientWithInterceptorWithObjects(interceptor.Funcs{ - Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { - if _, ok := obj.(*virtv1.VirtualMachineInstance); ok { - return kvvmiGetErr - } - return client.Get(ctx, key, obj, opts...) - }, - }, vm, deploy) - Expect(err).NotTo(HaveOccurred()) - - migrationCalled := false - h := NewFirmwareHandler(interceptClient, &firmwareMigrationStub{ - onceMigrate: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { - migrationCalled = true - return true, nil - }, - }, firmwareImage, virtControllerNamespace, virtControllerName) - - _, err = h.Handle(ctx, vm) - Expect(err).To(MatchError(kvvmiGetErr)) - Expect(migrationCalled).To(BeFalse()) - }) - - It("should not call migration when kvvmi is not running", func() { + It("should call migration when kvvmi is not running", func() { vm := newVMNeedMigrate() kvvmi := newKVVMI(virtv1.Failed) deploy := newVirtController(true, firmwareImage) @@ -213,7 +184,7 @@ var _ = Describe("TestFirmwareHandler", func() { _, err := h.Handle(ctx, vm) Expect(err).NotTo(HaveOccurred()) - Expect(migrationCalled).To(BeFalse()) + Expect(migrationCalled).To(BeTrue()) }) It("should continue processing when kvvmi is not found", func() { @@ -253,8 +224,7 @@ var _ = Describe("TestFirmwareHandler", func() { It("should return error when virt-controller deployment get fails", func() { vm := newVMNeedMigrate() - kvvmi := newKVVMI(virtv1.Running) - fakeClient = setupFirmwareEnvironment(vm, kvvmi) + fakeClient = setupFirmwareEnvironment(vm) h := NewFirmwareHandler(fakeClient, &firmwareMigrationStub{ onceMigrate: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { @@ -364,6 +334,22 @@ var _ = Describe("TestFirmwareHandler", func() { } Expect(getVirtLauncherImage(deploy)).To(Equal(firmwareImage)) }) + + It("should return launcher image from command arguments", func() { + deploy := &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "virt-controller", + Command: []string{"virt-controller", "--launcher-image", firmwareImage}, + }}, + }, + }, + }, + } + Expect(getVirtLauncherImage(deploy)).To(Equal(firmwareImage)) + }) }) It("should return firmware handler name", func() { From 6d098c7d1a72c8b56e494eb084d8d4fe72df3d9d Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Fri, 27 Mar 2026 17:25:09 +0200 Subject: [PATCH 7/7] test(workload-updater): remove obsolete kvvmi checks from firmware tests Signed-off-by: Daniil Antoshin --- .../internal/handler/firmware_test.go | 57 +++---------------- 1 file changed, 7 insertions(+), 50 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go index 1a78b74a97..32351fd2bf 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/firmware_test.go @@ -25,7 +25,6 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" @@ -80,22 +79,6 @@ var _ = Describe("TestFirmwareHandler", func() { return newVM(metav1.ConditionTrue) } - newKVVMI := func(phase virtv1.VirtualMachineInstancePhase) *virtv1.VirtualMachineInstance { - return &virtv1.VirtualMachineInstance{ - TypeMeta: metav1.TypeMeta{ - APIVersion: virtv1.GroupVersion.String(), - Kind: "VirtualMachineInstance", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - }, - Status: virtv1.VirtualMachineInstanceStatus{ - Phase: phase, - }, - } - } - setupFirmwareEnvironment := func(vm *v1alpha2.VirtualMachine, objs ...client.Object) client.Client { allObjects := []client.Object{vm} allObjects = append(allObjects, objs...) @@ -136,12 +119,8 @@ var _ = Describe("TestFirmwareHandler", func() { } DescribeTable("FirmwareHandler should return serviceCompleteErr if migration executed", - func(vm *v1alpha2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, deploy *appsv1.Deployment, needMigrate bool) { - objs := []client.Object{deploy} - if kvvmi != nil { - objs = append(objs, kvvmi) - } - fakeClient = setupFirmwareEnvironment(vm, objs...) + func(vm *v1alpha2.VirtualMachine, deploy *appsv1.Deployment, needMigrate bool) { + fakeClient = setupFirmwareEnvironment(vm, deploy) mockMigration := &firmwareMigrationStub{ onceMigrate: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { @@ -159,35 +138,13 @@ var _ = Describe("TestFirmwareHandler", func() { Expect(err).NotTo(HaveOccurred()) } }, - Entry("Migration should be executed", newVMNeedMigrate(), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), true), - Entry("Migration should be executed when kvvmi is not found", newVMNeedMigrate(), nil, newVirtController(true, firmwareImage), true), - Entry("Migration not should be executed", newVMUpToDate(), newKVVMI(virtv1.Running), newVirtController(true, firmwareImage), false), - Entry("Migration should be executed even when kvvmi is stopped", newVMNeedMigrate(), newKVVMI(virtv1.Succeeded), newVirtController(true, firmwareImage), true), - Entry("Migration should be executed even when kvvmi is pending", newVMNeedMigrate(), newKVVMI(virtv1.Pending), newVirtController(true, firmwareImage), true), - Entry("Migration not should be executed because virt-controller not ready", newVMNeedMigrate(), newKVVMI(virtv1.Running), newVirtController(false, firmwareImage), false), - Entry("Migration not should be executed because virt-controller ready but has wrong image", newVMNeedMigrate(), newKVVMI(virtv1.Running), newVirtController(true, "wrong-image"), false), + Entry("Migration should be executed", newVMNeedMigrate(), newVirtController(true, firmwareImage), true), + Entry("Migration not should be executed", newVMUpToDate(), newVirtController(true, firmwareImage), false), + Entry("Migration not should be executed because virt-controller not ready", newVMNeedMigrate(), newVirtController(false, firmwareImage), false), + Entry("Migration not should be executed because virt-controller ready but has wrong image", newVMNeedMigrate(), newVirtController(true, "wrong-image"), false), ) - It("should call migration when kvvmi is not running", func() { - vm := newVMNeedMigrate() - kvvmi := newKVVMI(virtv1.Failed) - deploy := newVirtController(true, firmwareImage) - fakeClient = setupFirmwareEnvironment(vm, kvvmi, deploy) - - migrationCalled := false - h := NewFirmwareHandler(fakeClient, &firmwareMigrationStub{ - onceMigrate: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { - migrationCalled = true - return true, nil - }, - }, firmwareImage, virtControllerNamespace, virtControllerName) - - _, err := h.Handle(ctx, vm) - Expect(err).NotTo(HaveOccurred()) - Expect(migrationCalled).To(BeTrue()) - }) - - It("should continue processing when kvvmi is not found", func() { + It("should call migration when update is needed and virt-controller is ready", func() { vm := newVMNeedMigrate() deploy := newVirtController(true, firmwareImage) fakeClient = setupFirmwareEnvironment(vm, deploy)