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:
parent
df0a80220d
commit
ee7858ffca
|
@ -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
|
||||
|
|
|
@ -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})
|
||||
|
|
|
@ -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']
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue