Don't delete neutron port when attach failed

Currently, when attaching neutron pre-existing
port to an instance, if the attach failed, it
will also be deleted in Neutron side due to
bad judgement of the who created the port by
reading not up-to-date info_cache.

The workflow starts at:
https://github.com/openstack/nova/blob/9ed0d6114/nova/network/neutronv2/api.py#L881
ordered_ports and preexisting_port_ids are
the same when attaching a preexisting port
to an instance and it calls
https://github.com/openstack/nova/blob/9ed0d6114/nova/network/base_api.py#L246
which calls back into the neutronv2 api code
https://github.com/openstack/nova/blob/9ed0d6114/nova/network/neutronv2/api.py#L1274
and at this point, compute_utils.refresh_info_cache_for_instance(context,
instance) won't have the newly attached port in it(see
debug log: http://paste.openstack.org/show/613232/)
because _build_network_info_model() is going to
process it. The instance obj in memoryt with old
info_cache will be used at rollback process and
causing the miss-judging.

This patch fixed it by updating instance.info_cache
to the new ic after it is created.

 Conflicts:
	doc/notification_samples/instance-shutdown-end.json

Co-Authored-By: huangtianhua@huawei.com
Change-Id: Ib323b74d4ea1e874b476ab5addfc6bc79cb7c751
closes-bug: #1645175
(cherry picked from commit 115cf068a6d48cdf8b0d20a3c5a779bb8120aa9b)

4 files changed, 31 insertions, 20 deletions

diff --git a/doc/notification_samples/instance-shutdown-end.json b/doc/notification_samples/instance-shutdown-end.json
index 2083b66c2f..ccf01c0672 100644
--- a/doc/notification_samples/instance-shutdown-end.json
+++ b/doc/notification_samples/instance-shutdown-end.json
@@ -11,20 +11,7 @@
             "fault":null,
             "host":"compute",
             "host_name":"some-server",
-            "ip_addresses": [{
-                "nova_object.name": "IpPayload",
-                "nova_object.namespace": "nova",
-                "nova_object.version": "1.0",
-                "nova_object.data": {
-                    "mac": "fa:16:3e:4c:2c:30",
-                    "address": "192.168.1.3",
-                    "port_uuid": "ce531f90-199f-48c0-816c-13e38010b442",
-                    "meta": {},
-                    "version": 4,
-                    "label": "private-network",
-                    "device_name": "tapce531f90-19"
-                }
-            }],
+            "ip_addresses": [],
             "kernel_id":"",
             "launched_at":"2012-10-29T13:42:11Z",
             "image_uuid": "155d900f-4e14-4e4c-a73d-069cbf4541e6",
diff --git a/nova/network/base_api.py b/nova/network/base_api.py
index 3b6251ceaa..f3167decfa 100644
--- a/nova/network/base_api.py
+++ b/nova/network/base_api.py
@@ -53,6 +53,7 @@ def update_instance_cache_with_nw_info(impl, context, instance,
         ic = objects.InstanceInfoCache.new(context, instance.uuid)
         ic.network_info = nw_info
         ic.save(update_cells=update_cells)
+        instance.info_cache = ic
     except Exception:
         with excutils.save_and_reraise_exception():
             LOG.exception(_LE('Failed storing info cache'), instance=instance)
diff --git a/nova/tests/unit/network/test_api.py b/nova/tests/unit/network/test_api.py
index 476578717e..3fb0aad248 100644
--- a/nova/tests/unit/network/test_api.py
+++ b/nova/tests/unit/network/test_api.py
@@ -15,6 +15,7 @@
 
 """Tests for network API."""
 
+import copy
 import itertools
 import uuid
 
@@ -559,7 +560,7 @@ class ApiTestCase(test.TestCase):
 
 
 @mock.patch('nova.network.api.API')
-@mock.patch('nova.db.instance_info_cache_update', return_value=fake_info_cache)
+@mock.patch('nova.db.instance_info_cache_update')
 class TestUpdateInstanceCache(test.NoDBTestCase):
     def setUp(self):
         super(TestUpdateInstanceCache, self).setUp()
@@ -572,13 +573,16 @@ class TestUpdateInstanceCache(test.NoDBTestCase):
 
     def test_update_nw_info_none(self, db_mock, api_mock):
         api_mock._get_instance_nw_info.return_value = self.nw_info
-
+        info_cache = copy.deepcopy(fake_info_cache)
+        info_cache.update({'network_info': self.nw_json})
+        db_mock.return_value = info_cache
         base_api.update_instance_cache_with_nw_info(api_mock, self.context,
                                                self.instance, None)
         api_mock._get_instance_nw_info.assert_called_once_with(self.context,
                                                                 self.instance)
         db_mock.assert_called_once_with(self.context, self.instance.uuid,
                                         {'network_info': self.nw_json})
+        self.assertEqual(self.nw_info, self.instance.info_cache.network_info)
 
     def test_update_nw_info_none_instance_deleted(self, db_mock, api_mock):
         instance = objects.Instance(uuid=FAKE_UUID, deleted=True)
@@ -587,23 +591,29 @@ class TestUpdateInstanceCache(test.NoDBTestCase):
         self.assertFalse(api_mock.called)
 
     def test_update_nw_info_one_network(self, db_mock, api_mock):
-        api_mock._get_instance_nw_info.return_value = self.nw_info
+        info_cache = copy.deepcopy(fake_info_cache)
+        info_cache.update({'network_info': self.nw_json})
+        db_mock.return_value = info_cache
         base_api.update_instance_cache_with_nw_info(api_mock, self.context,
                                                self.instance, self.nw_info)
         self.assertFalse(api_mock._get_instance_nw_info.called)
         db_mock.assert_called_once_with(self.context, self.instance.uuid,
                                         {'network_info': self.nw_json})
+        self.assertEqual(self.nw_info, self.instance.info_cache.network_info)
 
     def test_update_nw_info_empty_list(self, db_mock, api_mock):
-        api_mock._get_instance_nw_info.return_value = self.nw_info
+        new_nw_info = network_model.NetworkInfo([])
+        db_mock.return_value = fake_info_cache
         base_api.update_instance_cache_with_nw_info(api_mock, self.context,
-                                                self.instance,
-                                                network_model.NetworkInfo([]))
+                                                self.instance, new_nw_info)
         self.assertFalse(api_mock._get_instance_nw_info.called)
         db_mock.assert_called_once_with(self.context, self.instance.uuid,
                                         {'network_info': '[]'})
+        self.assertEqual(new_nw_info, self.instance.info_cache.network_info)
 
     def test_decorator_return_object(self, db_mock, api_mock):
+        db_mock.return_value = fake_info_cache
+
         @base_api.refresh_cache
         def func(self, context, instance):
             return network_model.NetworkInfo([])
@@ -613,6 +623,8 @@ class TestUpdateInstanceCache(test.NoDBTestCase):
                                         {'network_info': '[]'})
 
     def test_decorator_return_none(self, db_mock, api_mock):
+        db_mock.return_value = fake_info_cache
+
         @base_api.refresh_cache
         def func(self, context, instance):
             pass
diff --git a/releasenotes/notes/bug-1645175-b1ef3ad9a3e44ed6.yaml b/releasenotes/notes/bug-1645175-b1ef3ad9a3e44ed6.yaml
new file mode 100644
index 0000000000..f5453bcbd7
--- /dev/null
+++ b/releasenotes/notes/bug-1645175-b1ef3ad9a3e44ed6.yaml
@@ -0,0 +1,11 @@
+---
+other:
+  - |
+     ``instance.shutdown.end`` versioned notification will
+     have an empty ``ip_addresses`` field since the network
+     resources associated with the instance are deallocated
+     before this notification is sent, which is actually
+     more accurate. Consumers should rely on the
+     instance.shutdown.start notification if they need the
+     network information for the instance when it is being
+     deleted.