From 329cd1649703ac801b95fce28afa99b67f9f45aa Mon Sep 17 00:00:00 2001 From: Esha Seth Date: Fri, 4 Nov 2016 05:48:32 -0400 Subject: [PATCH] Add a no-op wait method to NetworkInfo The normal deploy flow uses a NetworkInfoAsyncWrapper for network allocation, and because of that many places have to call that class's wait method to make sure it has completed. During a reschedule where the network was allocated by a previous build attempt, a NetworkInfo instance is retrieved instead, which does not have a wait method. This then results in an exception complaining the missing method when it is called. This fix addresses that by adding a no-op wait method to the NetworkInfo class. Alternatively could have used isinstance or hasattr to avoid making wait calls on NetworkInfo, but that could be problematic to maintain as more places need to make wait calls in the future and may not know to make the isinstance/hasattr check. This fixes a regression issue caused by 61fc1b9ee11e416aecbf3a29e1d150a53fc890e8 , which reverted the previous fix made under 24a04c405ab2c98e52ea1edf8775489907526c6d Change-Id: Id7a71b2eb46ea7df19e7da0afbc0eafa87cac965 Closes-Bug: 1636109 (cherry picked from commit 1b351ef5356e0472aef51221ff02376dd8b42954) --- nova/network/model.py | 11 ++++++++++- nova/tests/unit/compute/test_compute_mgr.py | 15 +++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/nova/network/model.py b/nova/network/model.py index 36cd85499c38..e2183b77b95a 100644 --- a/nova/network/model.py +++ b/nova/network/model.py @@ -467,6 +467,15 @@ class NetworkInfo(list): network_info = jsonutils.loads(network_info) return cls([VIF.hydrate(vif) for vif in network_info]) + def wait(self, do_raise=True): + """Wait for asynchronous call to finish.""" + # There is no asynchronous call for this class, so this is a no-op + # here, but subclasses may override to provide asynchronous + # capabilities. Must be defined here in the parent class so that code + # which works with both parent and subclass types can reference this + # method. + pass + def json(self): return jsonutils.dumps(self) @@ -529,7 +538,7 @@ class NetworkInfoAsyncWrapper(NetworkInfo): return self._sync_wrapper(fn, *args, **kwargs) def wait(self, do_raise=True): - """Wait for async call to finish.""" + """Wait for asynchronous call to finish.""" if self._gt is not None: try: # NOTE(comstud): This looks funky, but this object is diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index d1d2be915380..37c44040e258 100755 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -4280,11 +4280,12 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): system_metadata={}, expected_attrs=['system_metadata']) - self.compute._build_networks_for_instance(self.context, instance, - self.requested_networks, self.security_groups) + nw_info_obj = self.compute._build_networks_for_instance(self.context, + instance, self.requested_networks, self.security_groups) mock_allocate.assert_called_once_with(self.context, instance, self.requested_networks, None, self.security_groups, None) + self.assertTrue(hasattr(nw_info_obj, 'wait'), "wait must be there") @mock.patch.object(manager.ComputeManager, '_allocate_network') @mock.patch.object(network_api.API, 'get_instance_nw_info') @@ -4293,11 +4294,12 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): system_metadata=dict(network_allocated='False'), expected_attrs=['system_metadata']) - self.compute._build_networks_for_instance(self.context, instance, - self.requested_networks, self.security_groups) + nw_info_obj = self.compute._build_networks_for_instance(self.context, + instance, self.requested_networks, self.security_groups) mock_allocate.assert_called_once_with(self.context, instance, self.requested_networks, None, self.security_groups, None) + self.assertTrue(hasattr(nw_info_obj, 'wait'), "wait must be there") @mock.patch.object(network_api.API, 'setup_instance_network_on_host') @mock.patch.object(manager.ComputeManager, '_allocate_network') @@ -4311,8 +4313,9 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): def fake_network_info(): return network_model.NetworkInfo([{'address': '123.123.123.123'}]) - mock_get.return_value = network_model.NetworkInfoAsyncWrapper( - fake_network_info) + # this should be a NetworkInfo, not NetworkInfoAsyncWrapper, to match + # what get_instance_nw_info really returns + mock_get.return_value = fake_network_info() self.compute._build_networks_for_instance(self.context, instance, self.requested_networks, self.security_groups)