[Kimchi-devel] [PATCH V2 6/6] Fix Kimchi vm tests according to new messages sent from backend

[Crístian Viana]

AFAIU, here is what happens in the method "test_vm_update": the VMs 
"kimchi-vm1" and "kimchi-vm2" are created; the commands to *delete those 
VMS by their names* are deferred in the transaction; *the VM 
"kimchi-vm1" is renamed to "пeω-∨м"*; the command to *delete "пeω-∨м" by 
its name* is deferred in the transaction.

Notice how the VM is deleted twice. Both "kimchi-vm1" and "пeω-∨м" are 
the same VMs, with different names in different moments. The command 
"vm_delete" fails for two reasons: 1) the VM "kimchi-vm1" does not exist 
anymore when "vm_delete" is executed (NotFoundError); 2) even if it did, 
you cannot delete a VM twice (also NotFoundError).
So the correct solution would be to not delete the VM twice, and keep 
track of the names they have during the tests. If the VMs are renamed, 
the deferred commands will fail in the future when they execute.

The same analysis can be done to work out a better solution for the 
"vm_stop" case. That function "_rollback_wrapper" looks very hacky and 
there should be a way not to need it.

Am 26-02-2014 16:05, schrieb Rodrigo Trujillo:
> With new backend messaging system, when a VM is not found the backend
> raises an exception which was breaking Rollback functions. This patch
> fixes this problem.
>
> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
> ---
>   tests/test_model.py | 29 +++++++++++++++++++++++------
>   1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/tests/test_model.py b/tests/test_model.py
> index 74e2424..d48377d 100644
> --- a/tests/test_model.py
> +++ b/tests/test_model.py
> @@ -547,14 +547,17 @@ class ModelTests(unittest.TestCase):
>               params_2 = {'name': 'kimchi-vm2', 'template': '/templates/test'}
>               inst.vms_create(params_1)
>               inst.vms_create(params_2)
> -            rollback.prependDefer(inst.vm_delete, 'kimchi-vm1')
> -            rollback.prependDefer(inst.vm_delete, 'kimchi-vm2')
> +            rollback.prependDefer(self._rollback_wrapper, inst.vm_delete,
> +                                  'kimchi-vm1')
> +            rollback.prependDefer(self._rollback_wrapper, inst.vm_delete,
> +                                  'kimchi-vm2')
>   
>               vms = inst.vms_get_list()
>               self.assertTrue('kimchi-vm1' in vms)
>   
>               inst.vm_start('kimchi-vm1')
> -            rollback.prependDefer(inst.vm_stop, 'kimchi-vm1')
> +            rollback.prependDefer(self._rollback_wrapper, inst.vm_stop,
> +                                  'kimchi-vm1')
>   
>               info = inst.vm_lookup('kimchi-vm1')
>               self.assertEquals('running', info['state'])
> @@ -569,7 +572,8 @@ class ModelTests(unittest.TestCase):
>                                 'kimchi-vm1', {'name': 'kimchi-vm2'})
>               inst.vm_update('kimchi-vm1', params)
>               self.assertEquals(info['uuid'], inst.vm_lookup(u'пeω-∨м')['uuid'])
> -            rollback.prependDefer(inst.vm_delete, u'пeω-∨м')
> +            rollback.prependDefer(self._rollback_wrapper, inst.vm_delete,
> +                                  u'пeω-∨м')
>   
>       @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
>       def test_network(self):
> @@ -758,6 +762,17 @@ class ModelTests(unittest.TestCase):
>                             inst.task_lookup(taskid)['message'])
>           self.assertEquals('failed', inst.task_lookup(taskid)['status'])
>   
> +    # This wrapper function is needed due to the new backend messaging in
> +    # vm model. vm_stop and vm_delete raise exception if vm is not found.
> +    # These functions are called after vm has been deleted if test finishes
> +    # correctly, then NofFoundError exception is raised and rollback breaks
> +    def _rollback_wrapper(self, func, vmname):
> +        try:
> +            func(vmname)
> +        except NotFoundError:
> +            # VM has been deleted already
> +            return
> +
>       @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
>       def test_delete_running_vm(self):
>           inst = model.Model(objstore_loc=self.tmp_store)
> @@ -769,10 +784,12 @@ class ModelTests(unittest.TestCase):
>   
>               params = {'name': u'kīмсhī-∨м', 'template': u'/templates/test'}
>               inst.vms_create(params)
> -            rollback.prependDefer(inst.vm_delete, u'kīмсhī-∨м')
> +            rollback.prependDefer(self._rollback_wrapper, inst.vm_delete,
> +                                  u'kīмсhī-∨м')
>   
>               inst.vm_start(u'kīмсhī-∨м')
> -            rollback.prependDefer(inst.vm_stop, u'kīмсhī-∨м')
> +            rollback.prependDefer(self._rollback_wrapper, inst.vm_stop,
> +                                  u'kīмсhī-∨м')
>   
>               inst.vm_delete(u'kīмсhī-∨м')
>   

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140226/302fb9dd/attachment.html>