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

Conflicts:
      nova/tests/unit/network/test_neutronv2.py
Conflicts due to mox removal patches merged in train. I basically needed
to redo the change in test_neutronv2.py as the merge conflict was huge.

Change-Id: Icc631cf2e81a5c78cb7fb1d0b625d19bd8f5a274
Closes-Bug: #1849657
(cherry picked from commit aab4b7a0e2)
(cherry picked from commit b6989836dd)

nova/network/neutronv2/api.py

@@ -1054,6 +1054,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
@@ -1061,9 +1065,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)
@@ -1113,10 +1123,6 @@ class API(base_api.NetworkAPI):
         available_macs = _filter_hypervisor_macs(
             instance, requested_ports_dict, macs)
 
-        # 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,

nova/tests/unit/network/test_neutronv2.py

@@ -497,7 +497,6 @@ class _TestNeutronv2Common(test.TestCase):
                                     mock_refresh, mock_populate, net_idx=1,
                                     requested_networks=None, macs=None,
                                     exception=None,
-                                    get_client_admin_call=True,
                                     context=None,
                                     **kwargs):
         ctxt = context or self.context
@@ -651,12 +650,9 @@ class _TestNeutronv2Common(test.TestCase):
                 ctxt, self.instance, False, requested_networks,
                 macs=original_macs, 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()
 
@@ -848,6 +844,10 @@ class TestNeutronv2Base(_TestNeutronv2Common):
         self._stub_allocate_for_instance_show_port(nets, ports, fixed_ips,
             macs, req_net_ids, ordered_networks, **kwargs)
 
+        neutronapi.get_client(
+            mox.IgnoreArg(), admin=True).AndReturn(
+            self.moxed_client)
+
         if kwargs.get('_break') == 'pre_list_networks':
             self.mox.ReplayAll()
             return api
@@ -871,10 +871,6 @@ class TestNeutronv2Base(_TestNeutronv2Common):
         self._stub_allocate_for_instance_create_port(
             ordered_networks, fixed_ips, nets)
 
-        neutronapi.get_client(
-            mox.IgnoreArg(), admin=True).AndReturn(
-            self.moxed_client)
-
         preexisting_port_ids = []
         ports_in_requested_net_order = []
         nets_in_requested_net_order = []
@@ -1289,6 +1285,8 @@ class TestNeutronv2(TestNeutronv2Base):
             shared=False).AndReturn(
                 {'networks': model.NetworkInfo([])})
         neutronapi.get_client(mox.IgnoreArg()).AndReturn(self.moxed_client)
+        neutronapi.get_client(
+            mox.IgnoreArg(), admin=True).AndReturn(self.moxed_client)
         self.moxed_client.list_networks(shared=True).AndReturn(
             {'networks': model.NetworkInfo([])})
         self.mox.ReplayAll()
@@ -1380,6 +1378,8 @@ class TestNeutronv2(TestNeutronv2Base):
             objects=[objects.NetworkRequest(network_id=net['id'])
                      for net in (self.nets2[0], self.nets2[1])])
         neutronapi.get_client(mox.IgnoreArg()).AndReturn(self.moxed_client)
+        neutronapi.get_client(
+            mox.IgnoreArg(), admin=True).AndReturn(self.moxed_client)
         self.moxed_client.list_networks(
             id=[uuids.my_netid1, uuids.my_netid2]).AndReturn(
             {'networks': self.nets2})
@@ -1406,6 +1406,8 @@ class TestNeutronv2(TestNeutronv2Base):
                                                         **self.instance)
         api = neutronapi.API()
         neutronapi.get_client(mox.IgnoreArg()).AndReturn(self.moxed_client)
+        neutronapi.get_client(
+            mox.IgnoreArg(), admin=True).AndReturn(self.moxed_client)
         self.mox.StubOutWithMock(api, '_get_available_networks')
         # Make sure we get an empty list and then bail out of the rest
         # of the function
@@ -1437,6 +1439,8 @@ class TestNeutronv2(TestNeutronv2Base):
         self.instance = fake_instance.fake_instance_obj(self.context,
                                                         **self.instance)
         neutronapi.get_client(mox.IgnoreArg()).AndReturn(self.moxed_client)
+        neutronapi.get_client(
+            mox.IgnoreArg(), admin=True).AndReturn(self.moxed_client)
         # no networks in the tenant
         self.moxed_client.list_networks(
             tenant_id=self.instance.project_id,
@@ -1458,6 +1462,8 @@ class TestNeutronv2(TestNeutronv2Base):
         self.instance = fake_instance.fake_instance_obj(self.context,
                                                         **self.instance)
         neutronapi.get_client(mox.IgnoreArg()).AndReturn(self.moxed_client)
+        neutronapi.get_client(
+            mox.IgnoreArg(), admin=True).AndReturn(self.moxed_client)
         # network found in the tenant
         self.moxed_client.list_networks(
             tenant_id=self.instance.project_id,
@@ -3609,8 +3615,7 @@ class TestNeutronv2WithMock(_TestNeutronv2Common):
     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)
 
     @mock.patch('nova.network.neutronv2.api.API._unbind_ports')
     def test_allocate_for_instance_not_enough_macs_via_ports(self,
@@ -3650,8 +3655,7 @@ class TestNeutronv2WithMock(_TestNeutronv2Common):
 
     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_invalid_network_id(self):
         requested_networks = objects.NetworkRequestList(
@@ -3660,7 +3664,6 @@ class TestNeutronv2WithMock(_TestNeutronv2Common):
         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_port_in_use(self):
@@ -3670,7 +3673,6 @@ class TestNeutronv2WithMock(_TestNeutronv2Common):
         self._test_allocate_for_instance(
             requested_networks=requested_networks,
             exception=exception.PortInUse,
-            get_client_admin_call=False,
             _break='pre_list_networks', _device=True)
 
     def test_allocate_for_instance_port_not_found(self):
@@ -3680,7 +3682,6 @@ class TestNeutronv2WithMock(_TestNeutronv2Common):
         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):
@@ -3690,7 +3691,6 @@ class TestNeutronv2WithMock(_TestNeutronv2Common):
         self._test_allocate_for_instance(
             requested_networks=requested_networks,
             exception=exception.PortNotUsable,
-            get_client_admin_call=False,
             _break='pre_list_networks')
 
     def test_allocate_for_instance_with_externalnet_admin_ctx(self):
@@ -6765,6 +6765,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_populate_mac_address_skip_if_none(self):
         api = neutronapi.API()
         port_req_body = {}