Merge "Reject server operations with extended resource req"
This commit is contained in:
commit
02b2d3e96e
|
@ -120,6 +120,13 @@ class EvacuateController(wsgi.Controller):
|
|||
msg = _("The target host can't be the same one.")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
if self.network_api.instance_has_extended_resource_request(id):
|
||||
msg = _(
|
||||
"The evacuate server operation with port having extended "
|
||||
"resource request, like a port with both QoS minimum "
|
||||
"bandwidth and packet rate policies, is not yet supported.")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
try:
|
||||
self.compute_api.evacuate(context, instance, host,
|
||||
on_shared_storage, password, force)
|
||||
|
|
|
@ -56,6 +56,13 @@ class MigrateServerController(wsgi.Controller):
|
|||
body['migrate'] is not None):
|
||||
host_name = body['migrate'].get('host')
|
||||
|
||||
if self.network_api.instance_has_extended_resource_request(id):
|
||||
msg = _(
|
||||
"The migrate server operation with port having extended "
|
||||
"resource request, like a port with both QoS minimum "
|
||||
"bandwidth and packet rate policies, is not yet supported.")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
try:
|
||||
self.compute_api.resize(req.environ['nova.context'], instance,
|
||||
host_name=host_name)
|
||||
|
@ -120,6 +127,13 @@ class MigrateServerController(wsgi.Controller):
|
|||
disk_over_commit = strutils.bool_from_string(disk_over_commit,
|
||||
strict=True)
|
||||
|
||||
if self.network_api.instance_has_extended_resource_request(id):
|
||||
msg = _(
|
||||
"The live migrate server operation with port having extended "
|
||||
"resource request, like a port with both QoS minimum "
|
||||
"bandwidth and packet rate policies, is not yet supported.")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
try:
|
||||
self.compute_api.live_migrate(context, instance, block_migration,
|
||||
disk_over_commit, host, force,
|
||||
|
|
|
@ -1028,6 +1028,15 @@ class ServersController(wsgi.Controller):
|
|||
target={'user_id': instance.user_id,
|
||||
'project_id': instance.project_id})
|
||||
|
||||
if self.network_api.instance_has_extended_resource_request(
|
||||
instance_id
|
||||
):
|
||||
msg = _(
|
||||
"The resize server operation with port having extended "
|
||||
"resource request, like a port with both QoS minimum "
|
||||
"bandwidth and packet rate policies, is not yet supported.")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
try:
|
||||
self.compute_api.resize(context, instance, flavor_id,
|
||||
auto_disk_config=auto_disk_config)
|
||||
|
|
|
@ -23,7 +23,9 @@ from nova.api.openstack.compute.schemas import shelve as shelve_schemas
|
|||
from nova.api.openstack import wsgi
|
||||
from nova.api import validation
|
||||
from nova.compute import api as compute
|
||||
from nova.compute import vm_states
|
||||
from nova import exception
|
||||
from nova.i18n import _
|
||||
from nova.network import neutron
|
||||
from nova.policies import shelve as shelve_policies
|
||||
|
||||
|
@ -105,6 +107,17 @@ class ShelveController(wsgi.Controller):
|
|||
if support_az and unshelve_dict:
|
||||
new_az = unshelve_dict['availability_zone']
|
||||
|
||||
if (
|
||||
instance.vm_state == vm_states.SHELVED_OFFLOADED and
|
||||
self.network_api.instance_has_extended_resource_request(id)
|
||||
):
|
||||
msg = _(
|
||||
"The unshelve server operation on a shelve offloaded server "
|
||||
"with port having extended resource request, like a "
|
||||
"port with both QoS minimum bandwidth and packet rate "
|
||||
"policies, is not yet supported.")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
try:
|
||||
self.compute_api.unshelve(context, instance, new_az=new_az)
|
||||
except (exception.InstanceIsLocked,
|
||||
|
|
|
@ -5051,6 +5051,21 @@ class API:
|
|||
self.volume_api.attachment_delete(
|
||||
context, new_attachment_id)
|
||||
|
||||
def support_port_attach(self, context, port):
|
||||
"""Returns false if neutron is configured with extended resource
|
||||
request and the port has resource request.
|
||||
|
||||
This function is only here temporary to help mocking this check in the
|
||||
functional test environment.
|
||||
"""
|
||||
if not self.network_api._has_extended_resource_request_extension(
|
||||
context
|
||||
):
|
||||
return True
|
||||
|
||||
resource_request = port.get('resource_request', {})
|
||||
return not resource_request.get('request_groups', [])
|
||||
|
||||
@check_instance_lock
|
||||
@check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED,
|
||||
vm_states.STOPPED],
|
||||
|
@ -5087,6 +5102,9 @@ class API:
|
|||
network_model.VNIC_TYPE_ACCELERATOR_DIRECT_PHYSICAL):
|
||||
raise exception.ForbiddenPortsWithAccelerator()
|
||||
|
||||
if not self.support_port_attach(context, port):
|
||||
raise exception.AttachWithExtendedQoSPolicyNotSupported()
|
||||
|
||||
return self.compute_rpcapi.attach_interface(context,
|
||||
instance=instance, network_id=network_id, port_id=port_id,
|
||||
requested_ip=requested_ip, tag=tag)
|
||||
|
|
|
@ -1944,6 +1944,13 @@ class AttachInterfaceWithQoSPolicyNotSupported(AttachInterfaceNotSupported):
|
|||
"instance %(instance_uuid)s.")
|
||||
|
||||
|
||||
class AttachWithExtendedQoSPolicyNotSupported(AttachInterfaceNotSupported):
|
||||
msg_fmt = _(
|
||||
"The interface attach server operation with port having extended "
|
||||
"resource request, like a port with both QoS minimum bandwidth and "
|
||||
"packet rate policies, is not yet supported.")
|
||||
|
||||
|
||||
class NetworksWithQoSPolicyNotSupported(Invalid):
|
||||
msg_fmt = _("Using networks with QoS policy is not supported for "
|
||||
"instance %(instance_uuid)s. (Network ID is %(network_id)s)")
|
||||
|
|
|
@ -1002,6 +1002,30 @@ class API:
|
|||
else:
|
||||
return bool(resource_request)
|
||||
|
||||
def instance_has_extended_resource_request(self, instance_uuid):
|
||||
# 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()
|
||||
|
||||
if not self._has_extended_resource_request_extension(admin_context):
|
||||
# Short circuit if the extended resource request API extension is
|
||||
# not available
|
||||
return False
|
||||
|
||||
# So neutron supports the extended resource request but does the
|
||||
# instance has a port with such request
|
||||
search_opts = {'device_id': instance_uuid,
|
||||
'fields': [constants.RESOURCE_REQUEST]}
|
||||
ports = self.list_ports(
|
||||
admin_context, **search_opts).get('ports', [])
|
||||
|
||||
for port in ports:
|
||||
resource_request = port.get(constants.RESOURCE_REQUEST) or {}
|
||||
if resource_request.get(constants.REQUEST_GROUPS, []):
|
||||
return True
|
||||
return False
|
||||
|
||||
def allocate_for_instance(self, context, instance,
|
||||
requested_networks,
|
||||
security_groups=None, bind_host_id=None,
|
||||
|
|
|
@ -128,6 +128,17 @@ class ResourceRequestNeutronFixture(NeutronFixture):
|
|||
|
||||
|
||||
class ExtendedResourceRequestNeutronFixture(ResourceRequestNeutronFixture):
|
||||
@classmethod
|
||||
def create_with_existing_neutron_state(cls, existing_fixture):
|
||||
"""Creates a new fixture but initialize it from an existing neutron
|
||||
fixture to carry over the state from it.
|
||||
"""
|
||||
fixture = cls(existing_fixture.test)
|
||||
fixture._ports = existing_fixture._ports
|
||||
fixture._networks = existing_fixture._networks
|
||||
fixture._subnets = existing_fixture._subnets
|
||||
return fixture
|
||||
|
||||
def list_extensions(self, *args, **kwargs):
|
||||
extensions = super().list_extensions(*args, **kwargs)
|
||||
extensions['extensions'].append(
|
||||
|
@ -2620,18 +2631,16 @@ class ExtendedResourceRequestTempNegativeTest(
|
|||
are expected to be removed when support for the extension is implemented
|
||||
in nova.
|
||||
"""
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.neutron = self.useFixture(
|
||||
ExtendedResourceRequestNeutronFixture(self))
|
||||
|
||||
def test_boot(self):
|
||||
"""The neutron fixture used in this test class enables the
|
||||
"""The neutron fixture used in this test enables the
|
||||
extended-resource-request API extension. This results in any new
|
||||
server to boot. This is harsh but without nova support for this
|
||||
extension there is no way that this extension is helpful. So treat
|
||||
this as a deployment configuration error.
|
||||
"""
|
||||
self.neutron = self.useFixture(
|
||||
ExtendedResourceRequestNeutronFixture(self))
|
||||
ex = self.assertRaises(
|
||||
client.OpenStackApiException,
|
||||
self._create_server,
|
||||
|
@ -2646,3 +2655,81 @@ class ExtendedResourceRequestTempNegativeTest(
|
|||
'Neutron.',
|
||||
str(ex)
|
||||
)
|
||||
|
||||
def _test_operation(self, op_name, op_callable):
|
||||
# boot a server with a qos port still using the old Neutron resource
|
||||
# request API extension
|
||||
server = self._create_server(
|
||||
flavor=self.flavor,
|
||||
networks=[{'port': self.neutron.port_with_resource_request['id']}],
|
||||
)
|
||||
self._wait_for_state_change(server, 'ACTIVE')
|
||||
|
||||
# enable the new extended-resource-request Neutron API extension by
|
||||
# replacing the old neutron fixture with a new one that enables the
|
||||
# extension. Note that we are carrying over the state of the neutron
|
||||
# to the new extension to keep the port bound to the server.
|
||||
self.neutron = self.useFixture(
|
||||
ExtendedResourceRequestNeutronFixture.
|
||||
create_with_existing_neutron_state(self.neutron))
|
||||
|
||||
# nova does not support this Neutron API extension yet so the
|
||||
# operation fails
|
||||
ex = self.assertRaises(
|
||||
client.OpenStackApiException,
|
||||
op_callable,
|
||||
server,
|
||||
)
|
||||
self.assertEqual(400, ex.response.status_code)
|
||||
self.assertIn(
|
||||
f'The {op_name} with port having extended resource request, like '
|
||||
f'a port with both QoS minimum bandwidth and packet rate '
|
||||
f'policies, is not yet supported.',
|
||||
str(ex)
|
||||
)
|
||||
|
||||
def test_resize(self):
|
||||
self._test_operation(
|
||||
'resize server operation',
|
||||
lambda server: self._resize_server(
|
||||
server, self.flavor_with_group_policy['id']
|
||||
)
|
||||
)
|
||||
|
||||
def test_migrate(self):
|
||||
self._test_operation(
|
||||
'migrate server operation',
|
||||
lambda server: self._migrate_server(server),
|
||||
)
|
||||
|
||||
def test_live_migrate(self):
|
||||
self._test_operation(
|
||||
'live migrate server operation',
|
||||
lambda server: self._live_migrate(server),
|
||||
)
|
||||
|
||||
def test_evacuate(self):
|
||||
self._test_operation(
|
||||
'evacuate server operation',
|
||||
lambda server: self._evacuate_server(server),
|
||||
)
|
||||
|
||||
def test_interface_attach(self):
|
||||
self._test_operation(
|
||||
'interface attach server operation',
|
||||
lambda server: self._attach_interface(
|
||||
server,
|
||||
self.neutron.port_with_sriov_resource_request['id'],
|
||||
),
|
||||
)
|
||||
|
||||
def test_unshelve_after_shelve_offload(self):
|
||||
|
||||
def shelve_offload_then_unshelve(server):
|
||||
self._shelve_server(server, expected_state='SHELVED_OFFLOADED')
|
||||
self._unshelve_server(server)
|
||||
|
||||
self._test_operation(
|
||||
'unshelve server operation on a shelve offloaded server',
|
||||
lambda server: shelve_offload_then_unshelve(server),
|
||||
)
|
||||
|
|
|
@ -12,6 +12,7 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import fixtures
|
||||
import mock
|
||||
from oslo_utils.fixture import uuidsentinel as uuids
|
||||
import testtools
|
||||
|
@ -66,6 +67,12 @@ class EvacuateTestV21(test.NoDBTestCase):
|
|||
self.stub_out('nova.compute.api.API.get', fake_compute_api_get)
|
||||
self.stub_out('nova.compute.api.HostAPI.service_get_by_compute_host',
|
||||
fake_service_get_by_compute_host)
|
||||
self.mock_neutron_extension_list = self.useFixture(
|
||||
fixtures.MockPatch(
|
||||
'nova.network.neutron.API._refresh_neutron_extensions_cache'
|
||||
)
|
||||
).mock
|
||||
self.mock_neutron_extension_list.return_value = {'extensions': []}
|
||||
self.UUID = uuids.fake
|
||||
for _method in self._methods:
|
||||
self.stub_out('nova.compute.api.API.%s' % _method,
|
||||
|
|
|
@ -13,6 +13,7 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import fixtures
|
||||
import mock
|
||||
from oslo_utils.fixture import uuidsentinel as uuids
|
||||
from oslo_utils import uuidutils
|
||||
|
@ -44,6 +45,12 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
|
|||
self.stub_out('nova.api.openstack.compute.migrate_server.'
|
||||
'MigrateServerController',
|
||||
lambda *a, **kw: self.controller)
|
||||
self.mock_neutron_extension_list = self.useFixture(
|
||||
fixtures.MockPatch(
|
||||
'nova.network.neutron.API._refresh_neutron_extensions_cache'
|
||||
)
|
||||
).mock
|
||||
self.mock_neutron_extension_list.return_value = {'extensions': []}
|
||||
|
||||
def _get_migration_body(self, **kwargs):
|
||||
return {'os-migrateLive': self._get_params(**kwargs)}
|
||||
|
|
|
@ -14,6 +14,7 @@
|
|||
# under the License.
|
||||
|
||||
import ddt
|
||||
import fixtures
|
||||
import mock
|
||||
from oslo_utils.fixture import uuidsentinel as uuids
|
||||
from oslo_utils import uuidutils
|
||||
|
@ -99,6 +100,12 @@ class ServerActionsControllerTestV21(test.TestCase):
|
|||
self.controller.compute_api, 'compute_task_api')
|
||||
mock_conductor.start()
|
||||
self.addCleanup(mock_conductor.stop)
|
||||
self.mock_neutron_extension_list = self.useFixture(
|
||||
fixtures.MockPatch(
|
||||
'nova.network.neutron.API._refresh_neutron_extensions_cache'
|
||||
)
|
||||
).mock
|
||||
self.mock_neutron_extension_list.return_value = {'extensions': []}
|
||||
|
||||
def _get_controller(self):
|
||||
return self.servers.ServersController()
|
||||
|
|
|
@ -12,6 +12,7 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import fixtures
|
||||
import mock
|
||||
|
||||
import ddt
|
||||
|
@ -104,6 +105,12 @@ class UnshelveServerControllerTestV277(test.NoDBTestCase):
|
|||
|
||||
def setUp(self):
|
||||
super(UnshelveServerControllerTestV277, self).setUp()
|
||||
self.mock_neutron_extension_list = self.useFixture(
|
||||
fixtures.MockPatch(
|
||||
'nova.network.neutron.API._refresh_neutron_extensions_cache'
|
||||
)
|
||||
).mock
|
||||
self.mock_neutron_extension_list.return_value = {'extensions': []}
|
||||
self.controller = shelve_v21.ShelveController()
|
||||
self.req = fakes.HTTPRequest.blank(
|
||||
'/%s/servers/a/action' % fakes.FAKE_PROJECT_ID,
|
||||
|
|
|
@ -7204,6 +7204,10 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
|||
mock_get_service.assert_called_once_with(
|
||||
self.context, instance.host, 'nova-compute')
|
||||
|
||||
@mock.patch(
|
||||
'nova.network.neutron.API._has_extended_resource_request_extension',
|
||||
new=mock.Mock(return_value=False)
|
||||
)
|
||||
@mock.patch('nova.compute.rpcapi.ComputeAPI.attach_interface')
|
||||
@mock.patch('nova.objects.service.Service.get_by_host_and_binary')
|
||||
@mock.patch('nova.compute.api.API._record_action_start')
|
||||
|
|
|
@ -6667,6 +6667,86 @@ class TestAPI(TestAPIBase):
|
|||
)
|
||||
|
||||
|
||||
class TestInstanceHasExtendedResourceRequest(TestAPIBase):
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
patcher = mock.patch.object(neutronapi, 'get_client')
|
||||
self.addCleanup(patcher.stop)
|
||||
self.mock_client = patcher.start().return_value
|
||||
self.extension = {
|
||||
"extensions": [
|
||||
{
|
||||
"name": constants.RESOURCE_REQUEST_GROUPS_EXTENSION,
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
def test_no_extension(self):
|
||||
self.mock_client.list_extensions.return_value = {
|
||||
"extensions": []
|
||||
}
|
||||
|
||||
self.assertFalse(
|
||||
self.api.instance_has_extended_resource_request(uuids.instance))
|
||||
|
||||
self.mock_client.list_extensions.assert_called_once_with()
|
||||
self.mock_client.list_ports.assert_not_called()
|
||||
|
||||
def test_no_port(self):
|
||||
self.mock_client.list_extensions.return_value = self.extension
|
||||
self.mock_client.list_ports.return_value = {
|
||||
"ports": []
|
||||
}
|
||||
|
||||
self.assertFalse(
|
||||
self.api.instance_has_extended_resource_request(uuids.instance))
|
||||
|
||||
self.mock_client.list_extensions.assert_called_once_with()
|
||||
self.mock_client.list_ports.assert_called_once_with(
|
||||
device_id=uuids.instance,
|
||||
fields=['resource_request'])
|
||||
|
||||
def test_port_without_request(self):
|
||||
self.mock_client.list_extensions.return_value = self.extension
|
||||
self.mock_client.list_ports.return_value = {
|
||||
"ports": [
|
||||
{"resource_request": {}}
|
||||
]
|
||||
}
|
||||
|
||||
self.assertFalse(
|
||||
self.api.instance_has_extended_resource_request(uuids.instance))
|
||||
|
||||
self.mock_client.list_extensions.assert_called_once_with()
|
||||
self.mock_client.list_ports.assert_called_once_with(
|
||||
device_id=uuids.instance,
|
||||
fields=['resource_request'])
|
||||
|
||||
def test_port_with_request(self):
|
||||
self.mock_client.list_extensions.return_value = self.extension
|
||||
self.mock_client.list_ports.return_value = {
|
||||
"ports": [
|
||||
{
|
||||
"resource_request": {
|
||||
"request_groups": [
|
||||
{
|
||||
"CUSTOM_FOO": 1000,
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
self.assertTrue(
|
||||
self.api.instance_has_extended_resource_request(uuids.instance))
|
||||
|
||||
self.mock_client.list_extensions.assert_called_once_with()
|
||||
self.mock_client.list_ports.assert_called_once_with(
|
||||
device_id=uuids.instance,
|
||||
fields=['resource_request'])
|
||||
|
||||
|
||||
class TestAPIModuleMethods(test.NoDBTestCase):
|
||||
|
||||
def test_gather_port_ids_and_networks_wrong_params(self):
|
||||
|
|
|
@ -41,6 +41,7 @@ class BasePolicyTest(test.TestCase):
|
|||
|
||||
def setUp(self):
|
||||
super(BasePolicyTest, self).setUp()
|
||||
self.useFixture(fixtures.NeutronFixture(self))
|
||||
self.policy = self.useFixture(fixtures.RealPolicyFixture())
|
||||
|
||||
self.admin_project_id = uuids.admin_project_id
|
||||
|
|
Loading…
Reference in New Issue