Use admin neutron client to query ports for binding

The compute service updates the binding:profile of the neutron port
during server create. If the port has resource_request then the
'allocation' key need to point to the resource provider the port is
allocating resources. Unfortunately this code used a non admin client to
query the port data and therefore if the original server create request
was sent by a non admin user the returned port does not have its
resource_request filled and as a consequence nova does not add the
allocation key to the binding profile.

This patch makes sure that the port is queried with an admin client.

There is a tempest test change that reproduces the issue:
https://review.opendev.org/#/c/690934

Change-Id: Icc631cf2e81a5c78cb7fb1d0b625d19bd8f5a274
Closes-Bug: #1849657
This commit is contained in:
Balazs Gibizer 2019-10-24 17:01:02 +02:00 committed by Matt Riedemann
parent e79a0effc6
commit aab4b7a0e2
2 changed files with 36 additions and 29 deletions

View File

@ -965,6 +965,10 @@ class API(base_api.NetworkAPI):
# We do not want to create a new neutron session for each call
neutron = get_client(context)
# We always need admin_client to build nw_info,
# we sometimes need it when updating ports
admin_client = get_client(context, admin=True)
#
# Validate ports and networks with neutron. The requested_ports_dict
# variable is a dict, keyed by port ID, of ports that were on the user
@ -972,9 +976,15 @@ class API(base_api.NetworkAPI):
# NetworkRequest objects for any networks or ports specifically
# requested by the user, which again may be empty.
#
# NOTE(gibi): we use the admin_client here to ensure that the returned
# ports has the resource_request attribute filled as later we use this
# information to decide when to add allocation key to the port binding.
# See bug 1849657.
requested_ports_dict, ordered_networks = (
self._validate_requested_port_ids(
context, instance, neutron, requested_networks, attach=attach))
context, instance, admin_client, requested_networks,
attach=attach))
nets = self._validate_requested_network_ids(
context, instance, neutron, requested_networks, ordered_networks)
@ -1022,10 +1032,6 @@ class API(base_api.NetworkAPI):
# Update existing and newly created ports
#
# We always need admin_client to build nw_info,
# we sometimes need it when updating ports
admin_client = get_client(context, admin=True)
ordered_nets, ordered_port_ids, preexisting_port_ids, \
created_port_ids = self._update_ports_for_instance(
context, instance,

View File

@ -468,7 +468,6 @@ class TestNeutronv2Base(test.TestCase):
mock_refresh, mock_populate, net_idx=1,
requested_networks=None,
exception=None,
get_client_admin_call=True,
context=None,
**kwargs):
ctxt = context or self.context
@ -614,12 +613,9 @@ class TestNeutronv2Base(test.TestCase):
ctxt, self.instance, False, requested_networks,
bind_host_id=bind_host_id)
if get_client_admin_call:
mock_get_client.assert_has_calls([
mock.call(ctxt), mock.call(ctxt, admin=True)],
any_order=True)
else:
mock_get_client.assert_called_once_with(mock.ANY)
mock_get_client.assert_has_calls([
mock.call(ctxt), mock.call(ctxt, admin=True)],
any_order=True)
mock_refresh.assert_not_called()
@ -1177,8 +1173,7 @@ class TestNeutronv2(TestNeutronv2Base):
def test_allocate_for_instance_2(self):
# Allocate one port in two networks env.
self._test_allocate_for_instance(
net_idx=2, exception=exception.NetworkAmbiguous,
get_client_admin_call=False)
net_idx=2, exception=exception.NetworkAmbiguous)
def test_allocate_for_instance_accepts_only_portid(self):
# Make sure allocate_for_instance works when only a portid is provided
@ -1199,8 +1194,7 @@ class TestNeutronv2(TestNeutronv2Base):
def test_allocate_for_instance_without_requested_networks(self):
self._test_allocate_for_instance(
net_idx=3, exception=exception.NetworkAmbiguous,
get_client_admin_call=False)
net_idx=3, exception=exception.NetworkAmbiguous)
def test_allocate_for_instance_with_requested_non_available_network(self):
"""verify that a non available network is ignored.
@ -1240,7 +1234,6 @@ class TestNeutronv2(TestNeutronv2Base):
# raised.
self._test_allocate_for_instance_with_virtual_interface(
net_idx=4, exception=exception.SecurityGroupCannotBeApplied,
get_client_admin_call=False,
_break='post_list_extensions')
def test_allocate_for_instance_with_invalid_network_id(self):
@ -1250,7 +1243,6 @@ class TestNeutronv2(TestNeutronv2Base):
self._test_allocate_for_instance(net_idx=9,
requested_networks=requested_networks,
exception=exception.NetworkNotFound,
get_client_admin_call=False,
_break='post_list_networks')
def test_allocate_for_instance_with_requested_networks_with_fixedip(self):
@ -1279,7 +1271,9 @@ class TestNeutronv2(TestNeutronv2Base):
nwinfo = self.api.allocate_for_instance(self.context, self.instance,
False, None)
self.assertEqual(0, len(nwinfo))
mock_get_client.assert_called_once_with(self.context)
mock_get_client.assert_has_calls([
mock.call(self.context),
mock.call(self.context, admin=True)])
mocked_client.list_networks.assert_has_calls([
mock.call(tenant_id=self.instance.project_id, shared=False),
mock.call(shared=True)])
@ -1389,7 +1383,9 @@ class TestNeutronv2(TestNeutronv2Base):
self.api.allocate_for_instance,
self.context, self.instance, False,
requested_networks=requested_networks)
mock_get_client.assert_called_once_with(self.context)
mock_get_client.assert_has_calls([
mock.call(self.context),
mock.call(self.context, admin=True)])
mocked_client.list_networks.assert_called_once_with(
id=[uuids.my_netid1, uuids.my_netid2])
mocked_client.create_port.assert_called_once_with(port_req_body)
@ -1410,7 +1406,9 @@ class TestNeutronv2(TestNeutronv2Base):
self.assertRaises(test.TestingException,
self.api.allocate_for_instance, self.context,
self.instance, False, requested_networks)
mock_get_client.assert_called_once_with(self.context)
mock_get_client.assert_has_calls([
mock.call(self.context),
mock.call(self.context, admin=True)])
mock_get_available.assert_called_once_with(
self.context, self.instance.project_id, [],
neutron=mocked_client, auto_allocate=False)
@ -1429,9 +1427,8 @@ class TestNeutronv2(TestNeutronv2Base):
objects=[objects.NetworkRequest(port_id=uuids.portid_1)])
self._test_allocate_for_instance(
requested_networks=requested_networks,
exception=exception.PortInUse,
get_client_admin_call=False,
_break='pre_list_networks', _device=True)
exception=exception.PortInUse, _break='pre_list_networks',
_device=True)
def test_allocate_for_instance_port_not_found(self):
# If a port is not found, an exception should be raised.
@ -1440,7 +1437,6 @@ class TestNeutronv2(TestNeutronv2Base):
self._test_allocate_for_instance(
requested_networks=requested_networks,
exception=exception.PortNotFound,
get_client_admin_call=False,
_break='pre_list_networks')
def test_allocate_for_instance_port_invalid_tenantid(self):
@ -1450,7 +1446,6 @@ class TestNeutronv2(TestNeutronv2Base):
self._test_allocate_for_instance(
requested_networks=requested_networks,
exception=exception.PortNotUsable,
get_client_admin_call=False,
_break='pre_list_networks')
@mock.patch.object(neutronapi, 'get_client')
@ -1471,7 +1466,9 @@ class TestNeutronv2(TestNeutronv2Base):
self.assertRaises(exception.ExternalNetworkAttachForbidden,
self.api.allocate_for_instance, self.context,
self.instance, False, None)
mock_get_client.assert_called_once_with(self.context)
mock_get_client.assert_has_calls([
mock.call(self.context),
mock.call(self.context, admin=True)])
mocked_client.list_networks.assert_has_calls([
mock.call(tenant_id=self.instance.project_id, shared=False),
mock.call(shared=True)])
@ -1496,7 +1493,9 @@ class TestNeutronv2(TestNeutronv2Base):
exception.NetworkAmbiguous,
self.api.allocate_for_instance,
self.context, self.instance, False, None)
mock_get_client.assert_called_once_with(self.context)
mock_get_client.assert_has_calls([
mock.call(self.context),
mock.call(self.context, admin=True)])
mocked_client.list_networks.assert_has_calls([
mock.call(tenant_id=self.instance.project_id, shared=False),
mock.call(shared=True)])
@ -6811,6 +6810,9 @@ class TestAllocateForInstance(test.NoDBTestCase):
self.assertEqual(result[0], {"id": uuids.created})
self.assertEqual(result[1], {"id": uuids.preexist})
mock_validate_ports.assert_called_once_with(
self.context, self.instance, "admin", None, attach=False)
def test_ensure_no_port_binding_failure_raises(self):
port = {
'id': uuids.port_id,
@ -7180,7 +7182,6 @@ class TestNeutronv2NeutronHostnameDNS(TestNeutronv2Base):
self._test_allocate_for_instance(
requested_networks=requested_networks,
exception=exception.PortNotUsableDNS,
get_client_admin_call=False,
dns_extension=True,
_break='pre_list_networks',
_dns_name='my-instance')