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
(cherry picked from commit aab4b7a0e2)
This commit is contained in:
Balazs Gibizer
parent
47e5e89cea
commit
b6989836dd
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -516,7 +516,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
|
||||
|
|
@ -662,12 +661,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()
|
||||
|
||||
|
|
@ -2600,8 +2596,7 @@ class TestNeutronv2WithMock(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
|
||||
|
|
@ -2622,8 +2617,7 @@ class TestNeutronv2WithMock(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.
|
||||
|
|
@ -2663,7 +2657,6 @@ class TestNeutronv2WithMock(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):
|
||||
|
|
@ -2673,7 +2666,6 @@ class TestNeutronv2WithMock(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):
|
||||
|
|
@ -2702,7 +2694,9 @@ class TestNeutronv2WithMock(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)])
|
||||
|
|
@ -2812,7 +2806,9 @@ class TestNeutronv2WithMock(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)
|
||||
|
|
@ -2833,7 +2829,9 @@ class TestNeutronv2WithMock(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)
|
||||
|
|
@ -2852,9 +2850,8 @@ class TestNeutronv2WithMock(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.
|
||||
|
|
@ -2863,7 +2860,6 @@ class TestNeutronv2WithMock(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):
|
||||
|
|
@ -2873,7 +2869,6 @@ class TestNeutronv2WithMock(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')
|
||||
|
|
@ -2894,7 +2889,9 @@ class TestNeutronv2WithMock(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)])
|
||||
|
|
@ -2919,7 +2916,9 @@ class TestNeutronv2WithMock(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)])
|
||||
|
|
@ -6737,6 +6736,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,
|
||||
|
|
@ -7108,7 +7110,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')
|
||||
|
|
|
|||
Loading…
Reference in New Issue