Merge "Use admin neutron client to see if instance has qos ports"

nova/api/openstack/common.py

@@ -28,6 +28,7 @@ from nova.api.openstack import api_version_request
 from nova.compute import task_states
 from nova.compute import vm_states
 import nova.conf
+from nova import context as nova_context
 from nova import exception
 from nova.i18n import _
 from nova.network.neutronv2 import constants
@@ -573,8 +574,7 @@ def supports_port_resource_request_during_move(req):
     return False
 
 
-def instance_has_port_with_resource_request(
-        context, instance_uuid, network_api):
+def instance_has_port_with_resource_request(instance_uuid, network_api):
 
     # TODO(gibi): Use instance.info_cache to see if there is VIFs with
     # allocation key in the profile. If there is no such VIF for an instance
@@ -583,7 +583,12 @@ def instance_has_port_with_resource_request(
     # offloaded then we still have to hit neutron.
     search_opts = {'device_id': instance_uuid,
                    'fields': [constants.RESOURCE_REQUEST]}
-    ports = network_api.list_ports(context, **search_opts).get('ports', [])
+    # NOTE(gibi): We need to use an admin context to query neutron ports as
+    # neutron does not fill the resource_request field in the port response if
+    # we query with a non admin context.
+    admin_context = nova_context.get_admin_context()
+    ports = network_api.list_ports(
+        admin_context, **search_opts).get('ports', [])
     for port in ports:
         if port.get(constants.RESOURCE_REQUEST):
             return True

nova/api/openstack/compute/migrate_server.py

@@ -59,7 +59,7 @@ class MigrateServerController(wsgi.Controller):
                                        expected_attrs=['flavor'])
 
         if common.instance_has_port_with_resource_request(
-                context, instance.uuid, self.network_api):
+                instance.uuid, self.network_api):
             # TODO(gibi): Remove when nova only supports compute newer than
             # Train
             source_service = objects.Service.get_by_host_and_binary(
@@ -132,7 +132,7 @@ class MigrateServerController(wsgi.Controller):
         # extra API call to neutron when we support move operations with ports
         # having resource requests.
         if (common.instance_has_port_with_resource_request(
-                    context, instance.uuid, self.network_api) and not
+                    instance.uuid, self.network_api) and not
                 common.supports_port_resource_request_during_move(req)):
             msg = _("The os-migrateLive action on a server with ports having "
                     "resource requests, like a port with a QoS minimum "

nova/api/openstack/compute/servers.py

@@ -941,7 +941,7 @@ class ServersController(wsgi.Controller):
                             'project_id': instance.project_id})
 
         if common.instance_has_port_with_resource_request(
-                context, instance_id, self.network_api):
+                instance_id, self.network_api):
             # TODO(gibi): Remove when nova only supports compute newer than
             # Train
             source_service = objects.Service.get_by_host_and_binary(

nova/api/openstack/compute/shelve.py

@@ -97,7 +97,7 @@ class ShelveController(wsgi.Controller):
         # having resource requests.
         if (instance.vm_state == vm_states.SHELVED_OFFLOADED and
                 common.instance_has_port_with_resource_request(
-                    context, instance.uuid, self.network_api) and
+                    instance.uuid, self.network_api) and
                 not common.supports_port_resource_request_during_move(
                     req)):
             msg = _("The unshelve action on a server with ports having "

nova/tests/fixtures.py

@@ -1697,7 +1697,20 @@ class NeutronFixture(fixtures.Fixture):
             self.fake_delete_port_binding)
 
         self.test.stub_out('nova.network.neutronv2.api.get_client',
-                           lambda *args, **kwargs: self)
+                           self._get_client)
+
+    def _get_client(self, context, admin=False):
+        # NOTE(gibi): This is a hack. As we return the same fixture for each
+        # get_client call there is no way to later know that a call came
+        # through which client. We store the parameters of the last get_client
+        # call. Later we should return a new client object from this call that
+        # is wrapping the fixture and this client can remember how it was
+        # initialized.
+
+        # This logic is copied from nova.network.neutronv2.api._get_auth_plugin
+        self.is_admin_client = (admin or
+                                (context.is_admin and not context.auth_token))
+        return self
 
     @staticmethod
     def fake_create_port_binding(context, client, port_id, data):
@@ -1769,8 +1782,15 @@ class NeutronFixture(fixtures.Fixture):
             del self._ports[port_id]
 
     def list_ports(self, retrieve_all=True, **_params):
-        return {'ports': self._list_resource(
-            self._ports, retrieve_all, **_params)}
+        ports = self._list_resource(self._ports, retrieve_all, **_params)
+        if not self.is_admin_client:
+            # Neutron returns None instead of the real resource_request if
+            # the ports are queried by a non-admin. So simulate this behavior
+            # here
+            for port in ports:
+                if 'resource_request' in port:
+                    port['resource_request'] = None
+        return {'ports': ports}
 
     def show_network(self, network_id, **_params):
         if network_id not in self._networks:

nova/tests/functional/integrated_helpers.py

@@ -426,7 +426,7 @@ class ProviderUsageBaseTestCase(test.TestCase, InstanceHelperMixin):
         self.flags(compute_driver=self.compute_driver)
         super(ProviderUsageBaseTestCase, self).setUp()
 
-        self.useFixture(policy_fixture.RealPolicyFixture())
+        self.policy = self.useFixture(policy_fixture.RealPolicyFixture())
         self.neutron = self.useFixture(nova_fixtures.NeutronFixture(self))
         self.useFixture(nova_fixtures.AllServicesCurrent())
 

nova/tests/functional/test_servers.py

@@ -5974,6 +5974,31 @@ class UnsupportedPortResourceRequestBasedSchedulingTest(
         self._wait_for_state_change(self.admin_api, server, 'ACTIVE')
 
 
+class NonAdminUnsupportedPortResourceRequestBasedSchedulingTest(
+        UnsupportedPortResourceRequestBasedSchedulingTest):
+
+    def setUp(self):
+        super(
+            NonAdminUnsupportedPortResourceRequestBasedSchedulingTest,
+            self).setUp()
+        # switch to non admin api
+        self.api = self.api_fixture.api
+        self.api.microversion = self.microversion
+
+        # allow non-admin to call the operations
+        self.policy.set_rules({
+            'os_compute_api:os-evacuate': '@',
+            'os_compute_api:servers:create': '@',
+            'os_compute_api:servers:create:attach_network': '@',
+            'os_compute_api:servers:show': '@',
+            'os_compute_api:os-attach-interfaces': '@',
+            'os_compute_api:os-attach-interfaces:create': '@',
+            'os_compute_api:os-shelve:shelve': '@',
+            'os_compute_api:os-shelve:unshelve': '@',
+            'os_compute_api:os-migrate-server:migrate_live': '@',
+        })
+
+
 class PortResourceRequestBasedSchedulingTest(
         PortResourceRequestBasedSchedulingTestBase):
     """Tests creating a server with a pre-existing port that has a resource

nova/tests/unit/api/openstack/compute/test_migrate_server.py

@@ -314,8 +314,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
             instance['uuid'], body={'migrate': None})
 
         mock_has_res_req.assert_called_once_with(
-            self.req.environ['nova.context'], instance['uuid'],
-            self.controller.network_api)
+            instance['uuid'], self.controller.network_api)
         mock_get_service.assert_called_once_with(
             self.req.environ['nova.context'], instance['host'], 'nova-compute')
 

nova/tests/unit/api/openstack/compute/test_server_actions.py

@@ -1260,7 +1260,6 @@ class ServerActionsControllerTestV21(test.TestCase):
                           self.req, FAKE_UUID, body=body)
 
         mock_has_res_req.assert_called_once_with(
-            self.req.environ['nova.context'], FAKE_UUID,
-            self.controller.network_api)
+            FAKE_UUID, self.controller.network_api)
         mock_get_service.assert_called_once_with(
             self.req.environ['nova.context'], 'fake_host', 'nova-compute')

nova/tests/unit/api/openstack/test_common.py

@@ -27,7 +27,9 @@ import webob.multidict
 from nova.api.openstack import common
 from nova.compute import task_states
 from nova.compute import vm_states
+from nova import context
 from nova import exception
+from nova import network
 from nova import test
 from nova.tests.unit.api.openstack import fakes
 
@@ -449,6 +451,23 @@ class MiscFunctionsTest(test.TestCase):
         self.assertRaises(exception.InvalidInput, common.is_all_tenants,
                           search_opts)
 
+    def test_instance_has_port_with_resource_request(self):
+        network_api = mock.Mock(spec=network.API())
+        network_api.list_ports.return_value = {'ports': [
+            {'resource_request': mock.sentinel.request}
+        ]}
+        res = common.instance_has_port_with_resource_request(
+            mock.sentinel.uuid, network_api)
+
+        self.assertTrue(res)
+        network_api.list_ports.assert_called_once_with(
+            test.MatchType(context.RequestContext),
+            device_id=mock.sentinel.uuid, fields=['resource_request'])
+        # assert that the neutron call uses an admin context
+        ctxt = network_api.mock_calls[0][1][0]
+        self.assertTrue(ctxt.is_admin)
+        self.assertIsNone(ctxt.auth_token)
+
 
 class TestCollectionLinks(test.NoDBTestCase):
     """Tests the _get_collection_links method."""