Address infinite loop in nova compute when getting network info

Move the refresh_cache decorator to get_instance_nw_info.
In fact, _get_instance_nw_info is called by the decorator
itself, potentially causing an infinite loop in case nw_info
is not of the expected value. This is also inline with
the method's purpose, as stated by the docstring. At the
same time, ensure that delete_port_for_instance and
allocate_for_instance continue to refresh the cache by
using the decorated version.

Also, add a couple of debug traces that are friendly to
the developer.

Partial-bug: 1235435
Related-bug: 1251792

Change-Id: I06f8634ea241d05ac8fbcc290adf0cb23829f3e4
(cherry picked from commit 2e520496c3)

nova/compute/manager.py

@@ -4297,7 +4297,7 @@ class ComputeManager(manager.SchedulerDependentManager):
         This is implemented by keeping a cache of uuids of instances
         that live on this host.  On each call, we pop one off of a
         list, pull the DB record, and try the call to the network API.
-        If anything errors, we don't care.  It's possible the instance
+        If anything errors don't fail, as it's possible the instance
         has been deleted, etc.
         """
         heal_interval = CONF.heal_instance_info_cache_interval
@@ -4338,9 +4338,8 @@ class ComputeManager(manager.SchedulerDependentManager):
             self._get_instance_nw_info(context, instance)
             LOG.debug(_('Updated the info_cache for instance'),
                       instance=instance)
-        except Exception:
-            # We don't care about any failures
-            pass
+        except Exception as e:
+            LOG.debug(_("An error occurred: %s"), e)
 
     @periodic_task.periodic_task
     def _poll_rebooting_instances(self, context):

nova/network/api.py

@@ -68,6 +68,7 @@ def refresh_cache(f):
 def update_instance_cache_with_nw_info(api, context, instance, nw_info=None,
                                        update_cells=True):
     try:
+        LOG.debug(_('Updating cache with info: %s'), nw_info)
         if not isinstance(nw_info, network_model.NetworkInfo):
             nw_info = None
         if not nw_info:

nova/network/neutronv2/api.py

@@ -357,7 +357,7 @@ class API(base.Base):
                             msg = _("Failed to delete port %s")
                             LOG.exception(msg, port_id)
 
-        nw_info = self._get_instance_nw_info(context, instance, networks=nets)
+        nw_info = self.get_instance_nw_info(context, instance, networks=nets)
         # NOTE(danms): Only return info about ports we created in this run.
         # In the initial allocation case, this will be everything we created,
         # and in later runs will only be what was created that time. Thus,
@@ -438,7 +438,7 @@ class API(base.Base):
             LOG.exception(_("Failed to delete neutron port %s") %
                           port_id)
 
-        return self._get_instance_nw_info(context, instance)
+        return self.get_instance_nw_info(context, instance)
 
     def list_ports(self, context, **search_opts):
         """List ports for the client based on search options."""
@@ -448,6 +448,7 @@ class API(base.Base):
         """Return the port for the client given the port id."""
         return neutronv2.get_client(context).show_port(port_id)
 
+    @refresh_cache
     def get_instance_nw_info(self, context, instance, networks=None):
         """Return network information for specified instance
            and update cache.
@@ -458,8 +459,9 @@ class API(base.Base):
         return result
 
     def _get_instance_nw_info(self, context, instance, networks=None):
-        LOG.debug(_('get_instance_nw_info() for %s'),
-                  instance['display_name'])
+        # keep this caching-free version of the get_instance_nw_info method
+        # because it is used by the caching logic itself.
+        LOG.debug(_('get_instance_nw_info() for %s'), instance['display_name'])
         nw_info = self._build_network_info_model(context, instance, networks)
         return network_model.NetworkInfo.hydrate(nw_info)
 

nova/tests/network/test_neutronv2.py

@@ -257,7 +257,7 @@ class TestNeutronv2Base(test.TestCase):
 
     def _stub_allocate_for_instance(self, net_idx=1, **kwargs):
         api = neutronapi.API()
-        self.mox.StubOutWithMock(api, '_get_instance_nw_info')
+        self.mox.StubOutWithMock(api, 'get_instance_nw_info')
         has_portbinding = False
         has_extra_dhcp_opts = False
         # Note: (dkehn) this option check should be removed as soon as support
@@ -372,10 +372,10 @@ class TestNeutronv2Base(test.TestCase):
                 self.moxed_client.create_port(
                     MyComparator(port_req_body)).AndReturn(res_port)
 
-        api._get_instance_nw_info(mox.IgnoreArg(),
-                                  self.instance,
-                                  networks=nets).AndReturn(
-                                        self._returned_nw_info)
+        api.get_instance_nw_info(mox.IgnoreArg(),
+                                 self.instance,
+                                 networks=nets).AndReturn(
+                                       self._returned_nw_info)
         self.mox.ReplayAll()
         return api