openstack
/
nova
Code Issues Proposed changes

Handle network-changed event for a specific port 

It turns out that when Neutron sends the "network-changed"
event it has a port in scope and can provide the port ID.

Older versions of Neutron wouldn't send this, but if it's
provided, we can try to optimize the network info cache
refresh and scope it to just that single port, rather than
build the entire cache all over again when the other ports
in the cache may not have changed at all since the last
time the cache was refreshed.

This can be especially beneficial for instances with a
relatively large number of ports which are being migrated.

Depends-On: Ifdaef05208d09ddd9587fed6214cf388e5265ba4

Change-Id: I023b5b1ccb248e68189f62ba0ff75d41093c1f60
Partial-Bug: #1691602
(cherry picked from commit 537f480927)
This commit is contained in:
Matt Riedemann 2017-05-17 22:48:29 -04:00
parent 3bb2d97c17
commit c8756d5a4a
4 changed files with 199 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
nova/compute/manager.py
View File

@ -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',

70
nova/network/neutronv2/api.py
View File

@ -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:

6
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -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,

128
nova/tests/unit/network/test_neutronv2.py
View File

@ -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)