neutron: handle binding:profile=None during migration

The port binding:profile handling in _setup_migration_port_profile
and _update_port_binding_for_instance is not handling when the
binding:profile is set on the port but the value is None, which
results in a NoneType error. Neutron API does not define a format
for this field, or whether or not it will be specified on the port,
and if so, if it's an empty dict or None, so let's be safe and
make sure we handle None.

Because of how many places we access the binding:profile on
a port, this adds a common helper utility.

Co-Authored-By: Eric M Gonzalez <eric@awnix.com>

Change-Id: I564bac88fad6cc47ccbf7425b1ab39899fdc1c2e
Closes-Bug: #1717365
(cherry picked from commit 8ac7be36be)
(cherry picked from commit c393e49075)
This commit is contained in:
Matt Riedemann 2017-09-14 18:04:20 -04:00
parent 891f9ecc2a
commit b257720720
2 changed files with 80 additions and 7 deletions

View File

@ -74,6 +74,19 @@ def _load_auth_plugin(conf):
raise neutron_client_exc.Unauthorized(message=err_msg)
def _get_binding_profile(port):
"""Convenience method to get the binding:profile from the port
The binding:profile in the port is undefined in the networking service
API and is dependent on backend configuration. This means it could be
an empty dict, None, or have some values.
:param port: dict port response body from the networking service API
:returns: The port binding:profile dict; empty if not set on the port
"""
return port.get(BINDING_PROFILE, {}) or {}
@profiler.trace_cls("neutron_api")
class ClientWrapper(clientv20.Client):
"""A Neutron client wrapper class.
@ -256,7 +269,7 @@ class API(base_api.NetworkAPI):
# If the port already has a migration profile and if
# it is to be torn down, then we need to clean up
# the migration profile.
port_profile = p.get(BINDING_PROFILE)
port_profile = _get_binding_profile(p)
if not port_profile:
continue
if MIGRATING_ATTR in port_profile:
@ -277,7 +290,7 @@ class API(base_api.NetworkAPI):
# the given 'host'.
host_id = p.get(BINDING_HOST_ID)
if host_id != host:
port_profile = p.get(BINDING_PROFILE, {})
port_profile = _get_binding_profile(p)
port_profile[MIGRATING_ATTR] = host
self._update_port_with_migration_profile(
instance, p['id'], port_profile, admin_client)
@ -2160,7 +2173,7 @@ class API(base_api.NetworkAPI):
mtu=network_mtu
)
network['subnets'] = subnets
port_profile = port.get(BINDING_PROFILE)
port_profile = _get_binding_profile(port)
if port_profile:
physical_network = port_profile.get('physical_network')
if physical_network:
@ -2259,7 +2272,7 @@ class API(base_api.NetworkAPI):
vnic_type=current_neutron_port.get('binding:vnic_type',
network_model.VNIC_TYPE_NORMAL),
type=current_neutron_port.get('binding:vif_type'),
profile=current_neutron_port.get(BINDING_PROFILE),
profile=_get_binding_profile(current_neutron_port),
details=current_neutron_port.get('binding:vif_details'),
ovs_interfaceid=ovs_interfaceid,
devname=devname,
@ -2425,7 +2438,7 @@ class API(base_api.NetworkAPI):
ports = data['ports']
for p in ports:
updates = {}
binding_profile = p.get(BINDING_PROFILE, {})
binding_profile = _get_binding_profile(p)
# If the host hasn't changed, like in the case of resizing to the
# same host, there is nothing to do.

View File

@ -3024,8 +3024,11 @@ class TestNeutronv2(TestNeutronv2Base):
self.assertEqual(requested_ports[index].get('binding:vif_details'),
nw_info.get('details'))
self.assertEqual(
requested_ports[index].get(neutronapi.BINDING_PROFILE),
nw_info.get('profile'))
# If the requested port does not define a binding:profile, or
# has it set to None, we default to an empty dict to avoid
# NoneType errors.
requested_ports[index].get(neutronapi.BINDING_PROFILE) or {},
nw_info.get('profile'))
index += 1
self.assertFalse(nw_infos[0]['active'])
@ -3910,6 +3913,31 @@ class TestNeutronv2WithMock(test.TestCase):
neutronapi.BINDING_PROFILE: {
'fake_profile': 'fake_data'}}})
@mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
def test_update_port_bindings_for_instance_binding_profile_none(
self, get_client_mock):
"""Tests _update_port_binding_for_instance when the binding:profile
value is None.
"""
instance = fake_instance.fake_instance_obj(self.context)
self.api._has_port_binding_extension = mock.Mock(return_value=True)
fake_ports = {'ports': [
{'id': uuids.portid,
neutronapi.BINDING_PROFILE: None,
neutronapi.BINDING_HOST_ID: instance.host}]}
list_ports_mock = mock.Mock(return_value=fake_ports)
get_client_mock.return_value.list_ports = list_ports_mock
update_port_mock = mock.Mock()
get_client_mock.return_value.update_port = update_port_mock
self.api._update_port_binding_for_instance(self.context, instance,
'my-host')
# Assert that update_port was called on the port with a
# different host but with no binding profile.
update_port_mock.assert_called_once_with(
uuids.portid, {'port': {neutronapi.BINDING_HOST_ID: 'my-host'}})
@mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
def test_update_port_bindings_for_instance_same_host(self,
get_client_mock):
@ -4166,6 +4194,38 @@ class TestNeutronv2WithMock(test.TestCase):
uuids.port_id,
port_data)
@mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
def test_update_port_profile_for_migration_teardown_false_none_profile(
self, get_client_mock):
"""Tests setup_networks_on_host when migrating the port to the
destination host and the binding:profile is None in the port.
"""
instance = fake_instance.fake_instance_obj(self.context)
self.api._has_port_binding_extension = mock.Mock(return_value=True)
# We test with an instance host and destination_host where the
# port will be moving but with binding:profile set to None.
get_ports = {
'ports': [
{'id': uuids.port_id,
neutronapi.BINDING_HOST_ID: instance.host,
neutronapi.BINDING_PROFILE: None}
]
}
self.api.list_ports = mock.Mock(return_value=get_ports)
update_port_mock = mock.Mock()
get_client_mock.return_value.update_port = update_port_mock
migrate_profile = {neutronapi.MIGRATING_ATTR: 'my-new-host'}
port_data = {
'port': {
neutronapi.BINDING_PROFILE: migrate_profile
}
}
self.api.setup_networks_on_host(
self.context, instance, host='my-new-host', teardown=False)
update_port_mock.assert_called_once_with(
uuids.port_id,
port_data)
@mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
def test__setup_migration_port_profile_called_on_teardown_false(
self, get_client_mock):