Make libvirt LoopingCalls actually wait()

 * Adds wait() to all the LoopingCall timers
 * Fixes loopingCalls in baremetal/proxy.py as well
 * Includes failing test to verify result from destroy
 * Fixes tests depending on improper behavior
 * Fixes bug 1015355

Change-Id: I9e1914c446170e49f0aab76c7745b55d12132425

3 files changed, 73 insertions, 17 deletions

diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index 3ac89135f..ccf271bed 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -1644,9 +1644,11 @@ class LibvirtConnTestCase(test.TestCase):
 
     @test.skip_if(missing_libvirt(), "Test requires libvirt")
     def test_immediate_delete(self):
+        def fake_lookup_by_name(instance_name):
+            raise exception.InstanceNotFound()
+
         conn = connection.LibvirtDriver(False)
-        self.mox.StubOutWithMock(connection.LibvirtDriver, '_conn')
-        connection.LibvirtDriver._conn.lookupByName = lambda x: None
+        self.stubs.Set(conn, '_lookup_by_name', fake_lookup_by_name)
 
         instance = db.instance_create(self.context, self.test_instance)
         conn.destroy(instance, {})
@@ -1665,12 +1667,58 @@ class LibvirtConnTestCase(test.TestCase):
         def fake_lookup_by_name(instance_name):
             return mock
 
+        def fake_get_info(instance_name):
+            return {'state': power_state.SHUTDOWN}
+
         conn = connection.LibvirtDriver(False)
         self.stubs.Set(conn, '_lookup_by_name', fake_lookup_by_name)
+        self.stubs.Set(conn, 'get_info', fake_get_info)
         instance = {"name": "instancename", "id": "instanceid",
                     "uuid": "875a8070-d0b9-4949-8b31-104d125c9a64"}
         conn.destroy(instance, [])
 
+    @test.skip_if(missing_libvirt(), "Test requires libvirt")
+    def test_private_destroy(self):
+        """Ensure Instance not found skips undefine"""
+        mock = self.mox.CreateMock(libvirt.virDomain)
+        mock.destroy()
+        self.mox.ReplayAll()
+
+        def fake_lookup_by_name(instance_name):
+            return mock
+
+        def fake_get_info(instance_name):
+            return {'state': power_state.SHUTDOWN}
+
+        conn = connection.LibvirtDriver(False)
+        self.stubs.Set(conn, '_lookup_by_name', fake_lookup_by_name)
+        self.stubs.Set(conn, 'get_info', fake_get_info)
+        instance = {"name": "instancename", "id": "instanceid",
+                    "uuid": "875a8070-d0b9-4949-8b31-104d125c9a64"}
+        result = conn._destroy(instance)
+        self.assertTrue(result)
+
+    @test.skip_if(missing_libvirt(), "Test requires libvirt")
+    def test_private_destroy_not_found(self):
+        """Ensure Instance not found skips undefine"""
+        mock = self.mox.CreateMock(libvirt.virDomain)
+        mock.destroy()
+        self.mox.ReplayAll()
+
+        def fake_lookup_by_name(instance_name):
+            return mock
+
+        def fake_get_info(instance_name):
+            raise exception.InstanceNotFound()
+
+        conn = connection.LibvirtDriver(False)
+        self.stubs.Set(conn, '_lookup_by_name', fake_lookup_by_name)
+        self.stubs.Set(conn, 'get_info', fake_get_info)
+        instance = {"name": "instancename", "id": "instanceid",
+                    "uuid": "875a8070-d0b9-4949-8b31-104d125c9a64"}
+        result = conn._destroy(instance)
+        self.assertFalse(result)
+
     def test_available_least_handles_missing(self):
         """Ensure destroy calls managedSaveRemove for saved instance"""
         conn = connection.LibvirtDriver(False)
@@ -2668,6 +2716,9 @@ class LibvirtDriverTestCase(test.TestCase):
         def fake_execute(*args, **kwargs):
             pass
 
+        def fake_get_info(instance):
+            return {'state': power_state.RUNNING}
+
         self.flags(use_cow_images=True)
         self.stubs.Set(connection.disk, 'extend', fake_extend)
         self.stubs.Set(self.libvirtconnection, 'to_xml', fake_to_xml)
@@ -2681,13 +2732,14 @@ class LibvirtDriverTestCase(test.TestCase):
         self.stubs.Set(utils, 'execute', fake_execute)
         fw = base_firewall.NoopFirewallDriver()
         self.stubs.Set(self.libvirtconnection, 'firewall_driver', fw)
+        self.stubs.Set(self.libvirtconnection, 'get_info',
+                       fake_get_info)
 
         ins_ref = self._create_instance()
 
-        ref = self.libvirtconnection.finish_migration(
+        self.libvirtconnection.finish_migration(
                       context.get_admin_context(), None, ins_ref,
                       disk_info_text, None, None, None)
-        self.assertTrue(isinstance(ref, eventlet.event.Event))
 
     def test_finish_revert_migration(self):
         """Test for nova.virt.libvirt.connection.LivirtConnection
@@ -2705,6 +2757,9 @@ class LibvirtDriverTestCase(test.TestCase):
         def fake_enable_hairpin(instance):
             pass
 
+        def fake_get_info(instance):
+            return {'state': power_state.RUNNING}
+
         self.stubs.Set(self.libvirtconnection, 'plug_vifs', fake_plug_vifs)
         self.stubs.Set(utils, 'execute', fake_execute)
         fw = base_firewall.NoopFirewallDriver()
@@ -2713,6 +2768,8 @@ class LibvirtDriverTestCase(test.TestCase):
                        fake_create_domain)
         self.stubs.Set(self.libvirtconnection, '_enable_hairpin',
                        fake_enable_hairpin)
+        self.stubs.Set(self.libvirtconnection, 'get_info',
+                       fake_get_info)
 
         with utils.tempdir() as tmpdir:
             self.flags(instances_path=tmpdir)
@@ -2724,8 +2781,7 @@ class LibvirtDriverTestCase(test.TestCase):
             f = open(libvirt_xml_path, 'w')
             f.close()
 
-            ref = self.libvirtconnection.finish_revert_migration(ins_ref, None)
-            self.assertTrue(isinstance(ref, eventlet.event.Event))
+            self.libvirtconnection.finish_revert_migration(ins_ref, None)
 
 
 class LibvirtNonblockingTestCase(test.TestCase):
diff --git a/nova/virt/baremetal/proxy.py b/nova/virt/baremetal/proxy.py
index 1c5729422..f1aa908bb 100644
--- a/nova/virt/baremetal/proxy.py
+++ b/nova/virt/baremetal/proxy.py
@@ -174,7 +174,7 @@ class ProxyConnection(driver.ComputeDriver):
                 LOG.exception(_('_wait_for_reboot failed'), instance=instance)
                 timer.stop()
         timer.f = _wait_for_reboot
-        return timer.start(interval=0.5)
+        return timer.start(interval=0.5).wait()
 
     @exception.wrap_exception
     def rescue(self, context, instance, network_info):
@@ -207,7 +207,7 @@ class ProxyConnection(driver.ComputeDriver):
                 LOG.exception(_('_wait_for_rescue failed'), instance=instance)
                 timer.stop()
         timer.f = _wait_for_rescue
-        return timer.start(interval=0.5)
+        return timer.start(interval=0.5).wait()
 
     @exception.wrap_exception
     def unrescue(self, instance, network_info):
@@ -275,7 +275,7 @@ class ProxyConnection(driver.ComputeDriver):
             timer.stop()
         timer.f = _wait_for_boot
 
-        return timer.start(interval=0.5)
+        return timer.start(interval=0.5).wait()
 
     def get_console_output(self, instance):
         console_log = os.path.join(FLAGS.instances_path, instance['name'],
diff --git a/nova/virt/libvirt/connection.py b/nova/virt/libvirt/connection.py
index 6f0f1f630..59a32ee5b 100644
--- a/nova/virt/libvirt/connection.py
+++ b/nova/virt/libvirt/connection.py
@@ -464,7 +464,7 @@ class LibvirtDriver(driver.ComputeDriver):
                 raise utils.LoopingCallDone(True)
 
         timer = utils.LoopingCall(_wait_for_destroy)
-        return timer.start(interval=0.5)
+        return timer.start(interval=0.5).wait()
 
     def destroy(self, instance, network_info, block_device_info=None):
         self._destroy(instance)
@@ -823,7 +823,7 @@ class LibvirtDriver(driver.ComputeDriver):
                          instance=instance)
                 self._create_domain(domain=dom)
                 timer = utils.LoopingCall(self._wait_for_running, instance)
-                return timer.start(interval=0.5)
+                return timer.start(interval=0.5).wait()
             greenthread.sleep(1)
         return False
 
@@ -854,7 +854,7 @@ class LibvirtDriver(driver.ComputeDriver):
                 raise utils.LoopingCallDone
 
         timer = utils.LoopingCall(_wait_for_reboot)
-        return timer.start(interval=0.5)
+        return timer.start(interval=0.5).wait()
 
     @exception.wrap_exception()
     def pause(self, instance):
@@ -879,7 +879,7 @@ class LibvirtDriver(driver.ComputeDriver):
         dom = self._lookup_by_name(instance['name'])
         self._create_domain(domain=dom)
         timer = utils.LoopingCall(self._wait_for_running, instance)
-        return timer.start(interval=0.5)
+        return timer.start(interval=0.5).wait()
 
     @exception.wrap_exception()
     def suspend(self, instance):
@@ -991,7 +991,7 @@ class LibvirtDriver(driver.ComputeDriver):
                 raise utils.LoopingCallDone
 
         timer = utils.LoopingCall(_wait_for_boot)
-        return timer.start(interval=0.5)
+        return timer.start(interval=0.5).wait()
 
     def _flush_libvirt_console(self, pty):
         out, err = utils.execute('dd',
@@ -2250,7 +2250,7 @@ class LibvirtDriver(driver.ComputeDriver):
                 post_method(ctxt, instance_ref, dest, block_migration)
 
         timer.f = wait_for_live_migration
-        return timer.start(interval=0.5)
+        return timer.start(interval=0.5).wait()
 
     def pre_live_migration(self, block_device_info):
         """Preparation live migration.
@@ -2591,7 +2591,7 @@ class LibvirtDriver(driver.ComputeDriver):
                                     block_device_info=None)
         self._create_domain_and_network(xml, instance, network_info)
         timer = utils.LoopingCall(self._wait_for_running, instance)
-        return timer.start(interval=0.5)
+        return timer.start(interval=0.5).wait()
 
     @exception.wrap_exception()
     def finish_revert_migration(self, instance, network_info):
@@ -2607,7 +2607,7 @@ class LibvirtDriver(driver.ComputeDriver):
         self._create_domain_and_network(xml, instance, network_info)
 
         timer = utils.LoopingCall(self._wait_for_running, instance)
-        return timer.start(interval=0.5)
+        return timer.start(interval=0.5).wait()
 
     def confirm_migration(self, migration, instance, network_info):
         """Confirms a resize, destroying the source VM"""