Use admin neutron client to see if instance has qos ports
The nova-api checks at each move* operation if the instance has qos port attached as not all the move operations are supported for such servers. Nova uses the request context to initialize the neutron client for the port query. However neutron does not return the value of the resource_request of the port if it is queried with a non admin client. This causes that if the move operation is initiated by a non admin then nova thinks that the ports do not have resource request. This patch creates an admin context for this neutron query. The new functional tests are not added before this patch in a regression test like way as existing functional tests are reused with different setup and doing that without the fix causes a lot of different failure scenarios. Note that neutron fixture is changed to simulate the different behavior in case of different request context are used to initialize the client. *: Note that Id5f2f4f22b856c989e2eef8ed56b9829d1bcefb6 removed the check for evacuate in Ussuri but exists in Train and Stein. Change-Id: I3cf6eb4654663865d9258c38f05cd05974ffcf9d Closes-Bug: #1850280
This commit is contained in:
parent
38a214466f
commit
8999769605
|
@ -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
|
||||
|
|
|
@ -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 "
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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 "
|
||||
|
|
|
@ -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:
|
||||
|
|
|
@ -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())
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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')
|
||||
|
||||
|
|
|
@ -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')
|
||||
|
|
|
@ -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."""
|
||||
|
|
Loading…
Reference in New Issue