From ca7e5e6a222b2bd56f380bb9bda2a4edab1b413b Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Mon, 26 Jan 2026 18:37:47 +0300 Subject: [PATCH 1/5] 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 0bf20457edd0da779eb33034f7610ed7e619c0e3 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Thu, 29 Jan 2026 11:36:22 +0300 Subject: [PATCH 2/5] 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 0f510669ad081e2e66441ee38b3e484228dffe3b Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Thu, 29 Jan 2026 18:37:05 +0300 Subject: [PATCH 3/5] 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 fed2a40b5c7151b774fc73bb0d37da13148fbcb9 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Thu, 29 Jan 2026 18:56:25 +0300 Subject: [PATCH 4/5] 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 aa3817c451826c168e22942f96cb5cf70724d66e Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Thu, 29 Jan 2026 19:22:15 +0300 Subject: [PATCH 5/5] 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 }