Skip to content
This repository has been archived by the owner on Feb 7, 2024. It is now read-only.

Commit

Permalink
Merge pull request #492 from mansam/defer-storing-vm-status-til-shutdown
Browse files Browse the repository at this point in the history 
Defer storing the VM power status until immediately before stopping it
  • Loading branch information
@fabiendupont
fabiendupont authored May 11, 2021
2 parents 019be36 + fe1ed21 commit 66c7829
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 41 deletions.
35 changes: 20 additions & 15 deletions pkg/controller/virtualmachineimport/virtualmachineimport_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,18 @@ func (r *ReconcileVirtualMachineImport) Reconcile(request reconcile.Request) (re

// don't stop the VM during a warm import unless it's time to finalize
if !shouldWarmImport(provider, instance) || shouldFinalizeWarmImport(instance) {
if _, ok := instance.Annotations[sourceVMInitialState]; !ok {
vmStatus, err := provider.GetVMStatus()
if err != nil {
return reconcile.Result{}, err
}

log.Info("Storing source VM status", "status", vmStatus)
err = r.storeSourceVMStatus(instance, string(vmStatus))
if err != nil {
return reconcile.Result{}, err
}
}
// Stop the VM
if err = provider.StopVM(instance, r.client); err != nil {
return reconcile.Result{}, err
Expand Down Expand Up @@ -610,11 +622,11 @@ func (r *ReconcileVirtualMachineImport) importDisks(provider provider.Provider,
}
if valid {
log.Info("Creating data volume", "DataVolume.Name", dv.Name, "VM.Name", vmName)
if _, err = r.createDataVolume(provider, mapper, instance, &dv, vmName); err != nil {
if err = r.endDiskImportFailed(provider, instance, foundDv, err.Error()); err != nil {
if _, createErr := r.createDataVolume(provider, mapper, instance, &dv, vmName); createErr != nil {
if err = r.endDiskImportFailed(provider, instance, foundDv, createErr.Error()); err != nil {
return false, err
}
return false, err
return false, createErr
}
} else {
// If disk status is wrong, end the import as failed:
Expand Down Expand Up @@ -1031,7 +1043,11 @@ func shouldReconcile(instance *v2vv1.VirtualMachineImport) bool {
}

func shouldStartVM(instance *v2vv1.VirtualMachineImport) bool {
return instance.Spec.StartVM != nil && *instance.Spec.StartVM && conditions.HasSucceededConditionOfReason(instance.Status.Conditions, v2vv1.VirtualMachineReady)
return conditions.HasSucceededConditionOfReason(instance.Status.Conditions, v2vv1.VirtualMachineReady) &&
((instance.Spec.StartVM != nil && *instance.Spec.StartVM) ||
(instance.Spec.StartVM == nil &&
instance.Spec.Warm &&
instance.Annotations[sourceVMInitialState] == string(provider.VMStatusUp)))
}

func shouldConvertGuest(provider provider.Provider, instance *v2vv1.VirtualMachineImport) bool {
Expand Down Expand Up @@ -1591,17 +1607,6 @@ func (r *ReconcileVirtualMachineImport) validate(instance *v2vv1.VirtualMachineI
r.recorder.Eventf(instance, corev1.EventTypeNormal, EventImportBlocked, "Virtual Machine %s/%s import blocked: %s", instance.Namespace, vmName, message)
return false, nil
}

vmStatus, err := provider.GetVMStatus()
if err != nil {
return true, err
}

logger.Info("Storing source VM status", "status", vmStatus)
err = r.storeSourceVMStatus(instance, string(vmStatus))
if err != nil {
return true, err
}
} else {
logger.Info("VirtualMachineImport has already been validated positively. Skipping re-validation")
}
Expand Down
33 changes: 7 additions & 26 deletions pkg/controller/virtualmachineimport/virtualmachineimport_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ var _ = Describe("Reconcile steps", func() {
})

It("should fail update vmimport with conditions: ", func() {
statusPatch = func(ctx context.Context, obj runtime.Object, patch client.Patch) error {
update = func(ctx context.Context, obj runtime.Object, opts ...client.UpdateOption) error {
return fmt.Errorf("Not modified")
}

Expand All @@ -480,31 +480,6 @@ var _ = Describe("Reconcile steps", func() {
Expect(err).To(BeNil())
Expect(validated).To(Equal(false))
})

It("should fail with vm status: ", func() {
getVMStatus = func() (provider.VMStatus, error) {
return "", fmt.Errorf("Not found")
}

validated, err := reconciler.validate(instance, mock)

Expect(err).To(Not(BeNil()))
Expect(validated).To(Equal(true))
})

It("should fail to store vm status: ", func() {
statusPatch = func(ctx context.Context, obj runtime.Object, patch client.Patch) error {
if obj.(*v2vv1.VirtualMachineImport).Annotations[sourceVMInitialState] == string(provider.VMStatusDown) {
return fmt.Errorf("Not modified")
}
return nil
}

validated, err := reconciler.validate(instance, mock)

Expect(err).To(Not(BeNil()))
Expect(validated).To(Equal(true))
})
})

Describe("createVM step", func() {
Expand Down Expand Up @@ -1401,6 +1376,8 @@ var _ = Describe("Reconcile steps", func() {
Type: v2vv1.MappingRulesVerified,
})
obj.(*v2vv1.VirtualMachineImport).Status.Conditions = conditions
annotations := map[string]string{"vmimport.v2v.kubevirt.io/source-vm-initial-state": "up"}
obj.(*v2vv1.VirtualMachineImport).Annotations = annotations
name := "test"
obj.(*v2vv1.VirtualMachineImport).Spec.TargetVMName = &name
case *corev1.Secret:
Expand Down Expand Up @@ -1531,6 +1508,7 @@ var _ = Describe("Reconcile steps", func() {
get = func(ctx context.Context, key client.ObjectKey, obj runtime.Object) error {
switch obj.(type) {
case *v2vv1.VirtualMachineImport:

obj.(*v2vv1.VirtualMachineImport).Spec = v2vv1.VirtualMachineImportSpec{
Source: v2vv1.VirtualMachineImportSourceSpec{
Ovirt: &v2vv1.VirtualMachineImportOvirtSourceSpec{},
Expand Down Expand Up @@ -1609,6 +1587,8 @@ var _ = Describe("Reconcile steps", func() {
Type: v2vv1.MappingRulesVerified,
})
obj.(*v2vv1.VirtualMachineImport).Status.Conditions = conditions
annotations := map[string]string{"vmimport.v2v.kubevirt.io/source-vm-initial-state": "up"}
obj.(*v2vv1.VirtualMachineImport).Annotations = annotations
name := "test"
obj.(*v2vv1.VirtualMachineImport).Spec.TargetVMName = &name
case *corev1.Secret:
Expand Down Expand Up @@ -1781,6 +1761,7 @@ var _ = Describe("Reconcile steps", func() {
)
BeforeEach(func() {
config = &v2vv1.VirtualMachineImport{}
config.Annotations = map[string]string{"vmimport.v2v.kubevirt.io/source-vm-initial-state": "up"}
config.Spec = v2vv1.VirtualMachineImportSpec{
Source: v2vv1.VirtualMachineImportSourceSpec{
Ovirt: &v2vv1.VirtualMachineImportOvirtSourceSpec{},
Expand Down

0 comments on commit 66c7829

Please sign in to comment.