From 7c1ef88e7968365f7e4ea3b3480a9e5f51c56d89 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Tue, 12 May 2026 14:09:26 +0200 Subject: [PATCH 1/6] feat(vmop): add operation supersede flow Signed-off-by: Daniil Antoshin --- api/core/v1alpha2/vmopcondition/condition.go | 3 + .../powerstate/internal/service/restart.go | 4 + .../vmop/powerstate/internal/service/stop.go | 4 + .../pkg/controller/vmop/service/service.go | 138 +++++++++++++++--- .../controller/vmop/service/service_test.go | 103 +++++++++++++ .../controller/vmop/supersede/supersede.go | 48 ++++++ .../vmop/supersede/supersede_test.go | 81 ++++++++++ .../pkg/controller/vmop/vmop_webhook.go | 27 ++++ 8 files changed, 391 insertions(+), 17 deletions(-) create mode 100644 images/virtualization-artifact/pkg/controller/vmop/service/service_test.go create mode 100644 images/virtualization-artifact/pkg/controller/vmop/supersede/supersede.go create mode 100644 images/virtualization-artifact/pkg/controller/vmop/supersede/supersede_test.go diff --git a/api/core/v1alpha2/vmopcondition/condition.go b/api/core/v1alpha2/vmopcondition/condition.go index 9326c306ec..9234b68ced 100644 --- a/api/core/v1alpha2/vmopcondition/condition.go +++ b/api/core/v1alpha2/vmopcondition/condition.go @@ -59,6 +59,9 @@ const ( // ReasonNotReadyToBeExecuted is a ReasonCompleted indicating that the operation is not ready to be executed. ReasonNotReadyToBeExecuted ReasonCompleted = "NotReadyToBeExecuted" + // ReasonSuperseded is a ReasonCompleted indicating that the operation has been superseded by another operation. + ReasonSuperseded ReasonCompleted = "Superseded" + // ReasonRestartInProgress is a ReasonCompleted indicating that the restart signal has been sent and restart is in progress. ReasonRestartInProgress ReasonCompleted = "RestartInProgress" diff --git a/images/virtualization-artifact/pkg/controller/vmop/powerstate/internal/service/restart.go b/images/virtualization-artifact/pkg/controller/vmop/powerstate/internal/service/restart.go index 60578cfabb..0d205a6b81 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/powerstate/internal/service/restart.go +++ b/images/virtualization-artifact/pkg/controller/vmop/powerstate/internal/service/restart.go @@ -19,6 +19,7 @@ package service import ( "context" + apierrors "k8s.io/apimachinery/pkg/api/errors" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -53,6 +54,9 @@ func (o RestartOperation) Execute(ctx context.Context) error { if o.vmop.Spec.Force != nil && *o.vmop.Spec.Force { kvvmi := &virtv1.VirtualMachineInstance{} err = o.client.Get(ctx, key, kvvmi) + if apierrors.IsNotFound(err) { + return kvvmutil.AddStartAnnotation(ctx, o.client, kvvm) + } if err != nil { return err } diff --git a/images/virtualization-artifact/pkg/controller/vmop/powerstate/internal/service/stop.go b/images/virtualization-artifact/pkg/controller/vmop/powerstate/internal/service/stop.go index db15bce3eb..333a46aede 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/powerstate/internal/service/stop.go +++ b/images/virtualization-artifact/pkg/controller/vmop/powerstate/internal/service/stop.go @@ -19,6 +19,7 @@ package service import ( "context" + apierrors "k8s.io/apimachinery/pkg/api/errors" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -42,6 +43,9 @@ type StopOperation struct { func (o StopOperation) Execute(ctx context.Context) error { kvvmi := &virtv1.VirtualMachineInstance{} err := o.client.Get(ctx, virtualMachineKeyByVmop(o.vmop), kvvmi) + if apierrors.IsNotFound(err) { + return nil + } if err != nil { return err } diff --git a/images/virtualization-artifact/pkg/controller/vmop/service/service.go b/images/virtualization-artifact/pkg/controller/vmop/service/service.go index 66a3d9d714..da8224dbb3 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/service/service.go +++ b/images/virtualization-artifact/pkg/controller/vmop/service/service.go @@ -19,18 +19,22 @@ package service import ( "context" "fmt" + "sort" "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/ptr" + "k8s.io/client-go/util/retry" + virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + kvvmutil "github.com/deckhouse/virtualization-controller/pkg/common/kvvm" "github.com/deckhouse/virtualization-controller/pkg/common/object" commonvmop "github.com/deckhouse/virtualization-controller/pkg/common/vmop" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/supersede" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" @@ -49,32 +53,54 @@ func NewBaseVMOPService(client client.Client, recorder eventrecord.EventRecorder } func (s *BaseVMOPService) ShouldExecuteOrSetFailedPhase(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (bool, error) { - should, err := s.ShouldExecute(ctx, vmop) + return s.ShouldExecuteOrSupersedeOrSetFailedPhase(ctx, vmop) +} + +func (s *BaseVMOPService) ShouldExecuteOrSupersedeOrSetFailedPhase(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (bool, error) { + blockers, err := s.findOlderActiveVMOPs(ctx, vmop) if err != nil { return false, err } - if should { + if len(blockers) == 0 { return true, nil } - vmop.Status.Phase = v1alpha2.VMOPPhaseFailed - conditions.SetCondition( - conditions.NewConditionBuilder(vmopcondition.TypeCompleted). - Generation(vmop.GetGeneration()). - Reason(vmopcondition.ReasonNotReadyToBeExecuted). - Message("VMOP cannot be executed now. Previously created operation should finish first."). - Status(metav1.ConditionFalse), - &vmop.Status.Conditions) - return false, nil + for i := range blockers { + oldVMOP := &blockers[i] + if !supersede.CanSupersede(oldVMOP, vmop) { + s.setNotReadyToBeExecuted(vmop) + return false, nil + } + } + + for i := range blockers { + oldVMOP := &blockers[i] + if err := s.cleanupSupersededOperation(ctx, oldVMOP); err != nil { + return false, err + } + if err := s.markSuperseded(ctx, oldVMOP, vmop); err != nil { + return false, err + } + } + + return true, nil } func (s *BaseVMOPService) ShouldExecute(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (bool, error) { - var vmopList v1alpha2.VirtualMachineOperationList - err := s.client.List(ctx, &vmopList, client.InNamespace(vmop.GetNamespace())) + blockers, err := s.findOlderActiveVMOPs(ctx, vmop) if err != nil { return false, err } + return len(blockers) == 0, nil +} + +func (s *BaseVMOPService) findOlderActiveVMOPs(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) ([]v1alpha2.VirtualMachineOperation, error) { + var vmopList v1alpha2.VirtualMachineOperationList + if err := s.client.List(ctx, &vmopList, client.InNamespace(vmop.GetNamespace())); err != nil { + return nil, err + } + blockers := make([]v1alpha2.VirtualMachineOperation, 0) for _, other := range vmopList.Items { if other.Spec.VirtualMachine != vmop.Spec.VirtualMachine { continue @@ -85,12 +111,90 @@ func (s *BaseVMOPService) ShouldExecute(ctx context.Context, vmop *v1alpha2.Virt if other.GetUID() == vmop.GetUID() { continue } - if other.CreationTimestamp.Before(ptr.To(vmop.CreationTimestamp)) { - return false, nil + if isOlderVMOP(&other, vmop) { + blockers = append(blockers, other) } } - return true, nil + sort.SliceStable(blockers, func(i, j int) bool { + return isOlderVMOP(&blockers[i], &blockers[j]) + }) + + return blockers, nil +} + +func isOlderVMOP(a, b *v1alpha2.VirtualMachineOperation) bool { + if !a.CreationTimestamp.Equal(&b.CreationTimestamp) { + return a.CreationTimestamp.Before(&b.CreationTimestamp) + } + return a.GetName() < b.GetName() +} + +func (s *BaseVMOPService) setNotReadyToBeExecuted(vmop *v1alpha2.VirtualMachineOperation) { + vmop.Status.Phase = v1alpha2.VMOPPhaseFailed + conditions.SetCondition( + conditions.NewConditionBuilder(vmopcondition.TypeCompleted). + Generation(vmop.GetGeneration()). + Reason(vmopcondition.ReasonNotReadyToBeExecuted). + Message("VMOP cannot be executed now. Previously created operation should finish first."). + Status(metav1.ConditionFalse), + &vmop.Status.Conditions) +} + +func (s *BaseVMOPService) cleanupSupersededOperation(ctx context.Context, oldVMOP *v1alpha2.VirtualMachineOperation) error { + key := types.NamespacedName{Name: oldVMOP.Spec.VirtualMachine, Namespace: oldVMOP.Namespace} + + switch oldVMOP.Spec.Type { + case v1alpha2.VMOPTypeStart: + kvvm := &virtv1.VirtualMachine{} + if err := s.client.Get(ctx, key, kvvm); err != nil { + return client.IgnoreNotFound(err) + } + return kvvmutil.RemoveStartAnnotation(ctx, s.client, kvvm) + case v1alpha2.VMOPTypeRestart: + kvvm := &virtv1.VirtualMachine{} + if err := s.client.Get(ctx, key, kvvm); err != nil { + return client.IgnoreNotFound(err) + } + if err := kvvmutil.RemoveRestartAnnotation(ctx, s.client, kvvm); err != nil { + return err + } + return kvvmutil.RemoveStartAnnotation(ctx, s.client, kvvm) + case v1alpha2.VMOPTypeMigrate, v1alpha2.VMOPTypeEvict: + mig := &virtv1.VirtualMachineInstanceMigration{} + if err := s.client.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("vmop-%s", oldVMOP.Name), Namespace: oldVMOP.Namespace}, mig); err != nil { + return client.IgnoreNotFound(err) + } + return client.IgnoreNotFound(s.client.Delete(ctx, mig)) + case v1alpha2.VMOPTypeStop: + return nil + default: + return nil + } +} + +func (s *BaseVMOPService) markSuperseded(ctx context.Context, oldVMOP, newVMOP *v1alpha2.VirtualMachineOperation) error { + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + current := &v1alpha2.VirtualMachineOperation{} + if err := s.client.Get(ctx, types.NamespacedName{Name: oldVMOP.Name, Namespace: oldVMOP.Namespace}, current); err != nil { + return client.IgnoreNotFound(err) + } + if commonvmop.IsFinished(current) { + return nil + } + + base := current.DeepCopy() + current.Status.Phase = v1alpha2.VMOPPhaseCompleted + conditions.SetCondition( + conditions.NewConditionBuilder(vmopcondition.TypeCompleted). + Generation(current.GetGeneration()). + Reason(vmopcondition.ReasonSuperseded). + Message(fmt.Sprintf("Superseded by %s with type %s", newVMOP.Name, newVMOP.Spec.Type)). + Status(metav1.ConditionTrue), + ¤t.Status.Conditions) + + return s.client.Status().Patch(ctx, current, client.MergeFrom(base)) + }) } func (s *BaseVMOPService) Init(vmop *v1alpha2.VirtualMachineOperation) { diff --git a/images/virtualization-artifact/pkg/controller/vmop/service/service_test.go b/images/virtualization-artifact/pkg/controller/vmop/service/service_test.go new file mode 100644 index 0000000000..f226910ffd --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/service/service_test.go @@ -0,0 +1,103 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "testing" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + + "github.com/deckhouse/virtualization-controller/pkg/common/testutil" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/eventrecord" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" +) + +func TestService(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "VMOP Service Suite") +} + +var _ = Describe("BaseVMOPService", func() { + Describe("ShouldExecuteOrSupersedeOrSetFailedPhase", func() { + It("marks an allowed older operation as superseded", func(ctx SpecContext) { + oldVMOP := newVMOP("old-start", v1alpha2.VMOPTypeStart, false, time.Now().Add(-time.Minute)) + newVMOP := newVMOP("new-stop", v1alpha2.VMOPTypeStop, false, time.Now()) + + client, err := testutil.NewFakeClientWithObjects(oldVMOP) + Expect(err).NotTo(HaveOccurred()) + + svc := NewBaseVMOPService(client, &eventrecord.EventRecorderLoggerMock{}) + should, err := svc.ShouldExecuteOrSupersedeOrSetFailedPhase(ctx, newVMOP) + Expect(err).NotTo(HaveOccurred()) + Expect(should).To(BeTrue()) + + changed := &v1alpha2.VirtualMachineOperation{} + Expect(client.Get(ctx, types.NamespacedName{Name: oldVMOP.Name, Namespace: oldVMOP.Namespace}, changed)).To(Succeed()) + Expect(changed.Status.Phase).To(Equal(v1alpha2.VMOPPhaseCompleted)) + + completed, found := conditions.GetCondition(vmopcondition.TypeCompleted, changed.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(completed.Status).To(Equal(metav1.ConditionTrue)) + Expect(completed.Reason).To(Equal(vmopcondition.ReasonSuperseded.String())) + }) + + It("fails a forbidden newer operation", func(ctx SpecContext) { + oldVMOP := newVMOP("old-stop", v1alpha2.VMOPTypeStop, false, time.Now().Add(-time.Minute)) + newVMOP := newVMOP("new-start", v1alpha2.VMOPTypeStart, false, time.Now()) + + client, err := testutil.NewFakeClientWithObjects(oldVMOP) + Expect(err).NotTo(HaveOccurred()) + + svc := NewBaseVMOPService(client, &eventrecord.EventRecorderLoggerMock{}) + should, err := svc.ShouldExecuteOrSupersedeOrSetFailedPhase(ctx, newVMOP) + Expect(err).NotTo(HaveOccurred()) + Expect(should).To(BeFalse()) + Expect(newVMOP.Status.Phase).To(Equal(v1alpha2.VMOPPhaseFailed)) + + completed, found := conditions.GetCondition(vmopcondition.TypeCompleted, newVMOP.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(completed.Status).To(Equal(metav1.ConditionFalse)) + Expect(completed.Reason).To(Equal(vmopcondition.ReasonNotReadyToBeExecuted.String())) + }) + }) +}) + +func newVMOP(name string, vmopType v1alpha2.VMOPType, force bool, createdAt time.Time) *v1alpha2.VirtualMachineOperation { + return &v1alpha2.VirtualMachineOperation{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + UID: types.UID(name), + CreationTimestamp: metav1.NewTime(createdAt), + }, + Spec: v1alpha2.VirtualMachineOperationSpec{ + Type: vmopType, + VirtualMachine: "test-vm", + Force: ptr.To(force), + }, + Status: v1alpha2.VirtualMachineOperationStatus{ + Phase: v1alpha2.VMOPPhaseInProgress, + }, + } +} diff --git a/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede.go b/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede.go new file mode 100644 index 0000000000..9591d81add --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede.go @@ -0,0 +1,48 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package supersede + +import ( + "k8s.io/utils/ptr" + + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +func CanSupersede(oldVMOP, newVMOP *v1alpha2.VirtualMachineOperation) bool { + if oldVMOP == nil || newVMOP == nil { + return false + } + if oldVMOP.Spec.VirtualMachine != newVMOP.Spec.VirtualMachine { + return false + } + + newForce := ptr.Deref(newVMOP.Spec.Force, false) + + switch oldVMOP.Spec.Type { + case v1alpha2.VMOPTypeStart: + return newVMOP.Spec.Type == v1alpha2.VMOPTypeStop || newVMOP.Spec.Type == v1alpha2.VMOPTypeRestart + case v1alpha2.VMOPTypeStop: + oldForce := ptr.Deref(oldVMOP.Spec.Force, false) + return !oldForce && newVMOP.Spec.Type == v1alpha2.VMOPTypeStop && newForce + case v1alpha2.VMOPTypeMigrate, v1alpha2.VMOPTypeEvict: + return newVMOP.Spec.Type == v1alpha2.VMOPTypeStop + case v1alpha2.VMOPTypeRestart: + return newVMOP.Spec.Type == v1alpha2.VMOPTypeStop + default: + return false + } +} diff --git a/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede_test.go b/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede_test.go new file mode 100644 index 0000000000..3674700dd2 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede_test.go @@ -0,0 +1,81 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package supersede + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/utils/ptr" + + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +func TestSupersede(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Supersede Suite") +} + +var _ = Describe("CanSupersede", func() { + DescribeTable("allowed combinations", + func(oldType v1alpha2.VMOPType, oldForce bool, newType v1alpha2.VMOPType, newForce bool) { + Expect(CanSupersede(vmop(oldType, oldForce), vmop(newType, newForce))).To(BeTrue()) + }, + Entry("start by stop", v1alpha2.VMOPTypeStart, false, v1alpha2.VMOPTypeStop, false), + Entry("start by force stop", v1alpha2.VMOPTypeStart, false, v1alpha2.VMOPTypeStop, true), + Entry("start by restart", v1alpha2.VMOPTypeStart, false, v1alpha2.VMOPTypeRestart, false), + Entry("start by force restart", v1alpha2.VMOPTypeStart, false, v1alpha2.VMOPTypeRestart, true), + Entry("stop by force stop", v1alpha2.VMOPTypeStop, false, v1alpha2.VMOPTypeStop, true), + Entry("migrate by stop", v1alpha2.VMOPTypeMigrate, false, v1alpha2.VMOPTypeStop, false), + Entry("migrate by force stop", v1alpha2.VMOPTypeMigrate, false, v1alpha2.VMOPTypeStop, true), + Entry("evict by stop", v1alpha2.VMOPTypeEvict, false, v1alpha2.VMOPTypeStop, false), + Entry("restart by stop", v1alpha2.VMOPTypeRestart, false, v1alpha2.VMOPTypeStop, false), + Entry("force restart by force stop", v1alpha2.VMOPTypeRestart, true, v1alpha2.VMOPTypeStop, true), + ) + + DescribeTable("forbidden combinations", + func(oldType v1alpha2.VMOPType, oldForce bool, newType v1alpha2.VMOPType, newForce bool) { + Expect(CanSupersede(vmop(oldType, oldForce), vmop(newType, newForce))).To(BeFalse()) + }, + Entry("start by start", v1alpha2.VMOPTypeStart, false, v1alpha2.VMOPTypeStart, false), + Entry("stop by stop", v1alpha2.VMOPTypeStop, false, v1alpha2.VMOPTypeStop, false), + Entry("force stop by force stop", v1alpha2.VMOPTypeStop, true, v1alpha2.VMOPTypeStop, true), + Entry("stop by start", v1alpha2.VMOPTypeStop, false, v1alpha2.VMOPTypeStart, false), + Entry("migrate by migrate", v1alpha2.VMOPTypeMigrate, false, v1alpha2.VMOPTypeMigrate, false), + Entry("restore by stop", v1alpha2.VMOPTypeRestore, false, v1alpha2.VMOPTypeStop, true), + Entry("clone by stop", v1alpha2.VMOPTypeClone, false, v1alpha2.VMOPTypeStop, true), + ) + + It("denies operations for different virtual machines", func() { + oldVMOP := vmop(v1alpha2.VMOPTypeStart, false) + newVMOP := vmop(v1alpha2.VMOPTypeStop, false) + newVMOP.Spec.VirtualMachine = "another-vm" + + Expect(CanSupersede(oldVMOP, newVMOP)).To(BeFalse()) + }) +}) + +func vmop(vmopType v1alpha2.VMOPType, force bool) *v1alpha2.VirtualMachineOperation { + return &v1alpha2.VirtualMachineOperation{ + Spec: v1alpha2.VirtualMachineOperationSpec{ + Type: vmopType, + VirtualMachine: "test-vm", + Force: ptr.To(force), + }, + } +} diff --git a/images/virtualization-artifact/pkg/controller/vmop/vmop_webhook.go b/images/virtualization-artifact/pkg/controller/vmop/vmop_webhook.go index 7889c376ae..b655002c86 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/vmop_webhook.go +++ b/images/virtualization-artifact/pkg/controller/vmop/vmop_webhook.go @@ -31,6 +31,7 @@ import ( "github.com/deckhouse/deckhouse/pkg/log" commonvmop "github.com/deckhouse/virtualization-controller/pkg/common/vmop" "github.com/deckhouse/virtualization-controller/pkg/controller/validator" + "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/supersede" "github.com/deckhouse/virtualization-controller/pkg/featuregates" "github.com/deckhouse/virtualization-controller/pkg/version" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -43,6 +44,7 @@ func NewValidator(c client.Client, log *log.Logger) admission.CustomValidator { ).WithCreateValidators( &nodeSelectorValidator{}, &localStorageMigrationValidator{client: c}, + &activeVMOPValidator{client: c}, ) } @@ -81,6 +83,31 @@ type localStorageMigrationValidator struct { client client.Client } +type activeVMOPValidator struct { + client client.Client +} + +func (v *activeVMOPValidator) ValidateCreate(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (admission.Warnings, error) { + var vmopList v1alpha2.VirtualMachineOperationList + if err := v.client.List(ctx, &vmopList, client.InNamespace(vmop.Namespace)); err != nil { + return nil, fmt.Errorf("failed to list VirtualMachineOperations: %w", err) + } + + for _, other := range vmopList.Items { + if other.Spec.VirtualMachine != vmop.Spec.VirtualMachine { + continue + } + if commonvmop.IsFinished(&other) { + continue + } + if !supersede.CanSupersede(&other, vmop) { + return nil, fmt.Errorf("VMOP cannot be executed now. Previously created operation %q should finish first", other.Name) + } + } + + return nil, nil +} + func (v *localStorageMigrationValidator) ValidateCreate(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (admission.Warnings, error) { if version.GetEdition() != version.EditionCE { return nil, nil From ba5f68868935045a99f927e32744f21ef4fac2ce Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Tue, 12 May 2026 15:41:06 +0200 Subject: [PATCH 2/6] test(api, vmop): add supersede e2e coverage Signed-off-by: Daniil Antoshin test(api, vmop): align supersede e2e with framework style Signed-off-by: Daniil Antoshin --- test/e2e/vmop/supersede.go | 187 +++++++++++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 test/e2e/vmop/supersede.go diff --git a/test/e2e/vmop/supersede.go b/test/e2e/vmop/supersede.go new file mode 100644 index 0000000000..e5df7f0a2f --- /dev/null +++ b/test/e2e/vmop/supersede.go @@ -0,0 +1,187 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package vmop + +import ( + "context" + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + crclient "sigs.k8s.io/controller-runtime/pkg/client" + + vdbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vd" + vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" + vmopbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vmop" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" + "github.com/deckhouse/virtualization/test/e2e/internal/framework" + "github.com/deckhouse/virtualization/test/e2e/internal/label" + "github.com/deckhouse/virtualization/test/e2e/internal/object" + "github.com/deckhouse/virtualization/test/e2e/internal/util" +) + +var _ = Describe("VirtualMachineOperationSupersede", label.Slow(), func() { + DescribeTable("supersedes active operation", func(runPolicy v1alpha2.RunPolicy, initialPhase v1alpha2.MachinePhase, oldType v1alpha2.VMOPType, oldForce bool, newType v1alpha2.VMOPType, newForce bool, finalPhase v1alpha2.MachinePhase) { + f := framework.NewFramework(fmt.Sprintf("vmop-supersede-%s-%s", oldType, newType)) + DeferCleanup(f.After) + f.Before() + + t := newSupersedeTest(f) + + By("Environment preparation", func() { + t.GenerateResources(runPolicy) + err := f.CreateWithDeferredDeletion(context.Background(), t.VDRoot, t.VM) + Expect(err).NotTo(HaveOccurred()) + util.UntilObjectPhase(string(initialPhase), framework.LongTimeout, t.VM) + }) + + By("Creating an active operation", func() { + t.OldVMOP = t.NewVMOP(oldType, oldForce) + err := f.CreateWithDeferredDeletion(context.Background(), t.OldVMOP) + Expect(err).NotTo(HaveOccurred()) + }) + + By("Creating a superseding operation", func() { + t.SupersedingVMOP = t.NewVMOP(newType, newForce) + err := f.CreateWithDeferredDeletion(context.Background(), t.SupersedingVMOP) + Expect(err).NotTo(HaveOccurred()) + }) + + By("Checking the old operation is marked as superseded", func() { + t.ExpectSuperseded(t.OldVMOP) + }) + + By("Checking the new operation completes normally", func() { + util.UntilObjectPhase(string(v1alpha2.VMOPPhaseCompleted), framework.LongTimeout, t.SupersedingVMOP) + util.UntilObjectPhase(string(finalPhase), framework.MiddleTimeout, t.VM) + }) + }, + Entry( + "Start is superseded by Stop", + v1alpha2.ManualPolicy, + v1alpha2.MachineStopped, + v1alpha2.VMOPTypeStart, + false, + v1alpha2.VMOPTypeStop, + false, + v1alpha2.MachineStopped, + ), + Entry( + "Stop is superseded by force Stop", + v1alpha2.AlwaysOnUnlessStoppedManually, + v1alpha2.MachineRunning, + v1alpha2.VMOPTypeStop, + false, + v1alpha2.VMOPTypeStop, + true, + v1alpha2.MachineStopped, + ), + ) + + It("rejects Start while Stop is active", func() { + f := framework.NewFramework("vmop-supersede-stop-start") + DeferCleanup(f.After) + f.Before() + + t := newSupersedeTest(f) + + By("Environment preparation", func() { + t.GenerateResources(v1alpha2.AlwaysOnUnlessStoppedManually) + err := f.CreateWithDeferredDeletion(context.Background(), t.VDRoot, t.VM) + Expect(err).NotTo(HaveOccurred()) + util.UntilObjectPhase(string(v1alpha2.MachineRunning), framework.LongTimeout, t.VM) + }) + + By("Creating an active Stop operation", func() { + t.OldVMOP = t.NewVMOP(v1alpha2.VMOPTypeStop, false) + err := f.CreateWithDeferredDeletion(context.Background(), t.OldVMOP) + Expect(err).NotTo(HaveOccurred()) + }) + + By("Checking Start is rejected by admission", func() { + newStart := t.NewVMOP(v1alpha2.VMOPTypeStart, false) + err := f.CreateWithDeferredDeletion(context.Background(), newStart) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("VMOP cannot be executed now")) + Expect(err.Error()).To(ContainSubstring(t.OldVMOP.Name)) + }) + }) +}) + +type supersedeTest struct { + Framework *framework.Framework + + VM *v1alpha2.VirtualMachine + VDRoot *v1alpha2.VirtualDisk + OldVMOP *v1alpha2.VirtualMachineOperation + SupersedingVMOP *v1alpha2.VirtualMachineOperation +} + +func newSupersedeTest(f *framework.Framework) *supersedeTest { + return &supersedeTest{ + Framework: f, + } +} + +func (t *supersedeTest) GenerateResources(runPolicy v1alpha2.RunPolicy) { + t.VDRoot = object.NewVDFromCVI("vd-root", t.Framework.Namespace().Name, object.PrecreatedCVIAlpineBIOS, + vdbuilder.WithSize(ptr.To(resource.MustParse("512Mi"))), + ) + + t.VM = object.NewMinimalVM("vm-", t.Framework.Namespace().Name, + vmbuilder.WithBlockDeviceRefs(v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.DiskDevice, + Name: t.VDRoot.Name, + }), + vmbuilder.WithRunPolicy(runPolicy), + ) +} + +func (t *supersedeTest) NewVMOP(vmopType v1alpha2.VMOPType, force bool) *v1alpha2.VirtualMachineOperation { + opts := []vmopbuilder.Option{ + vmopbuilder.WithGenerateName(fmt.Sprintf("%s-supersede-%s-", util.VmopE2ePrefix, string(vmopType))), + vmopbuilder.WithNamespace(t.VM.Namespace), + vmopbuilder.WithType(vmopType), + vmopbuilder.WithVirtualMachine(t.VM.Name), + } + + if force { + opts = append(opts, vmopbuilder.WithForce(ptr.To(true))) + } + + return vmopbuilder.New(opts...) +} + +func (t *supersedeTest) ExpectSuperseded(vmop *v1alpha2.VirtualMachineOperation) { + GinkgoHelper() + + util.UntilObjectPhase(string(v1alpha2.VMOPPhaseCompleted), framework.LongTimeout, vmop) + util.UntilConditionStatus(vmopcondition.TypeCompleted.String(), string(metav1.ConditionTrue), framework.ShortTimeout, vmop) + util.UntilConditionReason(vmopcondition.TypeCompleted.String(), vmopcondition.ReasonSuperseded.String(), framework.ShortTimeout, vmop) + + err := t.Framework.Clients.GenericClient().Get(context.Background(), crclient.ObjectKeyFromObject(vmop), vmop) + Expect(err).NotTo(HaveOccurred()) + completed, exists := conditions.GetCondition(vmopcondition.TypeCompleted, vmop.Status.Conditions) + Expect(exists).To(BeTrue()) + Expect(completed.Status).To(Equal(metav1.ConditionTrue)) + Expect(completed.Reason).To(Equal(vmopcondition.ReasonSuperseded.String())) +} From 0697ecdedd726838cefece392adca08cb29fe259 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Tue, 12 May 2026 16:39:14 +0200 Subject: [PATCH 3/6] test(api, vmop): cover force stop supersede service flow Signed-off-by: Daniil Antoshin --- .../controller/vmop/service/service_test.go | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/images/virtualization-artifact/pkg/controller/vmop/service/service_test.go b/images/virtualization-artifact/pkg/controller/vmop/service/service_test.go index f226910ffd..688333994a 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/service/service_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/service/service_test.go @@ -62,6 +62,28 @@ var _ = Describe("BaseVMOPService", func() { Expect(completed.Reason).To(Equal(vmopcondition.ReasonSuperseded.String())) }) + It("marks an older stop operation as superseded by force stop", func(ctx SpecContext) { + oldVMOP := newVMOP("old-stop", v1alpha2.VMOPTypeStop, false, time.Now().Add(-time.Minute)) + newVMOP := newVMOP("new-force-stop", v1alpha2.VMOPTypeStop, true, time.Now()) + + client, err := testutil.NewFakeClientWithObjects(oldVMOP) + Expect(err).NotTo(HaveOccurred()) + + svc := NewBaseVMOPService(client, &eventrecord.EventRecorderLoggerMock{}) + should, err := svc.ShouldExecuteOrSupersedeOrSetFailedPhase(ctx, newVMOP) + Expect(err).NotTo(HaveOccurred()) + Expect(should).To(BeTrue()) + + changed := &v1alpha2.VirtualMachineOperation{} + Expect(client.Get(ctx, types.NamespacedName{Name: oldVMOP.Name, Namespace: oldVMOP.Namespace}, changed)).To(Succeed()) + Expect(changed.Status.Phase).To(Equal(v1alpha2.VMOPPhaseCompleted)) + + completed, found := conditions.GetCondition(vmopcondition.TypeCompleted, changed.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(completed.Status).To(Equal(metav1.ConditionTrue)) + Expect(completed.Reason).To(Equal(vmopcondition.ReasonSuperseded.String())) + }) + It("fails a forbidden newer operation", func(ctx SpecContext) { oldVMOP := newVMOP("old-stop", v1alpha2.VMOPTypeStop, false, time.Now().Add(-time.Minute)) newVMOP := newVMOP("new-start", v1alpha2.VMOPTypeStart, false, time.Now()) From a87393d26e27387ac81dac8540e2e472dfc2c0c7 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Wed, 13 May 2026 17:14:00 +0200 Subject: [PATCH 4/6] docs(vmop): describe operation supersede behavior Signed-off-by: Daniil Antoshin --- docs/USER_GUIDE.md | 4 ++++ docs/USER_GUIDE.ru.md | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 4d1bc68edc..61a9f6aa1d 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -1756,6 +1756,10 @@ A list of possible operations is given in the table below: | `d8 v evict` | `Evict` | Evict the VM to another host | | `d8 v migrate` | `Migrate` | Migrate the VM to another host | +Only one active operation is executed for a VM at a time. If a new operation is compatible with an already active operation, it can supersede the older operation. The older operation is completed with `status.phase: Completed` and the `Completed` condition reason `Superseded`, while the new operation continues execution. For example, `Stop` can supersede an active `Start`, and `Stop` with `force: true` can supersede a regular `Stop`. + +If operations are incompatible, the new operation is rejected until the active operation finishes. Restore and clone operations do not supersede other VM operations. + How to perform the operation in the web interface: - Go to the "Projects" tab and select the desired project. diff --git a/docs/USER_GUIDE.ru.md b/docs/USER_GUIDE.ru.md index a5a6322270..043528b50f 100644 --- a/docs/USER_GUIDE.ru.md +++ b/docs/USER_GUIDE.ru.md @@ -1774,6 +1774,10 @@ d8 v restart linux-vm | `d8 v evict` | `Evict` | Выселить ВМ на другой узел | | `d8 v migrate` | `Migrate` | Мигрировать ВМ на другой узел | +Для одной ВМ одновременно выполняется только одна активная операция. Если новая операция совместима с уже активной операцией, она может вытеснить более старую операцию. Более старая операция завершается с `status.phase: Completed` и причиной `Superseded` в условии `Completed`, а новая операция продолжает выполнение. Например, `Stop` может вытеснить активную операцию `Start`, а `Stop` с `force: true` может вытеснить обычную операцию `Stop`. + +Если операции несовместимы, новая операция отклоняется до завершения активной операции. Операции восстановления и клонирования не вытесняют другие операции ВМ. + Как выполнить операцию в веб-интерфейсе: - Перейдите на вкладку «Проекты» и выберите нужный проект. From 65d5a577a9354274ab2aeacb93329806436cd2f6 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Thu, 14 May 2026 13:04:56 +0200 Subject: [PATCH 5/6] test(api, vmop): remove supersede e2e coverage Signed-off-by: Daniil Antoshin --- test/e2e/vmop/supersede.go | 187 ------------------------------------- 1 file changed, 187 deletions(-) delete mode 100644 test/e2e/vmop/supersede.go diff --git a/test/e2e/vmop/supersede.go b/test/e2e/vmop/supersede.go deleted file mode 100644 index e5df7f0a2f..0000000000 --- a/test/e2e/vmop/supersede.go +++ /dev/null @@ -1,187 +0,0 @@ -/* -Copyright 2026 Flant JSC - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package vmop - -import ( - "context" - "fmt" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" - crclient "sigs.k8s.io/controller-runtime/pkg/client" - - vdbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vd" - vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" - vmopbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vmop" - "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" - "github.com/deckhouse/virtualization/test/e2e/internal/framework" - "github.com/deckhouse/virtualization/test/e2e/internal/label" - "github.com/deckhouse/virtualization/test/e2e/internal/object" - "github.com/deckhouse/virtualization/test/e2e/internal/util" -) - -var _ = Describe("VirtualMachineOperationSupersede", label.Slow(), func() { - DescribeTable("supersedes active operation", func(runPolicy v1alpha2.RunPolicy, initialPhase v1alpha2.MachinePhase, oldType v1alpha2.VMOPType, oldForce bool, newType v1alpha2.VMOPType, newForce bool, finalPhase v1alpha2.MachinePhase) { - f := framework.NewFramework(fmt.Sprintf("vmop-supersede-%s-%s", oldType, newType)) - DeferCleanup(f.After) - f.Before() - - t := newSupersedeTest(f) - - By("Environment preparation", func() { - t.GenerateResources(runPolicy) - err := f.CreateWithDeferredDeletion(context.Background(), t.VDRoot, t.VM) - Expect(err).NotTo(HaveOccurred()) - util.UntilObjectPhase(string(initialPhase), framework.LongTimeout, t.VM) - }) - - By("Creating an active operation", func() { - t.OldVMOP = t.NewVMOP(oldType, oldForce) - err := f.CreateWithDeferredDeletion(context.Background(), t.OldVMOP) - Expect(err).NotTo(HaveOccurred()) - }) - - By("Creating a superseding operation", func() { - t.SupersedingVMOP = t.NewVMOP(newType, newForce) - err := f.CreateWithDeferredDeletion(context.Background(), t.SupersedingVMOP) - Expect(err).NotTo(HaveOccurred()) - }) - - By("Checking the old operation is marked as superseded", func() { - t.ExpectSuperseded(t.OldVMOP) - }) - - By("Checking the new operation completes normally", func() { - util.UntilObjectPhase(string(v1alpha2.VMOPPhaseCompleted), framework.LongTimeout, t.SupersedingVMOP) - util.UntilObjectPhase(string(finalPhase), framework.MiddleTimeout, t.VM) - }) - }, - Entry( - "Start is superseded by Stop", - v1alpha2.ManualPolicy, - v1alpha2.MachineStopped, - v1alpha2.VMOPTypeStart, - false, - v1alpha2.VMOPTypeStop, - false, - v1alpha2.MachineStopped, - ), - Entry( - "Stop is superseded by force Stop", - v1alpha2.AlwaysOnUnlessStoppedManually, - v1alpha2.MachineRunning, - v1alpha2.VMOPTypeStop, - false, - v1alpha2.VMOPTypeStop, - true, - v1alpha2.MachineStopped, - ), - ) - - It("rejects Start while Stop is active", func() { - f := framework.NewFramework("vmop-supersede-stop-start") - DeferCleanup(f.After) - f.Before() - - t := newSupersedeTest(f) - - By("Environment preparation", func() { - t.GenerateResources(v1alpha2.AlwaysOnUnlessStoppedManually) - err := f.CreateWithDeferredDeletion(context.Background(), t.VDRoot, t.VM) - Expect(err).NotTo(HaveOccurred()) - util.UntilObjectPhase(string(v1alpha2.MachineRunning), framework.LongTimeout, t.VM) - }) - - By("Creating an active Stop operation", func() { - t.OldVMOP = t.NewVMOP(v1alpha2.VMOPTypeStop, false) - err := f.CreateWithDeferredDeletion(context.Background(), t.OldVMOP) - Expect(err).NotTo(HaveOccurred()) - }) - - By("Checking Start is rejected by admission", func() { - newStart := t.NewVMOP(v1alpha2.VMOPTypeStart, false) - err := f.CreateWithDeferredDeletion(context.Background(), newStart) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("VMOP cannot be executed now")) - Expect(err.Error()).To(ContainSubstring(t.OldVMOP.Name)) - }) - }) -}) - -type supersedeTest struct { - Framework *framework.Framework - - VM *v1alpha2.VirtualMachine - VDRoot *v1alpha2.VirtualDisk - OldVMOP *v1alpha2.VirtualMachineOperation - SupersedingVMOP *v1alpha2.VirtualMachineOperation -} - -func newSupersedeTest(f *framework.Framework) *supersedeTest { - return &supersedeTest{ - Framework: f, - } -} - -func (t *supersedeTest) GenerateResources(runPolicy v1alpha2.RunPolicy) { - t.VDRoot = object.NewVDFromCVI("vd-root", t.Framework.Namespace().Name, object.PrecreatedCVIAlpineBIOS, - vdbuilder.WithSize(ptr.To(resource.MustParse("512Mi"))), - ) - - t.VM = object.NewMinimalVM("vm-", t.Framework.Namespace().Name, - vmbuilder.WithBlockDeviceRefs(v1alpha2.BlockDeviceSpecRef{ - Kind: v1alpha2.DiskDevice, - Name: t.VDRoot.Name, - }), - vmbuilder.WithRunPolicy(runPolicy), - ) -} - -func (t *supersedeTest) NewVMOP(vmopType v1alpha2.VMOPType, force bool) *v1alpha2.VirtualMachineOperation { - opts := []vmopbuilder.Option{ - vmopbuilder.WithGenerateName(fmt.Sprintf("%s-supersede-%s-", util.VmopE2ePrefix, string(vmopType))), - vmopbuilder.WithNamespace(t.VM.Namespace), - vmopbuilder.WithType(vmopType), - vmopbuilder.WithVirtualMachine(t.VM.Name), - } - - if force { - opts = append(opts, vmopbuilder.WithForce(ptr.To(true))) - } - - return vmopbuilder.New(opts...) -} - -func (t *supersedeTest) ExpectSuperseded(vmop *v1alpha2.VirtualMachineOperation) { - GinkgoHelper() - - util.UntilObjectPhase(string(v1alpha2.VMOPPhaseCompleted), framework.LongTimeout, vmop) - util.UntilConditionStatus(vmopcondition.TypeCompleted.String(), string(metav1.ConditionTrue), framework.ShortTimeout, vmop) - util.UntilConditionReason(vmopcondition.TypeCompleted.String(), vmopcondition.ReasonSuperseded.String(), framework.ShortTimeout, vmop) - - err := t.Framework.Clients.GenericClient().Get(context.Background(), crclient.ObjectKeyFromObject(vmop), vmop) - Expect(err).NotTo(HaveOccurred()) - completed, exists := conditions.GetCondition(vmopcondition.TypeCompleted, vmop.Status.Conditions) - Expect(exists).To(BeTrue()) - Expect(completed.Status).To(Equal(metav1.ConditionTrue)) - Expect(completed.Reason).To(Equal(vmopcondition.ReasonSuperseded.String())) -} From c7f0496882d3c2bcea35aa2711a15bbcd686b4aa Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Thu, 14 May 2026 13:42:33 +0200 Subject: [PATCH 6/6] feat(vmop): update supersede matrix Signed-off-by: Daniil Antoshin --- docs/USER_GUIDE.md | 2 +- docs/USER_GUIDE.ru.md | 2 +- .../controller/vmop/service/service_test.go | 73 ++++++++++++ .../controller/vmop/supersede/supersede.go | 18 ++- .../vmop/supersede/supersede_test.go | 110 +++++++++++++----- 5 files changed, 170 insertions(+), 35 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 61a9f6aa1d..8396837ae5 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -1756,7 +1756,7 @@ A list of possible operations is given in the table below: | `d8 v evict` | `Evict` | Evict the VM to another host | | `d8 v migrate` | `Migrate` | Migrate the VM to another host | -Only one active operation is executed for a VM at a time. If a new operation is compatible with an already active operation, it can supersede the older operation. The older operation is completed with `status.phase: Completed` and the `Completed` condition reason `Superseded`, while the new operation continues execution. For example, `Stop` can supersede an active `Start`, and `Stop` with `force: true` can supersede a regular `Stop`. +Only one active operation is executed for a VM at a time. If a new operation is compatible with an already active operation, it can supersede the older operation. The older operation is completed with `status.phase: Completed` and the `Completed` condition reason `Superseded`, while the new operation continues execution. For example, `Stop` can supersede an active `Start`, `Stop` with `force: true` can supersede a regular `Stop`, and `Restart` can supersede an active `Migrate` or `Evict`. If operations are incompatible, the new operation is rejected until the active operation finishes. Restore and clone operations do not supersede other VM operations. diff --git a/docs/USER_GUIDE.ru.md b/docs/USER_GUIDE.ru.md index 043528b50f..aac9890d65 100644 --- a/docs/USER_GUIDE.ru.md +++ b/docs/USER_GUIDE.ru.md @@ -1774,7 +1774,7 @@ d8 v restart linux-vm | `d8 v evict` | `Evict` | Выселить ВМ на другой узел | | `d8 v migrate` | `Migrate` | Мигрировать ВМ на другой узел | -Для одной ВМ одновременно выполняется только одна активная операция. Если новая операция совместима с уже активной операцией, она может вытеснить более старую операцию. Более старая операция завершается с `status.phase: Completed` и причиной `Superseded` в условии `Completed`, а новая операция продолжает выполнение. Например, `Stop` может вытеснить активную операцию `Start`, а `Stop` с `force: true` может вытеснить обычную операцию `Stop`. +Для одной ВМ одновременно выполняется только одна активная операция. Если новая операция совместима с уже активной операцией, она может вытеснить более старую операцию. Более старая операция завершается с `status.phase: Completed` и причиной `Superseded` в условии `Completed`, а новая операция продолжает выполнение. Например, `Stop` может вытеснить активную операцию `Start`, `Stop` с `force: true` может вытеснить обычную операцию `Stop`, а `Restart` может вытеснить активную операцию `Migrate` или `Evict`. Если операции несовместимы, новая операция отклоняется до завершения активной операции. Операции восстановления и клонирования не вытесняют другие операции ВМ. diff --git a/images/virtualization-artifact/pkg/controller/vmop/service/service_test.go b/images/virtualization-artifact/pkg/controller/vmop/service/service_test.go index 688333994a..7affa0750b 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/service/service_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/service/service_test.go @@ -22,9 +22,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" + virtv1 "kubevirt.io/api/core/v1" "github.com/deckhouse/virtualization-controller/pkg/common/testutil" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" @@ -84,6 +86,65 @@ var _ = Describe("BaseVMOPService", func() { Expect(completed.Reason).To(Equal(vmopcondition.ReasonSuperseded.String())) }) + It("marks an older stop operation as superseded by force restart", func(ctx SpecContext) { + oldVMOP := newVMOP("old-stop", v1alpha2.VMOPTypeStop, false, time.Now().Add(-time.Minute)) + newVMOP := newVMOP("new-force-restart", v1alpha2.VMOPTypeRestart, true, time.Now()) + + client, err := testutil.NewFakeClientWithObjects(oldVMOP) + Expect(err).NotTo(HaveOccurred()) + + svc := NewBaseVMOPService(client, &eventrecord.EventRecorderLoggerMock{}) + should, err := svc.ShouldExecuteOrSupersedeOrSetFailedPhase(ctx, newVMOP) + Expect(err).NotTo(HaveOccurred()) + Expect(should).To(BeTrue()) + + changed := &v1alpha2.VirtualMachineOperation{} + Expect(client.Get(ctx, types.NamespacedName{Name: oldVMOP.Name, Namespace: oldVMOP.Namespace}, changed)).To(Succeed()) + Expect(changed.Status.Phase).To(Equal(v1alpha2.VMOPPhaseCompleted)) + + completed, found := conditions.GetCondition(vmopcondition.TypeCompleted, changed.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(completed.Status).To(Equal(metav1.ConditionTrue)) + Expect(completed.Reason).To(Equal(vmopcondition.ReasonSuperseded.String())) + }) + + Describe("migration cleanup for restart supersede", func() { + entries := []struct { + name string + oldType v1alpha2.VMOPType + newForce bool + }{ + {name: "migrate by restart", oldType: v1alpha2.VMOPTypeMigrate}, + {name: "migrate by force restart", oldType: v1alpha2.VMOPTypeMigrate, newForce: true}, + {name: "evict by restart", oldType: v1alpha2.VMOPTypeEvict}, + {name: "evict by force restart", oldType: v1alpha2.VMOPTypeEvict, newForce: true}, + } + + for _, entry := range entries { + It("deletes migration side effect for "+entry.name, func(ctx SpecContext) { + oldVMOP := newVMOP("old-migration", entry.oldType, false, time.Now().Add(-time.Minute)) + newVMOP := newVMOP("new-restart", v1alpha2.VMOPTypeRestart, entry.newForce, time.Now()) + migration := newMigrationForVMOP(oldVMOP) + + client, err := testutil.NewFakeClientWithObjects(oldVMOP, migration) + Expect(err).NotTo(HaveOccurred()) + + svc := NewBaseVMOPService(client, &eventrecord.EventRecorderLoggerMock{}) + should, err := svc.ShouldExecuteOrSupersedeOrSetFailedPhase(ctx, newVMOP) + Expect(err).NotTo(HaveOccurred()) + Expect(should).To(BeTrue()) + + changed := &v1alpha2.VirtualMachineOperation{} + Expect(client.Get(ctx, types.NamespacedName{Name: oldVMOP.Name, Namespace: oldVMOP.Namespace}, changed)).To(Succeed()) + Expect(changed.Status.Phase).To(Equal(v1alpha2.VMOPPhaseCompleted)) + + deletedMigration := &virtv1.VirtualMachineInstanceMigration{} + err = client.Get(ctx, types.NamespacedName{Name: migration.Name, Namespace: migration.Namespace}, deletedMigration) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }) + } + }) + It("fails a forbidden newer operation", func(ctx SpecContext) { oldVMOP := newVMOP("old-stop", v1alpha2.VMOPTypeStop, false, time.Now().Add(-time.Minute)) newVMOP := newVMOP("new-start", v1alpha2.VMOPTypeStart, false, time.Now()) @@ -105,6 +166,18 @@ var _ = Describe("BaseVMOPService", func() { }) }) +func newMigrationForVMOP(vmop *v1alpha2.VirtualMachineOperation) *virtv1.VirtualMachineInstanceMigration { + return &virtv1.VirtualMachineInstanceMigration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmop-" + vmop.Name, + Namespace: vmop.Namespace, + }, + Spec: virtv1.VirtualMachineInstanceMigrationSpec{ + VMIName: vmop.Spec.VirtualMachine, + }, + } +} + func newVMOP(name string, vmopType v1alpha2.VMOPType, force bool, createdAt time.Time) *v1alpha2.VirtualMachineOperation { return &v1alpha2.VirtualMachineOperation{ ObjectMeta: metav1.ObjectMeta{ diff --git a/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede.go b/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede.go index 9591d81add..a486af4109 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede.go +++ b/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede.go @@ -30,18 +30,26 @@ func CanSupersede(oldVMOP, newVMOP *v1alpha2.VirtualMachineOperation) bool { return false } + oldForce := ptr.Deref(oldVMOP.Spec.Force, false) newForce := ptr.Deref(newVMOP.Spec.Force, false) switch oldVMOP.Spec.Type { case v1alpha2.VMOPTypeStart: - return newVMOP.Spec.Type == v1alpha2.VMOPTypeStop || newVMOP.Spec.Type == v1alpha2.VMOPTypeRestart + return newVMOP.Spec.Type == v1alpha2.VMOPTypeStop case v1alpha2.VMOPTypeStop: - oldForce := ptr.Deref(oldVMOP.Spec.Force, false) - return !oldForce && newVMOP.Spec.Type == v1alpha2.VMOPTypeStop && newForce + if oldForce { + return false + } + return newVMOP.Spec.Type == v1alpha2.VMOPTypeStop && newForce || + newVMOP.Spec.Type == v1alpha2.VMOPTypeRestart && newForce case v1alpha2.VMOPTypeMigrate, v1alpha2.VMOPTypeEvict: - return newVMOP.Spec.Type == v1alpha2.VMOPTypeStop + return newVMOP.Spec.Type == v1alpha2.VMOPTypeStop || newVMOP.Spec.Type == v1alpha2.VMOPTypeRestart case v1alpha2.VMOPTypeRestart: - return newVMOP.Spec.Type == v1alpha2.VMOPTypeStop + if oldForce { + return false + } + return newVMOP.Spec.Type == v1alpha2.VMOPTypeStop && newForce || + newVMOP.Spec.Type == v1alpha2.VMOPTypeRestart && newForce default: return false } diff --git a/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede_test.go b/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede_test.go index 3674700dd2..524b82ea4f 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/supersede/supersede_test.go @@ -17,6 +17,7 @@ limitations under the License. package supersede import ( + "fmt" "testing" . "github.com/onsi/ginkgo/v2" @@ -32,34 +33,13 @@ func TestSupersede(t *testing.T) { } var _ = Describe("CanSupersede", func() { - DescribeTable("allowed combinations", - func(oldType v1alpha2.VMOPType, oldForce bool, newType v1alpha2.VMOPType, newForce bool) { - Expect(CanSupersede(vmop(oldType, oldForce), vmop(newType, newForce))).To(BeTrue()) - }, - Entry("start by stop", v1alpha2.VMOPTypeStart, false, v1alpha2.VMOPTypeStop, false), - Entry("start by force stop", v1alpha2.VMOPTypeStart, false, v1alpha2.VMOPTypeStop, true), - Entry("start by restart", v1alpha2.VMOPTypeStart, false, v1alpha2.VMOPTypeRestart, false), - Entry("start by force restart", v1alpha2.VMOPTypeStart, false, v1alpha2.VMOPTypeRestart, true), - Entry("stop by force stop", v1alpha2.VMOPTypeStop, false, v1alpha2.VMOPTypeStop, true), - Entry("migrate by stop", v1alpha2.VMOPTypeMigrate, false, v1alpha2.VMOPTypeStop, false), - Entry("migrate by force stop", v1alpha2.VMOPTypeMigrate, false, v1alpha2.VMOPTypeStop, true), - Entry("evict by stop", v1alpha2.VMOPTypeEvict, false, v1alpha2.VMOPTypeStop, false), - Entry("restart by stop", v1alpha2.VMOPTypeRestart, false, v1alpha2.VMOPTypeStop, false), - Entry("force restart by force stop", v1alpha2.VMOPTypeRestart, true, v1alpha2.VMOPTypeStop, true), - ) - - DescribeTable("forbidden combinations", - func(oldType v1alpha2.VMOPType, oldForce bool, newType v1alpha2.VMOPType, newForce bool) { - Expect(CanSupersede(vmop(oldType, oldForce), vmop(newType, newForce))).To(BeFalse()) - }, - Entry("start by start", v1alpha2.VMOPTypeStart, false, v1alpha2.VMOPTypeStart, false), - Entry("stop by stop", v1alpha2.VMOPTypeStop, false, v1alpha2.VMOPTypeStop, false), - Entry("force stop by force stop", v1alpha2.VMOPTypeStop, true, v1alpha2.VMOPTypeStop, true), - Entry("stop by start", v1alpha2.VMOPTypeStop, false, v1alpha2.VMOPTypeStart, false), - Entry("migrate by migrate", v1alpha2.VMOPTypeMigrate, false, v1alpha2.VMOPTypeMigrate, false), - Entry("restore by stop", v1alpha2.VMOPTypeRestore, false, v1alpha2.VMOPTypeStop, true), - Entry("clone by stop", v1alpha2.VMOPTypeClone, false, v1alpha2.VMOPTypeStop, true), - ) + Describe("matrix combinations", func() { + for _, entry := range supersedeMatrixEntries() { + It(entry.name, func() { + Expect(CanSupersede(vmop(entry.oldType, entry.oldForce), vmop(entry.newType, entry.newForce))).To(Equal(entry.expected)) + }) + } + }) It("denies operations for different virtual machines", func() { oldVMOP := vmop(v1alpha2.VMOPTypeStart, false) @@ -68,8 +48,82 @@ var _ = Describe("CanSupersede", func() { Expect(CanSupersede(oldVMOP, newVMOP)).To(BeFalse()) }) + + DescribeTable("denies nil operations", + func(oldVMOP, newVMOP *v1alpha2.VirtualMachineOperation) { + Expect(CanSupersede(oldVMOP, newVMOP)).To(BeFalse()) + }, + Entry("nil old operation", nil, vmop(v1alpha2.VMOPTypeStop, false)), + Entry("nil new operation", vmop(v1alpha2.VMOPTypeStart, false), nil), + Entry("nil operations", nil, nil), + ) }) +type supersedeMatrixEntry struct { + name string + oldType v1alpha2.VMOPType + oldForce bool + newType v1alpha2.VMOPType + newForce bool + expected bool +} + +func supersedeMatrixEntries() []supersedeMatrixEntry { + vmopTypes := []v1alpha2.VMOPType{ + v1alpha2.VMOPTypeStart, + v1alpha2.VMOPTypeStop, + v1alpha2.VMOPTypeMigrate, + v1alpha2.VMOPTypeEvict, + v1alpha2.VMOPTypeRestart, + v1alpha2.VMOPTypeRestore, + v1alpha2.VMOPTypeClone, + } + forces := []bool{false, true} + + var entries []supersedeMatrixEntry + for _, oldType := range vmopTypes { + for _, oldForce := range forces { + for _, newType := range vmopTypes { + for _, newForce := range forces { + entries = append(entries, supersedeMatrixEntry{ + name: fmt.Sprintf("%s force=%t by %s force=%t", oldType, oldForce, newType, newForce), + oldType: oldType, + oldForce: oldForce, + newType: newType, + newForce: newForce, + expected: expectedCanSupersede(oldType, oldForce, newType, newForce), + }) + } + } + } + } + + return entries +} + +func expectedCanSupersede(oldType v1alpha2.VMOPType, oldForce bool, newType v1alpha2.VMOPType, newForce bool) bool { + switch oldType { + case v1alpha2.VMOPTypeStart: + return newType == v1alpha2.VMOPTypeStop + case v1alpha2.VMOPTypeStop: + if oldForce { + return false + } + return newType == v1alpha2.VMOPTypeStop && newForce || + newType == v1alpha2.VMOPTypeRestart && newForce + case v1alpha2.VMOPTypeMigrate, v1alpha2.VMOPTypeEvict: + return newType == v1alpha2.VMOPTypeStop || newType == v1alpha2.VMOPTypeRestart + case v1alpha2.VMOPTypeRestart: + if oldForce { + return false + } + return newType == v1alpha2.VMOPTypeStop && newForce || + newType == v1alpha2.VMOPTypeRestart && newForce + default: + return false + } +} + func vmop(vmopType v1alpha2.VMOPType, force bool) *v1alpha2.VirtualMachineOperation { return &v1alpha2.VirtualMachineOperation{ Spec: v1alpha2.VirtualMachineOperationSpec{