diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index c2bab9da0dfa..5b64ef5e48e0 100644 --- a/nova/api/openstack/common.py +++ b/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 diff --git a/nova/api/openstack/compute/evacuate.py b/nova/api/openstack/compute/evacuate.py index 180da2a0d19a..f366ba8c8c66 100644 --- a/nova/api/openstack/compute/evacuate.py +++ b/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 " diff --git a/nova/api/openstack/compute/migrate_server.py b/nova/api/openstack/compute/migrate_server.py index dd5eff8e3a54..f67e10a50c5b 100644 --- a/nova/api/openstack/compute/migrate_server.py +++ b/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 " diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 2148fc99ff66..b7dae423665d 100644 --- a/nova/api/openstack/compute/servers.py +++ b/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( diff --git a/nova/api/openstack/compute/shelve.py b/nova/api/openstack/compute/shelve.py index ae8bea989ce2..d26a58001e03 100644 --- a/nova/api/openstack/compute/shelve.py +++ b/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 " diff --git a/nova/tests/fixtures.py b/nova/tests/fixtures.py index 4944afc6739d..30d76b2c123e 100644 --- a/nova/tests/fixtures.py +++ b/nova/tests/fixtures.py @@ -1512,7 +1512,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): @@ -1577,6 +1590,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): diff --git a/nova/tests/functional/integrated_helpers.py b/nova/tests/functional/integrated_helpers.py index 767b5104e918..4a9b6715e60c 100644 --- a/nova/tests/functional/integrated_helpers.py +++ b/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()) diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index 2cdd330c5a5e..7cc393ee18a5 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -5902,6 +5902,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 diff --git a/nova/tests/unit/api/openstack/compute/test_migrate_server.py b/nova/tests/unit/api/openstack/compute/test_migrate_server.py index 2bd91c6385ab..99572af1bd1c 100644 --- a/nova/tests/unit/api/openstack/compute/test_migrate_server.py +++ b/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') diff --git a/nova/tests/unit/api/openstack/compute/test_server_actions.py b/nova/tests/unit/api/openstack/compute/test_server_actions.py index da3813ba7820..ae4a0dbde5e4 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_server_actions.py @@ -1240,7 +1240,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') diff --git a/nova/tests/unit/api/openstack/test_common.py b/nova/tests/unit/api/openstack/test_common.py index 9eecb1fa3193..4329ca9da66d 100644 --- a/nova/tests/unit/api/openstack/test_common.py +++ b/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."""