Change SpawnIsSynchronous fixture return

In normal usage utils.spawn returns an eventlet.greenthread.GreenThread
object which has a few methods defined on it. Currently the wait() and
link() methods are used in Nova. However the SpawnIsSynchronous fixture
was returning the results of wait() directly which led to an interface
mismatch. Normally utils.spawn_n returns a greenlet object which does
not allow checking the return but in testing we have never blocked that
capability before. This could be enhanced later to be strict for spawn_n
if we choose, but it is handy to have raised exceptions pass through in
testing.

SpawnIsSynchronous now returns a GreenThread object lookalike which has
wait() and link() methods that work as expected. This allows for the
removal of the wait() method from network/model.py:NetworkInfo.

A few mocks which emulated the improper behavior were also updated.

Change-Id: I1ded81f93235e217822e385ddd983b24c6359a53

nova/network/model.py

@@ -464,17 +464,6 @@ class NetworkInfo(list):
     def json(self):
         return jsonutils.dumps(self)
 
-    def wait(self, do_raise=True):
-        """Return this NetworkInfo
-
-        This is useful to avoid type checking when it's not clear if you have a
-        NetworkInfo or NetworkInfoAsyncWrapper. It's also useful when
-        utils.spawn in NetworkInfoAsyncWrapper is patched to return a
-        NetworkInfo rather than a GreenThread and wait() should return the same
-        thing for both cases.
-        """
-        return self
-
 
 class NetworkInfoAsyncWrapper(NetworkInfo):
     """Wrapper around NetworkInfo that allows retrieving NetworkInfo

nova/tests/fixtures.py

@@ -460,15 +460,41 @@ class IndirectionAPIFixture(fixtures.Fixture):
         self.addCleanup(self.cleanup)
 
 
+class _FakeGreenThread(object):
+    def __init__(self, func, *args, **kwargs):
+        self._result = func(*args, **kwargs)
+
+    def cancel(self, *args, **kwargs):
+        # This method doesn't make sense for a synchronous call, it's just
+        # defined to satisfy the interface.
+        pass
+
+    def kill(self, *args, **kwargs):
+        # This method doesn't make sense for a synchronous call, it's just
+        # defined to satisfy the interface.
+        pass
+
+    def link(self, func, *args, **kwargs):
+        func(self, *args, **kwargs)
+
+    def unlink(self, func, *args, **kwargs):
+        # This method doesn't make sense for a synchronous call, it's just
+        # defined to satisfy the interface.
+        pass
+
+    def wait(self):
+        return self._result
+
+
 class SpawnIsSynchronousFixture(fixtures.Fixture):
     """Patch and restore the spawn_n utility method to be synchronous"""
 
     def setUp(self):
         super(SpawnIsSynchronousFixture, self).setUp()
         self.useFixture(fixtures.MonkeyPatch(
-            'nova.utils.spawn_n', lambda f, *a, **k: f(*a, **k)))
+            'nova.utils.spawn_n', _FakeGreenThread))
         self.useFixture(fixtures.MonkeyPatch(
-            'nova.utils.spawn', lambda f, *a, **k: f(*a, **k)))
+            'nova.utils.spawn', _FakeGreenThread))
 
 
 class BannedDBSchemaOperations(fixtures.Fixture):

nova/tests/unit/compute/test_compute_mgr.py

@@ -3068,6 +3068,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                                             'hosts': [[self.compute.host,
                                                        'fake-node']]}}
 
+        self.useFixture(fixtures.SpawnIsSynchronousFixture())
+
         def fake_network_info():
             return network_model.NetworkInfo([{'address': '1.2.3.4'}])
 
@@ -3081,8 +3083,6 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                     self.compute.driver, self.node)
         self.compute._resource_tracker_dict[self.node] = fake_rt
 
-        self.useFixture(fixtures.SpawnIsSynchronousFixture())
-
     def _do_build_instance_update(self, reschedule_update=False):
         self.mox.StubOutWithMock(self.instance, 'save')
         self.instance.save(
@@ -3652,7 +3652,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                     side_effect=[self.instance, self.instance, self.instance]),
                 mock.patch.object(self.compute,
                     '_build_networks_for_instance',
-                    return_value=network_model.NetworkInfo()),
+                    return_value=self.network_info),
                 mock.patch.object(self.compute,
                     '_notify_about_instance_usage'),
                 mock.patch.object(self.compute,
@@ -3696,7 +3696,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
 
             _shutdown_instance.assert_called_once_with(self.context,
                     self.instance, self.block_device_mapping,
-                    self.requested_networks, try_deallocate_networks=True)
+                    self.requested_networks, try_deallocate_networks=False)
 
     def test_reschedule_on_resources_unavailable(self):
         reason = 'resource unavailable'
@@ -3851,7 +3851,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
         self.mox.StubOutWithMock(self.compute, '_shutdown_instance')
         self.compute._build_networks_for_instance(self.context, self.instance,
                 self.requested_networks, self.security_groups).AndReturn(
-                        network_model.NetworkInfo([{'address': '1.2.3.4'}]))
+                        self.network_info)
         self.compute._shutdown_instance(self.context, self.instance,
                 self.block_device_mapping, self.requested_networks,
                 try_deallocate_networks=False)

nova/tests/unit/test_fixtures.py

@@ -342,6 +342,46 @@ class TestSpawnIsSynchronousFixture(testtools.TestCase):
         utils.spawn_n(tester.function, 'foo', bar='bar')
         tester.function.assert_called_once_with('foo', bar='bar')
 
+    def test_spawn_return_has_wait(self):
+        self.useFixture(fixtures.SpawnIsSynchronousFixture())
+        gt = utils.spawn(lambda x: '%s' % x, 'foo')
+        foo = gt.wait()
+        self.assertEqual('foo', foo)
+
+    def test_spawn_n_return_has_wait(self):
+        self.useFixture(fixtures.SpawnIsSynchronousFixture())
+        gt = utils.spawn_n(lambda x: '%s' % x, 'foo')
+        foo = gt.wait()
+        self.assertEqual('foo', foo)
+
+    def test_spawn_has_link(self):
+        self.useFixture(fixtures.SpawnIsSynchronousFixture())
+        gt = utils.spawn(mock.MagicMock)
+        passed_arg = 'test'
+        call_count = []
+
+        def fake(thread, param):
+            self.assertEqual(gt, thread)
+            self.assertEqual(passed_arg, param)
+            call_count.append(1)
+
+        gt.link(fake, passed_arg)
+        self.assertEqual(1, len(call_count))
+
+    def test_spawn_n_has_link(self):
+        self.useFixture(fixtures.SpawnIsSynchronousFixture())
+        gt = utils.spawn_n(mock.MagicMock)
+        passed_arg = 'test'
+        call_count = []
+
+        def fake(thread, param):
+            self.assertEqual(gt, thread)
+            self.assertEqual(passed_arg, param)
+            call_count.append(1)
+
+        gt.link(fake, passed_arg)
+        self.assertEqual(1, len(call_count))
+
 
 class TestBannedDBSchemaOperations(testtools.TestCase):
     def test_column(self):