From b7d2da746c5f9afe7d64a2f917ccfecde68c9e1c Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Mon, 26 Jan 2026 18:37:47 +0300 Subject: [PATCH 01/24] restore Signed-off-by: Valeriy Khorunzhin refactoring Signed-off-by: Valeriy Khorunzhin fix Signed-off-by: Valeriy Khorunzhin remove printf Signed-off-by: Valeriy Khorunzhin fix e2e Signed-off-by: Valeriy Khorunzhin total rewriting Signed-off-by: Valeriy Khorunzhin fixes Signed-off-by: Valeriy Khorunzhin tests Signed-off-by: Valeriy Khorunzhin fix linter Signed-off-by: Valeriy Khorunzhin fix linter 2 Signed-off-by: Valeriy Khorunzhin final -> completed Signed-off-by: Valeriy Khorunzhin resolve Signed-off-by: Valeriy Khorunzhin --- api/core/v1alpha2/vmopcondition/condition.go | 45 --- .../vmop/snapshot/internal/common/common.go | 50 ---- .../snapshot/internal/handler/lifecycle.go | 64 +---- .../vmop/snapshot/internal/service/clone.go | 80 ++---- .../snapshot/internal/service/operation.go | 2 - .../vmop/snapshot/internal/service/restore.go | 81 ++---- .../internal/step/cleanup_snapshot_step.go | 56 +--- .../snapshot/internal/step/completed_step.go | 50 ++++ .../internal/step/create_snapshot_step.go | 13 +- .../internal/step/enter_maintenance_step.go | 13 +- .../internal/step/exit_maintenance_step.go | 20 +- .../snapshot/internal/step/final_step_test.go | 83 ++++++ .../internal/step/process_clone_step.go | 73 +---- .../internal/step/process_clone_step_test.go | 115 ++++++++ ...rocess_step.go => process_restore_step.go} | 44 +-- .../step/process_restore_step_test.go | 134 +++++++++ .../vmop/snapshot/internal/step/suite_test.go | 157 ++++++++++ .../internal/step/vmsnapshot_ready_step.go | 27 +- .../step/vmsnapshot_ready_step_test.go | 160 +++++++++++ .../internal/step/waiting_disk_ready_step.go | 115 ++++++++ .../step/waiting_disk_ready_step_test.go | 270 ++++++++++++++++++ test/e2e/vmop/restore.go | 6 - 22 files changed, 1202 insertions(+), 456 deletions(-) delete mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/common/common.go create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/final_step_test.go create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go rename images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/{process_step.go => process_restore_step.go} (64%) create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go diff --git a/api/core/v1alpha2/vmopcondition/condition.go b/api/core/v1alpha2/vmopcondition/condition.go index 173308e477..58d917ab5a 100644 --- a/api/core/v1alpha2/vmopcondition/condition.go +++ b/api/core/v1alpha2/vmopcondition/condition.go @@ -29,12 +29,6 @@ const ( // TypeSignalSent is a type for condition that indicates operation signal has been sent. TypeSignalSent Type = "SignalSent" - // TypeRestoreCompleted is a type for condition that indicates success of restore. - TypeRestoreCompleted Type = "RestoreCompleted" - - // TypeCloneCompleted is a type for condition that indicates success of clone. - TypeCloneCompleted Type = "CloneCompleted" - // TypeMaintenanceMode is a type for condition that indicates VMOP has put VM in maintenance mode. TypeMaintenanceMode Type = "MaintenanceMode" @@ -111,45 +105,6 @@ const ( ReasonOperationCompleted ReasonCompleted = "OperationCompleted" ) -// ReasonRestoreCompleted represents specific reasons for the 'RestoreCompleted' condition type. -type ReasonRestoreCompleted string - -func (r ReasonRestoreCompleted) String() string { - return string(r) -} - -const ( - // ReasonRestoreInProgress is a ReasonRestoreCompleted indicating that the restore operation is in progress. - ReasonRestoreOperationInProgress ReasonRestoreCompleted = "RestoreInProgress" - - // ReasonRestoreOperationCompleted is a ReasonRestoreCompleted indicating that the restore operation has completed successfully. - ReasonRestoreOperationCompleted ReasonRestoreCompleted = "RestoreCompleted" - - // ReasonDryRunOperationCompleted is a ReasonRestoreCompleted indicating that the restore dry run operation has completed successfully. - ReasonDryRunOperationCompleted ReasonRestoreCompleted = "RestoreDryRunCompleted" - - // ReasonRestoreOperationFailed is a ReasonRestoreCompleted indicating that operation has failed. - ReasonRestoreOperationFailed ReasonRestoreCompleted = "RestoreFailed" -) - -// ReasonCloneCompleted represents specific reasons for the 'CloneCompleted' condition type. -type ReasonCloneCompleted string - -func (r ReasonCloneCompleted) String() string { - return string(r) -} - -const ( - // ReasonCloneOperationInProgress is a ReasonCloneCompleted indicating that the clone operation is in progress. - ReasonCloneOperationInProgress ReasonCloneCompleted = "CloneInProgress" - - // ReasonCloneOperationCompleted is a ReasonCloneCompleted indicating that the clone operation has completed successfully. - ReasonCloneOperationCompleted ReasonCloneCompleted = "CloneCompleted" - - // ReasonCloneOperationFailed is a ReasonCloneCompleted indicating that clone operation has failed. - ReasonCloneOperationFailed ReasonCloneCompleted = "CloneFailed" -) - // ReasonCompleted represents specific reasons for the 'SignalSent' condition type. type ReasonSignalSent string diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/common/common.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/common/common.go deleted file mode 100644 index b01e6ab86e..0000000000 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/common/common.go +++ /dev/null @@ -1,50 +0,0 @@ -/* -Copyright 2025 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 common - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/service" - "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" -) - -func SetPhaseConditionToFailed(cb *conditions.ConditionBuilder, phase *v1alpha2.VMOPPhase, err error) { - *phase = v1alpha2.VMOPPhaseFailed - cb. - Status(metav1.ConditionFalse). - Reason(vmopcondition.ReasonRestoreOperationFailed). - Message(service.CapitalizeFirstLetter(err.Error()) + ".") -} - -func SetPhaseCloneConditionToFailed(cb *conditions.ConditionBuilder, phase *v1alpha2.VMOPPhase, err error) { - *phase = v1alpha2.VMOPPhaseFailed - cb. - Status(metav1.ConditionFalse). - Reason(vmopcondition.ReasonCloneOperationFailed). - Message(service.CapitalizeFirstLetter(err.Error()) + ".") -} - -func SetPhaseConditionCompleted(cb *conditions.ConditionBuilder, phase *v1alpha2.VMOPPhase, reason vmopcondition.ReasonRestoreCompleted, msg string) { - *phase = v1alpha2.VMOPPhaseCompleted - cb. - Status(metav1.ConditionTrue). - Reason(reason). - Message(service.CapitalizeFirstLetter(msg) + ".") -} diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/handler/lifecycle.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/handler/lifecycle.go index 25e70675dd..f9c2273649 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/handler/lifecycle.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/handler/lifecycle.go @@ -18,18 +18,14 @@ package handler import ( "context" - "fmt" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/reconcile" commonvmop "github.com/deckhouse/virtualization-controller/pkg/common/vmop" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" genericservice "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/service" - "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/service" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" - "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" ) @@ -66,16 +62,22 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach return reconcile.Result{}, nil } - svcOp, err := h.svcOpCreator(vmop) - if err != nil { - return reconcile.Result{}, err + // Ignore if VMOP is in final state or failed. + if vmop.Status.Phase == v1alpha2.VMOPPhaseCompleted || vmop.Status.Phase == v1alpha2.VMOPPhaseFailed { + return reconcile.Result{}, nil } - // Ignore if VMOP is in final state. - if complete, _ := svcOp.IsComplete(); complete { + completed, completedFound := conditions.GetCondition(vmopcondition.TypeCompleted, vmop.Status.Conditions) + if completedFound && completed.Status == metav1.ConditionTrue { + vmop.Status.Phase = v1alpha2.VMOPPhaseCompleted return reconcile.Result{}, nil } + svcOp, err := h.svcOpCreator(vmop) + if err != nil { + return reconcile.Result{}, err + } + // 1.Initialize new VMOP resource: set phase to Pending and all conditions to Unknown. h.base.Init(vmop) @@ -87,8 +89,8 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach // 3. Operation already in progress. Check if the operation is completed. // Run execute until the operation is completed. - if svcOp.IsInProgress() { - return h.execute(ctx, vmop, svcOp) + if _, found := conditions.GetCondition(vmopcondition.TypeCompleted, vmop.Status.Conditions); found { + return svcOp.Execute(ctx) } // 4. VMOP is not in progress. @@ -108,46 +110,10 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach } // 6. The Operation is valid, and can be executed. - return h.execute(ctx, vmop, svcOp) + vmop.Status.Phase = v1alpha2.VMOPPhaseInProgress + return svcOp.Execute(ctx) } func (h LifecycleHandler) Name() string { return lifecycleHandlerName } - -func (h LifecycleHandler) execute(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation, svcOp service.Operation) (rec reconcile.Result, err error) { - log := logger.FromContext(ctx) - - completedCond := conditions.NewConditionBuilder(vmopcondition.TypeCompleted).Generation(vmop.GetGeneration()) - rec, err = svcOp.Execute(ctx) - if err != nil { - failMsg := fmt.Sprintf("%s is failed", vmop.Spec.Type) - log.Debug(failMsg, logger.SlogErr(err)) - failMsg = fmt.Errorf("%s: %w", failMsg, err).Error() - h.recorder.Event(vmop, corev1.EventTypeWarning, v1alpha2.ReasonErrVMOPFailed, failMsg) - - vmop.Status.Phase = v1alpha2.VMOPPhaseFailed - conditions.SetCondition(completedCond.Reason(vmopcondition.ReasonOperationFailed).Message(failMsg).Status(metav1.ConditionFalse), &vmop.Status.Conditions) - } else { - vmop.Status.Phase = v1alpha2.VMOPPhaseInProgress - reason := svcOp.GetInProgressReason() - conditions.SetCondition(completedCond.Reason(reason).Message("Wait for operation to complete.").Status(metav1.ConditionFalse), &vmop.Status.Conditions) - } - - isComplete, failMsg := svcOp.IsComplete() - if isComplete { - if failMsg != "" { - vmop.Status.Phase = v1alpha2.VMOPPhaseFailed - h.recorder.Event(vmop, corev1.EventTypeWarning, v1alpha2.ReasonErrVMOPFailed, failMsg) - - conditions.SetCondition(completedCond.Reason(vmopcondition.ReasonOperationFailed).Message(failMsg).Status(metav1.ConditionFalse), &vmop.Status.Conditions) - } else { - vmop.Status.Phase = v1alpha2.VMOPPhaseCompleted - h.recorder.Event(vmop, corev1.EventTypeNormal, v1alpha2.ReasonVMOPSucceeded, "VirtualMachineOperation completed") - - conditions.SetCondition(completedCond.Reason(vmopcondition.ReasonOperationCompleted).Message("VirtualMachineOperation succeeded.").Status(metav1.ConditionTrue), &vmop.Status.Conditions) - } - } - - return rec, err -} diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go index 86911c3216..d3f9463c84 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go @@ -20,6 +20,7 @@ import ( "context" "fmt" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -28,9 +29,9 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/common/steptaker" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/step" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" + "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" ) @@ -50,21 +51,10 @@ type CloneOperation struct { } func (o CloneOperation) Execute(ctx context.Context) (reconcile.Result, error) { - cb := conditions.NewConditionBuilder(vmopcondition.TypeCloneCompleted) - defer func() { conditions.SetCondition(cb.Generation(o.vmop.Generation), &o.vmop.Status.Conditions) }() - - cond, exist := conditions.GetCondition(vmopcondition.TypeCloneCompleted, o.vmop.Status.Conditions) - if exist { - if cond.Status == metav1.ConditionUnknown { - cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonCloneOperationInProgress) - } else { - cb.Status(cond.Status).Reason(vmopcondition.ReasonRestoreCompleted(cond.Reason)).Message(cond.Message) - } - } + log := logger.FromContext(ctx) if o.vmop.Spec.Clone == nil { err := fmt.Errorf("clone specification is mandatory to start cloning") - cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonCloneOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } @@ -72,22 +62,40 @@ func (o CloneOperation) Execute(ctx context.Context) (reconcile.Result, error) { vm, err := object.FetchObject(ctx, vmKey, o.client, &v1alpha2.VirtualMachine{}) if err != nil { err := fmt.Errorf("failed to fetch the virtual machine %q: %w", vmKey.Name, err) - cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonCloneOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } if vm == nil { err := fmt.Errorf("virtual machine specified is not found") - cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonCloneOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } - return steptaker.NewStepTakers( - step.NewCleanupSnapshotStep(o.client, o.recorder, cb), - step.NewCreateSnapshotStep(o.client, o.recorder, cb), - step.NewVMSnapshotReadyStep(o.client, cb), - step.NewProcessCloneStep(o.client, o.recorder, cb), + o.vmop.Status.Phase = v1alpha2.VMOPPhaseInProgress + + result, err := steptaker.NewStepTakers( + step.NewCreateSnapshotStep(o.client, o.recorder), + step.NewVMSnapshotReadyStep(o.client), + step.NewProcessCloneStep(o.client, o.recorder), + step.NewWaitingDisksStep(o.client, o.recorder), + step.NewCleanupSnapshotStep(o.client, o.recorder), + step.NewCompletedStep(o.recorder), ).Run(ctx, o.vmop) + if err != nil { + failMsg := fmt.Sprintf("%s is failed", o.vmop.Spec.Type) + log.Debug(failMsg, logger.SlogErr(err)) + failMsg = fmt.Errorf("%s: %w", failMsg, err).Error() + o.recorder.Event(o.vmop, corev1.EventTypeWarning, v1alpha2.ReasonErrVMOPFailed, failMsg) + + o.vmop.Status.Phase = v1alpha2.VMOPPhaseFailed + conditions.SetCondition( + conditions.NewConditionBuilder(vmopcondition.TypeCompleted). + Reason(vmopcondition.ReasonOperationFailed). + Message(failMsg).Status(metav1.ConditionFalse), + &o.vmop.Status.Conditions, + ) + } + + return result, err } func (o CloneOperation) IsApplicableForVMPhase(phase v1alpha2.MachinePhase) bool { @@ -101,35 +109,3 @@ func (o CloneOperation) IsApplicableForRunPolicy(runPolicy v1alpha2.RunPolicy) b func (o CloneOperation) GetInProgressReason() vmopcondition.ReasonCompleted { return vmopcondition.ReasonCloneInProgress } - -func (o CloneOperation) IsInProgress() bool { - snapshotCondition, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, o.vmop.Status.Conditions) - if found && snapshotCondition.Status != metav1.ConditionUnknown { - return true - } - - cloneCondition, found := conditions.GetCondition(vmopcondition.TypeCloneCompleted, o.vmop.Status.Conditions) - if found && cloneCondition.Status != metav1.ConditionUnknown { - return true - } - - return false -} - -func (o CloneOperation) IsComplete() (bool, string) { - cloneCondition, ok := conditions.GetCondition(vmopcondition.TypeCloneCompleted, o.vmop.Status.Conditions) - if !ok { - return false, "" - } - - snapshotCondition, ok := conditions.GetCondition(vmopcondition.TypeSnapshotReady, o.vmop.Status.Conditions) - if !ok { - return false, "" - } - - if cloneCondition.Reason == string(vmopcondition.ReasonCloneOperationFailed) && snapshotCondition.Reason == string(vmopcondition.ReasonSnapshotCleanedUp) { - return true, cloneCondition.Message - } - - return cloneCondition.Status == metav1.ConditionTrue && snapshotCondition.Reason == string(vmopcondition.ReasonSnapshotCleanedUp), "" -} diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/operation.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/operation.go index a012c58a2a..e118f5fa57 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/operation.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/operation.go @@ -33,8 +33,6 @@ type Operation interface { IsApplicableForVMPhase(phase v1alpha2.MachinePhase) bool IsApplicableForRunPolicy(runPolicy v1alpha2.RunPolicy) bool GetInProgressReason() vmopcondition.ReasonCompleted - IsInProgress() bool - IsComplete() (bool, string) } func NewOperationService(client client.Client, recorder eventrecord.EventRecorderLogger, vmop *v1alpha2.VirtualMachineOperation) (Operation, error) { diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/restore.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/restore.go index ea7fd58d76..3ca3bb229a 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/restore.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/restore.go @@ -20,6 +20,7 @@ import ( "context" "fmt" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -28,9 +29,9 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/common/steptaker" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/step" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" + "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" ) @@ -50,27 +51,15 @@ type RestoreOperation struct { } func (o RestoreOperation) Execute(ctx context.Context) (reconcile.Result, error) { - cb := conditions.NewConditionBuilder(vmopcondition.TypeRestoreCompleted) - defer func() { conditions.SetCondition(cb.Generation(o.vmop.Generation), &o.vmop.Status.Conditions) }() - - cond, exist := conditions.GetCondition(vmopcondition.TypeRestoreCompleted, o.vmop.Status.Conditions) - if exist { - if cond.Status == metav1.ConditionUnknown { - cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonRestoreOperationInProgress) - } else { - cb.Status(cond.Status).Reason(vmopcondition.ReasonRestoreCompleted(cond.Reason)).Message(cond.Message) - } - } + log := logger.FromContext(ctx) if o.vmop.Spec.Restore == nil { err := fmt.Errorf("restore specification is nil") - cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonRestoreOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } if o.vmop.Spec.Restore.VirtualMachineSnapshotName == "" { err := fmt.Errorf("virtual machine snapshot name is required") - cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonRestoreOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } @@ -78,22 +67,40 @@ func (o RestoreOperation) Execute(ctx context.Context) (reconcile.Result, error) vm, err := object.FetchObject(ctx, vmKey, o.client, &v1alpha2.VirtualMachine{}) if err != nil { err := fmt.Errorf("failed to fetch the virtual machine %q: %w", vmKey.Name, err) - cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonRestoreOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } if vm == nil { err := fmt.Errorf("virtual machine is nil") - cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonRestoreOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } - return steptaker.NewStepTakers( - step.NewVMSnapshotReadyStep(o.client, cb), - step.NewEnterMaintenanceStep(o.client, o.recorder, cb), - step.NewProcessRestoreStep(o.client, o.recorder, cb), - step.NewExitMaintenanceStep(o.client, o.recorder, cb), + o.vmop.Status.Phase = v1alpha2.VMOPPhaseInProgress + + result, err := steptaker.NewStepTakers( + step.NewVMSnapshotReadyStep(o.client), + step.NewEnterMaintenanceStep(o.client, o.recorder), + step.NewProcessRestoreStep(o.client, o.recorder), + step.NewWaitingDisksStep(o.client, o.recorder), + step.NewExitMaintenanceStep(o.client, o.recorder), + step.NewCompletedStep(o.recorder), ).Run(ctx, o.vmop) + if err != nil { + failMsg := fmt.Sprintf("%s is failed", o.vmop.Spec.Type) + log.Debug(failMsg, logger.SlogErr(err)) + failMsg = fmt.Errorf("%s: %w", failMsg, err).Error() + o.recorder.Event(o.vmop, corev1.EventTypeWarning, v1alpha2.ReasonErrVMOPFailed, failMsg) + + o.vmop.Status.Phase = v1alpha2.VMOPPhaseFailed + conditions.SetCondition( + conditions.NewConditionBuilder(vmopcondition.TypeCompleted). + Reason(vmopcondition.ReasonOperationFailed). + Message(failMsg).Status(metav1.ConditionFalse), + &o.vmop.Status.Conditions, + ) + } + + return result, err } func (o RestoreOperation) IsApplicableForVMPhase(phase v1alpha2.MachinePhase) bool { @@ -107,35 +114,3 @@ func (o RestoreOperation) IsApplicableForRunPolicy(runPolicy v1alpha2.RunPolicy) func (o RestoreOperation) GetInProgressReason() vmopcondition.ReasonCompleted { return vmopcondition.ReasonRestoreInProgress } - -func (o RestoreOperation) IsInProgress() bool { - maintenanceModeCondition, found := conditions.GetCondition(vmopcondition.TypeMaintenanceMode, o.vmop.Status.Conditions) - if found && maintenanceModeCondition.Status != metav1.ConditionUnknown { - return true - } - - completedCondition, found := conditions.GetCondition(vmopcondition.TypeRestoreCompleted, o.vmop.Status.Conditions) - if found && completedCondition.Status != metav1.ConditionUnknown { - return true - } - - return false -} - -func (o RestoreOperation) IsComplete() (bool, string) { - rc, ok := conditions.GetCondition(vmopcondition.TypeRestoreCompleted, o.vmop.Status.Conditions) - if !ok { - return false, "" - } - - if o.vmop.Spec.Restore.Mode == v1alpha2.SnapshotOperationModeDryRun { - return rc.Status == metav1.ConditionTrue, "" - } - - mc, ok := conditions.GetCondition(vmopcondition.TypeMaintenanceMode, o.vmop.Status.Conditions) - if !ok { - return false, "" - } - - return rc.Status == metav1.ConditionTrue && mc.Status == metav1.ConditionFalse, "" -} diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go index fc77e368da..a9bc047a84 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go @@ -38,27 +38,32 @@ import ( type CleanupSnapshotStep struct { client client.Client recorder eventrecord.EventRecorderLogger - cb *conditions.ConditionBuilder } func NewCleanupSnapshotStep( client client.Client, recorder eventrecord.EventRecorderLogger, - cb *conditions.ConditionBuilder, ) *CleanupSnapshotStep { return &CleanupSnapshotStep{ client: client, recorder: recorder, - cb: cb, } } func (s CleanupSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { - canProceed := canProceedWithCleanup(vmop) - if !canProceed { + rcb := conditions.NewConditionBuilder(vmopcondition.TypeSnapshotReady) + + snapshotCondition, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + if found && snapshotCondition.Reason == string(vmopcondition.ReasonSnapshotCleanedUp) { return nil, nil } + for _, status := range vmop.Status.Resources { + if status.Status != v1alpha2.SnapshotResourceStatusCompleted { + return nil, nil + } + } + snapshotName, ok := vmop.Annotations[annotations.AnnVMOPSnapshotName] if !ok { return nil, nil @@ -94,44 +99,5 @@ func (s CleanupSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMac &vmop.Status.Conditions, ) - return &reconcile.Result{}, nil -} - -const ( - NoCleanup = false - Cleanup = true -) - -func canProceedWithCleanup(vmop *v1alpha2.VirtualMachineOperation) bool { - // Do not clean up if vmop is marked as "snapshot was cleaned before". - snapshotCondition, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) - if found && snapshotCondition.Reason == string(vmopcondition.ReasonSnapshotCleanedUp) { - return NoCleanup - } - - cloneCondition, found := conditions.GetCondition(vmopcondition.TypeCloneCompleted, vmop.Status.Conditions) - // Do not clean up in uncertain states of the clone condition. - if !found || cloneCondition.Status == metav1.ConditionUnknown { - return NoCleanup - } - - switch cloneCondition.Reason { - case string(vmopcondition.ReasonCloneOperationInProgress): - // No clean up while clone is in progress. - return NoCleanup - case string(vmopcondition.ReasonCloneOperationCompleted): - // Cleanup if definitely completed. - return cloneCondition.Status == metav1.ConditionTrue - case string(vmopcondition.ReasonCloneOperationFailed): - // Should clean up, but also check resources ... - } - - // Do not clean up if some resources are still in progress. - for _, status := range vmop.Status.Resources { - if status.Status == v1alpha2.SnapshotResourceStatusInProgress { - return NoCleanup - } - } - - return Cleanup + return nil, nil } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go new file mode 100644 index 0000000000..4f5cc836bb --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go @@ -0,0 +1,50 @@ +/* +Copyright 2025 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 step + +import ( + "context" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "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" +) + +type CompletedStep struct { + recorder eventrecord.EventRecorderLogger +} + +func NewCompletedStep( + recorder eventrecord.EventRecorderLogger, +) *CompletedStep { + return &CompletedStep{ + recorder: recorder, + } +} + +func (s CompletedStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { + cb := conditions.NewConditionBuilder(vmopcondition.TypeCompleted).Status(metav1.ConditionTrue).Reason(vmopcondition.ReasonOperationCompleted) + conditions.SetCondition(cb, &vmop.Status.Conditions) + vmop.Status.Phase = v1alpha2.VMOPPhaseCompleted + s.recorder.Event(vmop, corev1.EventTypeNormal, v1alpha2.ReasonVMOPSucceeded, "VirtualMachineOperation is successful completed") + return &reconcile.Result{}, nil +} diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step.go index 1d625c47c7..93ae64a5fd 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step.go @@ -30,7 +30,6 @@ import ( "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/controller/vmop/snapshot/internal/common" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" @@ -40,14 +39,12 @@ import ( type CreateSnapshotStep struct { client client.Client recorder eventrecord.EventRecorderLogger - cb *conditions.ConditionBuilder } -func NewCreateSnapshotStep(client client.Client, recorder eventrecord.EventRecorderLogger, cb *conditions.ConditionBuilder) *CreateSnapshotStep { +func NewCreateSnapshotStep(client client.Client, recorder eventrecord.EventRecorderLogger) *CreateSnapshotStep { return &CreateSnapshotStep{ client: client, recorder: recorder, - cb: cb, } } @@ -55,8 +52,8 @@ func (s CreateSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach rcb := conditions.NewConditionBuilder(vmopcondition.TypeSnapshotReady) if snapshotCondition, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions); found { - if snapshotCondition.Reason == string(vmopcondition.ReasonSnapshotCleanedUp) { - return &reconcile.Result{}, nil + if snapshotCondition.Status == metav1.ConditionTrue || snapshotCondition.Reason == string(vmopcondition.ReasonSnapshotCleanedUp) { + return nil, nil } } @@ -64,7 +61,6 @@ func (s CreateSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach vmSnapshotKey := types.NamespacedName{Namespace: vmop.Namespace, Name: snapshotName} vmSnapshot, err := object.FetchObject(ctx, vmSnapshotKey, s.client, &v1alpha2.VirtualMachineSnapshot{}) if err != nil { - common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } @@ -77,7 +73,6 @@ func (s CreateSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach ) vmsReadyCondition, _ := conditions.GetCondition(vmscondition.VirtualMachineSnapshotReadyType, vmSnapshot.Status.Conditions) err = fmt.Errorf("virtual machine %q have invalid state: %s", vmop.Spec.VirtualMachine, vmsReadyCondition.Message) - common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, fmt.Errorf("virtual machine snapshot %q is in failed phase: %w. Try again with new VMOP Clone operation", vmSnapshotKey.Name, err) case v1alpha2.VirtualMachineSnapshotPhaseReady: conditions.SetCondition( @@ -98,7 +93,6 @@ func (s CreateSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach var snapshotList v1alpha2.VirtualMachineSnapshotList err := s.client.List(ctx, &snapshotList, client.InNamespace(vmop.Namespace)) if err != nil { - common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } @@ -146,7 +140,6 @@ func (s CreateSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach err = s.client.Create(ctx, snapshot) if err != nil { - common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, fmt.Errorf("failed to create VirtualMachineSnapshot: %w", err) } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go index 923bf715ed..b1678dd900 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go @@ -29,7 +29,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/common" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" @@ -39,18 +38,15 @@ import ( type EnterMaintenanceStep struct { client client.Client recorder eventrecord.EventRecorderLogger - cb *conditions.ConditionBuilder } func NewEnterMaintenanceStep( client client.Client, recorder eventrecord.EventRecorderLogger, - cb *conditions.ConditionBuilder, ) *EnterMaintenanceStep { return &EnterMaintenanceStep{ client: client, recorder: recorder, - cb: cb, } } @@ -59,16 +55,16 @@ func (s EnterMaintenanceStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMa return nil, nil } + if vmop.Status.Resources != nil { + return nil, nil + } + vmKey := types.NamespacedName{Namespace: vmop.Namespace, Name: vmop.Spec.VirtualMachine} vm, err := object.FetchObject(ctx, vmKey, s.client, &v1alpha2.VirtualMachine{}) if err != nil { return nil, fmt.Errorf("failed to fetch the virtual machine %q: %w", vmKey.Name, err) } - if s.cb.Condition().Status == metav1.ConditionTrue { - return nil, nil - } - maintenanceCondition, found := conditions.GetCondition(vmcondition.TypeMaintenance, vm.Status.Conditions) if found && maintenanceCondition.Status == metav1.ConditionTrue && maintenanceCondition.Reason == vmcondition.ReasonMaintenanceRestore.String() { if vm.Status.Phase != v1alpha2.MachineStopped && vm.Status.Phase != v1alpha2.MachinePending { @@ -103,7 +99,6 @@ func (s EnterMaintenanceStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMa } s.recorder.Event(vmop, corev1.EventTypeWarning, v1alpha2.ReasonErrVMOPFailed, "Failed to enter maintenance mode: "+err.Error()) - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/exit_maintenance_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/exit_maintenance_step.go index b8a9ad79db..0ce2d8e8e7 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/exit_maintenance_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/exit_maintenance_step.go @@ -29,7 +29,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/common" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" @@ -39,30 +38,21 @@ import ( type ExitMaintenanceStep struct { client client.Client recorder eventrecord.EventRecorderLogger - cb *conditions.ConditionBuilder } func NewExitMaintenanceStep( client client.Client, recorder eventrecord.EventRecorderLogger, - cb *conditions.ConditionBuilder, ) *ExitMaintenanceStep { return &ExitMaintenanceStep{ client: client, recorder: recorder, - cb: cb, } } func (s ExitMaintenanceStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { if vmop.Spec.Restore.Mode == v1alpha2.SnapshotOperationModeDryRun { - return &reconcile.Result{}, nil - } - - for _, status := range vmop.Status.Resources { - if status.Status == v1alpha2.SnapshotResourceStatusInProgress { - return &reconcile.Result{}, nil - } + return nil, nil } vmKey := types.NamespacedName{Namespace: vmop.Namespace, Name: vmop.Spec.VirtualMachine} @@ -76,11 +66,6 @@ func (s ExitMaintenanceStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMac return &reconcile.Result{}, nil } - restoreCondition, found := conditions.GetCondition(vmopcondition.TypeRestoreCompleted, vmop.Status.Conditions) - if !found || restoreCondition.Status == metav1.ConditionFalse { - return &reconcile.Result{}, nil - } - // If a VM has a maintenance condition, set it to false. maintenanceVMCondition, maintenanceVMConditionFound := conditions.GetCondition(vmcondition.TypeMaintenance, vm.Status.Conditions) if maintenanceVMConditionFound && maintenanceVMCondition.Status == metav1.ConditionTrue { @@ -105,7 +90,6 @@ func (s ExitMaintenanceStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMac v1alpha2.ReasonErrVMOPFailed, "Failed to exit maintenance mode: "+err.Error(), ) - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } } @@ -125,5 +109,5 @@ func (s ExitMaintenanceStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMac ) } - return &reconcile.Result{}, nil + return nil, nil } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/final_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/final_step_test.go new file mode 100644 index 0000000000..1c006eefbb --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/final_step_test.go @@ -0,0 +1,83 @@ +/* +Copyright 2025 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 step + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/deckhouse/virtualization-controller/pkg/eventrecord" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +var _ = Describe("CompletedStep", func() { + var ( + ctx context.Context + recorder *eventrecord.EventRecorderLoggerMock + step *CompletedStep + ) + + BeforeEach(func() { + ctx = context.Background() + recorder = newNoOpRecorder() + }) + + It("should set completed phase and condition", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + + step = NewCompletedStep(recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(vmop.Status.Phase).To(Equal(v1alpha2.VMOPPhaseCompleted)) + }) + + It("should be idempotent - multiple calls produce same result", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + + step = NewCompletedStep(recorder) + + result1, err1 := step.Take(ctx, vmop) + phase1 := vmop.Status.Phase + conditions1 := len(vmop.Status.Conditions) + + result2, err2 := step.Take(ctx, vmop) + phase2 := vmop.Status.Phase + conditions2 := len(vmop.Status.Conditions) + + result3, err3 := step.Take(ctx, vmop) + phase3 := vmop.Status.Phase + conditions3 := len(vmop.Status.Conditions) + + Expect(err1).NotTo(HaveOccurred()) + Expect(err2).NotTo(HaveOccurred()) + Expect(err3).NotTo(HaveOccurred()) + Expect(result1).NotTo(BeNil()) + Expect(result2).NotTo(BeNil()) + Expect(result3).NotTo(BeNil()) + + Expect(phase1).To(Equal(phase2)) + Expect(phase2).To(Equal(phase3)) + Expect(phase1).To(Equal(v1alpha2.VMOPPhaseCompleted)) + + Expect(conditions1).To(Equal(conditions2)) + Expect(conditions2).To(Equal(conditions3)) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go index 4c6ccdeed5..47cd929427 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go @@ -21,79 +21,63 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "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/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/service/restorer" - "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/common" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" ) type ProcessCloneStep struct { client client.Client recorder eventrecord.EventRecorderLogger - cb *conditions.ConditionBuilder } func NewProcessCloneStep( client client.Client, recorder eventrecord.EventRecorderLogger, - cb *conditions.ConditionBuilder, ) *ProcessCloneStep { return &ProcessCloneStep{ client: client, recorder: recorder, - cb: cb, } } func (s ProcessCloneStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { - c, exist := conditions.GetCondition(s.cb.GetType(), vmop.Status.Conditions) - if exist { - if c.Status == metav1.ConditionTrue { - return &reconcile.Result{}, nil - } - - snapshotReadyCondition, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) - if found && snapshotReadyCondition.Status == metav1.ConditionFalse { - return &reconcile.Result{}, nil - } - } - snapshotName, ok := vmop.Annotations[annotations.AnnVMOPSnapshotName] if !ok { err := fmt.Errorf("snapshot name annotation not found") - common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } vmSnapshotKey := types.NamespacedName{Namespace: vmop.Namespace, Name: snapshotName} vmSnapshot, err := object.FetchObject(ctx, vmSnapshotKey, s.client, &v1alpha2.VirtualMachineSnapshot{}) if err != nil { - common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } + if vmSnapshot == nil { + return &reconcile.Result{}, nil + } + restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} restorerSecret, err := object.FetchObject(ctx, restorerSecretKey, s.client, &corev1.Secret{}) if err != nil { - common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } + if restorerSecret == nil { + return &reconcile.Result{}, nil + } + snapshotResources := restorer.NewSnapshotResources(s.client, v1alpha2.VMOPTypeClone, vmop.Spec.Clone.Mode, restorerSecret, vmSnapshot, string(vmop.UID)) err = snapshotResources.Prepare(ctx) if err != nil { - common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } @@ -109,55 +93,18 @@ func (s ProcessCloneStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachin statuses, err := snapshotResources.Validate(ctx) vmop.Status.Resources = statuses if err != nil { - s.cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonCloneOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) - return &reconcile.Result{}, nil + return &reconcile.Result{}, err } if vmop.Spec.Clone.Mode == v1alpha2.SnapshotOperationModeDryRun { - s.cb.Status(metav1.ConditionTrue).Reason(vmopcondition.ReasonCloneOperationCompleted).Message("The virtual machine can be cloned from the snapshot.") return &reconcile.Result{}, nil } statuses, err = snapshotResources.Process(ctx) vmop.Status.Resources = statuses if err != nil { - common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } - for _, status := range vmop.Status.Resources { - if status.Kind != v1alpha2.VirtualDiskKind { - continue - } - - var vd v1alpha2.VirtualDisk - vdKey := types.NamespacedName{Namespace: vmop.Namespace, Name: status.Name} - err := s.client.Get(ctx, vdKey, &vd) - if err != nil { - return &reconcile.Result{}, fmt.Errorf("failed to get the `VirtualDisk`: %w", err) - } - - if vd.Annotations[annotations.AnnVMOPRestore] != string(vmop.UID) { - return &reconcile.Result{}, nil - } - - switch vd.Status.Phase { - case v1alpha2.DiskFailed: - // Clone can't proceed if disk is failed. - err = fmt.Errorf("virtual disk %q is in failed phase", vdKey.Name) - conditions.SetCondition(s.cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonSnapshotFailed).Message(service.CapitalizeFirstLetter(err.Error())), &vmop.Status.Conditions) - common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) - return &reconcile.Result{}, fmt.Errorf("virtual disk %q is in failed phase", vdKey.Name) - case v1alpha2.DiskReady: - // Disk is Ready, check the next one. - continue - default: - s.cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonCloneOperationInProgress).Message("Wait for VirtualDisks to be Ready.") - return &reconcile.Result{}, nil - } - } - - // All disks are Ready, mark Clone operation as Completed. - s.cb.Status(metav1.ConditionTrue).Reason(vmopcondition.ReasonCloneOperationCompleted).Message("The virtual machine has been cloned successfully.") - return &reconcile.Result{}, nil + return nil, nil } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go new file mode 100644 index 0000000000..11f59c9364 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go @@ -0,0 +1,115 @@ +/* +Copyright 2025 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 step + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + "github.com/deckhouse/virtualization-controller/pkg/common/testutil" + "github.com/deckhouse/virtualization-controller/pkg/eventrecord" +) + +var _ = Describe("ProcessCloneStep", func() { + var ( + ctx context.Context + fakeClient client.WithWatch + recorder *eventrecord.EventRecorderLoggerMock + step *ProcessCloneStep + ) + + BeforeEach(func() { + ctx = context.Background() + recorder = newNoOpRecorder() + }) + + Describe("Snapshot annotation check", func() { + It("should return error when snapshot annotation is not found", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + delete(vmop.Annotations, annotations.AnnVMOPSnapshotName) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessCloneStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("snapshot name annotation not found")) + Expect(result).NotTo(BeNil()) + }) + + It("should be idempotent - multiple calls with missing annotation return same error", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + delete(vmop.Annotations, annotations.AnnVMOPSnapshotName) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessCloneStep(fakeClient, recorder) + + _, err1 := step.Take(ctx, vmop) + _, err2 := step.Take(ctx, vmop) + _, err3 := step.Take(ctx, vmop) + + Expect(err1).To(HaveOccurred()) + Expect(err2).To(HaveOccurred()) + Expect(err3).To(HaveOccurred()) + Expect(err1.Error()).To(Equal(err2.Error())) + Expect(err2.Error()).To(Equal(err3.Error())) + }) + }) + + Describe("Snapshot not found", func() { + It("should wait when snapshot is not found", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessCloneStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + }) + }) + + Describe("Secret not found", func() { + It("should wait when restorer secret is not found", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessCloneStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + }) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go similarity index 64% rename from images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_step.go rename to images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go index eff8387e24..df191d4243 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go @@ -28,7 +28,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service/restorer" - "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/common" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" @@ -37,83 +36,66 @@ import ( type ProcessRestoreStep struct { client client.Client recorder eventrecord.EventRecorderLogger - cb *conditions.ConditionBuilder } func NewProcessRestoreStep( client client.Client, recorder eventrecord.EventRecorderLogger, - cb *conditions.ConditionBuilder, ) *ProcessRestoreStep { return &ProcessRestoreStep{ client: client, recorder: recorder, - cb: cb, } } func (s ProcessRestoreStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { - c, exist := conditions.GetCondition(s.cb.GetType(), vmop.Status.Conditions) - if exist { - if c.Status == metav1.ConditionTrue { - return nil, nil - } - - maintenanceModeCondition, found := conditions.GetCondition(vmopcondition.TypeMaintenanceMode, vmop.Status.Conditions) - if found && maintenanceModeCondition.Status == metav1.ConditionFalse { - return &reconcile.Result{}, nil - } + maintenanceModeCondition, found := conditions.GetCondition(vmopcondition.TypeMaintenanceMode, vmop.Status.Conditions) + if !found || maintenanceModeCondition.Status == metav1.ConditionFalse { + return &reconcile.Result{}, nil } vmSnapshotKey := types.NamespacedName{Namespace: vmop.Namespace, Name: vmop.Spec.Restore.VirtualMachineSnapshotName} vmSnapshot, err := object.FetchObject(ctx, vmSnapshotKey, s.client, &v1alpha2.VirtualMachineSnapshot{}) if err != nil { - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } + if vmSnapshot == nil { + return &reconcile.Result{}, nil + } + restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} restorerSecret, err := object.FetchObject(ctx, restorerSecretKey, s.client, &corev1.Secret{}) if err != nil { - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } + if restorerSecret == nil { + return &reconcile.Result{}, nil + } + snapshotResources := restorer.NewSnapshotResources(s.client, v1alpha2.VMOPTypeRestore, vmop.Spec.Restore.Mode, restorerSecret, vmSnapshot, string(vmop.UID)) err = snapshotResources.Prepare(ctx) if err != nil { - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } statuses, err := snapshotResources.Validate(ctx) vmop.Status.Resources = statuses if err != nil { - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } if vmop.Spec.Restore.Mode == v1alpha2.SnapshotOperationModeDryRun { - s.cb.Status(metav1.ConditionTrue).Reason(vmopcondition.ReasonRestoreOperationCompleted).Message("The virtual machine can be restored from the snapshot.") - common.SetPhaseConditionCompleted(s.cb, &vmop.Status.Phase, vmopcondition.ReasonDryRunOperationCompleted, "The virtual machine can be restored from the snapshot") - return &reconcile.Result{}, nil + return nil, nil } statuses, err = snapshotResources.Process(ctx) vmop.Status.Resources = statuses if err != nil { - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } - for _, status := range statuses { - if status.Status == v1alpha2.SnapshotResourceStatusInProgress { - return &reconcile.Result{}, nil - } - } - - s.cb.Status(metav1.ConditionTrue).Reason(vmopcondition.ReasonRestoreOperationCompleted).Message("The virtual machine has been restored from the snapshot successfully.") - - return &reconcile.Result{}, nil + return nil, nil } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go new file mode 100644 index 0000000000..292c130c70 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go @@ -0,0 +1,134 @@ +/* +Copyright 2025 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 step + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/deckhouse/virtualization-controller/pkg/common/testutil" + "github.com/deckhouse/virtualization-controller/pkg/eventrecord" +) + +var _ = Describe("ProcessRestoreStep", func() { + var ( + ctx context.Context + fakeClient client.WithWatch + recorder *eventrecord.EventRecorderLoggerMock + step *ProcessRestoreStep + ) + + BeforeEach(func() { + ctx = context.Background() + recorder = newNoOpRecorder() + }) + + Describe("Maintenance mode check", func() { + It("should wait when maintenance condition is not found", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessRestoreStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + }) + + It("should wait when maintenance condition is false", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + setMaintenanceCondition(vmop, metav1.ConditionFalse) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessRestoreStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + }) + + It("should be idempotent - multiple calls with maintenance false return same result", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + setMaintenanceCondition(vmop, metav1.ConditionFalse) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessRestoreStep(fakeClient, recorder) + + result1, err1 := step.Take(ctx, vmop) + result2, err2 := step.Take(ctx, vmop) + result3, err3 := step.Take(ctx, vmop) + + Expect(err1).NotTo(HaveOccurred()) + Expect(err2).NotTo(HaveOccurred()) + Expect(err3).NotTo(HaveOccurred()) + Expect(result1).NotTo(BeNil()) + Expect(result2).NotTo(BeNil()) + Expect(result3).NotTo(BeNil()) + }) + }) + + Describe("Snapshot not found", func() { + It("should wait when snapshot is not found", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + setMaintenanceCondition(vmop, metav1.ConditionTrue) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessRestoreStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + }) + }) + + Describe("Secret not found", func() { + It("should wait when restorer secret is not found", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + setMaintenanceCondition(vmop, metav1.ConditionTrue) + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessRestoreStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + }) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go new file mode 100644 index 0000000000..e95c6e2c0d --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go @@ -0,0 +1,157 @@ +/* +Copyright 2025 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 step + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + "github.com/deckhouse/virtualization-controller/pkg/eventrecord" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmscondition" +) + +func TestSteps(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "VMOP Snapshot Steps Suite") +} + +func newNoOpRecorder() *eventrecord.EventRecorderLoggerMock { + return &eventrecord.EventRecorderLoggerMock{ + EventFunc: func(_ client.Object, _, _, _ string) {}, + EventfFunc: func(_ client.Object, _, _, _ string, _ ...interface{}) {}, + AnnotatedEventfFunc: func(_ client.Object, _ map[string]string, _, _, _ string, _ ...interface{}) {}, + } +} + +//nolint:unparam // namespace is always "default" in tests, but kept for flexibility +func createVMSnapshot(namespace, name, secretName string, ready bool) *v1alpha2.VirtualMachineSnapshot { + vms := &v1alpha2.VirtualMachineSnapshot{ + TypeMeta: metav1.TypeMeta{ + Kind: v1alpha2.VirtualMachineSnapshotKind, + APIVersion: v1alpha2.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha2.VirtualMachineSnapshotSpec{ + VirtualMachineName: "test-vm", + }, + Status: v1alpha2.VirtualMachineSnapshotStatus{ + Phase: v1alpha2.VirtualMachineSnapshotPhaseReady, + VirtualMachineSnapshotSecretName: secretName, + }, + } + + if ready { + vms.Status.Conditions = []metav1.Condition{ + { + Type: string(vmscondition.VirtualMachineSnapshotReadyType), + Status: metav1.ConditionTrue, + }, + } + } + + return vms +} + +//nolint:unparam // namespace is always "default" in tests, but kept for flexibility +func createRestoreVMOP(namespace, name, vmName, snapshotName string) *v1alpha2.VirtualMachineOperation { + return &v1alpha2.VirtualMachineOperation{ + TypeMeta: metav1.TypeMeta{ + Kind: v1alpha2.VirtualMachineOperationKind, + APIVersion: v1alpha2.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + UID: types.UID("test-vmop-uid"), + }, + Spec: v1alpha2.VirtualMachineOperationSpec{ + Type: v1alpha2.VMOPTypeRestore, + VirtualMachine: vmName, + Restore: &v1alpha2.VirtualMachineOperationRestoreSpec{ + Mode: v1alpha2.SnapshotOperationModeStrict, + VirtualMachineSnapshotName: snapshotName, + }, + }, + } +} + +//nolint:unparam // namespace is always "default" in tests, but kept for flexibility +func createCloneVMOP(namespace, name, vmName, snapshotName string) *v1alpha2.VirtualMachineOperation { + vmop := &v1alpha2.VirtualMachineOperation{ + TypeMeta: metav1.TypeMeta{ + Kind: v1alpha2.VirtualMachineOperationKind, + APIVersion: v1alpha2.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + UID: types.UID("test-vmop-uid"), + Annotations: map[string]string{ + annotations.AnnVMOPSnapshotName: snapshotName, + }, + }, + Spec: v1alpha2.VirtualMachineOperationSpec{ + Type: v1alpha2.VMOPTypeClone, + VirtualMachine: vmName, + Clone: &v1alpha2.VirtualMachineOperationCloneSpec{ + Mode: v1alpha2.SnapshotOperationModeStrict, + Customization: &v1alpha2.VirtualMachineOperationCloneCustomization{ + NameSuffix: "-clone", + }, + }, + }, + } + return vmop +} + +func setMaintenanceCondition(vmop *v1alpha2.VirtualMachineOperation, status metav1.ConditionStatus) { + vmop.Status.Conditions = append(vmop.Status.Conditions, metav1.Condition{ + Type: string(vmopcondition.TypeMaintenanceMode), + Status: status, + }) +} + +//nolint:unparam // namespace is always "default" in tests, but kept for flexibility +func createVirtualDisk(namespace, name, ownerUID string, phase v1alpha2.DiskPhase) *v1alpha2.VirtualDisk { + return &v1alpha2.VirtualDisk{ + TypeMeta: metav1.TypeMeta{ + Kind: "VirtualDisk", + APIVersion: v1alpha2.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + annotations.AnnVMOPRestore: ownerUID, + }, + }, + Status: v1alpha2.VirtualDiskStatus{ + Phase: phase, + }, + } +} diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go index 05a9d511c6..d13f1d904d 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go @@ -28,23 +28,19 @@ import ( "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/controller/vmop/snapshot/internal/common" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmscondition" ) type VMSnapshotReadyStep struct { client client.Client - cb *conditions.ConditionBuilder } func NewVMSnapshotReadyStep( client client.Client, - cb *conditions.ConditionBuilder, ) *VMSnapshotReadyStep { return &VMSnapshotReadyStep{ client: client, - cb: cb, } } @@ -53,7 +49,6 @@ func (s VMSnapshotReadyStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMac if vmop.Spec.Type == v1alpha2.VMOPTypeRestore { if vmop.Spec.Restore.VirtualMachineSnapshotName == "" { err := fmt.Errorf("the virtual machine snapshot name is empty") - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } @@ -69,38 +64,24 @@ func (s VMSnapshotReadyStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMac vmSnapshotKey := types.NamespacedName{Namespace: vmop.Namespace, Name: vmopName} vmSnapshot, err := object.FetchObject(ctx, vmSnapshotKey, s.client, &v1alpha2.VirtualMachineSnapshot{}) if err != nil { - vmop.Status.Phase = v1alpha2.VMOPPhaseFailed - err := fmt.Errorf("failed to fetch the virtual machine snapshot %q: %w", vmSnapshotKey.Name, err) - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err } if vmSnapshot == nil { - vmop.Status.Phase = v1alpha2.VMOPPhaseFailed - err := fmt.Errorf("virtual machine snapshot %q is not found", vmSnapshotKey.Name) - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) - return &reconcile.Result{}, err + return &reconcile.Result{}, fmt.Errorf("virtual machine snapshot %q is not found", vmSnapshotKey.Name) } vmSnapshotReadyToUseCondition, exist := conditions.GetCondition(vmscondition.VirtualMachineSnapshotReadyType, vmSnapshot.Status.Conditions) if !exist { - vmop.Status.Phase = v1alpha2.VMOPPhaseFailed - err := fmt.Errorf("virtual machine snapshot %q is not ready to use", vmop.Spec.Restore.VirtualMachineSnapshotName) - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) - return &reconcile.Result{}, err + return &reconcile.Result{}, fmt.Errorf("virtual machine snapshot %q is not ready to use", vmopName) } if vmSnapshotReadyToUseCondition.Status != metav1.ConditionTrue { - vmop.Status.Phase = v1alpha2.VMOPPhaseFailed - err := fmt.Errorf("virtual machine snapshot %q is not ready to use", vmop.Spec.Restore.VirtualMachineSnapshotName) - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) - return &reconcile.Result{}, err + return &reconcile.Result{}, fmt.Errorf("virtual machine snapshot %q is not ready to use", vmopName) } if vmSnapshot.Status.VirtualMachineSnapshotSecretName == "" { - err := fmt.Errorf("snapshot secret name is empty") - common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) - return &reconcile.Result{}, err + return &reconcile.Result{}, fmt.Errorf("snapshot secret name is empty") } return nil, nil diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go new file mode 100644 index 0000000000..9507a09256 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go @@ -0,0 +1,160 @@ +/* +Copyright 2025 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 step + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + "github.com/deckhouse/virtualization-controller/pkg/common/testutil" +) + +var _ = Describe("VMSnapshotReadyStep", func() { + var ( + ctx context.Context + fakeClient client.WithWatch + step *VMSnapshotReadyStep + ) + + BeforeEach(func() { + ctx = context.Background() + }) + + Describe("Restore operation", func() { + It("should return error when snapshot name is empty", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "") + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewVMSnapshotReadyStep(fakeClient) + result, err := step.Take(ctx, vmop) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("snapshot name is empty")) + Expect(result).NotTo(BeNil()) + }) + + It("should return error when snapshot is not ready", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", false) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewVMSnapshotReadyStep(fakeClient) + result, err := step.Take(ctx, vmop) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("is not ready to use")) + Expect(result).NotTo(BeNil()) + }) + + It("should proceed when snapshot is ready", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewVMSnapshotReadyStep(fakeClient) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should be idempotent - multiple calls with same state return same result", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewVMSnapshotReadyStep(fakeClient) + + result1, err1 := step.Take(ctx, vmop) + result2, err2 := step.Take(ctx, vmop) + result3, err3 := step.Take(ctx, vmop) + + Expect(err1).NotTo(HaveOccurred()) + Expect(err2).NotTo(HaveOccurred()) + Expect(err3).NotTo(HaveOccurred()) + Expect(result1).To(BeNil()) + Expect(result2).To(BeNil()) + Expect(result3).To(BeNil()) + }) + + It("should be idempotent when snapshot is not ready", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", false) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewVMSnapshotReadyStep(fakeClient) + + for i := range 5 { + result, err := step.Take(ctx, vmop) + Expect(err).To(HaveOccurred(), "Iteration %d should return error", i) + Expect(err.Error()).To(ContainSubstring("is not ready to use")) + Expect(result).NotTo(BeNil(), "Iteration %d should return non-nil result", i) + } + }) + }) + + Describe("Clone operation", func() { + It("should wait when snapshot annotation is not set", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + delete(vmop.Annotations, annotations.AnnVMOPSnapshotName) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewVMSnapshotReadyStep(fakeClient) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + }) + + It("should proceed when snapshot is ready", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewVMSnapshotReadyStep(fakeClient) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go new file mode 100644 index 0000000000..9fb16159a5 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go @@ -0,0 +1,115 @@ +/* +Copyright 2025 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 step + +import ( + "context" + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "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/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/eventrecord" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" +) + +type WaitingDisksReadyStep struct { + client client.Client + recorder eventrecord.EventRecorderLogger +} + +func NewWaitingDisksStep( + client client.Client, + recorder eventrecord.EventRecorderLogger, +) *WaitingDisksReadyStep { + return &WaitingDisksReadyStep{ + client: client, + recorder: recorder, + } +} + +func (s WaitingDisksReadyStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { + switch vmop.Spec.Type { + case v1alpha2.VMOPTypeRestore: + if vmop.Spec.Restore.Mode == v1alpha2.SnapshotOperationModeDryRun { + return nil, nil + } + case v1alpha2.VMOPTypeClone: + if vmop.Spec.Clone.Mode == v1alpha2.SnapshotOperationModeDryRun { + return nil, nil + } + } + + cb := conditions.NewConditionBuilder(vmopcondition.TypeCompleted).Status(metav1.ConditionFalse) + switch vmop.Spec.Type { + case v1alpha2.VMOPTypeClone: + cb.Reason(vmopcondition.ReasonCloneInProgress) + case v1alpha2.VMOPTypeRestore: + cb.Reason(vmopcondition.ReasonRestoreInProgress) + } + + for _, status := range vmop.Status.Resources { + if status.Kind != v1alpha2.VirtualDiskKind { + continue + } + + var vd v1alpha2.VirtualDisk + vdKey := types.NamespacedName{Namespace: vmop.Namespace, Name: status.Name} + err := s.client.Get(ctx, vdKey, &vd) + if err != nil { + return &reconcile.Result{}, fmt.Errorf("failed to get the `VirtualDisk`: %w", err) + } + + if vd.Annotations[annotations.AnnVMOPRestore] != string(vmop.UID) { + // Skip disks that don't belong to this vmop + continue + } + + if vd.Status.Phase == v1alpha2.DiskFailed { + return &reconcile.Result{}, fmt.Errorf("virtual disk %q is in failed phase", vdKey.Name) + } + + switch vd.Status.Phase { + case v1alpha2.DiskFailed: + return &reconcile.Result{}, fmt.Errorf("virtual disk %q is in failed phase", vdKey.Name) + case v1alpha2.DiskReady: + // Disk is Ready, check the next one. + continue + case v1alpha2.DiskWaitForFirstConsumer: + if vmop.Spec.Type == v1alpha2.VMOPTypeClone { + cb.Message(fmt.Sprintf("%s operation is completed. Waiting for resource readiness. Waiting for cleanup.", vmop.Spec.Type)) + conditions.SetCondition(cb, &vmop.Status.Conditions) + return &reconcile.Result{}, nil // Should wait for disk ready. + } + continue + default: + cb.Message(fmt.Sprintf("%s operation is completed. Waiting for resource readiness. Waiting for cleanup.", vmop.Spec.Type)) + conditions.SetCondition(cb, &vmop.Status.Conditions) + return &reconcile.Result{}, nil + } + } + + cb.Message(fmt.Sprintf("%s operation is completed. Resources are ready. Waiting for cleanup.", vmop.Spec.Type)) + conditions.SetCondition(cb, &vmop.Status.Conditions) + + return nil, nil +} diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go new file mode 100644 index 0000000000..d31a9584b6 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go @@ -0,0 +1,270 @@ +/* +Copyright 2025 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 step + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/deckhouse/virtualization-controller/pkg/common/testutil" + "github.com/deckhouse/virtualization-controller/pkg/eventrecord" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +var _ = Describe("WaitingDisksReadyStep", func() { + var ( + ctx context.Context + fakeClient client.WithWatch + recorder *eventrecord.EventRecorderLoggerMock + step *WaitingDisksReadyStep + ) + + BeforeEach(func() { + ctx = context.Background() + recorder = newNoOpRecorder() + }) + + Describe("DryRun mode", func() { + It("should skip for restore in DryRun mode", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Spec.Restore.Mode = v1alpha2.SnapshotOperationModeDryRun + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewWaitingDisksStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should skip for clone in DryRun mode", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Spec.Clone.Mode = v1alpha2.SnapshotOperationModeDryRun + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewWaitingDisksStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + }) + + Describe("Waiting for disks", func() { + It("should wait when disk is not ready for clone", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Kind: v1alpha2.VirtualDiskKind, Name: "test-disk"}, + } + + vd := createVirtualDisk("default", "test-disk", "test-vmop-uid", v1alpha2.DiskProvisioning) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vd) + Expect(err).NotTo(HaveOccurred()) + + step = NewWaitingDisksStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + }) + + It("should proceed when disk is ready for clone", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Kind: v1alpha2.VirtualDiskKind, Name: "test-disk"}, + } + + vd := createVirtualDisk("default", "test-disk", "test-vmop-uid", v1alpha2.DiskReady) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vd) + Expect(err).NotTo(HaveOccurred()) + + step = NewWaitingDisksStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should accept WaitForFirstConsumer phase for restore", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Kind: v1alpha2.VirtualDiskKind, Name: "test-disk"}, + } + + vd := createVirtualDisk("default", "test-disk", "test-vmop-uid", v1alpha2.DiskWaitForFirstConsumer) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vd) + Expect(err).NotTo(HaveOccurred()) + + step = NewWaitingDisksStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should return error when disk is in failed phase", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Kind: v1alpha2.VirtualDiskKind, Name: "test-disk"}, + } + + vd := createVirtualDisk("default", "test-disk", "test-vmop-uid", v1alpha2.DiskFailed) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vd) + Expect(err).NotTo(HaveOccurred()) + + step = NewWaitingDisksStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed phase")) + Expect(result).NotTo(BeNil()) + }) + + It("should be idempotent - multiple calls with same state return same result", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Kind: v1alpha2.VirtualDiskKind, Name: "test-disk"}, + } + + vd := createVirtualDisk("default", "test-disk", "test-vmop-uid", v1alpha2.DiskReady) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vd) + Expect(err).NotTo(HaveOccurred()) + + step = NewWaitingDisksStep(fakeClient, recorder) + + result1, err1 := step.Take(ctx, vmop) + result2, err2 := step.Take(ctx, vmop) + result3, err3 := step.Take(ctx, vmop) + + Expect(err1).NotTo(HaveOccurred()) + Expect(err2).NotTo(HaveOccurred()) + Expect(err3).NotTo(HaveOccurred()) + Expect(result1).To(BeNil()) + Expect(result2).To(BeNil()) + Expect(result3).To(BeNil()) + }) + + It("should be idempotent when waiting for disk", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Kind: v1alpha2.VirtualDiskKind, Name: "test-disk"}, + } + + vd := createVirtualDisk("default", "test-disk", "test-vmop-uid", v1alpha2.DiskProvisioning) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vd) + Expect(err).NotTo(HaveOccurred()) + + step = NewWaitingDisksStep(fakeClient, recorder) + + for i := range 5 { + result, err := step.Take(ctx, vmop) + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil(), "Iteration %d should return non-nil result (waiting)", i) + } + }) + }) + + Describe("Disk ownership check with two disks", func() { + // Setup: vmop has two disks in resources: + // - "our-disk" belongs to this vmop (UID matches) + // - "other-disk" belongs to a different vmop (UID doesn't match) + + It("should wait when our disk is not ready (both disks not ready)", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Kind: v1alpha2.VirtualDiskKind, Name: "our-disk"}, + {Kind: v1alpha2.VirtualDiskKind, Name: "other-disk"}, + } + + ourDisk := createVirtualDisk("default", "our-disk", "test-vmop-uid", v1alpha2.DiskProvisioning) + otherDisk := createVirtualDisk("default", "other-disk", "different-vmop-uid", v1alpha2.DiskProvisioning) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, ourDisk, otherDisk) + Expect(err).NotTo(HaveOccurred()) + + step = NewWaitingDisksStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil(), "should wait because our disk is not ready") + }) + + It("should wait when our disk is not ready (other disk is ready)", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Kind: v1alpha2.VirtualDiskKind, Name: "our-disk"}, + {Kind: v1alpha2.VirtualDiskKind, Name: "other-disk"}, + } + + ourDisk := createVirtualDisk("default", "our-disk", "test-vmop-uid", v1alpha2.DiskProvisioning) + otherDisk := createVirtualDisk("default", "other-disk", "different-vmop-uid", v1alpha2.DiskReady) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, ourDisk, otherDisk) + Expect(err).NotTo(HaveOccurred()) + + step = NewWaitingDisksStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil(), "should wait because our disk is not ready, regardless of other disk status") + }) + + It("should proceed when our disk is ready (other disk is not ready)", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Kind: v1alpha2.VirtualDiskKind, Name: "our-disk"}, + {Kind: v1alpha2.VirtualDiskKind, Name: "other-disk"}, + } + + ourDisk := createVirtualDisk("default", "our-disk", "test-vmop-uid", v1alpha2.DiskReady) + otherDisk := createVirtualDisk("default", "other-disk", "different-vmop-uid", v1alpha2.DiskProvisioning) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, ourDisk, otherDisk) + Expect(err).NotTo(HaveOccurred()) + + step = NewWaitingDisksStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil(), "should proceed because our disk is ready, we don't wait for other vmop's disks") + }) + }) +}) diff --git a/test/e2e/vmop/restore.go b/test/e2e/vmop/restore.go index bce10bc23f..6b3616a567 100644 --- a/test/e2e/vmop/restore.go +++ b/test/e2e/vmop/restore.go @@ -26,7 +26,6 @@ import ( . "github.com/onsi/gomega" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" crclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -41,7 +40,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" - "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" @@ -465,10 +463,6 @@ func (t *restoreModeTest) CheckVMAfterRestore( case v1alpha2.SnapshotOperationModeDryRun: err := t.Framework.Clients.GenericClient().Get(context.Background(), crclient.ObjectKeyFromObject(vmopRestore), vmopRestore) Expect(err).NotTo(HaveOccurred()) - restoreCompletedCondition, _ := conditions.GetCondition(vmopcondition.TypeRestoreCompleted, vmopRestore.Status.Conditions) - Expect(restoreCompletedCondition.Status).To(Equal(metav1.ConditionTrue)) - Expect(restoreCompletedCondition.Reason).To(Equal(vmopcondition.ReasonDryRunOperationCompleted.String())) - Expect(restoreCompletedCondition.Message).To(ContainSubstring("The virtual machine can be restored from the snapshot.")) t.CheckResourceReadyForRestore(vmopRestore, v1alpha2.VirtualMachineKind, vm.Name) t.CheckResourceReadyForRestore(vmopRestore, v1alpha2.VirtualDiskKind, vdRoot.Name) From 84019029cdc790547587668653be3c54656ce418 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Thu, 29 Jan 2026 11:36:22 +0300 Subject: [PATCH 02/24] fix after rebase Signed-off-by: Valeriy Khorunzhin --- .../vmop/snapshot/internal/step/cleanup_snapshot_step.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go index a9bc047a84..0134a72e29 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go @@ -69,8 +69,6 @@ func (s CleanupSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMac return nil, nil } - rcb := conditions.NewConditionBuilder(vmopcondition.TypeSnapshotReady) - vmSnapshotKey := types.NamespacedName{Namespace: vmop.Namespace, Name: snapshotName} vmSnapshot, err := object.FetchObject(ctx, vmSnapshotKey, s.client, &v1alpha2.VirtualMachineSnapshot{}) if err != nil { From 76ee6edbbc8e1d8adce90f15d2fa6bfa2d6309d1 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Thu, 29 Jan 2026 18:37:05 +0300 Subject: [PATCH 03/24] year) Signed-off-by: Valeriy Khorunzhin --- .../controller/vmop/snapshot/internal/step/completed_step.go | 2 +- .../step/{final_step_test.go => completed_step_test.go} | 2 +- .../vmop/snapshot/internal/step/process_clone_step_test.go | 2 +- .../vmop/snapshot/internal/step/process_restore_step_test.go | 2 +- .../vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go | 2 +- .../vmop/snapshot/internal/step/waiting_disk_ready_step_test.go | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) rename images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/{final_step_test.go => completed_step_test.go} (98%) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go index 4f5cc836bb..2b3c9b8ac7 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go @@ -1,5 +1,5 @@ /* -Copyright 2025 Flant JSC +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. diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/final_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step_test.go similarity index 98% rename from images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/final_step_test.go rename to images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step_test.go index 1c006eefbb..c07d980d21 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/final_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step_test.go @@ -1,5 +1,5 @@ /* -Copyright 2025 Flant JSC +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. diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go index 11f59c9364..feb1a4cd18 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go @@ -1,5 +1,5 @@ /* -Copyright 2025 Flant JSC +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. diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go index 292c130c70..e3ae8c63b5 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go @@ -1,5 +1,5 @@ /* -Copyright 2025 Flant JSC +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. diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go index 9507a09256..09ece46660 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go @@ -1,5 +1,5 @@ /* -Copyright 2025 Flant JSC +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. diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go index d31a9584b6..0658274141 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go @@ -1,5 +1,5 @@ /* -Copyright 2025 Flant JSC +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. From f7aab38ae875e60d55feb9062861e330621139d3 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Thu, 29 Jan 2026 18:56:25 +0300 Subject: [PATCH 04/24] year... Signed-off-by: Valeriy Khorunzhin --- .../pkg/controller/vmop/snapshot/internal/step/suite_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go index e95c6e2c0d..d27c521640 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go @@ -1,5 +1,5 @@ /* -Copyright 2025 Flant JSC +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. From ccbef5ff1c8c4129efe6dc5dce9114415d4114be Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Thu, 29 Jan 2026 19:22:15 +0300 Subject: [PATCH 05/24] resolve Signed-off-by: Valeriy Khorunzhin --- .../vmop/snapshot/internal/step/enter_maintenance_step.go | 1 + .../vmop/snapshot/internal/step/process_clone_step_test.go | 4 ++-- .../vmop/snapshot/internal/step/vmsnapshot_ready_step.go | 4 ++-- .../vmop/snapshot/internal/step/waiting_disk_ready_step.go | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go index b1678dd900..a039bfdada 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go @@ -55,6 +55,7 @@ func (s EnterMaintenanceStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMa return nil, nil } + // Need to prevent reapplying the maintenance condition once the resources have already been restored. if vmop.Status.Resources != nil { return nil, nil } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go index feb1a4cd18..6b4f7c9b96 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go @@ -75,8 +75,8 @@ var _ = Describe("ProcessCloneStep", func() { Expect(err1).To(HaveOccurred()) Expect(err2).To(HaveOccurred()) Expect(err3).To(HaveOccurred()) - Expect(err1.Error()).To(Equal(err2.Error())) - Expect(err2.Error()).To(Equal(err3.Error())) + Expect(err2.Error()).To(Equal(err1.Error()), "err2 should be equal to the first error") + Expect(err3.Error()).To(Equal(err1.Error()), "err3 should be equal to the first error") }) }) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go index d13f1d904d..c4fd5bd6f2 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go @@ -73,11 +73,11 @@ func (s VMSnapshotReadyStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMac vmSnapshotReadyToUseCondition, exist := conditions.GetCondition(vmscondition.VirtualMachineSnapshotReadyType, vmSnapshot.Status.Conditions) if !exist { - return &reconcile.Result{}, fmt.Errorf("virtual machine snapshot %q is not ready to use", vmopName) + return &reconcile.Result{}, fmt.Errorf("virtual machine snapshot %q is not ready to use", vmSnapshot.Name) } if vmSnapshotReadyToUseCondition.Status != metav1.ConditionTrue { - return &reconcile.Result{}, fmt.Errorf("virtual machine snapshot %q is not ready to use", vmopName) + return &reconcile.Result{}, fmt.Errorf("virtual machine snapshot %q is not ready to use", vmSnapshot.Name) } if vmSnapshot.Status.VirtualMachineSnapshotSecretName == "" { diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go index 9fb16159a5..e550516b6b 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go @@ -96,13 +96,13 @@ func (s WaitingDisksReadyStep) Take(ctx context.Context, vmop *v1alpha2.VirtualM continue case v1alpha2.DiskWaitForFirstConsumer: if vmop.Spec.Type == v1alpha2.VMOPTypeClone { - cb.Message(fmt.Sprintf("%s operation is completed. Waiting for resource readiness. Waiting for cleanup.", vmop.Spec.Type)) + cb.Message("Clone operation is completed. Waiting for resource readiness.") conditions.SetCondition(cb, &vmop.Status.Conditions) return &reconcile.Result{}, nil // Should wait for disk ready. } continue default: - cb.Message(fmt.Sprintf("%s operation is completed. Waiting for resource readiness. Waiting for cleanup.", vmop.Spec.Type)) + cb.Message(fmt.Sprintf("%s operation is completed. Waiting for resource readiness.", vmop.Spec.Type)) conditions.SetCondition(cb, &vmop.Status.Conditions) return &reconcile.Result{}, nil } From b9432cf8557ae691d3dde1492dd253e9e50c6927 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Fri, 30 Jan 2026 12:03:08 +0300 Subject: [PATCH 06/24] resolve Signed-off-by: Valeriy Khorunzhin --- .../controller/vmop/snapshot/internal/handler/lifecycle.go | 2 +- .../vmop/snapshot/internal/step/completed_step_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/handler/lifecycle.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/handler/lifecycle.go index f9c2273649..2fef2b186a 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/handler/lifecycle.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/handler/lifecycle.go @@ -89,7 +89,7 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach // 3. Operation already in progress. Check if the operation is completed. // Run execute until the operation is completed. - if _, found := conditions.GetCondition(vmopcondition.TypeCompleted, vmop.Status.Conditions); found { + if vmop.Status.Phase == v1alpha2.VMOPPhaseInProgress { return svcOp.Execute(ctx) } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step_test.go index c07d980d21..b41abaf7f3 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step_test.go @@ -73,9 +73,9 @@ var _ = Describe("CompletedStep", func() { Expect(result2).NotTo(BeNil()) Expect(result3).NotTo(BeNil()) - Expect(phase1).To(Equal(phase2)) - Expect(phase2).To(Equal(phase3)) Expect(phase1).To(Equal(v1alpha2.VMOPPhaseCompleted)) + Expect(phase2).To(Equal(v1alpha2.VMOPPhaseCompleted)) + Expect(phase3).To(Equal(v1alpha2.VMOPPhaseCompleted)) Expect(conditions1).To(Equal(conditions2)) Expect(conditions2).To(Equal(conditions3)) From 79c0b849e420d82e78601d08aac462381dd88211 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Fri, 30 Jan 2026 12:03:24 +0300 Subject: [PATCH 07/24] tests Signed-off-by: Valeriy Khorunzhin --- .../step/cleanup_snapshot_step_test.go | 216 +++++++++++++ .../step/create_snapshot_step_test.go | 287 ++++++++++++++++++ 2 files changed, 503 insertions(+) create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step_test.go diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go new file mode 100644 index 0000000000..708fd319c0 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go @@ -0,0 +1,216 @@ +/* +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 step + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "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/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" +) + +var _ = Describe("CleanupSnapshotStep", func() { + var ( + ctx context.Context + fakeClient client.WithWatch + recorder *eventrecord.EventRecorderLoggerMock + step *CleanupSnapshotStep + ) + + BeforeEach(func() { + ctx = context.Background() + recorder = newNoOpRecorder() + }) + + Describe("Skip conditions", func() { + It("should skip when snapshot condition is already CleanedUp", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Conditions = []metav1.Condition{ + { + Type: string(vmopcondition.TypeSnapshotReady), + Status: metav1.ConditionFalse, + Reason: string(vmopcondition.ReasonSnapshotCleanedUp), + }, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCleanupSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should skip when not all resources are completed", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Name: "disk-1", Status: v1alpha2.SnapshotResourceStatusCompleted}, + {Name: "disk-2", Status: v1alpha2.SnapshotResourceStatusInProgress}, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCleanupSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should skip when snapshot name annotation is not present", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + delete(vmop.Annotations, annotations.AnnVMOPSnapshotName) + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Name: "disk-1", Status: v1alpha2.SnapshotResourceStatusCompleted}, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCleanupSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + }) + + Describe("Snapshot cleanup", func() { + It("should set CleanedUp condition when snapshot is not found", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Name: "disk-1", Status: v1alpha2.SnapshotResourceStatusCompleted}, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCleanupSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + + snapshotCond, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(snapshotCond.Reason).To(Equal(string(vmopcondition.ReasonSnapshotCleanedUp))) + }) + + It("should delete snapshot when it exists and is not terminating", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Name: "disk-1", Status: v1alpha2.SnapshotResourceStatusCompleted}, + } + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewCleanupSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + + snapshotCond, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(snapshotCond.Reason).To(Equal(string(vmopcondition.ReasonSnapshotCleanedUp))) + + var deletedSnapshot v1alpha2.VirtualMachineSnapshot + err = fakeClient.Get(ctx, types.NamespacedName{Namespace: "default", Name: "test-snapshot"}, &deletedSnapshot) + Expect(err).To(HaveOccurred()) + }) + + It("should set CleanedUp condition when snapshot is terminating", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Name: "disk-1", Status: v1alpha2.SnapshotResourceStatusCompleted}, + } + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + now := metav1.Now() + snapshot.DeletionTimestamp = &now + snapshot.Finalizers = []string{"test-finalizer"} + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewCleanupSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + + snapshotCond, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(snapshotCond.Reason).To(Equal(string(vmopcondition.ReasonSnapshotCleanedUp))) + }) + }) + + Describe("Idempotency", func() { + It("should be idempotent - multiple calls with CleanedUp condition return same result", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Conditions = []metav1.Condition{ + { + Type: string(vmopcondition.TypeSnapshotReady), + Status: metav1.ConditionFalse, + Reason: string(vmopcondition.ReasonSnapshotCleanedUp), + }, + } + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Name: "disk-1", Status: v1alpha2.SnapshotResourceStatusCompleted}, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCleanupSnapshotStep(fakeClient, recorder) + + result1, err1 := step.Take(ctx, vmop) + result2, err2 := step.Take(ctx, vmop) + result3, err3 := step.Take(ctx, vmop) + + Expect(err1).NotTo(HaveOccurred()) + Expect(err2).NotTo(HaveOccurred()) + Expect(err3).NotTo(HaveOccurred()) + Expect(result1).To(BeNil()) + Expect(result2).To(BeNil()) + Expect(result3).To(BeNil()) + }) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step_test.go new file mode 100644 index 0000000000..2b97121759 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step_test.go @@ -0,0 +1,287 @@ +/* +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 step + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "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/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" +) + +var _ = Describe("CreateSnapshotStep", func() { + var ( + ctx context.Context + fakeClient client.WithWatch + recorder *eventrecord.EventRecorderLoggerMock + step *CreateSnapshotStep + ) + + BeforeEach(func() { + ctx = context.Background() + recorder = newNoOpRecorder() + }) + + Describe("Skip conditions", func() { + It("should skip when snapshot condition is already True", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Conditions = []metav1.Condition{ + { + Type: string(vmopcondition.TypeSnapshotReady), + Status: metav1.ConditionTrue, + Reason: string(vmopcondition.ReasonSnapshotOperationReady), + }, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCreateSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should skip when snapshot condition reason is CleanedUp", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Conditions = []metav1.Condition{ + { + Type: string(vmopcondition.TypeSnapshotReady), + Status: metav1.ConditionFalse, + Reason: string(vmopcondition.ReasonSnapshotCleanedUp), + }, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCreateSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + }) + + Describe("Existing snapshot handling", func() { + It("should set failed condition when snapshot is in Failed phase", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", false) + snapshot.Status.Phase = v1alpha2.VirtualMachineSnapshotPhaseFailed + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewCreateSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).To(HaveOccurred()) + Expect(result).NotTo(BeNil()) + + snapshotCond, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(snapshotCond.Reason).To(Equal(string(vmopcondition.ReasonSnapshotFailed))) + }) + + It("should set ready condition when snapshot is in Ready phase", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + snapshot.Status.Phase = v1alpha2.VirtualMachineSnapshotPhaseReady + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewCreateSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + + snapshotCond, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(snapshotCond.Status).To(Equal(metav1.ConditionTrue)) + Expect(snapshotCond.Reason).To(Equal(string(vmopcondition.ReasonSnapshotOperationReady))) + }) + + It("should set in progress condition when snapshot exists but not ready or failed", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", false) + snapshot.Status.Phase = v1alpha2.VirtualMachineSnapshotPhaseInProgress + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewCreateSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + + snapshotCond, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(snapshotCond.Reason).To(Equal(string(vmopcondition.ReasonSnapshotInProgress))) + }) + + It("should set in progress condition when snapshot annotation exists but snapshot not found", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCreateSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + + snapshotCond, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(snapshotCond.Reason).To(Equal(string(vmopcondition.ReasonSnapshotInProgress))) + }) + }) + + Describe("Snapshot discovery", func() { + It("should add annotation when owned snapshot already exists", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "") + delete(vmop.Annotations, annotations.AnnVMOPSnapshotName) + + snapshot := createVMSnapshot("default", "existing-snapshot", "test-secret", false) + snapshot.OwnerReferences = []metav1.OwnerReference{ + { + APIVersion: v1alpha2.SchemeGroupVersion.String(), + Kind: v1alpha2.VirtualMachineOperationKind, + Name: vmop.Name, + UID: vmop.UID, + BlockOwnerDeletion: ptr.To(true), + }, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot) + Expect(err).NotTo(HaveOccurred()) + + step = NewCreateSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(vmop.Annotations[annotations.AnnVMOPSnapshotName]).To(Equal("existing-snapshot")) + }) + }) + + Describe("Snapshot creation", func() { + It("should create snapshot when none exists", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "") + delete(vmop.Annotations, annotations.AnnVMOPSnapshotName) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCreateSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + + Expect(vmop.Annotations).NotTo(BeNil()) + Expect(vmop.Annotations[annotations.AnnVMOPSnapshotName]).NotTo(BeEmpty()) + + snapshotCond, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(snapshotCond.Reason).To(Equal(string(vmopcondition.ReasonSnapshotInProgress))) + + var snapshotList v1alpha2.VirtualMachineSnapshotList + err = fakeClient.List(ctx, &snapshotList, client.InNamespace("default")) + Expect(err).NotTo(HaveOccurred()) + Expect(snapshotList.Items).To(HaveLen(1)) + + createdSnapshot := snapshotList.Items[0] + Expect(createdSnapshot.Spec.VirtualMachineName).To(Equal("test-vm")) + Expect(createdSnapshot.Spec.RequiredConsistency).To(BeTrue()) + Expect(createdSnapshot.Annotations[annotations.AnnVMOPUID]).To(Equal(string(vmop.UID))) + }) + + It("should create snapshot with nil annotations", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "") + vmop.Annotations = nil + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCreateSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(vmop.Annotations).NotTo(BeNil()) + Expect(vmop.Annotations[annotations.AnnVMOPSnapshotName]).NotTo(BeEmpty()) + }) + }) + + Describe("Idempotency", func() { + It("should be idempotent - multiple calls with ready snapshot return same result", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Conditions = []metav1.Condition{ + { + Type: string(vmopcondition.TypeSnapshotReady), + Status: metav1.ConditionTrue, + Reason: string(vmopcondition.ReasonSnapshotOperationReady), + }, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCreateSnapshotStep(fakeClient, recorder) + + result1, err1 := step.Take(ctx, vmop) + result2, err2 := step.Take(ctx, vmop) + result3, err3 := step.Take(ctx, vmop) + + Expect(err1).NotTo(HaveOccurred()) + Expect(err2).NotTo(HaveOccurred()) + Expect(err3).NotTo(HaveOccurred()) + Expect(result1).To(BeNil()) + Expect(result2).To(BeNil()) + Expect(result3).To(BeNil()) + }) + }) +}) From e6e28a5bcd7f93cfcff7d64e49428866574afab0 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Fri, 30 Jan 2026 12:23:44 +0300 Subject: [PATCH 08/24] teeeests Signed-off-by: Valeriy Khorunzhin --- .../step/enter_maintenance_step_test.go | 211 ++++++++++++++++++ .../step/exit_maintenance_step_test.go | 192 ++++++++++++++++ .../vmop/snapshot/internal/step/suite_test.go | 26 +++ 3 files changed, 429 insertions(+) create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step_test.go create mode 100644 images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/exit_maintenance_step_test.go diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step_test.go new file mode 100644 index 0000000000..16fcac6ba9 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step_test.go @@ -0,0 +1,211 @@ +/* +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 step + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "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/vmcondition" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" +) + +var _ = Describe("EnterMaintenanceStep", func() { + var ( + ctx context.Context + fakeClient client.WithWatch + recorder *eventrecord.EventRecorderLoggerMock + step *EnterMaintenanceStep + ) + + BeforeEach(func() { + ctx = context.Background() + recorder = newNoOpRecorder() + }) + + Describe("Skip conditions", func() { + It("should skip when mode is DryRun", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Spec.Restore.Mode = v1alpha2.SnapshotOperationModeDryRun + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewEnterMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should skip when resources are already set", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ + {Name: "disk-1", Status: v1alpha2.SnapshotResourceStatusInProgress}, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewEnterMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + }) + + Describe("VM maintenance mode handling", func() { + It("should wait when VM already in maintenance but not stopped", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineRunning) + setVMMaintenanceCondition(vm, metav1.ConditionTrue, vmcondition.ReasonMaintenanceRestore) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vm) + Expect(err).NotTo(HaveOccurred()) + + step = NewEnterMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + }) + + It("should set VMOP maintenance condition when VM is stopped and in maintenance", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineStopped) + setVMMaintenanceCondition(vm, metav1.ConditionTrue, vmcondition.ReasonMaintenanceRestore) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vm) + Expect(err).NotTo(HaveOccurred()) + + step = NewEnterMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + + maintenanceCond, found := conditions.GetCondition(vmopcondition.TypeMaintenanceMode, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(maintenanceCond.Status).To(Equal(metav1.ConditionTrue)) + Expect(maintenanceCond.Reason).To(Equal(string(vmopcondition.ReasonMaintenanceModeEnabled))) + }) + + It("should set VMOP maintenance condition when VM is pending and in maintenance", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachinePending) + setVMMaintenanceCondition(vm, metav1.ConditionTrue, vmcondition.ReasonMaintenanceRestore) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vm) + Expect(err).NotTo(HaveOccurred()) + + step = NewEnterMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + + maintenanceCond, found := conditions.GetCondition(vmopcondition.TypeMaintenanceMode, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(maintenanceCond.Status).To(Equal(metav1.ConditionTrue)) + }) + + It("should set VM maintenance condition and update VM status", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineStopped) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vm) + Expect(err).NotTo(HaveOccurred()) + + step = NewEnterMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + + var updatedVM v1alpha2.VirtualMachine + err = fakeClient.Get(ctx, types.NamespacedName{Namespace: "default", Name: "test-vm"}, &updatedVM) + Expect(err).NotTo(HaveOccurred()) + + maintenanceCond, found := conditions.GetCondition(vmcondition.TypeMaintenance, updatedVM.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(maintenanceCond.Status).To(Equal(metav1.ConditionTrue)) + Expect(maintenanceCond.Reason).To(Equal(vmcondition.ReasonMaintenanceRestore.String())) + + vmopMaintenanceCond, found := conditions.GetCondition(vmopcondition.TypeMaintenanceMode, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(vmopMaintenanceCond.Status).To(Equal(metav1.ConditionTrue)) + }) + + It("should wait when VM is not stopped after setting maintenance", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineRunning) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vm) + Expect(err).NotTo(HaveOccurred()) + + step = NewEnterMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + }) + }) + + Describe("Idempotency", func() { + It("should be idempotent - multiple calls with DryRun mode return same result", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Spec.Restore.Mode = v1alpha2.SnapshotOperationModeDryRun + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewEnterMaintenanceStep(fakeClient, recorder) + + result1, err1 := step.Take(ctx, vmop) + result2, err2 := step.Take(ctx, vmop) + result3, err3 := step.Take(ctx, vmop) + + Expect(err1).NotTo(HaveOccurred()) + Expect(err2).NotTo(HaveOccurred()) + Expect(err3).NotTo(HaveOccurred()) + Expect(result1).To(BeNil()) + Expect(result2).To(BeNil()) + Expect(result3).To(BeNil()) + }) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/exit_maintenance_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/exit_maintenance_step_test.go new file mode 100644 index 0000000000..12eee6746b --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/exit_maintenance_step_test.go @@ -0,0 +1,192 @@ +/* +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 step + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "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/vmcondition" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" +) + +var _ = Describe("ExitMaintenanceStep", func() { + var ( + ctx context.Context + fakeClient client.WithWatch + recorder *eventrecord.EventRecorderLoggerMock + step *ExitMaintenanceStep + ) + + BeforeEach(func() { + ctx = context.Background() + recorder = newNoOpRecorder() + }) + + Describe("Skip conditions", func() { + It("should skip when mode is DryRun", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Spec.Restore.Mode = v1alpha2.SnapshotOperationModeDryRun + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewExitMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + + It("should wait when VMOP maintenance condition is not found", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineStopped) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vm) + Expect(err).NotTo(HaveOccurred()) + + step = NewExitMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + }) + + It("should wait when VMOP maintenance condition is false", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + setMaintenanceCondition(vmop, metav1.ConditionFalse) + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineStopped) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vm) + Expect(err).NotTo(HaveOccurred()) + + step = NewExitMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + }) + }) + + Describe("VM maintenance mode exit", func() { + It("should set VM maintenance to false when VM has maintenance condition true", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + setMaintenanceCondition(vmop, metav1.ConditionTrue) + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineStopped) + setVMMaintenanceCondition(vm, metav1.ConditionTrue, vmcondition.ReasonMaintenanceRestore) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vm) + Expect(err).NotTo(HaveOccurred()) + + step = NewExitMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + + var updatedVM v1alpha2.VirtualMachine + err = fakeClient.Get(ctx, types.NamespacedName{Namespace: "default", Name: "test-vm"}, &updatedVM) + Expect(err).NotTo(HaveOccurred()) + + maintenanceCond, found := conditions.GetCondition(vmcondition.TypeMaintenance, updatedVM.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(maintenanceCond.Status).To(Equal(metav1.ConditionFalse)) + }) + + It("should set VMOP maintenance to false when VM has no maintenance condition", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + setMaintenanceCondition(vmop, metav1.ConditionTrue) + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineStopped) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vm) + Expect(err).NotTo(HaveOccurred()) + + step = NewExitMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + + maintenanceCond, found := conditions.GetCondition(vmopcondition.TypeMaintenanceMode, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(maintenanceCond.Status).To(Equal(metav1.ConditionFalse)) + Expect(maintenanceCond.Reason).To(Equal(string(vmopcondition.ReasonMaintenanceModeDisabled))) + }) + + It("should set VMOP maintenance to false when VM maintenance is already false", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + setMaintenanceCondition(vmop, metav1.ConditionTrue) + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineStopped) + setVMMaintenanceCondition(vm, metav1.ConditionFalse, vmcondition.ReasonMaintenanceRestore) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, vm) + Expect(err).NotTo(HaveOccurred()) + + step = NewExitMaintenanceStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + + maintenanceCond, found := conditions.GetCondition(vmopcondition.TypeMaintenanceMode, vmop.Status.Conditions) + Expect(found).To(BeTrue()) + Expect(maintenanceCond.Status).To(Equal(metav1.ConditionFalse)) + }) + }) + + Describe("Idempotency", func() { + It("should be idempotent - multiple calls with DryRun mode return same result", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Spec.Restore.Mode = v1alpha2.SnapshotOperationModeDryRun + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewExitMaintenanceStep(fakeClient, recorder) + + result1, err1 := step.Take(ctx, vmop) + result2, err2 := step.Take(ctx, vmop) + result3, err3 := step.Take(ctx, vmop) + + Expect(err1).NotTo(HaveOccurred()) + Expect(err2).NotTo(HaveOccurred()) + Expect(err3).NotTo(HaveOccurred()) + Expect(result1).To(BeNil()) + Expect(result2).To(BeNil()) + Expect(result3).To(BeNil()) + }) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go index d27c521640..0fed8b7bd5 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go @@ -28,6 +28,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmscondition" ) @@ -155,3 +156,28 @@ func createVirtualDisk(namespace, name, ownerUID string, phase v1alpha2.DiskPhas }, } } + +//nolint:unparam // namespace is always "default" in tests, but kept for flexibility +func createVirtualMachine(namespace, name string, phase v1alpha2.MachinePhase) *v1alpha2.VirtualMachine { + return &v1alpha2.VirtualMachine{ + TypeMeta: metav1.TypeMeta{ + Kind: v1alpha2.VirtualMachineKind, + APIVersion: v1alpha2.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Status: v1alpha2.VirtualMachineStatus{ + Phase: phase, + }, + } +} + +func setVMMaintenanceCondition(vm *v1alpha2.VirtualMachine, status metav1.ConditionStatus, reason vmcondition.MaintenanceReason) { + vm.Status.Conditions = append(vm.Status.Conditions, metav1.Condition{ + Type: string(vmcondition.TypeMaintenance), + Status: status, + Reason: string(reason), + }) +} From 971f529329ad7ab37085c674123dd3b946d54022 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Fri, 30 Jan 2026 13:00:48 +0300 Subject: [PATCH 09/24] unparam Signed-off-by: Valeriy Khorunzhin --- .../pkg/controller/vmop/snapshot/internal/step/suite_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go index 0fed8b7bd5..db4930355a 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go @@ -174,6 +174,7 @@ func createVirtualMachine(namespace, name string, phase v1alpha2.MachinePhase) * } } +//nolint:unparam // reason is always ReasonMaintenanceRestore in tests, but kept for flexibility func setVMMaintenanceCondition(vm *v1alpha2.VirtualMachine, status metav1.ConditionStatus, reason vmcondition.MaintenanceReason) { vm.Status.Conditions = append(vm.Status.Conditions, metav1.Condition{ Type: string(vmcondition.TypeMaintenance), From 245f57451cecdeab111874ac1b5a9644e85982de Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Tue, 3 Feb 2026 18:14:13 +0300 Subject: [PATCH 10/24] resolve Signed-off-by: Valeriy Khorunzhin --- .../vmop/snapshot/internal/service/clone.go | 2 +- .../internal/step/cleanup_snapshot_step.go | 10 +++++----- .../internal/step/create_snapshot_step.go | 17 ++++++++--------- .../internal/step/enter_maintenance_step.go | 2 +- .../internal/step/waiting_disk_ready_step.go | 6 ------ 5 files changed, 15 insertions(+), 22 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go index d3f9463c84..d2cdf21fbf 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go @@ -66,7 +66,7 @@ func (o CloneOperation) Execute(ctx context.Context) (reconcile.Result, error) { } if vm == nil { - err := fmt.Errorf("virtual machine specified is not found") + err := fmt.Errorf("specified virtual machine was not found") return reconcile.Result{}, err } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go index 0134a72e29..e75ec9e588 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go @@ -51,10 +51,10 @@ func NewCleanupSnapshotStep( } func (s CleanupSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { - rcb := conditions.NewConditionBuilder(vmopcondition.TypeSnapshotReady) + snapshotConditionBuilder := conditions.NewConditionBuilder(vmopcondition.TypeSnapshotReady) - snapshotCondition, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) - if found && snapshotCondition.Reason == string(vmopcondition.ReasonSnapshotCleanedUp) { + snapshotCondition, _ := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + if snapshotCondition.Reason == string(vmopcondition.ReasonSnapshotCleanedUp) { return nil, nil } @@ -77,7 +77,7 @@ func (s CleanupSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMac if vmSnapshot == nil { conditions.SetCondition( - rcb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonSnapshotCleanedUp).Message("Snapshot cleanup completed."), + snapshotConditionBuilder.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonSnapshotCleanedUp).Message("Snapshot cleanup completed."), &vmop.Status.Conditions, ) return &reconcile.Result{}, nil @@ -93,7 +93,7 @@ func (s CleanupSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMac } conditions.SetCondition( - rcb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonSnapshotCleanedUp).Message("Snapshot cleanup completed."), + snapshotConditionBuilder.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonSnapshotCleanedUp).Message("Snapshot cleanup completed."), &vmop.Status.Conditions, ) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step.go index 93ae64a5fd..94dec1e91b 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step.go @@ -49,12 +49,11 @@ func NewCreateSnapshotStep(client client.Client, recorder eventrecord.EventRecor } func (s CreateSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { - rcb := conditions.NewConditionBuilder(vmopcondition.TypeSnapshotReady) + snapshotConditionBuilder := conditions.NewConditionBuilder(vmopcondition.TypeSnapshotReady) - if snapshotCondition, found := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions); found { - if snapshotCondition.Status == metav1.ConditionTrue || snapshotCondition.Reason == string(vmopcondition.ReasonSnapshotCleanedUp) { - return nil, nil - } + snapshotCondition, _ := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + if snapshotCondition.Status == metav1.ConditionTrue || snapshotCondition.Reason == string(vmopcondition.ReasonSnapshotCleanedUp) { + return nil, nil } if snapshotName, exists := vmop.Annotations[annotations.AnnVMOPSnapshotName]; exists { @@ -68,7 +67,7 @@ func (s CreateSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach switch vmSnapshot.Status.Phase { case v1alpha2.VirtualMachineSnapshotPhaseFailed: conditions.SetCondition( - rcb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonSnapshotFailed).Message("Snapshot is failed."), + snapshotConditionBuilder.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonSnapshotFailed).Message("Snapshot is failed."), &vmop.Status.Conditions, ) vmsReadyCondition, _ := conditions.GetCondition(vmscondition.VirtualMachineSnapshotReadyType, vmSnapshot.Status.Conditions) @@ -76,7 +75,7 @@ func (s CreateSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach return &reconcile.Result{}, fmt.Errorf("virtual machine snapshot %q is in failed phase: %w. Try again with new VMOP Clone operation", vmSnapshotKey.Name, err) case v1alpha2.VirtualMachineSnapshotPhaseReady: conditions.SetCondition( - rcb.Status(metav1.ConditionTrue).Reason(vmopcondition.ReasonSnapshotOperationReady).Message("Snapshot is ready for clone operation."), + snapshotConditionBuilder.Status(metav1.ConditionTrue).Reason(vmopcondition.ReasonSnapshotOperationReady).Message("Snapshot is ready for clone operation."), &vmop.Status.Conditions, ) return nil, nil @@ -84,7 +83,7 @@ func (s CreateSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach } conditions.SetCondition( - rcb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonSnapshotInProgress).Message("Snapshot creation is in progress."), + snapshotConditionBuilder.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonSnapshotInProgress).Message("Snapshot creation is in progress."), &vmop.Status.Conditions, ) return &reconcile.Result{}, nil @@ -151,7 +150,7 @@ func (s CreateSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach s.recorder.Event(vmop, corev1.EventTypeNormal, "SnapshotCreated", fmt.Sprintf("Created snapshot %s for clone operation", snapshot.Name)) conditions.SetCondition( - rcb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonSnapshotInProgress).Message("Snapshot creation is in progress."), + snapshotConditionBuilder.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonSnapshotInProgress).Message("Snapshot creation is in progress."), &vmop.Status.Conditions, ) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go index a039bfdada..cdf4b6c78f 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go @@ -55,7 +55,7 @@ func (s EnterMaintenanceStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMa return nil, nil } - // Need to prevent reapplying the maintenance condition once the resources have already been restored. + // Resources contains the list of resources that are affected by the snapshot operation. if vmop.Status.Resources != nil { return nil, nil } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go index e550516b6b..4ec5153b56 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go @@ -25,7 +25,6 @@ import ( "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/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -79,11 +78,6 @@ func (s WaitingDisksReadyStep) Take(ctx context.Context, vmop *v1alpha2.VirtualM return &reconcile.Result{}, fmt.Errorf("failed to get the `VirtualDisk`: %w", err) } - if vd.Annotations[annotations.AnnVMOPRestore] != string(vmop.UID) { - // Skip disks that don't belong to this vmop - continue - } - if vd.Status.Phase == v1alpha2.DiskFailed { return &reconcile.Result{}, fmt.Errorf("virtual disk %q is in failed phase", vdKey.Name) } From 1dfe5cc64751fdc6458cb9cedeff81c29a4b5c1e Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Tue, 3 Feb 2026 18:34:02 +0300 Subject: [PATCH 11/24] resolve Signed-off-by: Valeriy Khorunzhin --- .../vmop/snapshot/internal/service/clone.go | 22 +++--- .../vmop/snapshot/internal/service/restore.go | 23 ++++--- .../internal/step/waiting_disk_ready_step.go | 6 +- .../step/waiting_disk_ready_step_test.go | 69 ------------------- 4 files changed, 32 insertions(+), 88 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go index d2cdf21fbf..8ffa1c85a5 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/clone.go @@ -29,6 +29,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/common/steptaker" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/step" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization-controller/pkg/logger" @@ -51,10 +52,20 @@ type CloneOperation struct { } func (o CloneOperation) Execute(ctx context.Context) (reconcile.Result, error) { + cb := conditions.NewConditionBuilder(vmopcondition.TypeCompleted) + + defer func() { + if cb.Condition().Reason == string(vmopcondition.ReasonOperationFailed) { + o.vmop.Status.Phase = v1alpha2.VMOPPhaseFailed + conditions.SetCondition(cb, &o.vmop.Status.Conditions) + } + }() + log := logger.FromContext(ctx) if o.vmop.Spec.Clone == nil { err := fmt.Errorf("clone specification is mandatory to start cloning") + cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } @@ -62,11 +73,13 @@ func (o CloneOperation) Execute(ctx context.Context) (reconcile.Result, error) { vm, err := object.FetchObject(ctx, vmKey, o.client, &v1alpha2.VirtualMachine{}) if err != nil { err := fmt.Errorf("failed to fetch the virtual machine %q: %w", vmKey.Name, err) + cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } if vm == nil { err := fmt.Errorf("specified virtual machine was not found") + cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } @@ -85,14 +98,7 @@ func (o CloneOperation) Execute(ctx context.Context) (reconcile.Result, error) { log.Debug(failMsg, logger.SlogErr(err)) failMsg = fmt.Errorf("%s: %w", failMsg, err).Error() o.recorder.Event(o.vmop, corev1.EventTypeWarning, v1alpha2.ReasonErrVMOPFailed, failMsg) - - o.vmop.Status.Phase = v1alpha2.VMOPPhaseFailed - conditions.SetCondition( - conditions.NewConditionBuilder(vmopcondition.TypeCompleted). - Reason(vmopcondition.ReasonOperationFailed). - Message(failMsg).Status(metav1.ConditionFalse), - &o.vmop.Status.Conditions, - ) + cb.Reason(vmopcondition.ReasonOperationFailed).Message(failMsg).Status(metav1.ConditionFalse) } return result, err diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/restore.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/restore.go index 3ca3bb229a..26c6624dab 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/restore.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/service/restore.go @@ -29,6 +29,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/common/steptaker" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/step" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization-controller/pkg/logger" @@ -51,15 +52,26 @@ type RestoreOperation struct { } func (o RestoreOperation) Execute(ctx context.Context) (reconcile.Result, error) { + cb := conditions.NewConditionBuilder(vmopcondition.TypeCompleted) + + defer func() { + if cb.Condition().Reason == string(vmopcondition.ReasonOperationFailed) { + o.vmop.Status.Phase = v1alpha2.VMOPPhaseFailed + conditions.SetCondition(cb, &o.vmop.Status.Conditions) + } + }() + log := logger.FromContext(ctx) if o.vmop.Spec.Restore == nil { err := fmt.Errorf("restore specification is nil") + cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } if o.vmop.Spec.Restore.VirtualMachineSnapshotName == "" { err := fmt.Errorf("virtual machine snapshot name is required") + cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } @@ -67,11 +79,13 @@ func (o RestoreOperation) Execute(ctx context.Context) (reconcile.Result, error) vm, err := object.FetchObject(ctx, vmKey, o.client, &v1alpha2.VirtualMachine{}) if err != nil { err := fmt.Errorf("failed to fetch the virtual machine %q: %w", vmKey.Name, err) + cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } if vm == nil { err := fmt.Errorf("virtual machine is nil") + cb.Status(metav1.ConditionFalse).Reason(vmopcondition.ReasonOperationFailed).Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, err } @@ -90,14 +104,7 @@ func (o RestoreOperation) Execute(ctx context.Context) (reconcile.Result, error) log.Debug(failMsg, logger.SlogErr(err)) failMsg = fmt.Errorf("%s: %w", failMsg, err).Error() o.recorder.Event(o.vmop, corev1.EventTypeWarning, v1alpha2.ReasonErrVMOPFailed, failMsg) - - o.vmop.Status.Phase = v1alpha2.VMOPPhaseFailed - conditions.SetCondition( - conditions.NewConditionBuilder(vmopcondition.TypeCompleted). - Reason(vmopcondition.ReasonOperationFailed). - Message(failMsg).Status(metav1.ConditionFalse), - &o.vmop.Status.Conditions, - ) + cb.Reason(vmopcondition.ReasonOperationFailed).Message(failMsg).Status(metav1.ConditionFalse) } return result, err diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go index 4ec5153b56..5affc1e15d 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step.go @@ -90,19 +90,19 @@ func (s WaitingDisksReadyStep) Take(ctx context.Context, vmop *v1alpha2.VirtualM continue case v1alpha2.DiskWaitForFirstConsumer: if vmop.Spec.Type == v1alpha2.VMOPTypeClone { - cb.Message("Clone operation is completed. Waiting for resource readiness.") + cb.Message("Waiting for resource readiness.") conditions.SetCondition(cb, &vmop.Status.Conditions) return &reconcile.Result{}, nil // Should wait for disk ready. } continue default: - cb.Message(fmt.Sprintf("%s operation is completed. Waiting for resource readiness.", vmop.Spec.Type)) + cb.Message("Waiting for resource readiness.") conditions.SetCondition(cb, &vmop.Status.Conditions) return &reconcile.Result{}, nil } } - cb.Message(fmt.Sprintf("%s operation is completed. Resources are ready. Waiting for cleanup.", vmop.Spec.Type)) + cb.Message("Resources are ready. Waiting for cleanup.") conditions.SetCondition(cb, &vmop.Status.Conditions) return nil, nil diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go index 0658274141..07bc4f8ab7 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/waiting_disk_ready_step_test.go @@ -198,73 +198,4 @@ var _ = Describe("WaitingDisksReadyStep", func() { } }) }) - - Describe("Disk ownership check with two disks", func() { - // Setup: vmop has two disks in resources: - // - "our-disk" belongs to this vmop (UID matches) - // - "other-disk" belongs to a different vmop (UID doesn't match) - - It("should wait when our disk is not ready (both disks not ready)", func() { - vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") - vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ - {Kind: v1alpha2.VirtualDiskKind, Name: "our-disk"}, - {Kind: v1alpha2.VirtualDiskKind, Name: "other-disk"}, - } - - ourDisk := createVirtualDisk("default", "our-disk", "test-vmop-uid", v1alpha2.DiskProvisioning) - otherDisk := createVirtualDisk("default", "other-disk", "different-vmop-uid", v1alpha2.DiskProvisioning) - - var err error - fakeClient, err = testutil.NewFakeClientWithObjects(vmop, ourDisk, otherDisk) - Expect(err).NotTo(HaveOccurred()) - - step = NewWaitingDisksStep(fakeClient, recorder) - result, err := step.Take(ctx, vmop) - - Expect(err).NotTo(HaveOccurred()) - Expect(result).NotTo(BeNil(), "should wait because our disk is not ready") - }) - - It("should wait when our disk is not ready (other disk is ready)", func() { - vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") - vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ - {Kind: v1alpha2.VirtualDiskKind, Name: "our-disk"}, - {Kind: v1alpha2.VirtualDiskKind, Name: "other-disk"}, - } - - ourDisk := createVirtualDisk("default", "our-disk", "test-vmop-uid", v1alpha2.DiskProvisioning) - otherDisk := createVirtualDisk("default", "other-disk", "different-vmop-uid", v1alpha2.DiskReady) - - var err error - fakeClient, err = testutil.NewFakeClientWithObjects(vmop, ourDisk, otherDisk) - Expect(err).NotTo(HaveOccurred()) - - step = NewWaitingDisksStep(fakeClient, recorder) - result, err := step.Take(ctx, vmop) - - Expect(err).NotTo(HaveOccurred()) - Expect(result).NotTo(BeNil(), "should wait because our disk is not ready, regardless of other disk status") - }) - - It("should proceed when our disk is ready (other disk is not ready)", func() { - vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") - vmop.Status.Resources = []v1alpha2.SnapshotResourceStatus{ - {Kind: v1alpha2.VirtualDiskKind, Name: "our-disk"}, - {Kind: v1alpha2.VirtualDiskKind, Name: "other-disk"}, - } - - ourDisk := createVirtualDisk("default", "our-disk", "test-vmop-uid", v1alpha2.DiskReady) - otherDisk := createVirtualDisk("default", "other-disk", "different-vmop-uid", v1alpha2.DiskProvisioning) - - var err error - fakeClient, err = testutil.NewFakeClientWithObjects(vmop, ourDisk, otherDisk) - Expect(err).NotTo(HaveOccurred()) - - step = NewWaitingDisksStep(fakeClient, recorder) - result, err := step.Take(ctx, vmop) - - Expect(err).NotTo(HaveOccurred()) - Expect(result).To(BeNil(), "should proceed because our disk is ready, we don't wait for other vmop's disks") - }) - }) }) From ccca318b4a45b69843ac25f5e9af280cdf64d486 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Tue, 3 Feb 2026 18:58:52 +0300 Subject: [PATCH 12/24] resolve 3 Signed-off-by: Valeriy Khorunzhin --- .../vmop/snapshot/internal/step/cleanup_snapshot_step.go | 2 +- .../vmop/snapshot/internal/step/cleanup_snapshot_step_test.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go index e75ec9e588..1591111b91 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go @@ -60,7 +60,7 @@ func (s CleanupSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMac for _, status := range vmop.Status.Resources { if status.Status != v1alpha2.SnapshotResourceStatusCompleted { - return nil, nil + return &reconcile.Result{}, nil } } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go index 708fd319c0..1f041c06db 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go @@ -84,7 +84,8 @@ var _ = Describe("CleanupSnapshotStep", func() { result, err := step.Take(ctx, vmop) Expect(err).NotTo(HaveOccurred()) - Expect(result).To(BeNil()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) }) It("should skip when snapshot name annotation is not present", func() { From 0ce78f43196fd41285734613ffa74b5f9a0e12b0 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 4 Feb 2026 02:34:52 +0300 Subject: [PATCH 13/24] fix resolve Signed-off-by: Valeriy Khorunzhin --- .../internal/step/cleanup_snapshot_step.go | 4 +++ .../step/cleanup_snapshot_step_test.go | 15 ++++++++++ .../internal/step/process_clone_step.go | 11 ++++--- .../internal/step/process_clone_step_test.go | 10 ++++--- .../internal/step/process_restore_step.go | 5 ++-- .../step/process_restore_step_test.go | 10 ++++--- .../internal/step/vmsnapshot_ready_step.go | 22 +++++++++----- .../step/vmsnapshot_ready_step_test.go | 30 +++++++++++++++++-- 8 files changed, 81 insertions(+), 26 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go index 1591111b91..a9bca1ae75 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step.go @@ -51,6 +51,10 @@ func NewCleanupSnapshotStep( } func (s CleanupSnapshotStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { + if vmop.Spec.Clone.Mode == v1alpha2.SnapshotOperationModeDryRun { + return nil, nil + } + snapshotConditionBuilder := conditions.NewConditionBuilder(vmopcondition.TypeSnapshotReady) snapshotCondition, _ := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go index 1f041c06db..30d2b35e96 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/cleanup_snapshot_step_test.go @@ -48,6 +48,21 @@ var _ = Describe("CleanupSnapshotStep", func() { }) Describe("Skip conditions", func() { + It("should skip when clone mode is DryRun", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Spec.Clone.Mode = v1alpha2.SnapshotOperationModeDryRun + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewCleanupSnapshotStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + It("should skip when snapshot condition is already CleanedUp", func() { vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") vmop.Status.Conditions = []metav1.Condition{ diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go index 47cd929427..876b9e8fe5 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go @@ -18,7 +18,7 @@ package step import ( "context" - "fmt" + "errors" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -50,8 +50,7 @@ func NewProcessCloneStep( func (s ProcessCloneStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { snapshotName, ok := vmop.Annotations[annotations.AnnVMOPSnapshotName] if !ok { - err := fmt.Errorf("snapshot name annotation not found") - return &reconcile.Result{}, err + return &reconcile.Result{}, errors.New("snapshot name annotation not found") } vmSnapshotKey := types.NamespacedName{Namespace: vmop.Namespace, Name: snapshotName} @@ -61,7 +60,7 @@ func (s ProcessCloneStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachin } if vmSnapshot == nil { - return &reconcile.Result{}, nil + return &reconcile.Result{}, errors.New("snapshot is not found") } restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} @@ -71,7 +70,7 @@ func (s ProcessCloneStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachin } if restorerSecret == nil { - return &reconcile.Result{}, nil + return &reconcile.Result{}, errors.New("restorer secret is not found") } snapshotResources := restorer.NewSnapshotResources(s.client, v1alpha2.VMOPTypeClone, vmop.Spec.Clone.Mode, restorerSecret, vmSnapshot, string(vmop.UID)) @@ -97,7 +96,7 @@ func (s ProcessCloneStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachin } if vmop.Spec.Clone.Mode == v1alpha2.SnapshotOperationModeDryRun { - return &reconcile.Result{}, nil + return nil, nil } statuses, err = snapshotResources.Process(ctx) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go index 6b4f7c9b96..2d495b303c 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go @@ -81,7 +81,7 @@ var _ = Describe("ProcessCloneStep", func() { }) Describe("Snapshot not found", func() { - It("should wait when snapshot is not found", func() { + It("should return error when snapshot is not found", func() { vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") var err error @@ -91,13 +91,14 @@ var _ = Describe("ProcessCloneStep", func() { step = NewProcessCloneStep(fakeClient, recorder) result, err := step.Take(ctx, vmop) - Expect(err).NotTo(HaveOccurred()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("snapshot is not found")) Expect(result).NotTo(BeNil()) }) }) Describe("Secret not found", func() { - It("should wait when restorer secret is not found", func() { + It("should return error when restorer secret is not found", func() { vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) @@ -108,7 +109,8 @@ var _ = Describe("ProcessCloneStep", func() { step = NewProcessCloneStep(fakeClient, recorder) result, err := step.Take(ctx, vmop) - Expect(err).NotTo(HaveOccurred()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("restorer secret is not found")) Expect(result).NotTo(BeNil()) }) }) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go index df191d4243..511655707b 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go @@ -18,6 +18,7 @@ package step import ( "context" + "errors" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -61,7 +62,7 @@ func (s ProcessRestoreStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach } if vmSnapshot == nil { - return &reconcile.Result{}, nil + return &reconcile.Result{}, errors.New("snapshot is not found") } restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} @@ -71,7 +72,7 @@ func (s ProcessRestoreStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach } if restorerSecret == nil { - return &reconcile.Result{}, nil + return &reconcile.Result{}, errors.New("restorer secret is not found") } snapshotResources := restorer.NewSnapshotResources(s.client, v1alpha2.VMOPTypeRestore, vmop.Spec.Restore.Mode, restorerSecret, vmSnapshot, string(vmop.UID)) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go index e3ae8c63b5..236b1eb1c9 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go @@ -98,7 +98,7 @@ var _ = Describe("ProcessRestoreStep", func() { }) Describe("Snapshot not found", func() { - It("should wait when snapshot is not found", func() { + It("should return error when snapshot is not found", func() { vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") setMaintenanceCondition(vmop, metav1.ConditionTrue) @@ -109,13 +109,14 @@ var _ = Describe("ProcessRestoreStep", func() { step = NewProcessRestoreStep(fakeClient, recorder) result, err := step.Take(ctx, vmop) - Expect(err).NotTo(HaveOccurred()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("snapshot is not found")) Expect(result).NotTo(BeNil()) }) }) Describe("Secret not found", func() { - It("should wait when restorer secret is not found", func() { + It("should return error when restorer secret is not found", func() { vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") setMaintenanceCondition(vmop, metav1.ConditionTrue) snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) @@ -127,7 +128,8 @@ var _ = Describe("ProcessRestoreStep", func() { step = NewProcessRestoreStep(fakeClient, recorder) result, err := step.Take(ctx, vmop) - Expect(err).NotTo(HaveOccurred()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("restorer secret is not found")) Expect(result).NotTo(BeNil()) }) }) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go index c4fd5bd6f2..ef8a394f17 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step.go @@ -18,6 +18,7 @@ package step import ( "context" + "errors" "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -29,6 +30,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "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/api/core/v1alpha2/vmscondition" ) @@ -45,23 +47,27 @@ func NewVMSnapshotReadyStep( } func (s VMSnapshotReadyStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { - var vmopName string + snapshotCondition, _ := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + if snapshotCondition.Reason == string(vmopcondition.ReasonSnapshotCleanedUp) { + return nil, nil + } + + var snapshotName string if vmop.Spec.Type == v1alpha2.VMOPTypeRestore { if vmop.Spec.Restore.VirtualMachineSnapshotName == "" { - err := fmt.Errorf("the virtual machine snapshot name is empty") - return &reconcile.Result{}, err + return &reconcile.Result{}, errors.New("the virtual machine snapshot name is empty") } - vmopName = vmop.Spec.Restore.VirtualMachineSnapshotName + snapshotName = vmop.Spec.Restore.VirtualMachineSnapshotName } else { - snapshotName, exist := vmop.Annotations[annotations.AnnVMOPSnapshotName] + var exist bool + snapshotName, exist = vmop.Annotations[annotations.AnnVMOPSnapshotName] if !exist { - return &reconcile.Result{}, nil + return &reconcile.Result{}, errors.New("snapshot name annotation not found") } - vmopName = snapshotName } - vmSnapshotKey := types.NamespacedName{Namespace: vmop.Namespace, Name: vmopName} + vmSnapshotKey := types.NamespacedName{Namespace: vmop.Namespace, Name: snapshotName} vmSnapshot, err := object.FetchObject(ctx, vmSnapshotKey, s.client, &v1alpha2.VirtualMachineSnapshot{}) if err != nil { return &reconcile.Result{}, err diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go index 09ece46660..5116eb211e 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/vmsnapshot_ready_step_test.go @@ -21,10 +21,12 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/common/testutil" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" ) var _ = Describe("VMSnapshotReadyStep", func() { @@ -38,6 +40,29 @@ var _ = Describe("VMSnapshotReadyStep", func() { ctx = context.Background() }) + Describe("Skip conditions", func() { + It("should skip when snapshot condition is already CleanedUp", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Status.Conditions = []metav1.Condition{ + { + Type: string(vmopcondition.TypeSnapshotReady), + Status: metav1.ConditionFalse, + Reason: string(vmopcondition.ReasonSnapshotCleanedUp), + }, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop) + Expect(err).NotTo(HaveOccurred()) + + step = NewVMSnapshotReadyStep(fakeClient) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + }) + }) + Describe("Restore operation", func() { It("should return error when snapshot name is empty", func() { vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "") @@ -127,7 +152,7 @@ var _ = Describe("VMSnapshotReadyStep", func() { }) Describe("Clone operation", func() { - It("should wait when snapshot annotation is not set", func() { + It("should return error when snapshot annotation is not set", func() { vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") delete(vmop.Annotations, annotations.AnnVMOPSnapshotName) @@ -138,7 +163,8 @@ var _ = Describe("VMSnapshotReadyStep", func() { step = NewVMSnapshotReadyStep(fakeClient) result, err := step.Take(ctx, vmop) - Expect(err).NotTo(HaveOccurred()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("snapshot name annotation not found")) Expect(result).NotTo(BeNil()) }) From 50cbb4c97f3192d7c90eea07a2b73660aaebce99 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 11 Feb 2026 18:24:36 +0300 Subject: [PATCH 14/24] resolve Signed-off-by: Valeriy Khorunzhin --- .../controller/vmop/snapshot/internal/step/completed_step.go | 2 +- .../vmop/snapshot/internal/step/enter_maintenance_step.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go index 2b3c9b8ac7..a6d4d2e1f5 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/completed_step.go @@ -45,6 +45,6 @@ func (s CompletedStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOp cb := conditions.NewConditionBuilder(vmopcondition.TypeCompleted).Status(metav1.ConditionTrue).Reason(vmopcondition.ReasonOperationCompleted) conditions.SetCondition(cb, &vmop.Status.Conditions) vmop.Status.Phase = v1alpha2.VMOPPhaseCompleted - s.recorder.Event(vmop, corev1.EventTypeNormal, v1alpha2.ReasonVMOPSucceeded, "VirtualMachineOperation is successful completed") + s.recorder.Event(vmop, corev1.EventTypeNormal, v1alpha2.ReasonVMOPSucceeded, "VirtualMachineOperation is successfully completed") return &reconcile.Result{}, nil } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go index cdf4b6c78f..1a9a47cabd 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/enter_maintenance_step.go @@ -55,7 +55,7 @@ func (s EnterMaintenanceStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMa return nil, nil } - // Resources contains the list of resources that are affected by the snapshot operation. + // If resources are already populated, restore is in progress or done; do not re-enter maintenance mode. if vmop.Status.Resources != nil { return nil, nil } From 279d3f7443c07e7e54f1aaf157fb5c3231b2e43a Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 11 Feb 2026 18:36:04 +0300 Subject: [PATCH 15/24] resolve test Signed-off-by: Valeriy Khorunzhin --- .../internal/step/create_snapshot_step_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step_test.go index 2b97121759..b47c46a205 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/create_snapshot_step_test.go @@ -67,6 +67,11 @@ var _ = Describe("CreateSnapshotStep", func() { Expect(err).NotTo(HaveOccurred()) Expect(result).To(BeNil()) + + snapshotReadyCondition, ok := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + Expect(ok).To(BeTrue()) + Expect(snapshotReadyCondition.Status).To(Equal(metav1.ConditionTrue)) + Expect(snapshotReadyCondition.Reason).To(Equal(string(vmopcondition.ReasonSnapshotOperationReady))) }) It("should skip when snapshot condition reason is CleanedUp", func() { @@ -88,6 +93,11 @@ var _ = Describe("CreateSnapshotStep", func() { Expect(err).NotTo(HaveOccurred()) Expect(result).To(BeNil()) + + snapshotReadyCondition, ok := conditions.GetCondition(vmopcondition.TypeSnapshotReady, vmop.Status.Conditions) + Expect(ok).To(BeTrue()) + Expect(snapshotReadyCondition.Status).To(Equal(metav1.ConditionFalse)) + Expect(snapshotReadyCondition.Reason).To(Equal(string(vmopcondition.ReasonSnapshotCleanedUp))) }) }) From 888cb2869a1a8b2c5da98d6a9f955ea6fb588bd9 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Tue, 17 Feb 2026 11:44:33 +0300 Subject: [PATCH 16/24] fix dryrun Signed-off-by: Valeriy Khorunzhin --- .../internal/step/process_restore_step.go | 2 +- .../step/process_restore_step_test.go | 21 +++++++++++++++++++ .../vmop/snapshot/internal/step/suite_test.go | 19 +++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go index 511655707b..5acd12c9f7 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go @@ -51,7 +51,7 @@ func NewProcessRestoreStep( func (s ProcessRestoreStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (*reconcile.Result, error) { maintenanceModeCondition, found := conditions.GetCondition(vmopcondition.TypeMaintenanceMode, vmop.Status.Conditions) - if !found || maintenanceModeCondition.Status == metav1.ConditionFalse { + if vmop.Spec.Restore.Mode != v1alpha2.SnapshotOperationModeDryRun && (!found || maintenanceModeCondition.Status == metav1.ConditionFalse) { return &reconcile.Result{}, nil } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go index 236b1eb1c9..92e04d7b57 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go @@ -27,6 +27,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/testutil" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" + "github.com/deckhouse/virtualization/api/core/v1alpha2" ) var _ = Describe("ProcessRestoreStep", func() { @@ -43,6 +44,26 @@ var _ = Describe("ProcessRestoreStep", func() { }) Describe("Maintenance mode check", func() { + It("Should pass if restore mode dryrun", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + vmop.Spec.Restore.Mode = v1alpha2.SnapshotOperationModeDryRun + + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineRunning) + restorerSecret := createRestorerSecret("default", "test-secret", vm) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot, restorerSecret) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessRestoreStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + Expect(vmop.Status.Resources).NotTo(BeNil()) + }) + It("should wait when maintenance condition is not found", func() { vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go index db4930355a..dbae132f3c 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go @@ -17,10 +17,12 @@ limitations under the License. package step import ( + "encoding/json" "testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -78,6 +80,23 @@ func createVMSnapshot(namespace, name, secretName string, ready bool) *v1alpha2. return vms } +func createRestorerSecret(namespace, name string, vm *v1alpha2.VirtualMachine) *corev1.Secret { + vmJSON, err := json.Marshal(vm) + if err != nil { + panic(err) + } + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: map[string][]byte{ + "vm": vmJSON, + }, + Type: "virtualmachine.virtualization.deckhouse.io/snapshot", + } +} + //nolint:unparam // namespace is always "default" in tests, but kept for flexibility func createRestoreVMOP(namespace, name, vmName, snapshotName string) *v1alpha2.VirtualMachineOperation { return &v1alpha2.VirtualMachineOperation{ From ab42d824e54df9a25632c11667ebf801e8920ae4 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 18 Feb 2026 17:19:50 +0300 Subject: [PATCH 17/24] fix restoring resources Signed-off-by: Valeriy Khorunzhin --- .../internal/step/process_clone_step.go | 6 ++ .../internal/step/process_clone_step_test.go | 71 +++++++++++++++++++ .../internal/step/process_restore_step.go | 6 ++ .../step/process_restore_step_test.go | 58 +++++++++++++++ .../vmop/snapshot/internal/step/suite_test.go | 32 +++++++++ 5 files changed, 173 insertions(+) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go index 876b9e8fe5..c4b70d2f96 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go @@ -105,5 +105,11 @@ func (s ProcessCloneStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachin return &reconcile.Result{}, err } + for _, status := range statuses { + if status.Status != v1alpha2.SnapshotResourceStatusCompleted { + return &reconcile.Result{}, nil + } + } + return nil, nil } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go index 2d495b303c..8a9a03544f 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go @@ -18,14 +18,20 @@ package step import ( "context" + "fmt" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/common/testutil" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" + "github.com/deckhouse/virtualization/api/core/v1alpha2" ) var _ = Describe("ProcessCloneStep", func() { @@ -114,4 +120,69 @@ var _ = Describe("ProcessCloneStep", func() { Expect(result).NotTo(BeNil()) }) }) + + Describe("Process completion", func() { + It("should complete when all resources are Completed after Process", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineRunning) + restorerSecret := createRestorerSecret("default", "test-secret", vm) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot, restorerSecret) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessCloneStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + Expect(vmop.Status.Resources).NotTo(BeEmpty()) + for _, status := range vmop.Status.Resources { + Expect(status.Status).To(Equal(v1alpha2.SnapshotResourceStatusCompleted)) + } + }) + + It("should requeue when not all resources are Completed after Process", func() { + vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineRunning) + + vmbda := createVMBDA("default", "test-vmbda", "test-vm") + restorerSecret := createRestorerSecretWithVMBDAs("default", "test-secret", vm, []*v1alpha2.VirtualMachineBlockDeviceAttachment{vmbda}) + + intercept := interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + if _, ok := obj.(*v1alpha2.VirtualMachineBlockDeviceAttachment); ok { + return apierrors.NewConflict( + schema.GroupResource{Resource: "virtualmachineblockdeviceattachments"}, + obj.GetName(), + fmt.Errorf("the object has been modified"), + ) + } + return c.Create(ctx, obj, opts...) + }, + } + + var err error + fakeClient, err = testutil.NewFakeClientWithInterceptorWithObjects(intercept, vmop, snapshot, restorerSecret) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessCloneStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + Expect(vmop.Status.Resources).NotTo(BeEmpty()) + + hasInProgress := false + for _, status := range vmop.Status.Resources { + if status.Status == v1alpha2.SnapshotResourceStatusInProgress { + hasInProgress = true + } + } + Expect(hasInProgress).To(BeTrue(), "expected at least one resource with InProgress status") + }) + }) }) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go index 5acd12c9f7..eb7d8e3447 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step.go @@ -98,5 +98,11 @@ func (s ProcessRestoreStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach return &reconcile.Result{}, err } + for _, status := range statuses { + if status.Status != v1alpha2.SnapshotResourceStatusCompleted { + return &reconcile.Result{}, nil + } + } + return nil, nil } diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go index 92e04d7b57..7c3050d2d1 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go @@ -28,6 +28,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/testutil" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" ) var _ = Describe("ProcessRestoreStep", func() { @@ -154,4 +155,61 @@ var _ = Describe("ProcessRestoreStep", func() { Expect(result).NotTo(BeNil()) }) }) + + Describe("Process completion", func() { + It("should complete when all resources are Completed after Process", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + setMaintenanceCondition(vmop, metav1.ConditionTrue) + + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineRunning) + restorerSecret := createRestorerSecret("default", "test-secret", vm) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot, restorerSecret) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessRestoreStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeNil()) + Expect(vmop.Status.Resources).NotTo(BeEmpty()) + for _, status := range vmop.Status.Resources { + Expect(status.Status).To(Equal(v1alpha2.SnapshotResourceStatusCompleted)) + } + }) + + It("should requeue when not all resources are Completed after Process", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") + setMaintenanceCondition(vmop, metav1.ConditionTrue) + + snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) + vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineRunning) + setVMMaintenanceCondition(vm, metav1.ConditionTrue, vmcondition.ReasonMaintenanceRestore) + + vmbda := createVMBDA("default", "test-vmbda", "test-vm") + restorerSecret := createRestorerSecretWithVMBDAs("default", "test-secret", vm, []*v1alpha2.VirtualMachineBlockDeviceAttachment{vmbda}) + + var err error + fakeClient, err = testutil.NewFakeClientWithObjects(vmop, snapshot, restorerSecret, vm, vmbda) + Expect(err).NotTo(HaveOccurred()) + + step = NewProcessRestoreStep(fakeClient, recorder) + result, err := step.Take(ctx, vmop) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(*result).To(Equal(reconcile.Result{})) + Expect(vmop.Status.Resources).NotTo(BeEmpty()) + + hasInProgress := false + for _, status := range vmop.Status.Resources { + if status.Status == v1alpha2.SnapshotResourceStatusInProgress { + hasInProgress = true + } + } + Expect(hasInProgress).To(BeTrue(), "expected at least one resource with InProgress status") + }) + }) }) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go index dbae132f3c..0762dd1dce 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go @@ -201,3 +201,35 @@ func setVMMaintenanceCondition(vm *v1alpha2.VirtualMachine, status metav1.Condit Reason: string(reason), }) } + +func createVMBDA(namespace, name, vmName string) *v1alpha2.VirtualMachineBlockDeviceAttachment { + return &v1alpha2.VirtualMachineBlockDeviceAttachment{ + TypeMeta: metav1.TypeMeta{ + Kind: v1alpha2.VirtualMachineBlockDeviceAttachmentKind, + APIVersion: v1alpha2.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha2.VirtualMachineBlockDeviceAttachmentSpec{ + VirtualMachineName: vmName, + BlockDeviceRef: v1alpha2.VMBDAObjectRef{ + Kind: v1alpha2.VMBDAObjectRefKindVirtualDisk, + Name: "test-disk", + }, + }, + } +} + +func createRestorerSecretWithVMBDAs(namespace, name string, vm *v1alpha2.VirtualMachine, vmbdas []*v1alpha2.VirtualMachineBlockDeviceAttachment) *corev1.Secret { + secret := createRestorerSecret(namespace, name, vm) + if len(vmbdas) > 0 { + vmbdasJSON, err := json.Marshal(vmbdas) + if err != nil { + panic(err) + } + secret.Data["vmbdas"] = vmbdasJSON + } + return secret +} From 51c9e559fbcd70f14f307fa11706ba6075461e67 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 18 Feb 2026 18:22:42 +0300 Subject: [PATCH 18/24] Update images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go Co-authored-by: Ivan Mikheykin Signed-off-by: Valeriy Khorunzhin --- .../pkg/controller/vmop/snapshot/internal/step/suite_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go index 0762dd1dce..68724a4907 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go @@ -222,6 +222,7 @@ func createVMBDA(namespace, name, vmName string) *v1alpha2.VirtualMachineBlockDe } } +//nolint:unparam // namespace is always "default" in tests, but kept for flexibility func createRestorerSecretWithVMBDAs(namespace, name string, vm *v1alpha2.VirtualMachine, vmbdas []*v1alpha2.VirtualMachineBlockDeviceAttachment) *corev1.Secret { secret := createRestorerSecret(namespace, name, vm) if len(vmbdas) > 0 { From eb588ecf32f53dd6d01cf91847e649e6fedfc11f Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 18 Feb 2026 18:23:03 +0300 Subject: [PATCH 19/24] Update images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go Co-authored-by: Ivan Mikheykin Signed-off-by: Valeriy Khorunzhin --- .../pkg/controller/vmop/snapshot/internal/step/suite_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go index 68724a4907..cbbac4eafd 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go @@ -202,6 +202,7 @@ func setVMMaintenanceCondition(vm *v1alpha2.VirtualMachine, status metav1.Condit }) } +//nolint:unparam // namespace is always "default" in tests, but kept for flexibility func createVMBDA(namespace, name, vmName string) *v1alpha2.VirtualMachineBlockDeviceAttachment { return &v1alpha2.VirtualMachineBlockDeviceAttachment{ TypeMeta: metav1.TypeMeta{ From 0c4c0f858a10514f7ca6244c5f100bf8ca3bcc38 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 18 Feb 2026 18:23:21 +0300 Subject: [PATCH 20/24] Update images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go Co-authored-by: Ivan Mikheykin Signed-off-by: Valeriy Khorunzhin --- .../vmop/snapshot/internal/step/process_restore_step_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go index 7c3050d2d1..cca8268a50 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go @@ -157,7 +157,7 @@ var _ = Describe("ProcessRestoreStep", func() { }) Describe("Process completion", func() { - It("should complete when all resources are Completed after Process", func() { + It("should complete after running Process if all resources are in the Completed state", func() { vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") setMaintenanceCondition(vmop, metav1.ConditionTrue) From 94d236442a07fdccbc0d417b31093fe3b04dd3a8 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 18 Feb 2026 18:23:31 +0300 Subject: [PATCH 21/24] Update images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go Co-authored-by: Ivan Mikheykin Signed-off-by: Valeriy Khorunzhin --- .../vmop/snapshot/internal/step/process_restore_step_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go index cca8268a50..08baa40d3e 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go @@ -180,7 +180,8 @@ var _ = Describe("ProcessRestoreStep", func() { } }) - It("should requeue when not all resources are Completed after Process", func() { + It("should requeue after running Process if there are resources in the Completed state", func() { + vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") setMaintenanceCondition(vmop, metav1.ConditionTrue) From 394461400565b43952a3ec8f8fe5b96bbc0276f1 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 18 Feb 2026 18:23:42 +0300 Subject: [PATCH 22/24] Update images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go Co-authored-by: Ivan Mikheykin Signed-off-by: Valeriy Khorunzhin --- .../vmop/snapshot/internal/step/process_clone_step_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go index 8a9a03544f..a571047e11 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go @@ -143,7 +143,7 @@ var _ = Describe("ProcessCloneStep", func() { } }) - It("should requeue when not all resources are Completed after Process", func() { + It("should requeue after running Process if there are resources in the Completed state", func() { vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineRunning) From e9d8c30bf1cc1a651255d0fef72cac7433a61369 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 18 Feb 2026 18:23:54 +0300 Subject: [PATCH 23/24] Update images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go Co-authored-by: Ivan Mikheykin Signed-off-by: Valeriy Khorunzhin --- .../vmop/snapshot/internal/step/process_clone_step_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go index a571047e11..465a0a36f0 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step_test.go @@ -122,7 +122,7 @@ var _ = Describe("ProcessCloneStep", func() { }) Describe("Process completion", func() { - It("should complete when all resources are Completed after Process", func() { + It("should complete after running Process if all resources are in the Completed state", func() { vmop := createCloneVMOP("default", "test-vmop", "test-vm", "test-snapshot") snapshot := createVMSnapshot("default", "test-snapshot", "test-secret", true) vm := createVirtualMachine("default", "test-vm", v1alpha2.MachineRunning) From 648e9870f269e59a246a497402156422d1fdb5cd Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 18 Feb 2026 18:37:02 +0300 Subject: [PATCH 24/24] fix linter Signed-off-by: Valeriy Khorunzhin --- .../vmop/snapshot/internal/step/process_restore_step_test.go | 1 - .../pkg/controller/vmop/snapshot/internal/step/suite_test.go | 2 -- 2 files changed, 3 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go index 08baa40d3e..678d36d569 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_restore_step_test.go @@ -181,7 +181,6 @@ var _ = Describe("ProcessRestoreStep", func() { }) It("should requeue after running Process if there are resources in the Completed state", func() { - vmop := createRestoreVMOP("default", "test-vmop", "test-vm", "test-snapshot") setMaintenanceCondition(vmop, metav1.ConditionTrue) diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go index cbbac4eafd..0762dd1dce 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/suite_test.go @@ -202,7 +202,6 @@ func setVMMaintenanceCondition(vm *v1alpha2.VirtualMachine, status metav1.Condit }) } -//nolint:unparam // namespace is always "default" in tests, but kept for flexibility func createVMBDA(namespace, name, vmName string) *v1alpha2.VirtualMachineBlockDeviceAttachment { return &v1alpha2.VirtualMachineBlockDeviceAttachment{ TypeMeta: metav1.TypeMeta{ @@ -223,7 +222,6 @@ func createVMBDA(namespace, name, vmName string) *v1alpha2.VirtualMachineBlockDe } } -//nolint:unparam // namespace is always "default" in tests, but kept for flexibility func createRestorerSecretWithVMBDAs(namespace, name string, vm *v1alpha2.VirtualMachine, vmbdas []*v1alpha2.VirtualMachineBlockDeviceAttachment) *corev1.Secret { secret := createRestorerSecret(namespace, name, vm) if len(vmbdas) > 0 {