Refresh instance network info on deletion

When deleting an instance, if the network info is empty, we should
refresh the info because we can't be sure the copy of the cache we
have when we fetched the instance to delete is up-to-date, i.e. if
we're racing to delete the server while it's building and the
network info cache was updated in the database after we started the
delete operation and got the instance from the DB, then we could
fail to unplug VIFs.

Closes-Bug: #1830081

Change-Id: I99601773406c61f93002e2f7cbb248cf73cef0ab
Signed-off-by: Arnaud Morin <arnaud.morin@corp.ovh.com>
(cherry picked from commit d4ed0d8b7a)

nova/compute/manager.py

@@ -2615,6 +2615,12 @@ class ComputeManager(manager.Manager):
 
         network_info = instance.get_network_info()
 
+        # NOTE(arnaudmorin) to avoid nova destroying the instance without
+        # unplugging the interface, refresh network_info if it is empty.
+        if not network_info:
+            network_info = self.network_api.get_instance_nw_info(
+                context, instance)
+
         # NOTE(vish) get bdms before destroying the instance
         vol_bdms = [bdm for bdm in bdms if bdm.is_volume]
         block_device_info = self._get_instance_block_device_info(

nova/tests/unit/compute/test_compute_mgr.py

@@ -4860,6 +4860,39 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
         mock_delete_instance.assert_called_once_with(
             self.context, instance, bdms)
 
+    @mock.patch('nova.context.RequestContext.elevated')
+    def test_terminate_instance_no_network_info(self, mock_elevated):
+        # Tests that we refresh the network info if it was empty
+        instance = fake_instance.fake_instance_obj(
+            self.context, vm_state=vm_states.ACTIVE)
+        empty_nw_info = network_model.NetworkInfo()
+        instance.info_cache = objects.InstanceInfoCache(
+            network_info=empty_nw_info)
+        vif = fake_network_cache_model.new_vif()
+        nw_info = network_model.NetworkInfo([vif])
+        bdms = objects.BlockDeviceMappingList()
+        elevated = context.get_admin_context()
+        mock_elevated.return_value = elevated
+
+        # Call shutdown instance
+        with test.nested(
+            mock.patch.object(self.compute.network_api, 'get_instance_nw_info',
+                              return_value=nw_info),
+            mock.patch.object(self.compute, '_get_instance_block_device_info'),
+            mock.patch.object(self.compute.driver, 'destroy')
+        ) as (
+            mock_nw_api_info, mock_get_bdi, mock_destroy
+        ):
+            self.compute._shutdown_instance(self.context, instance, bdms,
+                notify=False, try_deallocate_networks=False)
+
+        # Verify
+        mock_nw_api_info.assert_called_once_with(elevated, instance)
+        mock_get_bdi.assert_called_once_with(elevated, instance, bdms=bdms)
+        # destroy should have been called with the refresh network_info
+        mock_destroy.assert_called_once_with(
+            elevated, instance, nw_info, mock_get_bdi.return_value)
+
     @mock.patch('nova.compute.utils.notify_about_instance_action')
     @mock.patch.object(nova.compute.manager.ComputeManager,
                        '_notify_about_instance_usage')

nova/tests/unit/compute/test_compute_utils.py

@@ -61,7 +61,7 @@ CONF = nova.conf.CONF
 def create_instance(context, user_id='fake', project_id='fake', params=None):
     """Create a test instance."""
     flavor = flavors.get_flavor_by_name('m1.tiny')
-    net_info = model.NetworkInfo([])
+    net_info = model.NetworkInfo([model.VIF(id=uuids.port_id)])
     info_cache = objects.InstanceInfoCache(network_info=net_info)
     inst = objects.Instance(context=context,
                             image_ref=uuids.fake_image_ref,