From bbfb3bcf792b0d712ec59259479c8701b1e31722 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Fri, 11 Jan 2019 16:48:06 +0100 Subject: [PATCH] Reject resize with port having resource request Nova does not consider the resource request of a Neutron port as of now. So this patch makes sure that server resize request is rejected if it involves a port that has resource request. When the feature is ready on the nova side this limitation will be lifted. Note that the api sample tests for server actions needed to split into two test class. The floating ip tests are moved to a new class as those tests are not supporting Neutron yet. For the rest neutron is now used in the test. This was necessary as now the resize action depending on neutron a neturon only feature, i.e. listing ports. blueprint: bandwidth-resource-provider Change-Id: I61a3e8902a891bac36911812e4e7c080570e3850 --- nova/api/openstack/common.py | 25 +++++++++ nova/api/openstack/compute/servers.py | 9 ++++ .../api_sample_tests/test_servers.py | 7 +++ nova/tests/functional/test_servers.py | 28 ++++++++++ .../openstack/compute/test_server_actions.py | 53 +++++++++++++++---- .../api/openstack/compute/test_serversV21.py | 15 ++++-- 6 files changed, 123 insertions(+), 14 deletions(-) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index 4d33fb9e2ade..7a9da405d752 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -550,3 +550,28 @@ def supports_port_resource_request(req): port resource request support, False otherwise. """ return False + + +def supports_port_resource_request_during_move(req): + """Check to see if the requested API version is high enough for support + port resource request during move operation. + + NOTE: At the moment there is no such microversion that supports port + resource request during move. This function is added as a preparation for + that microversion. + + :param req: The incoming API request + :returns: True if the requested API microversion is high enough for + port resource request move support, False otherwise. + """ + return False + + +def instance_has_port_with_resource_request( + context, instance_uuid, network_api): + + search_opts = {'device_id': instance_uuid} + ports = network_api.list_ports(context, **search_opts).get('ports', []) + ports_with_resource_request = [port for port in ports + if port.get('resource_request', None)] + return bool(ports_with_resource_request) diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index dac494d861c6..58efc8f18dc3 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -41,6 +41,7 @@ from nova import context as nova_context from nova import exception from nova.i18n import _ from nova.image import api as image_api +from nova import network as network_api from nova import objects from nova.objects import service as service_obj from nova.policies import servers as server_policies @@ -76,6 +77,7 @@ class ServersController(wsgi.Controller): super(ServersController, self).__init__(**kwargs) self.compute_api = compute.API() + self.network_api = network_api.API() @wsgi.expected_errors((400, 403)) @validation.query_schema(schema_servers.query_params_v266, '2.66') @@ -806,6 +808,13 @@ class ServersController(wsgi.Controller): target={'user_id': instance.user_id, 'project_id': instance.project_id}) + if (common.instance_has_port_with_resource_request( + context, instance_id, self.network_api) and not + common.supports_port_resource_request_during_move(req)): + msg = _("The resize server operation with port having QoS policy " + "is not supported.") + raise exc.HTTPBadRequest(explanation=msg) + try: self.compute_api.resize(context, instance, flavor_id, **kwargs) except exception.InstanceUnknownCell as e: diff --git a/nova/tests/functional/api_sample_tests/test_servers.py b/nova/tests/functional/api_sample_tests/test_servers.py index ca21ca2c9a1e..8ee5635937f6 100644 --- a/nova/tests/functional/api_sample_tests/test_servers.py +++ b/nova/tests/functional/api_sample_tests/test_servers.py @@ -412,6 +412,7 @@ class _ServersActionsJsonTestMixin(object): class ServersActionsJsonTest(ServersSampleBase, _ServersActionsJsonTestMixin): + USE_NEUTRON = True def test_server_reboot_hard(self): uuid = self._post_server() @@ -463,6 +464,12 @@ class ServersActionsJsonTest(ServersSampleBase, _ServersActionsJsonTestMixin): 'server-action-confirm-resize', code=204) + +class ServersActionsJsonTestNovaNet( + ServersSampleBase, _ServersActionsJsonTestMixin): + # TODO(gibi): fix the tests to work with neutron as nova net is deprecated + USE_NEUTRON = False + def _wait_for_active_server(self, uuid): """Wait 10 seconds for the server to be ACTIVE, else fail. diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index b80402de2150..a22ef8c4c365 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -5458,3 +5458,31 @@ class PortResourceRequestBasedSchedulingTest( self.assertIn( 'Creating server with port having QoS policy is not supported.', six.text_type(ex)) + + def test_resize_server_with_port_resource_request_old_microversion(self): + server = self._create_server( + flavor=self.flavor, + networks=[{'port': self.neutron.port_1['id']}]) + self._wait_for_state_change(self.admin_api, server, 'ACTIVE') + + # We need to simulate that the above server has a port that has + # resource request, we cannot boot with such a port but legacy servers + # can exists with such a port. + bound_port = self.neutron._ports[self.neutron.port_1['id']] + fake_resource_request = self.neutron.port_with_resource_request[ + 'resource_request'] + bound_port['resource_request'] = fake_resource_request + + resize_req = { + 'resize': { + 'flavorRef': self.flavor['id'] + } + } + ex = self.assertRaises( + client.OpenStackApiException, + self.api.post_server_action, server['id'], resize_req) + + self.assertEqual(400, ex.response.status_code) + self.assertIn( + 'The resize server operation with port having QoS policy is not ' + 'supported.', six.text_type(ex)) 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 32615f0e20f1..977b631ea5b2 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_server_actions.py @@ -123,7 +123,10 @@ class ServerActionsControllerTestV21(test.TestCase): mock.patch.object(compute_api.API, method, side_effect=exception.InstanceIsLocked( instance_uuid=instance['uuid'])), - ) as (mock_get, mock_method): + mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', + return_value=False) + ) as (mock_get, mock_method, mock_port_check): controller_function = 'self.controller.' + action self.assertRaises(webob.exc.HTTPConflict, @@ -617,8 +620,11 @@ class ServerActionsControllerTestV21(test.TestCase): self.assertEqual(instance_meta['kernel_id'], uuids.kernel_image_id) self.assertEqual(instance_meta['ramdisk_id'], uuids.ramdisk_image_id) + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', return_value=False) @mock.patch.object(compute_api.API, 'rebuild') - def test_rebuild_instance_raise_auto_disk_config_exc(self, mock_rebuild): + def test_rebuild_instance_raise_auto_disk_config_exc( + self, mock_rebuild, mock_port_check): body = { "rebuild": { "imageRef": self._image_href, @@ -632,7 +638,10 @@ class ServerActionsControllerTestV21(test.TestCase): self.controller._action_rebuild, self.req, FAKE_UUID, body=body) - def test_resize_server(self): + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', + return_value=False) + def test_resize_server(self, mock_port_check): body = dict(resize=dict(flavorRef="http://localhost/3")) @@ -684,7 +693,9 @@ class ServerActionsControllerTestV21(test.TestCase): self.controller._action_resize, self.req, FAKE_UUID, body=body) - def test_resize_with_image_exceptions(self): + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', return_value=False) + def test_resize_with_image_exceptions(self, mock_port_check): body = dict(resize=dict(flavorRef="http://localhost/3")) self.resize_called = 0 image_id = 'fake_image_id' @@ -725,24 +736,33 @@ class ServerActionsControllerTestV21(test.TestCase): ' disk resize disabled.') self.assertEqual(self.resize_called, call_no + 1) + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', return_value=False) @mock.patch('nova.compute.api.API.resize', side_effect=exception.CannotResizeDisk(reason='')) - def test_resize_raises_cannot_resize_disk(self, mock_resize): + def test_resize_raises_cannot_resize_disk( + self, mock_resize, mock_port_check): body = dict(resize=dict(flavorRef="http://localhost/3")) self.assertRaises(webob.exc.HTTPBadRequest, self.controller._action_resize, self.req, FAKE_UUID, body=body) + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', return_value=False) @mock.patch('nova.compute.api.API.resize', side_effect=exception.FlavorNotFound(reason='', flavor_id='fake_id')) - def test_resize_raises_flavor_not_found(self, mock_resize): + def test_resize_raises_flavor_not_found( + self, mock_resize, mock_port_check): body = dict(resize=dict(flavorRef="http://localhost/3")) self.assertRaises(webob.exc.HTTPBadRequest, self.controller._action_resize, self.req, FAKE_UUID, body=body) - def test_resize_with_too_many_instances(self): + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', + return_value=False) + def test_resize_with_too_many_instances(self, mock_port_check): body = dict(resize=dict(flavorRef="http://localhost/3")) def fake_resize(*args, **kwargs): @@ -754,7 +774,9 @@ class ServerActionsControllerTestV21(test.TestCase): self.controller._action_resize, self.req, FAKE_UUID, body=body) - def test_resize_raises_conflict_on_invalid_state(self): + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', return_value=False) + def test_resize_raises_conflict_on_invalid_state(self, mock_port_check): body = dict(resize=dict(flavorRef="http://localhost/3")) def fake_resize(*args, **kwargs): @@ -768,17 +790,24 @@ class ServerActionsControllerTestV21(test.TestCase): self.controller._action_resize, self.req, FAKE_UUID, body=body) + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', + return_value=False) @mock.patch('nova.compute.api.API.resize', side_effect=exception.NoValidHost(reason='')) - def test_resize_raises_no_valid_host(self, mock_resize): + def test_resize_raises_no_valid_host(self, mock_resize, mock_port_check): body = dict(resize=dict(flavorRef="http://localhost/3")) self.assertRaises(webob.exc.HTTPBadRequest, self.controller._action_resize, self.req, FAKE_UUID, body=body) + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', + return_value=False) @mock.patch.object(compute_api.API, 'resize') - def test_resize_instance_raise_auto_disk_config_exc(self, mock_resize): + def test_resize_instance_raise_auto_disk_config_exc( + self, mock_resize, mock_port_check): mock_resize.side_effect = exception.AutoDiskConfigDisabledByImage( image='dummy') @@ -788,10 +817,12 @@ class ServerActionsControllerTestV21(test.TestCase): self.controller._action_resize, self.req, FAKE_UUID, body=body) + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', return_value=False) @mock.patch('nova.compute.api.API.resize', side_effect=exception.PciRequestAliasNotDefined( alias='fake_name')) - def test_resize_pci_alias_not_defined(self, mock_resize): + def test_resize_pci_alias_not_defined(self, mock_resize, mock_check_port): # Tests that PciRequestAliasNotDefined is translated to a 400 error. body = dict(resize=dict(flavorRef="http://localhost/3")) self.assertRaises(webob.exc.HTTPBadRequest, diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index 2347e2072bc2..f33e19d073de 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -7694,13 +7694,16 @@ class ServersPolicyEnforcementV21(test.NoDBTestCase): rule, rule_name, self.controller.update, self.req, FAKE_UUID, body=body) + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', return_value=False) @mock.patch('nova.api.openstack.compute.views.servers.ViewBuilder.show') @mock.patch.object(compute_api.API, 'update_instance') @mock.patch.object(common, 'get_instance') def test_update_overridden_policy_pass_with_same_user(self, get_instance_mock, update_instance_mock, - view_show_mock): + view_show_mock, + mock_port_check): instance = fake_instance.fake_instance_obj( self.req.environ['nova.context'], user_id=self.req.environ['nova.context'].user_id) @@ -7747,11 +7750,14 @@ class ServersPolicyEnforcementV21(test.NoDBTestCase): rule, rule_name, self.controller._action_resize, self.req, FAKE_UUID, body=body) + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', return_value=False) @mock.patch('nova.compute.api.API.resize') @mock.patch('nova.api.openstack.common.get_instance') def test_resize_overridden_policy_pass_with_same_project(self, get_instance_mock, - resize_mock): + resize_mock, + mock_post_check): instance = fake_instance.fake_instance_obj( self.req.environ['nova.context'], project_id=self.req.environ['nova.context'].project_id) @@ -7777,11 +7783,14 @@ class ServersPolicyEnforcementV21(test.NoDBTestCase): rule, rule_name, self.controller._action_resize, self.req, FAKE_UUID, body=body) + @mock.patch('nova.api.openstack.common.' + 'instance_has_port_with_resource_request', return_value=False) @mock.patch('nova.compute.api.API.resize') @mock.patch('nova.api.openstack.common.get_instance') def test_resize_overridden_policy_pass_with_same_user(self, get_instance_mock, - resize_mock): + resize_mock, + mock_port_check): instance = fake_instance.fake_instance_obj( self.req.environ['nova.context'], user_id=self.req.environ['nova.context'].user_id)