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:
parent
e79a0effc6
commit
aab4b7a0e2
|
@ -965,6 +965,10 @@ class API(base_api.NetworkAPI):
|
||||||
# We do not want to create a new neutron session for each call
|
# We do not want to create a new neutron session for each call
|
||||||
neutron = get_client(context)
|
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
|
# 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
|
# 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
|
# NetworkRequest objects for any networks or ports specifically
|
||||||
# requested by the user, which again may be empty.
|
# 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 = (
|
requested_ports_dict, ordered_networks = (
|
||||||
self._validate_requested_port_ids(
|
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(
|
nets = self._validate_requested_network_ids(
|
||||||
context, instance, neutron, requested_networks, ordered_networks)
|
context, instance, neutron, requested_networks, ordered_networks)
|
||||||
|
@ -1022,10 +1032,6 @@ class API(base_api.NetworkAPI):
|
||||||
# Update existing and newly created ports
|
# 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, \
|
ordered_nets, ordered_port_ids, preexisting_port_ids, \
|
||||||
created_port_ids = self._update_ports_for_instance(
|
created_port_ids = self._update_ports_for_instance(
|
||||||
context, instance,
|
context, instance,
|
||||||
|
|
|
@ -468,7 +468,6 @@ class TestNeutronv2Base(test.TestCase):
|
||||||
mock_refresh, mock_populate, net_idx=1,
|
mock_refresh, mock_populate, net_idx=1,
|
||||||
requested_networks=None,
|
requested_networks=None,
|
||||||
exception=None,
|
exception=None,
|
||||||
get_client_admin_call=True,
|
|
||||||
context=None,
|
context=None,
|
||||||
**kwargs):
|
**kwargs):
|
||||||
ctxt = context or self.context
|
ctxt = context or self.context
|
||||||
|
@ -614,12 +613,9 @@ class TestNeutronv2Base(test.TestCase):
|
||||||
ctxt, self.instance, False, requested_networks,
|
ctxt, self.instance, False, requested_networks,
|
||||||
bind_host_id=bind_host_id)
|
bind_host_id=bind_host_id)
|
||||||
|
|
||||||
if get_client_admin_call:
|
|
||||||
mock_get_client.assert_has_calls([
|
mock_get_client.assert_has_calls([
|
||||||
mock.call(ctxt), mock.call(ctxt, admin=True)],
|
mock.call(ctxt), mock.call(ctxt, admin=True)],
|
||||||
any_order=True)
|
any_order=True)
|
||||||
else:
|
|
||||||
mock_get_client.assert_called_once_with(mock.ANY)
|
|
||||||
|
|
||||||
mock_refresh.assert_not_called()
|
mock_refresh.assert_not_called()
|
||||||
|
|
||||||
|
@ -1177,8 +1173,7 @@ class TestNeutronv2(TestNeutronv2Base):
|
||||||
def test_allocate_for_instance_2(self):
|
def test_allocate_for_instance_2(self):
|
||||||
# Allocate one port in two networks env.
|
# Allocate one port in two networks env.
|
||||||
self._test_allocate_for_instance(
|
self._test_allocate_for_instance(
|
||||||
net_idx=2, exception=exception.NetworkAmbiguous,
|
net_idx=2, exception=exception.NetworkAmbiguous)
|
||||||
get_client_admin_call=False)
|
|
||||||
|
|
||||||
def test_allocate_for_instance_accepts_only_portid(self):
|
def test_allocate_for_instance_accepts_only_portid(self):
|
||||||
# Make sure allocate_for_instance works when only a portid is provided
|
# 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):
|
def test_allocate_for_instance_without_requested_networks(self):
|
||||||
self._test_allocate_for_instance(
|
self._test_allocate_for_instance(
|
||||||
net_idx=3, exception=exception.NetworkAmbiguous,
|
net_idx=3, exception=exception.NetworkAmbiguous)
|
||||||
get_client_admin_call=False)
|
|
||||||
|
|
||||||
def test_allocate_for_instance_with_requested_non_available_network(self):
|
def test_allocate_for_instance_with_requested_non_available_network(self):
|
||||||
"""verify that a non available network is ignored.
|
"""verify that a non available network is ignored.
|
||||||
|
@ -1240,7 +1234,6 @@ class TestNeutronv2(TestNeutronv2Base):
|
||||||
# raised.
|
# raised.
|
||||||
self._test_allocate_for_instance_with_virtual_interface(
|
self._test_allocate_for_instance_with_virtual_interface(
|
||||||
net_idx=4, exception=exception.SecurityGroupCannotBeApplied,
|
net_idx=4, exception=exception.SecurityGroupCannotBeApplied,
|
||||||
get_client_admin_call=False,
|
|
||||||
_break='post_list_extensions')
|
_break='post_list_extensions')
|
||||||
|
|
||||||
def test_allocate_for_instance_with_invalid_network_id(self):
|
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,
|
self._test_allocate_for_instance(net_idx=9,
|
||||||
requested_networks=requested_networks,
|
requested_networks=requested_networks,
|
||||||
exception=exception.NetworkNotFound,
|
exception=exception.NetworkNotFound,
|
||||||
get_client_admin_call=False,
|
|
||||||
_break='post_list_networks')
|
_break='post_list_networks')
|
||||||
|
|
||||||
def test_allocate_for_instance_with_requested_networks_with_fixedip(self):
|
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,
|
nwinfo = self.api.allocate_for_instance(self.context, self.instance,
|
||||||
False, None)
|
False, None)
|
||||||
self.assertEqual(0, len(nwinfo))
|
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([
|
mocked_client.list_networks.assert_has_calls([
|
||||||
mock.call(tenant_id=self.instance.project_id, shared=False),
|
mock.call(tenant_id=self.instance.project_id, shared=False),
|
||||||
mock.call(shared=True)])
|
mock.call(shared=True)])
|
||||||
|
@ -1389,7 +1383,9 @@ class TestNeutronv2(TestNeutronv2Base):
|
||||||
self.api.allocate_for_instance,
|
self.api.allocate_for_instance,
|
||||||
self.context, self.instance, False,
|
self.context, self.instance, False,
|
||||||
requested_networks=requested_networks)
|
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(
|
mocked_client.list_networks.assert_called_once_with(
|
||||||
id=[uuids.my_netid1, uuids.my_netid2])
|
id=[uuids.my_netid1, uuids.my_netid2])
|
||||||
mocked_client.create_port.assert_called_once_with(port_req_body)
|
mocked_client.create_port.assert_called_once_with(port_req_body)
|
||||||
|
@ -1410,7 +1406,9 @@ class TestNeutronv2(TestNeutronv2Base):
|
||||||
self.assertRaises(test.TestingException,
|
self.assertRaises(test.TestingException,
|
||||||
self.api.allocate_for_instance, self.context,
|
self.api.allocate_for_instance, self.context,
|
||||||
self.instance, False, requested_networks)
|
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(
|
mock_get_available.assert_called_once_with(
|
||||||
self.context, self.instance.project_id, [],
|
self.context, self.instance.project_id, [],
|
||||||
neutron=mocked_client, auto_allocate=False)
|
neutron=mocked_client, auto_allocate=False)
|
||||||
|
@ -1429,9 +1427,8 @@ class TestNeutronv2(TestNeutronv2Base):
|
||||||
objects=[objects.NetworkRequest(port_id=uuids.portid_1)])
|
objects=[objects.NetworkRequest(port_id=uuids.portid_1)])
|
||||||
self._test_allocate_for_instance(
|
self._test_allocate_for_instance(
|
||||||
requested_networks=requested_networks,
|
requested_networks=requested_networks,
|
||||||
exception=exception.PortInUse,
|
exception=exception.PortInUse, _break='pre_list_networks',
|
||||||
get_client_admin_call=False,
|
_device=True)
|
||||||
_break='pre_list_networks', _device=True)
|
|
||||||
|
|
||||||
def test_allocate_for_instance_port_not_found(self):
|
def test_allocate_for_instance_port_not_found(self):
|
||||||
# If a port is not found, an exception should be raised.
|
# If a port is not found, an exception should be raised.
|
||||||
|
@ -1440,7 +1437,6 @@ class TestNeutronv2(TestNeutronv2Base):
|
||||||
self._test_allocate_for_instance(
|
self._test_allocate_for_instance(
|
||||||
requested_networks=requested_networks,
|
requested_networks=requested_networks,
|
||||||
exception=exception.PortNotFound,
|
exception=exception.PortNotFound,
|
||||||
get_client_admin_call=False,
|
|
||||||
_break='pre_list_networks')
|
_break='pre_list_networks')
|
||||||
|
|
||||||
def test_allocate_for_instance_port_invalid_tenantid(self):
|
def test_allocate_for_instance_port_invalid_tenantid(self):
|
||||||
|
@ -1450,7 +1446,6 @@ class TestNeutronv2(TestNeutronv2Base):
|
||||||
self._test_allocate_for_instance(
|
self._test_allocate_for_instance(
|
||||||
requested_networks=requested_networks,
|
requested_networks=requested_networks,
|
||||||
exception=exception.PortNotUsable,
|
exception=exception.PortNotUsable,
|
||||||
get_client_admin_call=False,
|
|
||||||
_break='pre_list_networks')
|
_break='pre_list_networks')
|
||||||
|
|
||||||
@mock.patch.object(neutronapi, 'get_client')
|
@mock.patch.object(neutronapi, 'get_client')
|
||||||
|
@ -1471,7 +1466,9 @@ class TestNeutronv2(TestNeutronv2Base):
|
||||||
self.assertRaises(exception.ExternalNetworkAttachForbidden,
|
self.assertRaises(exception.ExternalNetworkAttachForbidden,
|
||||||
self.api.allocate_for_instance, self.context,
|
self.api.allocate_for_instance, self.context,
|
||||||
self.instance, False, None)
|
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([
|
mocked_client.list_networks.assert_has_calls([
|
||||||
mock.call(tenant_id=self.instance.project_id, shared=False),
|
mock.call(tenant_id=self.instance.project_id, shared=False),
|
||||||
mock.call(shared=True)])
|
mock.call(shared=True)])
|
||||||
|
@ -1496,7 +1493,9 @@ class TestNeutronv2(TestNeutronv2Base):
|
||||||
exception.NetworkAmbiguous,
|
exception.NetworkAmbiguous,
|
||||||
self.api.allocate_for_instance,
|
self.api.allocate_for_instance,
|
||||||
self.context, self.instance, False, None)
|
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([
|
mocked_client.list_networks.assert_has_calls([
|
||||||
mock.call(tenant_id=self.instance.project_id, shared=False),
|
mock.call(tenant_id=self.instance.project_id, shared=False),
|
||||||
mock.call(shared=True)])
|
mock.call(shared=True)])
|
||||||
|
@ -6811,6 +6810,9 @@ class TestAllocateForInstance(test.NoDBTestCase):
|
||||||
self.assertEqual(result[0], {"id": uuids.created})
|
self.assertEqual(result[0], {"id": uuids.created})
|
||||||
self.assertEqual(result[1], {"id": uuids.preexist})
|
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):
|
def test_ensure_no_port_binding_failure_raises(self):
|
||||||
port = {
|
port = {
|
||||||
'id': uuids.port_id,
|
'id': uuids.port_id,
|
||||||
|
@ -7180,7 +7182,6 @@ class TestNeutronv2NeutronHostnameDNS(TestNeutronv2Base):
|
||||||
self._test_allocate_for_instance(
|
self._test_allocate_for_instance(
|
||||||
requested_networks=requested_networks,
|
requested_networks=requested_networks,
|
||||||
exception=exception.PortNotUsableDNS,
|
exception=exception.PortNotUsableDNS,
|
||||||
get_client_admin_call=False,
|
|
||||||
dns_extension=True,
|
dns_extension=True,
|
||||||
_break='pre_list_networks',
|
_break='pre_list_networks',
|
||||||
_dns_name='my-instance')
|
_dns_name='my-instance')
|
||||||
|
|
Loading…
Reference in New Issue