refactor: extract method vm_ref_or_raise

Related to bp xenapi-volume-drivers

Imporve the code by extract method, and testing.

Change-Id: I8caca173266890bd99a360671e7af86759b89201

3 files changed, 69 insertions, 8 deletions

diff --git a/nova/tests/xenapi/test_vm_utils.py b/nova/tests/xenapi/test_vm_utils.py
index e7992d54f..c78d42fd3 100644
--- a/nova/tests/xenapi/test_vm_utils.py
+++ b/nova/tests/xenapi/test_vm_utils.py
@@ -1,9 +1,11 @@
+import mox
 from nova import exception
 from nova.tests.xenapi import stubs
 from nova.virt.xenapi import driver as xenapi_conn
 from nova.virt.xenapi import fake
 from nova.virt.xenapi import vm_utils
 from nova.virt.xenapi import volume_utils
+import unittest
 
 
 class GetInstanceForVdisForSrTestCase(stubs.XenAPITestBase):
@@ -81,3 +83,57 @@ class GetInstanceForVdisForSrTestCase(stubs.XenAPITestBase):
         self.assertRaises(exception.NovaException,
                           vm_utils.get_vdis_for_boot_from_vol,
                           driver._session, dev_params)
+
+
+class VMRefOrRaiseVMFoundTestCase(unittest.TestCase):
+
+    def test_lookup_call(self):
+        mock = mox.Mox()
+        mock.StubOutWithMock(vm_utils, 'lookup')
+
+        vm_utils.lookup('session', 'somename').AndReturn('ignored')
+
+        mock.ReplayAll()
+        vm_utils.vm_ref_or_raise('session', 'somename')
+        mock.VerifyAll()
+
+    def test_return_value(self):
+        mock = mox.Mox()
+        mock.StubOutWithMock(vm_utils, 'lookup')
+
+        vm_utils.lookup(mox.IgnoreArg(), mox.IgnoreArg()).AndReturn('vmref')
+
+        mock.ReplayAll()
+        self.assertEquals(
+            'vmref', vm_utils.vm_ref_or_raise('session', 'somename'))
+        mock.VerifyAll()
+
+
+class VMRefOrRaiseVMNotFoundTestCase(unittest.TestCase):
+
+    def test_exception_raised(self):
+        mock = mox.Mox()
+        mock.StubOutWithMock(vm_utils, 'lookup')
+
+        vm_utils.lookup('session', 'somename').AndReturn(None)
+
+        mock.ReplayAll()
+        self.assertRaises(
+            exception.InstanceNotFound,
+            lambda: vm_utils.vm_ref_or_raise('session', 'somename')
+        )
+        mock.VerifyAll()
+
+    def test_exception_msg_contains_vm_name(self):
+        mock = mox.Mox()
+        mock.StubOutWithMock(vm_utils, 'lookup')
+
+        vm_utils.lookup('session', 'somename').AndReturn(None)
+
+        mock.ReplayAll()
+        try:
+            vm_utils.vm_ref_or_raise('session', 'somename')
+        except exception.InstanceNotFound as e:
+            self.assertTrue(
+                'somename' in str(e))
+        mock.VerifyAll()
diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index b08dd1387..4879a857d 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -2221,3 +2221,10 @@ def move_disks(session, instance, disk_info):
     root_vdi_ref = session.call_xenapi('VDI.get_by_uuid', root_uuid)
 
     return {'uuid': root_uuid, 'ref': root_vdi_ref}
+
+
+def vm_ref_or_raise(session, instance_name):
+    vm_ref = lookup(session, instance_name)
+    if vm_ref is None:
+        raise exception.InstanceNotFound(instance_id=instance_name)
+    return vm_ref
diff --git a/nova/virt/xenapi/volumeops.py b/nova/virt/xenapi/volumeops.py
index 16da85b9d..c44e38f92 100644
--- a/nova/virt/xenapi/volumeops.py
+++ b/nova/virt/xenapi/volumeops.py
@@ -107,10 +107,9 @@ class VolumeOps(object):
 
     def attach_volume(self, connection_info, instance_name, mountpoint):
         """Attach volume storage to VM instance"""
-        # Before we start, check that the VM exists
-        vm_ref = vm_utils.lookup(self._session, instance_name)
-        if vm_ref is None:
-            raise exception.InstanceNotFound(instance_id=instance_name)
+
+        vm_ref = vm_utils.vm_ref_or_raise(self._session, instance_name)
+
         # NOTE: No Resource Pool concept so far
         LOG.debug(_("Attach_volume: %(connection_info)s, %(instance_name)s,"
                 " %(mountpoint)s") % locals())
@@ -196,10 +195,9 @@ class VolumeOps(object):
 
     def detach_volume(self, connection_info, instance_name, mountpoint):
         """Detach volume storage to VM instance"""
-        # Before we start, check that the VM exists
-        vm_ref = vm_utils.lookup(self._session, instance_name)
-        if vm_ref is None:
-            raise exception.InstanceNotFound(instance_id=instance_name)
+
+        vm_ref = vm_utils.vm_ref_or_raise(self._session, instance_name)
+
         # Detach VBD from VM
         LOG.debug(_("Detach_volume: %(instance_name)s, %(mountpoint)s")
                 % locals())