Merge "Use admin neutron client to see if instance has qos ports" into stable/stein

nova/api/openstack/common.py

@@ -29,6 +29,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 import objects
@@ -564,8 +565,7 @@ def supports_port_resource_request_during_move(req):
     return False
 
 
-def instance_has_port_with_resource_request(
+def instance_has_port_with_resource_request(instance_uuid, network_api):
-        context, 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
@@ -574,7 +574,12 @@ def instance_has_port_with_resource_request(
     # offloaded then we still have to hit neutron.
     search_opts = {'device_id': instance_uuid,
                    'fields': ['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('resource_request'):
             return True

nova/api/openstack/compute/evacuate.py

@@ -121,7 +121,7 @@ class EvacuateController(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 evacuate action on a server with ports having "
                     "resource requests, like a port with a QoS minimum "

nova/api/openstack/compute/migrate_server.py

@@ -58,7 +58,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 migrate action on a server with ports having "
                     "resource requests, like a port with a QoS minimum "
@@ -127,7 +127,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

@@ -909,7 +909,7 @@ class ServersController(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_id, self.network_api) and not
+                    instance_id, self.network_api) and not
                 common.supports_port_resource_request_during_move(req)):
             msg = _("The resize action on a server with ports having "
                     "resource requests, like a port with a QoS minimum "

nova/api/openstack/compute/shelve.py

@@ -88,7 +88,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)
+                    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

@@ -1473,7 +1473,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):
@@ -1538,6 +1551,14 @@ class NeutronFixture(fixtures.Fixture):
         _params.pop('fields', None)
         ports = [p for p in self._ports.values()
                  if all(p.get(opt) == _params[opt] for opt in _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': copy.deepcopy(ports)}
 
     def list_subnets(self, retrieve_all=True, **_params):

nova/tests/functional/integrated_helpers.py

@@ -408,7 +408,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())
 
@@ -417,10 +417,10 @@ class ProviderUsageBaseTestCase(test.TestCase, InstanceHelperMixin):
 
         placement = self.useFixture(func_fixtures.PlacementFixture())
         self.placement_api = placement.api
-        api_fixture = self.useFixture(nova_fixtures.OSAPIFixture(
+        self.api_fixture = self.useFixture(nova_fixtures.OSAPIFixture(
             api_version='v2.1'))
 
-        self.admin_api = api_fixture.admin_api
+        self.admin_api = self.api_fixture.admin_api
         self.admin_api.microversion = self.microversion
         self.api = self.admin_api
 

nova/tests/functional/test_servers.py

@@ -6504,6 +6504,33 @@ 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': '@',
+            'os_compute_api:os-migrate-server:migrate': '@',
+            'os_compute_api:servers:resize': '@',
+        })
+
+
 class PortResourceRequestBasedSchedulingTest(
         PortResourceRequestBasedSchedulingTestBase):
     """Tests creating a server with a pre-existing port that has a resource

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."""