From 1e49a1f2e2df59742e2e43330ce36d48e50dc3bf Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Fri, 20 Mar 2026 14:13:10 +0200 Subject: [PATCH] fix(livemigration): require force=true for AlwaysForced policy Fix validation to properly reject migrations without force=true when VM has liveMigrationPolicy=AlwaysForced. Previously, UI migrations without explicit force flag were allowed, while CLI evict failed as expected. Changes: - Remove early return in isApplicableForLiveMigrationPolicy that skipped validation when Force=nil - Update CalculateEffectivePolicy to check vmop.Spec.Force instead of nil - Update tests to expect Failed phase for AlwaysForced+force=nil Signed-off-by: Daniil Antoshin --- .../controller/vmop/migration/internal/handler/lifecycle.go | 5 ----- .../vmop/migration/internal/handler/lifecycle_test.go | 4 ++-- images/virtualization-artifact/pkg/livemigration/policy.go | 4 ++-- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle.go b/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle.go index 390fb9069d..dad39694b6 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle.go +++ b/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle.go @@ -312,11 +312,6 @@ func (h LifecycleHandler) syncOperationComplete(ctx context.Context, vmop *v1alp } func (h LifecycleHandler) isApplicableForLiveMigrationPolicy(vmop *v1alpha2.VirtualMachineOperation, vm *v1alpha2.VirtualMachine) (string, bool) { - // No problems if force flag is not specified. - if vmop.Spec.Force == nil { - return "", true - } - effectivePolicy, autoConverge, err := livemigration.CalculateEffectivePolicy(*vm, vmop) if err != nil { msg := fmt.Sprintf("Operation is invalid: %v", err) 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 491817229c..8279c0f991 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 @@ -152,10 +152,10 @@ var _ = Describe("LifecycleHandler", func() { ), // AlwaysForced cases. - Entry("is ok for AlwaysForced and force=nil", + Entry("should become Failed for AlwaysForced and force=nil", newVMOPEvictPending(), v1alpha2.AlwaysForcedMigrationPolicy, - v1alpha2.VMOPPhasePending, + v1alpha2.VMOPPhaseFailed, ), Entry("should become Failed for AlwaysForced and force=false", newVMOPEvictPending(vmopbuilder.WithForce(ptr.To(false))), diff --git a/images/virtualization-artifact/pkg/livemigration/policy.go b/images/virtualization-artifact/pkg/livemigration/policy.go index 2bc5fe7fab..1dad3de38e 100644 --- a/images/virtualization-artifact/pkg/livemigration/policy.go +++ b/images/virtualization-artifact/pkg/livemigration/policy.go @@ -62,8 +62,8 @@ func CalculateEffectivePolicy(vm v1alpha2.VirtualMachine, vmop *v1alpha2.Virtual return effectivePolicy, *autoConvergePtr, fmt.Errorf("force=true is not applicable for VM liveMigrationPolicy %s", effectivePolicy) } case v1alpha2.AlwaysForcedMigrationPolicy: - if vmop.Spec.Force != nil && !*vmop.Spec.Force { - return effectivePolicy, *autoConvergePtr, fmt.Errorf("force=false is not applicable for VM liveMigrationPolicy %s", effectivePolicy) + if vmop.Spec.Force == nil || !*vmop.Spec.Force { + return effectivePolicy, *autoConvergePtr, fmt.Errorf("force=true is required for VM liveMigrationPolicy %s", effectivePolicy) } } }