From 8694c1619d774bb8a6c23ed4c0f33df2084849bc Mon Sep 17 00:00:00 2001
From: Mark Goddard <mgoddard@cray.com>
Date: Thu, 1 Oct 2015 17:37:45 +0100
Subject: [PATCH] network: Don't repopulate instance info cache from Neutron
 ports

Allocation of network interfaces for an instance can result in
corruption of the instance info cache. The result is that the cache
may contain duplicate entries for network interfaces. This can cause
instance boot failure. This bug appears to be attributable to the
combined effects of the fixes for bugs #1467581 and #1407664.

This change reverts the fix for bug #1407664, whilst keeping a
modified version of the unit test that was added with it. It also
adds a second unit test.

Change-Id: I53d5284907d44ae8b5546993f8fd461b385c39e6
Closes-bug: #1501735
Related-bug: #1467581
Related-bug: #1407664
---
 nova/network/neutronv2/api.py             |  5 ---
 nova/tests/unit/network/test_neutronv2.py | 43 ++++++++++++++++++++---
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py
index 1f8c9b29783..e4d41d0f12f 100644
--- a/nova/network/neutronv2/api.py
+++ b/nova/network/neutronv2/api.py
@@ -1618,11 +1618,6 @@ def _build_network_info_model(self, context, instance, networks=None,
             current_neutron_port_map[current_neutron_port['id']] = (
                 current_neutron_port)
 
-        # In that case we should repopulate ports from the state of
-        # Neutron.
-        if not port_ids:
-            port_ids = current_neutron_port_map.keys()
-
         for port_id in port_ids:
             current_neutron_port = current_neutron_port_map.get(port_id)
             if current_neutron_port:
diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py
index d783360faa6..9f5d6b33a7a 100644
--- a/nova/tests/unit/network/test_neutronv2.py
+++ b/nova/tests/unit/network/test_neutronv2.py
@@ -726,6 +726,21 @@ def test_get_instance_nw_info_ignores_neutron_ports(self):
                                                None,
                                                None)
 
+    def test_get_instance_nw_info_ignores_neutron_ports_empty_cache(self):
+        # Tests that ports returned from neutron that match the same
+        # instance_id/device_id are ignored when the instance info cache is
+        # empty.
+        port_data2 = copy.copy(self.port_data2)
+
+        # set device_id on the ports to be the same.
+        port_data2[1]['device_id'] = port_data2[0]['device_id']
+        network_cache = {'info_cache': {'network_info': []}}
+
+        self._fake_get_instance_nw_info_helper(network_cache,
+                                               port_data2,
+                                               None,
+                                               None)
+
     def _fake_get_instance_nw_info_helper(self, network_cache,
                                           current_neutron_ports,
                                           networks=None, port_ids=None):
@@ -765,8 +780,26 @@ def _fake_get_instance_nw_info_helper(self, network_cache,
                      'tenant_id': iface['network']['meta']['tenant_id']}
                     for iface in ifaces]
         if networks is None:
-            self.moxed_client.list_networks(
-                id=net_ids).AndReturn({'networks': nets})
+            if ifaces:
+                self.moxed_client.list_networks(
+                    id=net_ids).AndReturn({'networks': nets})
+            else:
+                non_shared_nets = [
+                    {'id': iface['network']['id'],
+                     'name': iface['network']['label'],
+                     'tenant_id': iface['network']['meta']['tenant_id']}
+                    for iface in ifaces if not iface['shared']]
+                shared_nets = [
+                    {'id': iface['network']['id'],
+                     'name': iface['network']['label'],
+                     'tenant_id': iface['network']['meta']['tenant_id']}
+                    for iface in ifaces if iface['shared']]
+                self.moxed_client.list_networks(
+                    shared=False,
+                    tenant_id=self.instance['project_id']
+                        ).AndReturn({'networks': non_shared_nets})
+                self.moxed_client.list_networks(
+                    shared=True).AndReturn({'networks': shared_nets})
         else:
             networks = networks + [
                 dict(id=iface['network']['id'],
@@ -2668,6 +2701,8 @@ def test_build_network_info_model_empty(
             mock_nw_info_build_network,
             mock_nw_info_get_ips,
             mock_nw_info_get_subnets):
+        # An empty instance info network cache should not be populated from
+        # ports found in Neutron.
         api = neutronapi.API()
 
         fake_inst = objects.Instance()
@@ -2696,7 +2731,7 @@ def test_build_network_info_model_empty(
             tenant_id='fake', device_id='uuid').AndReturn(
                 {'ports': fake_ports})
 
-        mock_gather_port_ids_and_networks.return_value = (None, None)
+        mock_gather_port_ids_and_networks.return_value = ([], [])
         mock_get_preexisting_port_ids.return_value = []
         mock_nw_info_build_network.return_value = (None, None)
         mock_nw_info_get_ips.return_value = []
@@ -2707,7 +2742,7 @@ def test_build_network_info_model_empty(
 
         nw_infos = api._build_network_info_model(
             self.context, fake_inst)
-        self.assertEqual(1, len(nw_infos))
+        self.assertEqual(0, len(nw_infos))
 
     def test_get_subnets_from_port(self):
         api = neutronapi.API()