From 735c2181dc450195454cf4dc62a814ff1679abda Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Wed, 9 Jan 2019 17:47:20 +0100 Subject: [PATCH] Reject server create 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 create 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. Change-Id: I97f06d0ec34cbd75c182caaa686b8de5c777a576 blueprint: bandwidth-resource-provider --- nova/api/openstack/common.py | 15 +++++++ nova/api/openstack/compute/servers.py | 30 +++++++------ nova/compute/api.py | 47 +++++++++++--------- nova/exception.py | 5 +++ nova/tests/functional/test_servers.py | 16 +++++++ nova/tests/unit/compute/test_compute_api.py | 48 ++++++++++++++++++++- 6 files changed, 127 insertions(+), 34 deletions(-) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index d98387f09397..4d33fb9e2ade 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -535,3 +535,18 @@ def supports_multiattach_volume(req): volume multiattach support, False otherwise. """ return api_version_request.is_supported(req, '2.60') + + +def supports_port_resource_request(req): + """Check to see if the requested API version is high enough for resource + request + + NOTE: At the moment there is no such microversion that supports port + resource request. 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 support, False otherwise. + """ + return False diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 46ca50d32acd..dac494d861c6 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -558,19 +558,22 @@ class ServersController(wsgi.Controller): flavor_id, ctxt=context, read_deleted="no") supports_multiattach = common.supports_multiattach_volume(req) + supports_port_resource_request = \ + common.supports_port_resource_request(req) (instances, resv_id) = self.compute_api.create(context, - inst_type, - image_uuid, - display_name=name, - display_description=description, - availability_zone=availability_zone, - forced_host=host, forced_node=node, - metadata=server_dict.get('metadata', {}), - admin_password=password, - requested_networks=requested_networks, - check_server_group_quota=True, - supports_multiattach=supports_multiattach, - **create_kwargs) + inst_type, + image_uuid, + display_name=name, + display_description=description, + availability_zone=availability_zone, + forced_host=host, forced_node=node, + metadata=server_dict.get('metadata', {}), + admin_password=password, + requested_networks=requested_networks, + check_server_group_quota=True, + supports_multiattach=supports_multiattach, + supports_port_resource_request=supports_port_resource_request, + **create_kwargs) except (exception.QuotaError, exception.PortLimitExceeded) as error: raise exc.HTTPForbidden( @@ -645,7 +648,8 @@ class ServersController(wsgi.Controller): exception.SnapshotNotFound, exception.UnableToAutoAllocateNetwork, exception.MultiattachNotSupportedOldMicroversion, - exception.CertificateValidationFailed) as error: + exception.CertificateValidationFailed, + exception.ServerCreateWithQoSPortNotSupported) as error: raise exc.HTTPBadRequest(explanation=error.format_message()) except (exception.PortInUse, exception.InstanceExists, diff --git a/nova/compute/api.py b/nova/compute/api.py index e979daf14171..ac5ac60a13f9 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -748,7 +748,8 @@ class API(base.Base): metadata, access_ip_v4, access_ip_v6, requested_networks, config_drive, auto_disk_config, reservation_id, - max_count): + max_count, + supports_port_resource_request): """Verify all the input parameters regardless of the provisioning strategy being performed. """ @@ -813,6 +814,9 @@ class API(base.Base): context, requested_networks, pci_request_info) network_metadata, port_resource_requests = result + if port_resource_requests and not supports_port_resource_request: + raise exception.ServerCreateWithQoSPortNotSupported() + base_options = { 'reservation_id': reservation_id, 'image_ref': image_href, @@ -1114,7 +1118,8 @@ class API(base.Base): block_device_mapping, auto_disk_config, filter_properties, reservation_id=None, legacy_bdm=True, shutdown_terminate=False, check_server_group_quota=False, tags=None, - supports_multiattach=False, trusted_certs=None): + supports_multiattach=False, trusted_certs=None, + supports_port_resource_request=False): """Verify all the input parameters regardless of the provisioning strategy being performed and schedule the instance(s) for creation. @@ -1154,7 +1159,7 @@ class API(base.Base): key_name, key_data, security_groups, availability_zone, user_data, metadata, access_ip_v4, access_ip_v6, requested_networks, config_drive, auto_disk_config, - reservation_id, max_count) + reservation_id, max_count, supports_port_resource_request) # max_net_count is the maximum number of instances requested by the # user adjusted for any network quota constraints, including @@ -1689,7 +1694,8 @@ class API(base.Base): config_drive=None, auto_disk_config=None, scheduler_hints=None, legacy_bdm=True, shutdown_terminate=False, check_server_group_quota=False, tags=None, - supports_multiattach=False, trusted_certs=None): + supports_multiattach=False, trusted_certs=None, + supports_port_resource_request=False): """Provision instances, sending instance information to the scheduler. The scheduler will determine where the instance(s) go and will handle creating the DB entries. @@ -1715,22 +1721,23 @@ class API(base.Base): scheduler_hints, forced_host, forced_node, instance_type) return self._create_instance( - context, instance_type, - image_href, kernel_id, ramdisk_id, - min_count, max_count, - display_name, display_description, - key_name, key_data, security_groups, - availability_zone, user_data, metadata, - injected_files, admin_password, - access_ip_v4, access_ip_v6, - requested_networks, config_drive, - block_device_mapping, auto_disk_config, - filter_properties=filter_properties, - legacy_bdm=legacy_bdm, - shutdown_terminate=shutdown_terminate, - check_server_group_quota=check_server_group_quota, - tags=tags, supports_multiattach=supports_multiattach, - trusted_certs=trusted_certs) + context, instance_type, + image_href, kernel_id, ramdisk_id, + min_count, max_count, + display_name, display_description, + key_name, key_data, security_groups, + availability_zone, user_data, metadata, + injected_files, admin_password, + access_ip_v4, access_ip_v6, + requested_networks, config_drive, + block_device_mapping, auto_disk_config, + filter_properties=filter_properties, + legacy_bdm=legacy_bdm, + shutdown_terminate=shutdown_terminate, + check_server_group_quota=check_server_group_quota, + tags=tags, supports_multiattach=supports_multiattach, + trusted_certs=trusted_certs, + supports_port_resource_request=supports_port_resource_request) def _check_auto_disk_config(self, instance=None, image=None, **extra_instance_updates): diff --git a/nova/exception.py b/nova/exception.py index 27761383400c..13a6ae8eebfe 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -2153,6 +2153,11 @@ class NetworksWithQoSPolicyNotSupported(Invalid): "instance %(instance_uuid)s. (Network ID is %(network_id)s)") +class ServerCreateWithQoSPortNotSupported(Invalid): + msg_fmt = _("Creating server with port having QoS policy is not " + "supported.") + + class InvalidReservedMemoryPagesOption(Invalid): msg_fmt = _("The format of the option 'reserved_huge_pages' is invalid. " "(found '%(conf)s') Please refer to the nova " diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index 92764a722f3f..b80402de2150 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -5442,3 +5442,19 @@ class PortResourceRequestBasedSchedulingTest( self.assertEqual(500, server['fault']['code']) self.assertIn('Failed to allocate the network', server['fault']['message']) + + def test_create_server_with_port_resource_request_old_microversion(self): + server_req = self._build_minimal_create_server_request( + self.api, 'bandwidth-aware-server', + image_uuid='76fa36fc-c930-4bf3-8c8a-ea2a2420deb6', + flavor_id=self.flavor['id'], + networks=[{'port': self.neutron.port_with_resource_request['id']}]) + + ex = self.assertRaises( + client.OpenStackApiException, + self.api.post_server, {'server': server_req}) + + self.assertEqual(400, ex.response.status_code) + self.assertIn( + 'Creating server with port having QoS policy is not supported.', + six.text_type(ex)) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index a817ab6910c9..bbb5a0b60583 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -6402,6 +6402,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): requested_networks = objects.NetworkRequestList(objects=[ objects.NetworkRequest(network_id='none')]) max_count = 1 + supports_port_resource_request = False with mock.patch.object( self.compute_api.security_group_api, 'get', return_value={'id': uuids.secgroup_uuid}) as scget: @@ -6413,7 +6414,8 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): 'fake-display-name', 'fake-description', key_name, key_data, requested_secgroups, 'fake-az', user_data, metadata, access_ip_v4, access_ip_v6, requested_networks, - config_drive, auto_disk_config, reservation_id, max_count + config_drive, auto_disk_config, reservation_id, max_count, + supports_port_resource_request ) ) # Assert the neutron security group API get method was called once @@ -6423,6 +6425,50 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): self.assertItemsEqual(['default', uuids.secgroup_uuid], security_groups) + @mock.patch('nova.network.neutronv2.api.API.validate_networks') + @mock.patch('nova.network.neutronv2.api.API.create_resource_requests') + def test_validate_and_build_base_options_checks_resource_request( + self, mock_neutron_create_resource_requests, + mock_validate_network): + """Checks that validate_and_build_base_options raises if the request + contains port with resource request but API request does not use the + microversion enabling such support. + """ + instance_type = objects.Flavor(**test_flavor.fake_flavor) + boot_meta = metadata = {} + kernel_id = ramdisk_id = key_name = key_data = user_data = \ + access_ip_v4 = access_ip_v6 = config_drive = \ + auto_disk_config = reservation_id = None + requested_secgroups = ['default'] + requested_networks = objects.NetworkRequestList(objects=[ + objects.NetworkRequest(port_id=uuids.port_id)]) + mock_neutron_create_resource_requests.return_value = ( + None, [objects.RequestGroup()]) + max_count = 1 + + # This expected not to raise + supports_port_resource_request = True + self.compute_api._validate_and_build_base_options( + self.context, instance_type, boot_meta, uuids.image_href, + mock.sentinel.image_id, kernel_id, ramdisk_id, + 'fake-display-name', 'fake-description', key_name, + key_data, requested_secgroups, 'fake-az', user_data, + metadata, access_ip_v4, access_ip_v6, requested_networks, + config_drive, auto_disk_config, reservation_id, max_count, + supports_port_resource_request) + + supports_port_resource_request = False + self.assertRaises( + exception.ServerCreateWithQoSPortNotSupported, + self.compute_api._validate_and_build_base_options, + self.context, instance_type, boot_meta, uuids.image_href, + mock.sentinel.image_id, kernel_id, ramdisk_id, + 'fake-display-name', 'fake-description', key_name, + key_data, requested_secgroups, 'fake-az', user_data, + metadata, access_ip_v4, access_ip_v6, requested_networks, + config_drive, auto_disk_config, reservation_id, max_count, + supports_port_resource_request) + @mock.patch('nova.compute.api.API._record_action_start') @mock.patch.object(compute_rpcapi.ComputeAPI, 'attach_interface') def test_tagged_interface_attach(self, mock_attach, mock_record):