Don't overwrite binding-profile

Currently when providing existing direct port, nova-compute
will overwrite the binding-profile information with pci_vendor_info
and pci_slot. The binding-profile will be used to request
NIC capabilities for SR-IOV ports [1]. This also allows to distinguish
which neutron mechanism driver will bind the port [2].

This patch updates the behaviour that on update port it will update,
rather than overwrite, the binding-profile information with
pci_vendor_info and pci_slot. And on unbind port it will remove
only the pci_vendor_info and pci_slot from the port binding-profile
rather than unsetting the entire field.

[1] https://review.openstack.org/#/c/435954/
[2] https://review.openstack.org/#/c/499203/

Closes-Bug: #1719327

Change-Id: I80106707a037d567d0f690570f2cf9cfcd30d594
This commit is contained in:
Moshe Levi 2017-09-20 15:24:29 +03:00 committed by Edan David
parent df0a80220d
commit ee7858ffca
4 changed files with 117 additions and 14 deletions

View File

@ -15,6 +15,7 @@
# under the License.
#
import copy
import time
from keystoneauth1 import loading as ks_loading
@ -508,7 +509,17 @@ class API(base_api.NetworkAPI):
continue
port_req_body = {'port': {'device_id': '', 'device_owner': ''}}
port_req_body['port'][BINDING_HOST_ID] = None
port_req_body['port'][BINDING_PROFILE] = {}
port = self._show_port(context, port_id,
neutron_client=neutron,
fields=BINDING_PROFILE)
port_profile = port.get(BINDING_PROFILE, {})
# NOTE: We're doing this to remove the binding information
# for the physical device but don't want to overwrite the other
# information in the binding profile.
for profile_key in ('pci_vendor_info', 'pci_slot'):
if profile_key in port_profile:
del port_profile[profile_key]
port_req_body['port'][BINDING_PROFILE] = port_profile
if self._has_dns_extension():
port_req_body['port']['dns_name'] = ''
try:
@ -903,7 +914,7 @@ class API(base_api.NetworkAPI):
created_port_ids = self._update_ports_for_instance(
context, instance,
neutron, admin_client, requests_and_created_ports, nets,
bind_host_id, available_macs)
bind_host_id, available_macs, requested_ports_dict)
#
# Perform a full update of the network_info_cache,
@ -928,7 +939,7 @@ class API(base_api.NetworkAPI):
def _update_ports_for_instance(self, context, instance, neutron,
admin_client, requests_and_created_ports, nets,
bind_host_id, available_macs):
bind_host_id, available_macs, requested_ports_dict):
"""Update ports from network_requests.
Updates the pre-existing ports and the ones created in
@ -946,6 +957,8 @@ class API(base_api.NetworkAPI):
:param nets: a dict of network_id to networks returned from neutron
:param bind_host_id: a string for port['binding:host_id']
:param available_macs: a list of available mac addresses
:param requested_ports_dict: dict, keyed by port ID, of ports requested
by the user
:returns: tuple with the following::
* list of network dicts in their requested order
@ -980,6 +993,11 @@ class API(base_api.NetworkAPI):
zone = 'compute:%s' % instance.availability_zone
port_req_body = {'port': {'device_id': instance.uuid,
'device_owner': zone}}
if (requested_ports_dict and
request.port_id in requested_ports_dict and
requested_ports_dict[request.port_id].get(BINDING_PROFILE)):
port_req_body['port'][BINDING_PROFILE] = (
requested_ports_dict[request.port_id][BINDING_PROFILE])
try:
self._populate_neutron_extension_values(
context, instance, request.pci_request_id, port_req_body,
@ -1077,7 +1095,10 @@ class API(base_api.NetworkAPI):
if pci_request_id:
pci_dev = pci_manager.get_instance_pci_devs(
instance, pci_request_id).pop()
profile = self._get_pci_device_profile(pci_dev)
if port_req_body['port'].get(BINDING_PROFILE) is None:
port_req_body['port'][BINDING_PROFILE] = {}
profile = copy.deepcopy(port_req_body['port'][BINDING_PROFILE])
profile.update(self._get_pci_device_profile(pci_dev))
port_req_body['port'][BINDING_PROFILE] = profile
@staticmethod

View File

@ -1216,6 +1216,13 @@ class NeutronFixture(fixtures.Fixture):
else:
return None
def _filter_ports(self, **_params):
ports = copy.deepcopy(self._ports)
for opt in _params:
filtered_ports = [p for p in ports if p.get(opt) == _params[opt]]
ports = filtered_ports
return {'ports': ports}
def list_extensions(self, *args, **kwargs):
return copy.deepcopy({'extensions': self._extensions})
@ -1229,12 +1236,13 @@ class NeutronFixture(fixtures.Fixture):
for current_port in self._ports:
if current_port['id'] == port:
self._ports.remove(current_port)
return
def list_networks(self, retrieve_all=True, **_params):
return copy.deepcopy({'networks': self._networks})
def list_ports(self, retrieve_all=True, **_params):
return copy.deepcopy({'ports': self._ports})
return self._filter_ports(**_params)
def list_subnets(self, retrieve_all=True, **_params):
return copy.deepcopy({'subnets': self._subnets})

View File

@ -124,6 +124,13 @@ class InterfaceFullstackWithNeutron(test_servers.ServersTestBase):
found_server = self._wait_for_state_change(created_server, 'BUILD')
self.assertEqual('ACTIVE', found_server['status'])
post = {
'interfaceAttachment': {
'net_id': "3cb9bc59-5699-4588-a4b1-b87f96708bc6"
}
}
self.api.attach_interface(created_server_id, post)
response = self.api.get_port_interfaces(created_server_id)[0]
port_id = response['port_id']

View File

@ -1539,6 +1539,8 @@ class TestNeutronv2(TestNeutronv2Base):
self.moxed_client)
if requested_networks:
for net, fip, port, request_id in requested_networks:
self.moxed_client.show_port(port, fields='binding:profile'
).AndReturn({'port': ret_data[0]})
self.moxed_client.update_port(port)
for port in ports:
self.moxed_client.delete_port(port).InAnyOrder("delete_port_group")
@ -4320,16 +4322,16 @@ class TestNeutronv2WithMock(test.TestCase):
def test_unbind_ports_get_client(self, mock_neutron):
self._test_unbind_ports_get_client(mock_neutron)
def _test_unbind_ports(self, mock_neutron):
@mock.patch('nova.network.neutronv2.api.API._show_port')
def _test_unbind_ports(self, mock_neutron, mock_show):
mock_client = mock.Mock()
mock_update_port = mock.Mock()
mock_client.update_port = mock_update_port
mock_ctx = mock.Mock(is_admin=False)
mock_neutron.return_value = mock_client
ports = ["1", "2", "3"]
mock_show.side_effect = [{"id": "1"}, {"id": "2"}, {"id": "3"}]
api = neutronapi.API()
api._unbind_ports(mock_ctx, ports, mock_client)
api._unbind_ports(mock_ctx, ports, mock_neutron, mock_client)
body = {'port': {'device_id': '', 'device_owner': ''}}
body['port'][neutronapi.BINDING_HOST_ID] = None
@ -4644,11 +4646,13 @@ class TestNeutronv2WithMock(test.TestCase):
self.api.get_floating_ips_by_project,
self.context)
def test_unbind_ports_reset_dns_name(self):
@mock.patch('nova.network.neutronv2.api.API._show_port')
def test_unbind_ports_reset_dns_name(self, mock_show):
neutron = mock.Mock()
port_client = mock.Mock()
self.api.extensions = [constants.DNS_INTEGRATION]
ports = [uuids.port_id]
mock_show.return_value = {'id': uuids.port}
self.api._unbind_ports(self.context, ports, neutron, port_client)
port_req_body = {'port': {'binding:host_id': None,
'binding:profile': {},
@ -4658,6 +4662,29 @@ class TestNeutronv2WithMock(test.TestCase):
port_client.update_port.assert_called_once_with(
uuids.port_id, port_req_body)
@mock.patch('nova.network.neutronv2.api.API._show_port')
def test_unbind_ports_reset_binding_profile(self, mock_show):
neutron = mock.Mock()
port_client = mock.Mock()
ports = [uuids.port_id]
mock_show.return_value = {
'id': uuids.port,
'binding:profile': {'pci_vendor_info': '1377:0047',
'pci_slot': '0000:0a:00.1',
'physical_network': 'phynet1',
'capabilities': ['switchdev']}
}
self.api._unbind_ports(self.context, ports, neutron, port_client)
port_req_body = {'port': {'binding:host_id': None,
'binding:profile':
{'physical_network': 'phynet1',
'capabilities': ['switchdev']},
'device_id': '',
'device_owner': ''}
}
port_client.update_port.assert_called_once_with(
uuids.port_id, port_req_body)
def test_make_floating_ip_obj(self):
self._test_make_floating_ip_obj()
@ -4759,7 +4786,7 @@ class TestNeutronv2WithMock(test.TestCase):
self.api._update_ports_for_instance,
self.context, instance, ntrn, ntrn,
requests_and_created_ports, nets, bind_host_id=None,
available_macs=None)
available_macs=None, requested_ports_dict=None)
# assert the calls
mock_update_port.assert_has_calls([
mock.call(ntrn, instance, uuids.preexisting_port_id, mock.ANY),
@ -4788,12 +4815,14 @@ class TestNeutronv2WithMock(test.TestCase):
'172.24.4.227')
self.assertIsNone(fip)
@mock.patch('nova.network.neutronv2.api.API._show_port')
@mock.patch.object(neutronapi.LOG, 'exception')
def test_unbind_ports_portnotfound(self, mock_log):
def test_unbind_ports_portnotfound(self, mock_log, mock_show):
api = neutronapi.API()
neutron_client = mock.Mock()
neutron_client.update_port = mock.Mock(
side_effect=exceptions.PortNotFoundClient)
mock_show.return_value = {'id': uuids.port}
api._unbind_ports(self.context, [uuids.port_id],
neutron_client, neutron_client)
neutron_client.update_port.assert_called_once_with(
@ -4802,12 +4831,14 @@ class TestNeutronv2WithMock(test.TestCase):
'binding:profile': {}, 'binding:host_id': None}})
mock_log.assert_not_called()
@mock.patch('nova.network.neutronv2.api.API._show_port')
@mock.patch.object(neutronapi.LOG, 'exception')
def test_unbind_ports_unexpected_error(self, mock_log):
def test_unbind_ports_unexpected_error(self, mock_log, mock_show):
api = neutronapi.API()
neutron_client = mock.Mock()
neutron_client.update_port = mock.Mock(
side_effect=test.TestingException)
mock_show.return_value = {'id': uuids.port}
api._unbind_ports(self.context, [uuids.port_id],
neutron_client, neutron_client)
neutron_client.update_port.assert_called_once_with(
@ -4964,6 +4995,41 @@ class TestNeutronv2Portbinding(TestNeutronv2Base):
self.assertEqual(profile,
port_req_body['port'][neutronapi.BINDING_PROFILE])
@mock.patch.object(pci_whitelist.Whitelist, 'get_devspec')
@mock.patch.object(pci_manager, 'get_instance_pci_devs')
def test_populate_neutron_extension_values_binding_sriov_with_cap(self,
mock_get_instance_pci_devs,
mock_get_pci_device_devspec):
api = neutronapi.API()
host_id = 'my_host_id'
instance = {'host': host_id}
port_req_body = {'port': {
neutronapi.BINDING_PROFILE: {
'capabilities': ['switchdev']}}}
pci_req_id = 'my_req_id'
pci_dev = {'vendor_id': '1377',
'product_id': '0047',
'address': '0000:0a:00.1',
}
PciDevice = collections.namedtuple('PciDevice',
['vendor_id', 'product_id', 'address'])
mydev = PciDevice(**pci_dev)
profile = {'pci_vendor_info': '1377:0047',
'pci_slot': '0000:0a:00.1',
'physical_network': 'phynet1',
'capabilities': ['switchdev'],
}
mock_get_instance_pci_devs.return_value = [mydev]
devspec = mock.Mock()
devspec.get_tags.return_value = {'physical_network': 'phynet1'}
mock_get_pci_device_devspec.return_value = devspec
api._populate_neutron_binding_profile(instance,
pci_req_id, port_req_body)
self.assertEqual(profile,
port_req_body['port'][neutronapi.BINDING_PROFILE])
@mock.patch.object(pci_whitelist.Whitelist, 'get_devspec')
@mock.patch.object(pci_manager, 'get_instance_pci_devs')
def test_populate_neutron_extension_values_binding_sriov_fail(
@ -5554,6 +5620,7 @@ class TestAllocateForInstance(test.NoDBTestCase):
nets = {uuids.net1: net1, uuids.net2: net2}
bind_host_id = "bind_host_id"
available_macs = ["mac1", "mac2"]
requested_ports_dict = {uuids.port1: {}, uuids.port2: {}}
mock_neutron.list_extensions.return_value = {"extensions": [
{"name": "asdf"}]}
@ -5565,7 +5632,7 @@ class TestAllocateForInstance(test.NoDBTestCase):
created_port_ids = api._update_ports_for_instance(
self.context, self.instance,
mock_neutron, mock_admin, requests_and_created_ports, nets,
bind_host_id, available_macs)
bind_host_id, available_macs, requested_ports_dict)
# TODO(johngarbutt) need to build on this test so we can replace
# all the mox based tests