diff --git a/nova/compute/api.py b/nova/compute/api.py index 70c7bbceb7d4..0fec77fb55b9 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -846,7 +846,8 @@ class API(base.Base): 'progress': 0, 'pci_requests': pci_request_info, 'numa_topology': numa_topology, - 'system_metadata': system_metadata} + 'system_metadata': system_metadata, + 'port_resource_requests': port_resource_requests} options_from_image = self._inherit_properties_from_image( boot_meta, auto_disk_config) @@ -870,6 +871,7 @@ class API(base.Base): security_groups = self.security_group_api.populate_security_groups( security_groups) self.security_group_api.ensure_default(context) + port_resource_requests = base_options.pop('port_resource_requests') LOG.debug("Going to run %s instances...", num_instances) instances_to_build = [] try: @@ -883,7 +885,8 @@ class API(base.Base): base_options['numa_topology'], base_options['pci_requests'], filter_properties, instance_group, base_options['availability_zone'], - security_groups=security_groups) + security_groups=security_groups, + port_resource_requests=port_resource_requests) if block_device_mapping: # Record whether or not we are a BFV instance diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index ecc8634d6ce4..6a54bb1c7563 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -608,6 +608,11 @@ class ComputeTaskManager(base.Base): else: # This is not a reschedule, so we need to call the scheduler to # get appropriate hosts for the request. + # NOTE(gibi): We only call the scheduler if using cells v1 or + # we are rescheduling from a really old compute. In + # either case we do not support externally-defined resource + # requests, like port QoS. So no requested_resources are set + # on the RequestSpec here. host_lists = self._schedule_instances(context, spec_obj, instance_uuids, return_alternates=True) except Exception as exc: @@ -809,6 +814,11 @@ class ComputeTaskManager(base.Base): # when populate_filter_properties accepts it filter_properties = request_spec.\ to_legacy_filter_properties_dict() + + # TODO(gibi): We need to make sure that the + # requested_resources field is re calculated based on + # neutron ports. + # NOTE(cfriesen): Ensure that we restrict the scheduler to # the cell specified by the instance mapping. instance_mapping = \ @@ -984,6 +994,9 @@ class ComputeTaskManager(base.Base): # if we want to make sure that the next destination # is not forced to be the original host request_spec.reset_forced_destinations() + # TODO(gibi): We need to make sure that the + # requested_resources field is re calculated based on + # neutron ports. try: # if this is a rebuild of instance on the same host with # new image. diff --git a/nova/conductor/tasks/live_migrate.py b/nova/conductor/tasks/live_migrate.py index b543a91ffb3f..d06a4c427195 100644 --- a/nova/conductor/tasks/live_migrate.py +++ b/nova/conductor/tasks/live_migrate.py @@ -346,6 +346,9 @@ class LiveMigrationTask(base.TaskBase): # if we want to make sure that the next destination # is not forced to be the original host request_spec.reset_forced_destinations() + + # TODO(gibi): We need to make sure that the requested_resources field + # is re calculated based on neutron ports. scheduler_utils.setup_instance_group(self.context, request_spec) # We currently only support live migrating to hosts in the same diff --git a/nova/conductor/tasks/migrate.py b/nova/conductor/tasks/migrate.py index 7601b9e11c39..4998c96fa975 100644 --- a/nova/conductor/tasks/migrate.py +++ b/nova/conductor/tasks/migrate.py @@ -202,6 +202,9 @@ class MigrationTask(base.TaskBase): # is not forced to be the original host self.request_spec.reset_forced_destinations() + # TODO(gibi): We need to make sure that the requested_resources field + # is re calculated based on neutron ports. + self._restrict_request_spec_to_cell(legacy_props) # Once _preallocate_migration() is done, the source node allocation is diff --git a/nova/tests/functional/integrated_helpers.py b/nova/tests/functional/integrated_helpers.py index 083a85598e29..76de3b8b8733 100644 --- a/nova/tests/functional/integrated_helpers.py +++ b/nova/tests/functional/integrated_helpers.py @@ -488,16 +488,18 @@ class ProviderUsageBaseTestCase(test.TestCase, InstanceHelperMixin): url= ('/resource_providers/%s/inventories' % rp_uuid), version='1.15', body=inv_body).body - def _update_inventory(self, rp_uuid, inv_body): + def _update_inventory(self, rp_uuid, inv_body, version='1.20'): """This will update the inventory for a given resource provider. :param rp_uuid: UUID of the resource provider to update :param inv_body: inventory to set on the provider + :param version: the placement microversion used in the request :returns: APIResponse object with the results """ return self.placement_api.put( url= ('/resource_providers/%s/inventories' % rp_uuid), - body=inv_body).body + body=inv_body, + version=version).body def _get_resource_provider_by_uuid(self, rp_uuid): return self.placement_api.get( diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index 2324d3c9337b..67c0fe9bcf38 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -12,10 +12,12 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +from __future__ import absolute_import import collections import copy import datetime +import fixtures import time import zlib @@ -36,6 +38,7 @@ from nova import context from nova import exception from nova import objects from nova.objects import block_device as block_device_obj +from nova import rc_fields from nova.scheduler import weights from nova import test from nova.tests import fixtures as nova_fixtures @@ -5380,12 +5383,18 @@ class PortResourceRequestBasedSchedulingTestBase( compute_driver = 'fake.SmallFakeDriver' + CUSTOM_VNIC_TYPE_NORMAL = 'CUSTOM_VNIC_TYPE_NORMAL' + CUSTOM_PHYSNET_2 = 'CUSTOM_PHYSNET_2' + def setUp(self): super(PortResourceRequestBasedSchedulingTestBase, self).setUp() self.compute1 = self._start_compute('host1') self.compute1_rp_uuid = self._get_provider_uuid_by_host('host1') + self.ovs_bridge_rp_per_host = {} self.flavor = self.api.get_flavors()[0] + self._create_networking_rp_tree(self.compute1_rp_uuid) + def _create_server(self, flavor, networks): server_req = self._build_minimal_create_server_request( self.api, 'bandwidth-aware-server', @@ -5393,6 +5402,62 @@ class PortResourceRequestBasedSchedulingTestBase( flavor_id=flavor['id'], networks=networks) return self.api.post_server({'server': server_req}) + def _set_provider_inventories(self, rp_uuid, inventories): + rp = self.placement_api.get( + '/resource_providers/%s' % rp_uuid).body + inventories['resource_provider_generation'] = rp['generation'] + return self._update_inventory(rp_uuid, inventories) + + def _create_networking_rp_tree(self, compute_rp_uuid): + # let's simulate what the neutron ovs agent would do + + # we need uuid sentinel for the test to make pep8 happy but we need a + # unique one per compute so here is some ugliness + ovs_agent_rp_uuid = getattr(uuids, compute_rp_uuid + 'ovs agent') + agent_rp_req = { + "name": ovs_agent_rp_uuid, + "uuid": ovs_agent_rp_uuid, + "parent_provider_uuid": compute_rp_uuid + } + self.placement_api.post('/resource_providers', + body=agent_rp_req, + version='1.20') + ovs_bridge_rp_uuid = getattr(uuids, ovs_agent_rp_uuid + 'ovs br') + ovs_bridge_req = { + "name": ovs_bridge_rp_uuid, + "uuid": ovs_bridge_rp_uuid, + "parent_provider_uuid": ovs_agent_rp_uuid + } + self.placement_api.post('/resource_providers', + body=ovs_bridge_req, + version='1.20') + self.ovs_bridge_rp_per_host[compute_rp_uuid] = ovs_bridge_rp_uuid + + self._set_provider_inventories( + ovs_bridge_rp_uuid, + {"inventories": { + rc_fields.ResourceClass.NET_BW_IGR_KILOBIT_PER_SEC: + {"total": 10000}, + rc_fields.ResourceClass.NET_BW_EGR_KILOBIT_PER_SEC: + {"total": 10000}, + }}) + + self._create_trait(self.CUSTOM_VNIC_TYPE_NORMAL) + self._create_trait(self.CUSTOM_PHYSNET_2) + + self._set_provider_traits( + ovs_bridge_rp_uuid, + [self.CUSTOM_VNIC_TYPE_NORMAL, self.CUSTOM_PHYSNET_2]) + + def assertPortMatchesAllocation(self, port, allocations): + port_request = port['resource_request']['resources'] + for rc, amount in allocations.items(): + self.assertEqual(port_request[rc], amount, + 'port %s requested %d %s ' + 'resources but got allocation %d' % + (port['id'], port_request[rc], rc, + amount)) + class PortResourceRequestBasedSchedulingTest( PortResourceRequestBasedSchedulingTestBase): @@ -5648,3 +5713,127 @@ class PortResourceRequestBasedSchedulingTest( self.api.post_server_action(server['id'], {'unshelve': {}}) self._wait_for_state_change(self.admin_api, server, 'ACTIVE') + + +class PortResourceRequestBasedSchedulingTestIgnoreMicroversionCheck( + PortResourceRequestBasedSchedulingTestBase): + + def setUp(self): + super( + PortResourceRequestBasedSchedulingTestIgnoreMicroversionCheck, + self).setUp() + + # NOTE(gibi): This mock turns off the api microversion that prevents + # handling of instances operations if the request involves port's with + # resource request with old microversion. The new microversion does not + # exists yet as the whole feature is not read for end user consumption. + # This functional tests however would like to prove that some use cases + # already work. + self.useFixture( + fixtures.MockPatch( + 'nova.api.openstack.common.' + 'supports_port_resource_request', + return_value=True)) + + def test_boot_server_with_two_ports_one_having_resource_request(self): + non_qos_port = self.neutron.port_1 + qos_port = self.neutron.port_with_resource_request + + server = self._create_server( + flavor=self.flavor, + networks=[{'port': non_qos_port['id']}, + {'port': qos_port['id']}]) + server = self._wait_for_state_change(self.admin_api, server, 'ACTIVE') + + allocations = self.placement_api.get( + '/allocations/%s' % server['id']).body['allocations'] + + # We expect one set of allocations for the compute resources on the + # compute rp and one set for the networking resources on the ovs bridge + # rp due to the qos_port resource request + self.assertEqual(2, len(allocations)) + compute_allocations = allocations[self.compute1_rp_uuid]['resources'] + network_allocations = allocations[ + self.ovs_bridge_rp_per_host[self.compute1_rp_uuid]]['resources'] + + self.assertEqual(self._resources_from_flavor(self.flavor), + compute_allocations) + self.assertPortMatchesAllocation(qos_port, network_allocations) + + self.api.delete_server(server['id']) + self._wait_until_deleted(server) + fake_notifier.wait_for_versioned_notifications('instance.delete.end') + allocations = self._get_allocations_by_server_uuid(server['id']) + self.assertEqual(0, len(allocations)) + + +class PortResourceRequestReSchedulingTestIgnoreMicroversionCheck( + PortResourceRequestBasedSchedulingTestBase): + + compute_driver = 'fake.FakeRescheduleDriver' + + def setUp(self): + super( + PortResourceRequestReSchedulingTestIgnoreMicroversionCheck, + self).setUp() + self.compute2 = self._start_compute('host2') + self.compute2_rp_uuid = self._get_provider_uuid_by_host('host2') + self._create_networking_rp_tree(self.compute2_rp_uuid) + + # NOTE(gibi): This mock turns off the api microversion that prevents + # handling of instances operations if the request involves port's with + # resource request with old microversion. The new microversion does not + # exists yet as the whole feature is not read for end user consumption. + # This functional tests however would like to prove that some use cases + # already work. + self.useFixture( + fixtures.MockPatch( + 'nova.api.openstack.common.' + 'supports_port_resource_request', + return_value=True)) + + def test_boot_reschedule_success(self): + port = self.neutron.port_with_resource_request + + server = self._create_server( + flavor=self.flavor, + networks=[{'port': port['id']}]) + server = self._wait_for_state_change(self.admin_api, server, 'ACTIVE') + + dest_hostname = server['OS-EXT-SRV-ATTR:host'] + dest_compute_rp_uuid = self._get_provider_uuid_by_host(dest_hostname) + + failed_compute_rp = (self.compute1_rp_uuid + if dest_compute_rp_uuid == self.compute2_rp_uuid + else self.compute2_rp_uuid) + + allocations = self.placement_api.get( + '/allocations/%s' % server['id']).body['allocations'] + + # We expect one set of allocations for the compute resources on the + # compute rp and one set for the networking resources on the ovs bridge + # rp + self.assertEqual(2, len(allocations)) + compute_allocations = allocations[dest_compute_rp_uuid]['resources'] + network_allocations = allocations[ + self.ovs_bridge_rp_per_host[dest_compute_rp_uuid]]['resources'] + + self.assertEqual(self._resources_from_flavor(self.flavor), + compute_allocations) + self.assertPortMatchesAllocation(port, network_allocations) + + # assert that the allocations against the host where the spawn + # failed are cleaned up properly + self.assertEqual( + {'VCPU': 0, 'MEMORY_MB': 0, 'DISK_GB': 0}, + self._get_provider_usages(failed_compute_rp)) + self.assertEqual( + {'NET_BW_EGR_KILOBIT_PER_SEC': 0, 'NET_BW_IGR_KILOBIT_PER_SEC': 0}, + self._get_provider_usages( + self.ovs_bridge_rp_per_host[failed_compute_rp])) + + self.api.delete_server(server['id']) + self._wait_until_deleted(server) + fake_notifier.wait_for_versioned_notifications('instance.delete.end') + allocations = self._get_allocations_by_server_uuid(server['id']) + self.assertEqual(0, len(allocations)) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index d0d4396796ea..055c6321327f 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -4595,7 +4595,8 @@ class _ComputeAPIUnitTestMixIn(object): 'root_device_name': None, 'user_data': None, 'numa_topology': None, - 'pci_requests': None} + 'pci_requests': None, + 'port_resource_requests': None} security_groups = {} block_device_mapping = objects.BlockDeviceMappingList( objects=[objects.BlockDeviceMapping( @@ -4737,7 +4738,8 @@ class _ComputeAPIUnitTestMixIn(object): 'root_device_name': None, 'user_data': None, 'numa_topology': None, - 'pci_requests': None} + 'pci_requests': None, + 'port_resource_requests': None} security_groups = {} block_device_mappings = objects.BlockDeviceMappingList( objects=[objects.BlockDeviceMapping( @@ -4827,7 +4829,8 @@ class _ComputeAPIUnitTestMixIn(object): 'root_device_name': None, 'user_data': None, 'numa_topology': None, - 'pci_requests': None} + 'pci_requests': None, + 'port_resource_requests': None} security_groups = {} block_device_mapping = objects.BlockDeviceMappingList( objects=[objects.BlockDeviceMapping( @@ -4924,7 +4927,8 @@ class _ComputeAPIUnitTestMixIn(object): 'root_device_name': None, 'user_data': None, 'numa_topology': None, - 'pci_requests': None} + 'pci_requests': None, + 'port_resource_requests': None} security_groups = {} block_device_mapping = objects.BlockDeviceMappingList( objects=[objects.BlockDeviceMapping( @@ -5027,7 +5031,8 @@ class _ComputeAPIUnitTestMixIn(object): 'root_device_name': None, 'user_data': None, 'numa_topology': None, - 'pci_requests': None} + 'pci_requests': None, + 'port_resource_requests': None} security_groups = {} block_device_mapping = objects.BlockDeviceMappingList( objects=[objects.BlockDeviceMapping( @@ -5091,7 +5096,7 @@ class _ComputeAPIUnitTestMixIn(object): mock_objects.RequestSpec.from_components.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, - security_groups=secgroups) + security_groups=secgroups, port_resource_requests=mock.ANY) test() def _test_rescue(self, vm_state=vm_states.ACTIVE, rescue_password=None,