diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 6c8fee92a84d..0f7b4acefa94 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -7615,7 +7615,10 @@ class ComputeManager(manager.Manager): instance=instance) if event.name == 'network-changed': try: - self.network_api.get_instance_nw_info(context, instance) + LOG.debug('Refreshing instance network info cache due to ' + 'event %s.', event.key, instance=instance) + self.network_api.get_instance_nw_info( + context, instance, refresh_vif_id=event.tag) except exception.NotFound as e: LOG.info('Failed to process external instance event ' '%(event)s due to: %(error)s', diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index bb7ea3cb8d60..48322c303e86 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -1376,7 +1376,8 @@ class API(base_api.NetworkAPI): def _get_instance_nw_info(self, context, instance, networks=None, port_ids=None, admin_client=None, - preexisting_port_ids=None, **kwargs): + preexisting_port_ids=None, + refresh_vif_id=None, **kwargs): # NOTE(danms): This is an inner method intended to be called # by other code that updates instance nwinfo. It *must* be # called with the refresh_cache-%(instance_uuid) lock held! @@ -1387,7 +1388,8 @@ class API(base_api.NetworkAPI): compute_utils.refresh_info_cache_for_instance(context, instance) nw_info = self._build_network_info_model(context, instance, networks, port_ids, admin_client, - preexisting_port_ids) + preexisting_port_ids, + refresh_vif_id) return network_model.NetworkInfo.hydrate(nw_info) def _gather_port_ids_and_networks(self, context, instance, networks=None, @@ -2355,7 +2357,8 @@ class API(base_api.NetworkAPI): def _build_network_info_model(self, context, instance, networks=None, port_ids=None, admin_client=None, - preexisting_port_ids=None): + preexisting_port_ids=None, + refresh_vif_id=None): """Return list of ordered VIFs attached to instance. :param context: Request context. @@ -2373,6 +2376,10 @@ class API(base_api.NetworkAPI): an instance is de-allocated. Supplied list will be added to the cached list of preexisting port IDs for this instance. + :param refresh_vif_id: Optional port ID to refresh within the existing + cache rather than the entire cache. This can be + triggered via a "network-changed" server external event + from Neutron. """ search_opts = {'tenant_id': instance.project_id, @@ -2385,10 +2392,6 @@ class API(base_api.NetworkAPI): data = client.list_ports(**search_opts) current_neutron_ports = data.get('ports', []) - nw_info_refresh = networks is None and port_ids is None - networks, port_ids = self._gather_port_ids_and_networks( - context, instance, networks, port_ids, client) - nw_info = network_model.NetworkInfo() if preexisting_port_ids is None: preexisting_port_ids = [] @@ -2400,6 +2403,59 @@ class API(base_api.NetworkAPI): current_neutron_port_map[current_neutron_port['id']] = ( current_neutron_port) + # Figure out what kind of operation we're processing. If we're given + # a single port to refresh then we try to optimize and update just the + # information for that VIF in the existing cache rather than try to + # rebuild the entire thing. + if refresh_vif_id is not None: + # TODO(mriedem): Consider pulling this out into it's own method. + nw_info = instance.get_network_info() + if nw_info: + current_neutron_port = current_neutron_port_map.get( + refresh_vif_id) + if current_neutron_port: + # Get the network for the port. + networks = self._get_available_networks( + context, instance.project_id, + [current_neutron_port['network_id']], client) + # Build the VIF model given the latest port information. + refreshed_vif = self._build_vif_model( + context, client, current_neutron_port, networks, + preexisting_port_ids) + for index, vif in enumerate(nw_info): + if vif['id'] == refresh_vif_id: + # Update the existing entry. + nw_info[index] = refreshed_vif + LOG.debug('Updated VIF entry in instance network ' + 'info cache for port %s.', + refresh_vif_id, instance=instance) + break + else: + # If it wasn't in the existing cache, add it. + nw_info.append(refreshed_vif) + LOG.debug('Added VIF to instance network info cache ' + 'for port %s.', refresh_vif_id, + instance=instance) + else: + # This port is no longer associated with the instance, so + # simply remove it from the nw_info cache. + for index, vif in enumerate(nw_info): + if vif['id'] == refresh_vif_id: + LOG.info('Port %s from network info_cache is no ' + 'longer associated with instance in ' + 'Neutron. Removing from network ' + 'info_cache.', refresh_vif_id, + instance=instance) + del nw_info[index] + break + return nw_info + # else there is no existing cache and we need to build it + + nw_info_refresh = networks is None and port_ids is None + networks, port_ids = self._gather_port_ids_and_networks( + context, instance, networks, port_ids, client) + nw_info = network_model.NetworkInfo() + 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/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index a558aab1919f..9afeeae9778b 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -2836,7 +2836,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): self.compute.external_instance_event(self.context, instances, events) get_instance_nw_info.assert_called_once_with(self.context, - instances[0]) + instances[0], + refresh_vif_id='tag1') _process_instance_event.assert_called_once_with(instances[1], events[1]) _process_instance_vif_deleted_event.assert_called_once_with( @@ -2904,7 +2905,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): self.compute.external_instance_event(self.context, instances, events) get_instance_nw_info.assert_called_once_with(self.context, - instances[0]) + instances[0], + refresh_vif_id='tag1') update_instance_cache_with_nw_info.assert_called_once_with( self.compute.network_api, self.context, diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index e9aeebdef846..b1023bd7611b 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -6318,3 +6318,131 @@ class TestNeutronv2AutoAllocateNetwork(test.NoDBTestCase): ntrn, instance, uuids.port_id, port_req_body) do_test(self) + + +class TestGetInstanceNetworkInfo(test.NoDBTestCase): + """Tests rebuilding the network_info cache.""" + + def setUp(self): + super(TestGetInstanceNetworkInfo, self).setUp() + self.api = neutronapi.API() + self.context = context.RequestContext(uuids.user_id, uuids.project_id) + self.instance = fake_instance.fake_instance_obj(self.context) + client_mock = mock.patch('nova.network.neutronv2.api.get_client') + self.client = client_mock.start().return_value + self.addCleanup(client_mock.stop) + # This is a no-db set of tests and we don't care about refreshing the + # info_cache from the database so just mock it out. + refresh_info_cache_for_instance = mock.patch( + 'nova.compute.utils.refresh_info_cache_for_instance') + refresh_info_cache_for_instance.start() + self.addCleanup(refresh_info_cache_for_instance.stop) + + @staticmethod + def _get_vif_in_cache(info_cache, vif_id): + for vif in info_cache: + if vif['id'] == vif_id: + return vif + + @staticmethod + def _get_fake_info_cache(vif_ids, **kwargs): + """Returns InstanceInfoCache based on the list of provided VIF IDs""" + nwinfo = model.NetworkInfo( + [model.VIF(vif_id, **kwargs) for vif_id in vif_ids]) + return objects.InstanceInfoCache(network_info=nwinfo) + + @staticmethod + def _get_fake_port(port_id, **kwargs): + network_id = kwargs.get('network_id', uuids.network_id) + return {'id': port_id, 'network_id': network_id} + + def test_get_nw_info_refresh_vif_id_add_vif(self): + """Tests that a network-changed event occurred on a single port + which is not already in the cache so it's added. + """ + # The cache has one existing port. + self.instance.info_cache = self._get_fake_info_cache([uuids.old_port]) + # The instance has two ports, one old, one new. + self.client.list_ports.return_value = { + 'ports': [self._get_fake_port(uuids.old_port), + self._get_fake_port(uuids.new_port)]} + with test.nested( + mock.patch.object(self.api, '_get_available_networks', + return_value=[{'id': uuids.network_id}]), + mock.patch.object(self.api, '_build_vif_model', + return_value=model.VIF(uuids.new_port)), + # We should not get as far as calling _gather_port_ids_and_networks + mock.patch.object(self.api, '_gather_port_ids_and_networks', + new_callable=mock.NonCallableMock) + ) as ( + get_nets, build_vif, gather_ports + ): + nwinfo = self.api._get_instance_nw_info( + self.context, self.instance, refresh_vif_id=uuids.new_port) + get_nets.assert_called_once_with( + self.context, self.instance.project_id, + [uuids.network_id], self.client) + # Assert that the old and new ports are in the cache. + for port_id in (uuids.old_port, uuids.new_port): + self.assertIsNotNone(self._get_vif_in_cache(nwinfo, port_id)) + + def test_get_nw_info_refresh_vif_id_update_vif(self): + """Tests that a network-changed event occurred on a single port + which is already in the cache so it's updated. + """ + # The cache has two existing active VIFs. + self.instance.info_cache = self._get_fake_info_cache( + [uuids.old_port, uuids.new_port], active=True) + # The instance has two ports, one old, one new. + self.client.list_ports.return_value = { + 'ports': [self._get_fake_port(uuids.old_port), + self._get_fake_port(uuids.new_port)]} + with test.nested( + mock.patch.object(self.api, '_get_available_networks', + return_value=[{'id': uuids.network_id}]), + # Fake that the port is no longer active. + mock.patch.object(self.api, '_build_vif_model', + return_value=model.VIF( + uuids.new_port, active=False)), + # We should not get as far as calling _gather_port_ids_and_networks + mock.patch.object(self.api, '_gather_port_ids_and_networks', + new_callable=mock.NonCallableMock) + ) as ( + get_nets, build_vif, gather_ports + ): + nwinfo = self.api._get_instance_nw_info( + self.context, self.instance, refresh_vif_id=uuids.new_port) + get_nets.assert_called_once_with( + self.context, self.instance.project_id, + [uuids.network_id], self.client) + # Assert that the old and new ports are in the cache and that the + # old port is still active and the new port is not active. + old_vif = self._get_vif_in_cache(nwinfo, uuids.old_port) + self.assertIsNotNone(old_vif) + self.assertTrue(old_vif['active']) + new_vif = self._get_vif_in_cache(nwinfo, uuids.new_port) + self.assertIsNotNone(new_vif) + self.assertFalse(new_vif['active']) + + def test_get_nw_info_refresh_vif_id_remove_vif(self): + """Tests that a network-changed event occurred on a single port + which is already in the cache but not in the current list of ports + for the instance, so it's removed from the cache. + """ + # The cache has two existing VIFs. + self.instance.info_cache = self._get_fake_info_cache( + [uuids.old_port, uuids.removed_port]) + # The instance has one remaining port. + self.client.list_ports.return_value = { + 'ports': [self._get_fake_port(uuids.old_port)]} + # We should not get as far as calling _gather_port_ids_and_networks + with mock.patch.object( + self.api, '_gather_port_ids_and_networks', + new_callable=mock.NonCallableMock): + nwinfo = self.api._get_instance_nw_info( + self.context, self.instance, refresh_vif_id=uuids.removed_port) + # Assert that only the old port is still in the cache. + old_vif = self._get_vif_in_cache(nwinfo, uuids.old_port) + self.assertIsNotNone(old_vif) + removed_vif = self._get_vif_in_cache(nwinfo, uuids.removed_port) + self.assertIsNone(removed_vif)