From 191bdf2069086493be2a2e0351afa7f3efad7099 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Fri, 23 Jul 2021 18:23:28 +0200 Subject: [PATCH] Support move ops with extended resource request Nova re-generates the resource request of an instance for each server move operation (migrate, resize, evacuate, live-migrate, unshelve) to find (or validate) a target host for the instance move. This patch extends the this logic to support the extended resource request from neutron. As the changes in the neutron interface code is called from nova-compute service during the port binding the compute service version is bumped. And a check is added to the compute-api to reject the move operations with ports having extended resource request if there are old computes in the cluster. blueprint: qos-minimum-guaranteed-packet-rate Change-Id: Ibcf703e254e720b9a6de17527325758676628d48 --- .../source/port_with_resource_request.rst | 5 +- .../admin/ports-with-resource-requests.rst | 5 +- nova/api/openstack/compute/evacuate.py | 10 +- nova/api/openstack/compute/migrate_server.py | 30 +- nova/api/openstack/compute/servers.py | 24 +- nova/api/openstack/compute/shelve.py | 20 +- nova/compute/api.py | 37 ++- nova/conductor/manager.py | 23 +- nova/conductor/tasks/live_migrate.py | 20 +- nova/conductor/tasks/migrate.py | 7 +- nova/network/neutron.py | 81 +++-- nova/objects/service.py | 6 +- .../test_servers_resource_request.py | 111 ++++--- nova/tests/unit/compute/test_api.py | 63 +++- nova/tests/unit/compute/test_compute.py | 15 + nova/tests/unit/compute/test_shelve.py | 19 ++ .../unit/conductor/tasks/test_live_migrate.py | 71 +++- .../unit/conductor/tasks/test_migrate.py | 8 +- nova/tests/unit/conductor/test_conductor.py | 20 +- nova/tests/unit/network/test_neutron.py | 314 +++++++++++++++++- 20 files changed, 694 insertions(+), 195 deletions(-) diff --git a/api-guide/source/port_with_resource_request.rst b/api-guide/source/port_with_resource_request.rst index 0109ec799b..1ab9e9f366 100644 --- a/api-guide/source/port_with_resource_request.rst +++ b/api-guide/source/port_with_resource_request.rst @@ -45,9 +45,8 @@ Extended resource request Since neutron 19.0.0 (Xena), neutron implements an extended resource request format via the the ``port-resource-request-groups`` neutron API extension. As of nova 24.0.0 (Xena), nova does not fully support the new extension. If the -extension is enabled in neutron, then nova will reject server move -operations, as well as interface attach operation. Admins should not enable -this API extension in neutron. +extension is enabled in neutron, then nova will reject interface attach +operation. Admins should not enable this API extension in neutron. Please note that Nova only supports the server create operation if every nova-compute service also upgraded to Xena version and the diff --git a/doc/source/admin/ports-with-resource-requests.rst b/doc/source/admin/ports-with-resource-requests.rst index 3765b09afe..23fe8049bb 100644 --- a/doc/source/admin/ports-with-resource-requests.rst +++ b/doc/source/admin/ports-with-resource-requests.rst @@ -72,9 +72,8 @@ Extended resource request Since neutron 19.0.0 (Xena), neutron implements an extended resource request format via the the ``port-resource-request-groups`` neutron API extension. As of nova 24.0.0 (Xena), Nova does not fully support the new extension. If the -extension is enabled in neutron, then nova will reject server move operations, -as well as interface attach operation. Admins should not enable this API -extension in neutron. +extension is enabled in neutron, then nova will reject interface attach +operation. Admins should not enable this API extension in neutron. Please note that Nova only supports the server create operation if every nova-compute service also upgraded to Xena version and the diff --git a/nova/api/openstack/compute/evacuate.py b/nova/api/openstack/compute/evacuate.py index 6d014ef2f3..b09e592a2b 100644 --- a/nova/api/openstack/compute/evacuate.py +++ b/nova/api/openstack/compute/evacuate.py @@ -26,7 +26,6 @@ from nova.compute import api as compute import nova.conf from nova import exception from nova.i18n import _ -from nova.network import neutron from nova.policies import evacuate as evac_policies from nova import utils @@ -40,7 +39,6 @@ class EvacuateController(wsgi.Controller): super(EvacuateController, self).__init__() self.compute_api = compute.API() self.host_api = compute.HostAPI() - self.network_api = neutron.API() def _get_on_shared_storage(self, req, evacuate_body): if api_version_request.is_supported(req, min_version='2.14'): @@ -120,13 +118,6 @@ 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) @@ -136,6 +127,7 @@ class EvacuateController(wsgi.Controller): except ( exception.ComputeServiceInUse, exception.ForbiddenPortsWithAccelerator, + exception.ExtendedResourceRequestOldCompute, ) as e: raise exc.HTTPBadRequest(explanation=e.format_message()) except exception.ForbiddenWithAccelerators as e: diff --git a/nova/api/openstack/compute/migrate_server.py b/nova/api/openstack/compute/migrate_server.py index be2fe51ce5..ebaeccff3d 100644 --- a/nova/api/openstack/compute/migrate_server.py +++ b/nova/api/openstack/compute/migrate_server.py @@ -26,7 +26,6 @@ from nova.api import validation from nova.compute import api as compute from nova import exception from nova.i18n import _ -from nova.network import neutron from nova.policies import migrate_server as ms_policies LOG = logging.getLogger(__name__) @@ -36,7 +35,6 @@ class MigrateServerController(wsgi.Controller): def __init__(self): super(MigrateServerController, self).__init__() self.compute_api = compute.API() - self.network_api = neutron.API() @wsgi.response(202) @wsgi.expected_errors((400, 403, 404, 409)) @@ -56,13 +54,6 @@ 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) @@ -81,9 +72,12 @@ class MigrateServerController(wsgi.Controller): 'migrate', id) except exception.InstanceNotFound as e: raise exc.HTTPNotFound(explanation=e.format_message()) - except (exception.ComputeHostNotFound, - exception.CannotMigrateToSameHost, - exception.ForbiddenPortsWithAccelerator) as e: + except ( + exception.ComputeHostNotFound, + exception.CannotMigrateToSameHost, + exception.ForbiddenPortsWithAccelerator, + exception.ExtendedResourceRequestOldCompute, + ) as e: raise exc.HTTPBadRequest(explanation=e.format_message()) @wsgi.response(202) @@ -127,13 +121,6 @@ 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, @@ -164,7 +151,10 @@ class MigrateServerController(wsgi.Controller): raise exc.HTTPConflict(explanation=e.format_message()) except exception.InstanceIsLocked as e: raise exc.HTTPConflict(explanation=e.format_message()) - except exception.ComputeHostNotFound as e: + except ( + exception.ComputeHostNotFound, + exception.ExtendedResourceRequestOldCompute, + )as e: raise exc.HTTPBadRequest(explanation=e.format_message()) except exception.InstanceInvalidState as state_error: common.raise_http_conflict_for_instance_invalid_state(state_error, diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index a297d11e57..4a52cff4e0 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -40,7 +40,6 @@ from nova import context as nova_context from nova import exception from nova.i18n import _ from nova.image import glance -from nova.network import neutron from nova import objects from nova.policies import servers as server_policies from nova import utils @@ -114,7 +113,6 @@ class ServersController(wsgi.Controller): def __init__(self): super(ServersController, self).__init__() self.compute_api = compute.API() - self.network_api = neutron.API() @wsgi.expected_errors((400, 403)) @validation.query_schema(schema_servers.query_params_v275, '2.75') @@ -1028,15 +1026,6 @@ 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) @@ -1063,11 +1052,14 @@ class ServersController(wsgi.Controller): msg = _("Image that the instance was started " "with could not be found.") raise exc.HTTPBadRequest(explanation=msg) - except (exception.AutoDiskConfigDisabledByImage, - exception.CannotResizeDisk, - exception.CannotResizeToSameFlavor, - exception.FlavorNotFound, - exception.ForbiddenPortsWithAccelerator) as e: + except ( + exception.AutoDiskConfigDisabledByImage, + exception.CannotResizeDisk, + exception.CannotResizeToSameFlavor, + exception.FlavorNotFound, + exception.ForbiddenPortsWithAccelerator, + exception.ExtendedResourceRequestOldCompute, + ) as e: raise exc.HTTPBadRequest(explanation=e.format_message()) except INVALID_FLAVOR_IMAGE_EXCEPTIONS as e: raise exc.HTTPBadRequest(explanation=e.format_message()) diff --git a/nova/api/openstack/compute/shelve.py b/nova/api/openstack/compute/shelve.py index 5bb589817b..b985d49a9a 100644 --- a/nova/api/openstack/compute/shelve.py +++ b/nova/api/openstack/compute/shelve.py @@ -23,10 +23,7 @@ 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 LOG = logging.getLogger(__name__) @@ -36,7 +33,6 @@ class ShelveController(wsgi.Controller): def __init__(self): super(ShelveController, self).__init__() self.compute_api = compute.API() - self.network_api = neutron.API() @wsgi.response(202) @wsgi.expected_errors((404, 403, 409, 400)) @@ -107,17 +103,6 @@ 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, @@ -128,5 +113,8 @@ class ShelveController(wsgi.Controller): common.raise_http_conflict_for_instance_invalid_state(state_error, 'unshelve', id) - except exception.InvalidRequest as e: + except ( + exception.InvalidRequest, + exception.ExtendedResourceRequestOldCompute, + ) as e: raise exc.HTTPBadRequest(explanation=e.format_message()) diff --git a/nova/compute/api.py b/nova/compute/api.py index db3e670d41..44e98edbb8 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -70,6 +70,7 @@ from nova.objects import fields as fields_obj from nova.objects import image_meta as image_meta_obj from nova.objects import keypair as keypair_obj from nova.objects import quotas as quotas_obj +from nova.objects import service as service_obj from nova.pci import request as pci_request from nova.policies import servers as servers_policies import nova.policy @@ -111,6 +112,7 @@ SUPPORT_ACCELERATOR_SERVICE_FOR_REBUILD = 53 SUPPORT_VNIC_TYPE_ACCELERATOR = 57 MIN_COMPUTE_BOOT_WITH_EXTENDED_RESOURCE_REQUEST = 58 +MIN_COMPUTE_MOVE_WITH_EXTENDED_RESOURCE_REQUEST = 59 # FIXME(danms): Keep a global cache of the cells we find the # first time we look. This needs to be refreshed on a timer or @@ -354,6 +356,20 @@ def block_port_accelerators(): return inner +def block_extended_resource_request(function): + @functools.wraps(function) + def inner(self, context, instance, *args, **kwargs): + if self.network_api.instance_has_extended_resource_request( + instance.uuid + ): + version = service_obj.get_minimum_version_all_cells( + context, ["nova-compute"]) + if version < MIN_COMPUTE_MOVE_WITH_EXTENDED_RESOURCE_REQUEST: + raise exception.ExtendedResourceRequestOldCompute() + return function(self, context, instance, *args, **kwargs) + return inner + + @profiler.trace_cls("compute_api") class API: """API for interacting with the compute manager.""" @@ -3801,16 +3817,18 @@ class API: # during the resize. if instance.get_network_info().has_port_with_allocation(): # TODO(gibi): do not directly overwrite the - # RequestSpec.requested_resources as others like cyborg might added + # RequestSpec.requested_resources and + # RequestSpec.request_level_paramsas others like cyborg might added # to things there already # NOTE(gibi): We need to collect the requested resource again as it # is intentionally not persisted in nova. Note that this needs to # be done here as the nova API code directly calls revert on the # dest compute service skipping the conductor. - port_res_req = ( + port_res_req, req_lvl_params = ( self.network_api.get_requested_resource_for_instance( context, instance.uuid)) reqspec.requested_resources = port_res_req + reqspec.request_level_params = req_lvl_params instance.task_state = task_states.RESIZE_REVERTING instance.save(expected_task_state=[None]) @@ -3941,8 +3959,11 @@ class API: min_compute_version, MIN_COMPUTE_CROSS_CELL_RESIZE) return False - if self.network_api.get_requested_resource_for_instance( - context, instance.uuid): + res_req, req_lvl_params = ( + self.network_api.get_requested_resource_for_instance( + context, instance.uuid) + ) + if res_req: LOG.info( 'Request is allowed by policy to perform cross-cell ' 'resize but the instance has ports with resource request ' @@ -3998,9 +4019,10 @@ class API: # TODO(stephenfin): This logic would be so much easier to grok if we # finally split resize and cold migration into separate code paths + @block_extended_resource_request + @block_port_accelerators() # FIXME(sean-k-mooney): Cold migrate and resize to different hosts # probably works but they have not been tested so block them for now - @block_port_accelerators() @reject_vdpa_instances(instance_actions.RESIZE) @block_accelerators() @check_instance_lock @@ -4335,6 +4357,7 @@ class API: "vol_zone": volume['availability_zone']} raise exception.MismatchVolumeAZException(reason=msg) + @block_extended_resource_request @check_instance_lock @check_instance_state(vm_state=[vm_states.SHELVED, vm_states.SHELVED_OFFLOADED]) @@ -5201,6 +5224,7 @@ class API: return _metadata + @block_extended_resource_request @block_port_accelerators() @reject_vdpa_instances(instance_actions.LIVE_MIGRATION) @block_accelerators() @@ -5334,8 +5358,9 @@ class API: self.compute_rpcapi.live_migration_abort(context, instance, migration.id) - # FIXME(sean-k-mooney): rebuild works but we have not tested evacuate yet + @block_extended_resource_request @block_port_accelerators() + # FIXME(sean-k-mooney): rebuild works but we have not tested evacuate yet @reject_vdpa_instances(instance_actions.EVACUATE) @reject_vtpm_instances(instance_actions.EVACUATE) @block_accelerators(until_service=SUPPORT_ACCELERATOR_SERVICE_FOR_REBUILD) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index d9b1d212ac..12737c9964 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -987,20 +987,21 @@ class ComputeTaskManager: # when populate_filter_properties accepts it filter_properties = request_spec.\ to_legacy_filter_properties_dict() - - external_resources = ( + res_req, req_lvl_params = ( self.network_api.get_requested_resource_for_instance( - context, instance.uuid)) + context, instance.uuid) + ) extra_specs = request_spec.flavor.extra_specs device_profile = extra_specs.get('accel:device_profile') - external_resources.extend( + res_req.extend( cyborg.get_device_profile_request_groups( context, device_profile) if device_profile else []) # NOTE(gibi): When other modules want to handle similar # non-nova resources then here we have to collect all # the external resource requests in a single list and # add them to the RequestSpec. - request_spec.requested_resources = external_resources + request_spec.requested_resources = res_req + request_spec.request_level_params = req_lvl_params # NOTE(cfriesen): Ensure that we restrict the scheduler to # the cell specified by the instance mapping. @@ -1184,14 +1185,13 @@ class ComputeTaskManager: # if we want to make sure that the next destination # is not forced to be the original host request_spec.reset_forced_destinations() - - external_resources = [] - external_resources += ( + res_req, req_lvl_params = ( self.network_api.get_requested_resource_for_instance( - context, instance.uuid)) + context, instance.uuid) + ) extra_specs = request_spec.flavor.extra_specs device_profile = extra_specs.get('accel:device_profile') - external_resources.extend( + res_req.extend( cyborg.get_device_profile_request_groups( context, device_profile) if device_profile else []) @@ -1199,7 +1199,8 @@ class ComputeTaskManager: # non-nova resources then here we have to collect all # the external resource requests in a single list and # add them to the RequestSpec. - request_spec.requested_resources = external_resources + request_spec.requested_resources = res_req + request_spec.request_level_params = req_lvl_params try: # if this is a rebuild of instance on the same host with diff --git a/nova/conductor/tasks/live_migrate.py b/nova/conductor/tasks/live_migrate.py index 9613c136a4..294abfe4e3 100644 --- a/nova/conductor/tasks/live_migrate.py +++ b/nova/conductor/tasks/live_migrate.py @@ -372,16 +372,13 @@ class LiveMigrationTask(base.TaskBase): self._update_migrate_vifs_from_bindings(self.migrate_data.vifs, bindings) - @staticmethod - def _get_port_profile_from_provider_mapping(port_id, provider_mappings): - if port_id in provider_mappings: - # NOTE(gibi): In the resource provider mapping there can be - # more than one RP fulfilling a request group. But resource - # requests of a Neutron port is always mapped to a - # numbered request group that is always fulfilled by one - # resource provider. So we only pass that single RP UUID - # here. - return {'allocation': provider_mappings[port_id][0]} + def _get_port_profile_from_provider_mapping( + self, port_id, provider_mappings + ): + allocation = self.network_api.get_binding_profile_allocation( + self.context, port_id, provider_mappings) + if allocation: + return {'allocation': allocation} else: return {} @@ -476,7 +473,7 @@ class LiveMigrationTask(base.TaskBase): # is not forced to be the original host request_spec.reset_forced_destinations() - port_res_req = ( + port_res_req, req_lvl_params = ( self.network_api.get_requested_resource_for_instance( self.context, self.instance.uuid)) # NOTE(gibi): When cyborg or other module wants to handle @@ -484,6 +481,7 @@ class LiveMigrationTask(base.TaskBase): # all the external resource requests in a single list and # add them to the RequestSpec. request_spec.requested_resources = port_res_req + request_spec.request_level_params = req_lvl_params scheduler_utils.setup_instance_group(self.context, request_spec) diff --git a/nova/conductor/tasks/migrate.py b/nova/conductor/tasks/migrate.py index 38e10ff4cc..6ff6206f65 100644 --- a/nova/conductor/tasks/migrate.py +++ b/nova/conductor/tasks/migrate.py @@ -249,12 +249,15 @@ class MigrationTask(base.TaskBase): scheduler_utils.populate_retry(legacy_props, self.instance.uuid) - port_res_req = self.network_api.get_requested_resource_for_instance( - self.context, self.instance.uuid) + port_res_req, req_lvl_params = ( + self.network_api.get_requested_resource_for_instance( + self.context, self.instance.uuid) + ) # NOTE(gibi): When cyborg or other module wants to handle similar # non-nova resources then here we have to collect all the external # resource requests in a single list and add them to the RequestSpec. self.request_spec.requested_resources = port_res_req + self.request_spec.request_level_params = req_lvl_params self._set_requested_destination_cell(legacy_props) diff --git a/nova/network/neutron.py b/nova/network/neutron.py index de512e8a51..60ca345353 100644 --- a/nova/network/neutron.py +++ b/nova/network/neutron.py @@ -1027,6 +1027,33 @@ class API: return True return False + def get_binding_profile_allocation( + self, + context: nova_context.RequestContext, + port_id: str, + resource_provider_mapping: ty.Dict[str, ty.List[str]], + ) -> ty.Union[None, str, ty.Dict[str, str]]: + """Calculate the value of the allocation key of the binding:profile + based on the allocated resources. + + :param context: the request context + :param port_id: the uuid of the neutron port + :param resource_provider_mapping: the mapping returned by the placement + defining which request group get allocated from which resource + providers + :returns: None if the port has no resource request. Returns a single + RP UUID if the port has a legacy resource request. Returns a dict + of request group id: resource provider UUID mapping if the port has + an extended resource request. + """ + neutron = get_client(context) + port = self._show_port(context, port_id, neutron_client=neutron) + if self._has_resource_request(context, port, neutron): + return self._get_binding_profile_allocation( + context, port, neutron, resource_provider_mapping) + else: + return None + def _get_binding_profile_allocation( self, context, port, neutron, resource_provider_mapping ): @@ -1698,7 +1725,7 @@ class API: neutron port only allocates from a single resource provider. """ neutron = get_client(context) - port_allocation = {} + port_allocation: ty.Dict = {} try: # NOTE(gibi): we need to read the port resource information from # neutron here as we might delete the port below @@ -2378,12 +2405,18 @@ class API: raise exception.NetworkNotFound(network_id=id_str) return ports_needed_per_instance - def get_requested_resource_for_instance(self, context, instance_uuid): + def get_requested_resource_for_instance( + self, + context: nova_context.RequestContext, + instance_uuid: str + ) -> ty.Tuple[ + ty.List['objects.RequestGroup'], 'objects.RequestLevelParams']: """Collect resource requests from the ports associated to the instance :param context: nova request context :param instance_uuid: The UUID of the instance - :return: A list of RequestGroup objects + :return: A two tuple with a list of RequestGroup objects and a + RequestLevelParams object. """ # NOTE(gibi): We need to use an admin client as otherwise a non admin @@ -2393,19 +2426,33 @@ class API: neutron = get_client(context, admin=True) # get the ports associated to this instance data = neutron.list_ports( - device_id=instance_uuid, fields=['id', 'resource_request']) + device_id=instance_uuid, fields=['id', constants.RESOURCE_REQUEST]) resource_requests = [] + request_level_params = objects.RequestLevelParams() + extended_rr = self.has_extended_resource_request_extension( + context, neutron) for port in data.get('ports', []): - if port.get('resource_request'): - # NOTE(gibi): explicitly orphan the RequestGroup by setting - # context=None as we never intended to save it to the DB. - resource_requests.append( - objects.RequestGroup.from_port_request( - context=None, port_uuid=port['id'], + resource_request = port.get(constants.RESOURCE_REQUEST) + if extended_rr and resource_request: + resource_requests.extend( + objects.RequestGroup.from_extended_port_request( + context=None, port_resource_request=port['resource_request'])) + request_level_params.extend_with( + objects.RequestLevelParams.from_port_request( + port_resource_request=resource_request)) + else: + # keep supporting the old format of the resource_request + if resource_request: + # NOTE(gibi): explicitly orphan the RequestGroup by setting + # context=None as we never intended to save it to the DB. + resource_requests.append( + objects.RequestGroup.from_port_request( + context=None, port_uuid=port['id'], + port_resource_request=port['resource_request'])) - return resource_requests + return resource_requests, request_level_params def validate_networks(self, context, requested_networks, num_instances): """Validate that the tenant can use the requested networks. @@ -3552,7 +3599,7 @@ class API: # allocation key in the port binding. However during resize, cold # migrate, evacuate and unshelve we have to set the binding here. # Also note that during unshelve no migration object is created. - if p.get('resource_request') and ( + if self._has_resource_request(context, p, neutron) and ( migration is None or not migration.is_live_migration ): if not provider_mappings: @@ -3579,13 +3626,9 @@ class API: "compute service but are required for ports with " "a resource request.")) - # NOTE(gibi): In the resource provider mapping there can be - # more than one RP fulfilling a request group. But resource - # requests of a Neutron port is always mapped to a - # numbered request group that is always fulfilled by one - # resource provider. So we only pass that single RP UUID here. - binding_profile[constants.ALLOCATION] = \ - provider_mappings[p['id']][0] + binding_profile[constants.ALLOCATION] = ( + self._get_binding_profile_allocation( + context, p, neutron, provider_mappings)) updates[constants.BINDING_PROFILE] = binding_profile port_updates.append((p['id'], updates)) diff --git a/nova/objects/service.py b/nova/objects/service.py index 33e6c7d43f..64a181dae3 100644 --- a/nova/objects/service.py +++ b/nova/objects/service.py @@ -31,7 +31,7 @@ LOG = logging.getLogger(__name__) # NOTE(danms): This is the global service version counter -SERVICE_VERSION = 58 +SERVICE_VERSION = 59 # NOTE(danms): This is our SERVICE_VERSION history. The idea is that any @@ -205,6 +205,10 @@ SERVICE_VERSION_HISTORY = ( # Version 58: Compute RPC v6.0: # Add support for booting with neutron extended resource request {'compute_rpc': '6.0'}, + # Version 59: Compute RPC v6.0: + # Add support for server move operations with neutron extended resource + # request + {'compute_rpc': '6.0'}, ) # This is used to raise an error at service startup if older than N-1 computes diff --git a/nova/tests/functional/test_servers_resource_request.py b/nova/tests/functional/test_servers_resource_request.py index 8be374b868..faf892ffc0 100644 --- a/nova/tests/functional/test_servers_resource_request.py +++ b/nova/tests/functional/test_servers_resource_request.py @@ -2567,8 +2567,6 @@ class ServerMoveWithPortResourceRequestTest( server, qos_normal_port, qos_sriov_port) -# TODO(gibi): This would show a lot of failing tests. Disable it for now -@unittest.skip class ServerMoveWithMultiGroupResourceRequestBasedSchedulingTest( ExtendedPortResourceRequestBasedSchedulingTestBase, ServerMoveWithPortResourceRequestTest, @@ -2583,15 +2581,6 @@ class ServerMoveWithMultiGroupResourceRequestBasedSchedulingTest( super().setUp() self.neutron = self.useFixture( MultiGroupResourceRequestNeutronFixture(self)) - # Turn off the blanket rejections of the extended resource request. - # This test class wants to prove that the extended resource request is - # supported. - patcher = mock.patch( - 'nova.network.neutron.API.instance_has_extended_resource_request', - return_value=False, - ) - self.addCleanup(patcher.stop) - patcher.start() class LiveMigrateAbortWithPortResourceRequestTest( @@ -2881,14 +2870,16 @@ class ExtendedResourceRequestOldCompute( hasn't been upgraded to a version that support extended resource request. So nova rejects the operations due to the old compute. """ + def setUp(self): + super().setUp() + self.neutron = self.useFixture( + ExtendedResourceRequestNeutronFixture(self)) @mock.patch.object( objects.service, 'get_minimum_version_all_cells', new=mock.Mock(return_value=57) ) def test_boot(self): - self.neutron = self.useFixture( - ExtendedResourceRequestNeutronFixture(self)) ex = self.assertRaises( client.OpenStackApiException, self._create_server, @@ -2903,6 +2894,63 @@ class ExtendedResourceRequestOldCompute( str(ex) ) + @mock.patch.object( + objects.service, 'get_minimum_version_all_cells', + new=mock.Mock(return_value=58) + ) + def _test_operation(self, op_callable): + # boot a server, service version 58 already supports that + server = self._create_server( + flavor=self.flavor, + networks=[{'port': self.neutron.port_with_resource_request['id']}], + ) + self._wait_for_state_change(server, 'ACTIVE') + + # still the move operations require service version 58 so they will + # fail + ex = self.assertRaises( + client.OpenStackApiException, + op_callable, + server, + ) + self.assertEqual(400, ex.response.status_code) + self.assertIn( + 'The port-resource-request-groups neutron API extension is not ' + 'supported by old nova compute service. Upgrade your compute ' + 'services to Xena (24.0.0) or later.', + str(ex) + ) + + def test_resize(self): + self._test_operation( + lambda server: self._resize_server( + server, self.flavor_with_group_policy['id'] + ) + ) + + def test_migrate(self): + self._test_operation( + lambda server: self._migrate_server(server), + ) + + def test_live_migrate(self): + self._test_operation( + lambda server: self._live_migrate(server), + ) + + def test_evacuate(self): + self._test_operation( + lambda server: self._evacuate_server(server), + ) + + 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( + lambda server: shelve_offload_then_unshelve(server), + ) + class ExtendedResourceRequestTempNegativeTest( PortResourceRequestBasedSchedulingTestBase): @@ -2943,32 +2991,6 @@ class ExtendedResourceRequestTempNegativeTest( 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', @@ -2977,14 +2999,3 @@ class ExtendedResourceRequestTempNegativeTest( 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), - ) diff --git a/nova/tests/unit/compute/test_api.py b/nova/tests/unit/compute/test_api.py index d0d7e5ba1d..a840c75629 100644 --- a/nova/tests/unit/compute/test_api.py +++ b/nova/tests/unit/compute/test_api.py @@ -86,6 +86,14 @@ class _ComputeAPIUnitTestMixIn(object): self.context = context.RequestContext(self.user_id, self.project_id) + self.useFixture( + fixtures.MonkeyPatch( + 'nova.network.neutron.API.' + 'instance_has_extended_resource_request', + mock.Mock(return_value=False), + ) + ) + def _get_vm_states(self, exclude_states=None): vm_state = set([vm_states.ACTIVE, vm_states.BUILDING, vm_states.PAUSED, vm_states.SUSPENDED, vm_states.RESCUED, vm_states.STOPPED, @@ -1751,7 +1759,7 @@ class _ComputeAPIUnitTestMixIn(object): @mock.patch('nova.virt.hardware.numa_get_constraints') @mock.patch('nova.network.neutron.API.get_requested_resource_for_instance', - return_value=[]) + return_value=([], objects.RequestLevelParams())) @mock.patch('nova.availability_zones.get_host_availability_zone', return_value='nova') @mock.patch('nova.objects.Quotas.check_deltas') @@ -1843,6 +1851,9 @@ class _ComputeAPIUnitTestMixIn(object): self.assertEqual( [], mock_get_reqspec.return_value.requested_resources) + self.assertEqual( + mock_get_requested_resources.return_value[1], + mock_get_reqspec.return_value.request_level_params) def test_revert_resize(self): self._test_revert_resize(same_flavor=False) @@ -2178,6 +2189,27 @@ class _ComputeAPIUnitTestMixIn(object): self.compute_api.resize, self.context, fake_inst, flavor_id=new_flavor.flavorid) + @mock.patch( + 'nova.compute.api.API.get_instance_host_status', + new=mock.Mock(return_value=fields_obj.HostStatus.UP) + ) + @mock.patch( + 'nova.objects.service.get_minimum_version_all_cells', + new=mock.Mock(return_value=58), + ) + @mock.patch( + 'nova.network.neutron.API.instance_has_extended_resource_request', + new=mock.Mock(return_value=True), + ) + def test_resize_with_extended_resource_request_old_compute(self): + fake_inst = self._create_instance_obj() + + self.assertRaises( + exception.ExtendedResourceRequestOldCompute, + self.compute_api.resize, + self.context, fake_inst, flavor_id=uuids.new_falvor + ) + def _test_migrate(self, *args, **kwargs): self._test_resize(*args, flavor_id_passed=False, **kwargs) @@ -2607,6 +2639,23 @@ class _ComputeAPIUnitTestMixIn(object): disk_over_commit=False) self.assertIsNone(instance.task_state) + @mock.patch( + 'nova.objects.service.get_minimum_version_all_cells', + new=mock.Mock(return_value=58), + ) + @mock.patch( + 'nova.network.neutron.API.instance_has_extended_resource_request', + new=mock.Mock(return_value=True), + ) + def test_live_migrate_with_extended_resource_request_old_compute(self): + instance = self._create_instance_obj() + self.assertRaises( + exception.ExtendedResourceRequestOldCompute, + self.compute_api.live_migrate, self.context, instance, + host_name='fake_host', block_migration='auto', + disk_over_commit=False + ) + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') @mock.patch.object(objects.Instance, 'save') @mock.patch.object(objects.InstanceAction, 'action_start') @@ -7696,8 +7745,10 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): mock_get_min_ver.assert_called_once_with( self.context, ['nova-compute']) - @mock.patch('nova.network.neutron.API.get_requested_resource_for_instance', - return_value=[objects.RequestGroup()]) + @mock.patch( + 'nova.network.neutron.API.get_requested_resource_for_instance', + return_value=([objects.RequestGroup()], objects.RequestLevelParams()) + ) @mock.patch('nova.objects.service.get_minimum_version_all_cells', return_value=compute_api.MIN_COMPUTE_CROSS_CELL_RESIZE) def test_allow_cross_cell_resize_false_port_with_resource_req( @@ -7716,8 +7767,10 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): self.context, ['nova-compute']) mock_get_res_req.assert_called_once_with(self.context, uuids.instance) - @mock.patch('nova.network.neutron.API.get_requested_resource_for_instance', - return_value=[]) + @mock.patch( + 'nova.network.neutron.API.get_requested_resource_for_instance', + return_value=([], objects.RequestLevelParams()) + ) @mock.patch('nova.objects.service.get_minimum_version_all_cells', return_value=compute_api.MIN_COMPUTE_CROSS_CELL_RESIZE) def test_allow_cross_cell_resize_true( diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 003114192b..ae71d3fd1f 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -12029,6 +12029,21 @@ class ComputeAPITestCase(BaseTestCase): host='fake_dest_host', on_shared_storage=True, admin_password=None) + @mock.patch( + 'nova.objects.service.get_minimum_version_all_cells', + new=mock.Mock(return_value=58), + ) + @mock.patch( + 'nova.network.neutron.API.instance_has_extended_resource_request', + new=mock.Mock(return_value=True), + ) + def test_evacuate_with_extended_resource_request_old_compute(self): + instance = self._create_fake_instance_obj(services=True) + self.assertRaises(exception.ExtendedResourceRequestOldCompute, + self.compute_api.evacuate, self.context.elevated(), instance, + host='fake_dest_host', on_shared_storage=True, + admin_password=None) + @mock.patch('nova.objects.MigrationList.get_by_filters') def test_get_migrations(self, mock_migration): migration = test_migration.fake_db_migration() diff --git a/nova/tests/unit/compute/test_shelve.py b/nova/tests/unit/compute/test_shelve.py index 12bf23bda2..3f3f470fa5 100644 --- a/nova/tests/unit/compute/test_shelve.py +++ b/nova/tests/unit/compute/test_shelve.py @@ -1028,6 +1028,25 @@ class ShelveComputeAPITestCase(test_compute.BaseTestCase): self.compute_api.unshelve, self.context, instance, new_az=new_az) + @mock.patch( + 'nova.objects.service.get_minimum_version_all_cells', + new=mock.Mock(return_value=58), + ) + @mock.patch( + 'nova.network.neutron.API.instance_has_extended_resource_request', + new=mock.Mock(return_value=True), + ) + def test_unshelve_offloaded_with_extended_resource_request_old_compute( + self + ): + instance = self._get_specify_state_instance( + vm_states.SHELVED_OFFLOADED) + + self.assertRaises( + exception.ExtendedResourceRequestOldCompute, + self.compute_api.unshelve, self.context, instance, new_az="az1" + ) + @mock.patch('nova.objects.BlockDeviceMappingList.get_by_instance_uuid', new_callable=mock.NonCallableMock) @mock.patch('nova.availability_zones.get_availability_zones') diff --git a/nova/tests/unit/conductor/tasks/test_live_migrate.py b/nova/tests/unit/conductor/tasks/test_live_migrate.py index e3bbcdc678..88f00d0d84 100644 --- a/nova/tests/unit/conductor/tasks/test_live_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_live_migrate.py @@ -78,7 +78,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): _p = mock.patch( 'nova.network.neutron.API.' 'get_requested_resource_for_instance', - return_value=[]) + return_value=([], objects.RequestLevelParams())) self.mock_get_res_req = _p.start() self.addCleanup(_p.stop) @@ -600,8 +600,11 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): self.assertRaises(exception.MigrationPreCheckError, self.task._call_livem_checks_on_host, {}, {}) + @mock.patch('nova.network.neutron.API.get_binding_profile_allocation') @mock.patch('nova.network.neutron.API.bind_ports_to_host') - def test_bind_ports_on_destination_merges_profiles(self, mock_bind_ports): + def test_bind_ports_on_destination_merges_profiles( + self, mock_bind_ports, mock_get_binding_profile_alloc + ): """Assert that if both the migration_data and the provider mapping contains binding profile related information then such information is merged in the resulting profile. @@ -615,6 +618,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): {'some-key': 'value'})) ]) provider_mappings = {uuids.port1: [uuids.dest_bw_rp]} + mock_get_binding_profile_alloc.return_value = uuids.dest_bw_rp self.task._bind_ports_on_destination('dest-host', provider_mappings) @@ -623,7 +627,59 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): vnic_types=None, port_profiles={uuids.port1: {'allocation': uuids.dest_bw_rp, 'some-key': 'value'}}) + mock_get_binding_profile_alloc.assert_called_once_with( + self.context, uuids.port1, provider_mappings) + @mock.patch('nova.network.neutron.API.get_binding_profile_allocation') + @mock.patch('nova.network.neutron.API.bind_ports_to_host') + def test_bind_ports_on_destination_merges_profiles_extended_res_req( + self, mock_bind_ports, mock_get_binding_profile_alloc + ): + """Assert that if both the migration_data and the provider mapping + contains binding profile related information and the port has extended + resource request then such information is merged in the resulting + profile. + """ + + self.task.migrate_data = objects.LibvirtLiveMigrateData( + vifs=[ + objects.VIFMigrateData( + port_id=uuids.port1, + profile_json=jsonutils.dumps( + {'some-key': 'value'})) + ]) + provider_mappings = { + uuids.bw_group: [uuids.dest_bw_rp], + uuids.pps_group: [uuids.dest_pps_rp], + uuids.accel_group: [uuids.cyborg_rp], + } + mock_get_binding_profile_alloc.return_value = { + uuids.bw_group: uuids.dest_bw_rp, + uuids.pps_group: uuids.dest_pps_rp, + } + + self.task._bind_ports_on_destination('dest-host', provider_mappings) + + mock_bind_ports.assert_called_once_with( + context=self.context, instance=self.instance, host='dest-host', + vnic_types=None, + port_profiles={ + uuids.port1: { + 'allocation': { + uuids.bw_group: uuids.dest_bw_rp, + uuids.pps_group: uuids.dest_pps_rp, + }, + 'some-key': 'value' + } + } + ) + mock_get_binding_profile_alloc.assert_called_once_with( + self.context, uuids.port1, provider_mappings) + + @mock.patch( + 'nova.network.neutron.API.get_binding_profile_allocation', + new=mock.Mock(return_value=None) + ) @mock.patch('nova.network.neutron.API.bind_ports_to_host') def test_bind_ports_on_destination_migration_data(self, mock_bind_ports): """Assert that if only the migration_data contains binding profile @@ -646,8 +702,11 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): vnic_types=None, port_profiles={uuids.port1: {'some-key': 'value'}}) + @mock.patch('nova.network.neutron.API.get_binding_profile_allocation') @mock.patch('nova.network.neutron.API.bind_ports_to_host') - def test_bind_ports_on_destination_provider_mapping(self, mock_bind_ports): + def test_bind_ports_on_destination_provider_mapping( + self, mock_bind_ports, mock_get_binding_profile_alloc + ): """Assert that if only the provider mapping contains binding profile related information then that is sent to neutron. """ @@ -658,6 +717,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): port_id=uuids.port1) ]) provider_mappings = {uuids.port1: [uuids.dest_bw_rp]} + mock_get_binding_profile_alloc.return_value = uuids.dest_bw_rp self.task._bind_ports_on_destination('dest-host', provider_mappings) @@ -665,6 +725,8 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): context=self.context, instance=self.instance, host='dest-host', vnic_types=None, port_profiles={uuids.port1: {'allocation': uuids.dest_bw_rp}}) + mock_get_binding_profile_alloc.assert_called_once_with( + self.context, uuids.port1, provider_mappings) @mock.patch( 'nova.compute.utils.' @@ -682,7 +744,8 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): self, mock_setup, mock_reset, mock_select, mock_check, mock_call, mock_fill_provider_mapping, mock_update_pci_req): resource_req = [objects.RequestGroup(requester_id=uuids.port_id)] - self.mock_get_res_req.return_value = resource_req + self.mock_get_res_req.return_value = ( + resource_req, objects.RequestLevelParams()) self.instance.pci_requests = objects.InstancePCIRequests(requests=[]) self.assertEqual(("host1", "node1", fake_limits1), diff --git a/nova/tests/unit/conductor/tasks/test_migrate.py b/nova/tests/unit/conductor/tasks/test_migrate.py index 9e29cd171e..145e54f884 100644 --- a/nova/tests/unit/conductor/tasks/test_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_migrate.py @@ -98,7 +98,7 @@ class MigrationTaskTestCase(test.NoDBTestCase): gbf_mock.return_value = objects.MigrationList() mock_get_resources = \ self.mock_network_api.get_requested_resource_for_instance - mock_get_resources.return_value = [] + mock_get_resources.return_value = ([], objects.RequestLevelParams()) if requested_destination: self.request_spec.requested_destination = objects.Destination( @@ -182,6 +182,10 @@ class MigrationTaskTestCase(test.NoDBTestCase): mock_get_resources.assert_called_once_with( self.context, self.instance.uuid) self.assertEqual([], self.request_spec.requested_resources) + self.assertEqual( + mock_get_resources.return_value[1], + self.request_spec.request_level_params, + ) def test_execute(self): self._test_execute() @@ -222,7 +226,7 @@ class MigrationTaskTestCase(test.NoDBTestCase): mock_gbf.return_value = objects.MigrationList() mock_get_resources = \ self.mock_network_api.get_requested_resource_for_instance - mock_get_resources.return_value = [] + mock_get_resources.return_value = ([], objects.RequestLevelParams()) # We just need this hook point to set a uuid on the # migration before we use it for teardown diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index d9cddfb641..87c8837ac1 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1635,12 +1635,16 @@ class _BaseTaskTestCase(object): mock_schedule.return_value = [[selection]] res_req = [objects.RequestGroup()] - mock_get_res_req.return_value = res_req + mock_get_res_req.return_value = (res_req, objects.RequestLevelParams()) self.conductor_manager.unshelve_instance( self.context, instance, request_spec) self.assertEqual(res_req, request_spec.requested_resources) + self.assertEqual( + mock_get_res_req.return_value[1], + request_spec.request_level_params, + ) mock_get_res_req.assert_called_once_with(self.context, instance.uuid) mock_schedule.assert_called_once_with( @@ -1673,7 +1677,7 @@ class _BaseTaskTestCase(object): limits=None) mock_schedule.return_value = [[selection]] - mock_get_res_req.return_value = [] + mock_get_res_req.return_value = ([], objects.RequestLevelParams()) dp_groups = [objects.RequestGroup(requester_id='deviceprofile2'), objects.RequestGroup(requester_id='deviceprofile3')] @@ -1684,6 +1688,10 @@ class _BaseTaskTestCase(object): self.context, instance, request_spec) self.assertEqual(dp_groups, request_spec.requested_resources) + self.assertEqual( + mock_get_res_req.return_value[1], + request_spec.request_level_params, + ) mock_get_res_req.assert_called_once_with(self.context, instance.uuid) mock_schedule.assert_called_once_with( @@ -1719,7 +1727,7 @@ class _BaseTaskTestCase(object): dp_groups = [objects.RequestGroup(requester_id='deviceprofile2'), objects.RequestGroup(requester_id='deviceprofile3')] mock_get_dp.return_value = dp_groups - mock_get_res_req.return_value = [] + mock_get_res_req.return_value = ([], objects.RequestLevelParams()) arqs = ["fake-arq-uuid"] mock_get_arqs.side_effect = exc.AcceleratorRequestBindingFailed( arqs=arqs, msg='') @@ -2016,7 +2024,7 @@ class _BaseTaskTestCase(object): mock.patch('nova.scheduler.utils.fill_provider_mapping'), mock.patch('nova.network.neutron.API.' 'get_requested_resource_for_instance', - return_value=[]) + return_value=([], objects.RequestLevelParams())) ) as (rebuild_mock, sig_mock, select_dest_mock, reset_fd, fill_rp_mapping_mock, get_req_res_mock): self.conductor_manager.rebuild_instance(context=self.context, @@ -2086,7 +2094,7 @@ class _BaseTaskTestCase(object): mock.patch('nova.scheduler.utils.fill_provider_mapping'), mock.patch('nova.network.neutron.API.' 'get_requested_resource_for_instance', - return_value=[]) + return_value=([], objects.RequestLevelParams())) ) as (rebuild_mock, sig_mock, select_dest_mock, reset_fd, fill_rp_mapping_mock, get_req_res_mock): self.conductor_manager.rebuild_instance(context=self.context, @@ -2160,7 +2168,7 @@ class _BaseTaskTestCase(object): mock.patch('nova.scheduler.utils.fill_provider_mapping'), mock.patch('nova.network.neutron.API.' 'get_requested_resource_for_instance', - return_value=[]) + return_value=([], objects.RequestLevelParams())) ) as (sig_mock, select_dest_mock, reset_fd, fill_rp_mapping_mock, get_req_res_mock): ex = self.assertRaises(exc.AcceleratorRequestBindingFailed, diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py index e511b925a8..005ef89259 100644 --- a/nova/tests/unit/network/test_neutron.py +++ b/nova/tests/unit/network/test_neutron.py @@ -4370,9 +4370,14 @@ class TestAPI(TestAPIBase): mock_client.update_port.assert_called_once_with('fake-port-id', port_req_body) + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_bindings_for_instance_with_migration_profile( - self, get_client_mock): + self, get_client_mock + ): instance = fake_instance.fake_instance_obj(self.context) self.api._has_port_binding_extension = mock.Mock(return_value=True) @@ -4406,9 +4411,14 @@ class TestAPI(TestAPIBase): constants.BINDING_PROFILE: { 'fake_profile': 'fake_data'}}}) + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_bindings_for_instance_binding_profile_none( - self, get_client_mock): + self, get_client_mock + ): """Tests _update_port_binding_for_instance when the binding:profile value is None. """ @@ -4435,9 +4445,14 @@ class TestAPI(TestAPIBase): 'compute:%s' % instance.availability_zone}}) + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) - def test_update_port_bindings_for_instance_same_host(self, - get_client_mock): + def test_update_port_bindings_for_instance_same_host( + self, get_client_mock + ): instance = fake_instance.fake_instance_obj(self.context) # We test two ports, one with the same host as the host passed in and @@ -4460,6 +4475,10 @@ class TestAPI(TestAPIBase): 'device_owner': 'compute:%s' % instance.availability_zone}}) + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec') @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_bindings_for_instance_with_pci(self, @@ -4567,6 +4586,10 @@ class TestAPI(TestAPIBase): instance.host, migration) + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec') @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_bindings_for_instance_with_pci_no_migration(self, @@ -4614,6 +4637,10 @@ class TestAPI(TestAPIBase): # No ports should be updated if the port's pci binding did not change. update_port_mock.assert_not_called() + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_bindings_for_instance_with_same_host_failed_vif_type( self, get_client_mock): @@ -4655,6 +4682,10 @@ class TestAPI(TestAPIBase): instance.availability_zone }}) + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_bindings_for_instance_with_diff_host_unbound_vif_type( self, get_client_mock): @@ -4688,6 +4719,10 @@ class TestAPI(TestAPIBase): instance.availability_zone }}) + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) @mock.patch.object(neutronapi.API, '_get_pci_mapping_for_migration') @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec') @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) @@ -4734,6 +4769,10 @@ class TestAPI(TestAPIBase): constants.BINDING_HOST_ID], 'new-host') + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_bindings_for_instance_with_resource_req( self, get_client_mock): @@ -4760,6 +4799,68 @@ class TestAPI(TestAPIBase): 'binding:profile': {'allocation': uuids.dest_compute_rp}, 'binding:host_id': 'new-host'}}) + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=True), + ) + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_update_port_bindings_for_instance_with_extended_resource_req( + self, get_client_mock + ): + instance = fake_instance.fake_instance_obj(self.context) + fake_ports = { + 'ports': [ + { + 'id': 'fake-port-1', + 'binding:vnic_type': 'normal', + constants.BINDING_HOST_ID: 'old-host', + constants.BINDING_PROFILE: { + 'allocation': { + uuids.group1: uuids.source_compute_bw_rp, + uuids.group2: uuids.source_compute_pps_rp, + }, + }, + 'resource_request': { + 'request_groups': [ + { + "id": uuids.group1 + }, + { + "id": uuids.group2 + }, + ], + }, + } + ] + } + migration = objects.Migration( + status='confirmed', migration_type='migration') + list_ports_mock = mock.Mock(return_value=fake_ports) + get_client_mock.return_value.list_ports = list_ports_mock + + self.api._update_port_binding_for_instance( + self.context, instance, 'new-host', migration, + { + uuids.group1: [uuids.dest_compute_bw_rp], + uuids.group2: [uuids.dest_compute_pps_rp], + } + ) + get_client_mock.return_value.update_port.assert_called_once_with( + 'fake-port-1', + { + 'port': { + 'device_owner': 'compute:None', + 'binding:profile': { + 'allocation': { + uuids.group1: uuids.dest_compute_bw_rp, + uuids.group2: uuids.dest_compute_pps_rp, + } + }, + 'binding:host_id': 'new-host' + } + } + ) + @mock.patch.object(neutronapi, 'get_client') def test_update_port_bindings_for_instance_with_resource_req_unshelve( self, get_client_mock): @@ -4785,6 +4886,10 @@ class TestAPI(TestAPIBase): 'binding:profile': {'allocation': uuids.dest_compute_rp}, 'binding:host_id': 'new-host'}}) + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_bindings_for_instance_with_resource_req_no_mapping( self, get_client_mock): @@ -4811,6 +4916,10 @@ class TestAPI(TestAPIBase): "are required for ports with a resource request.", str(ex)) + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_bindings_for_instance_with_resource_req_live_mig( self, get_client_mock): @@ -6620,27 +6729,34 @@ class TestAPI(TestAPIBase): ]} mock_client.list_ports.return_value = ports - request_groups = self.api.get_requested_resource_for_instance( - self.context, uuids.inst1) + request_groups, req_lvl_params = ( + self.api.get_requested_resource_for_instance( + self.context, uuids.inst1) + ) mock_client.list_ports.assert_called_with( device_id=uuids.inst1, fields=['id', 'resource_request']) self.assertEqual([], request_groups) + self.assertEqual([], req_lvl_params.same_subtree) @mock.patch('nova.network.neutron.get_client') def test_get_requested_resource_for_instance_no_ports( - self, mock_get_client): + self, mock_get_client + ): mock_client = mock_get_client.return_value ports = {'ports': []} mock_client.list_ports.return_value = ports - request_groups = self.api.get_requested_resource_for_instance( - self.context, uuids.inst1) + request_groups, req_lvl_params = ( + self.api.get_requested_resource_for_instance( + self.context, uuids.inst1) + ) mock_client.list_ports.assert_called_with( device_id=uuids.inst1, fields=['id', 'resource_request']) self.assertEqual([], request_groups) + self.assertEqual([], req_lvl_params.same_subtree) @mock.patch('nova.network.neutron.get_client') def test_get_requested_resource_for_instance_with_multiple_ports( @@ -6662,8 +6778,10 @@ class TestAPI(TestAPIBase): ]} mock_client.list_ports.return_value = ports - request_groups = self.api.get_requested_resource_for_instance( - self.context, uuids.inst1) + request_groups, req_lvl_params = ( + self.api.get_requested_resource_for_instance( + self.context, uuids.inst1) + ) mock_client.list_ports.assert_called_with( device_id=uuids.inst1, fields=['id', 'resource_request']) @@ -6674,9 +6792,97 @@ class TestAPI(TestAPIBase): self.assertEqual( uuids.port1, request_groups[0].requester_id) + self.assertEqual([], req_lvl_params.same_subtree) mock_get_client.assert_called_once_with(self.context, admin=True) + @mock.patch( + "nova.network.neutron.API.has_extended_resource_request_extension", + new=mock.Mock(return_value=True), + ) + @mock.patch('nova.network.neutron.get_client') + def test_get_requested_resource_for_instance_with_multiple_ports_extended( + self, mock_get_client): + mock_client = mock_get_client.return_value + + ports = {'ports': [ + { + 'id': uuids.port1, + 'device_id': uuids.isnt1, + 'resource_request': { + 'request_groups': [ + { + 'id': uuids.group1, + 'resources': { + 'NET_BW_EGR_KILOBIT_PER_SEC': 10000 + } + }, + { + 'id': uuids.group2, + 'resources': { + 'NET_KILOPACKET_PER_SEC': 100 + } + } + ], + 'same_subtree': [uuids.group1, uuids.group2], + } + }, + { + 'id': uuids.port2, + 'device_id': uuids.isnt1, + 'resource_request': { + 'request_groups': [ + { + 'id': uuids.group3, + 'resources': { + 'NET_BW_EGR_KILOBIT_PER_SEC': 20000 + } + }, + ], + 'same_subtree': [uuids.group3], + } + }, + { + 'id': uuids.port3, + 'device_id': uuids.isnt1, + 'resource_request': {} + }, + ]} + mock_client.list_ports.return_value = ports + + request_groups, req_lvl_params = ( + self.api.get_requested_resource_for_instance( + self.context, uuids.inst1) + ) + + mock_client.list_ports.assert_called_with( + device_id=uuids.inst1, fields=['id', 'resource_request']) + self.assertEqual(3, len(request_groups)) + self.assertEqual( + {'NET_BW_EGR_KILOBIT_PER_SEC': 10000}, + request_groups[0].resources) + self.assertEqual( + uuids.group1, + request_groups[0].requester_id) + self.assertEqual( + {'NET_KILOPACKET_PER_SEC': 100}, + request_groups[1].resources) + self.assertEqual( + uuids.group2, + request_groups[1].requester_id) + self.assertEqual( + {'NET_BW_EGR_KILOBIT_PER_SEC': 20000}, + request_groups[2].resources) + self.assertEqual( + uuids.group3, + request_groups[2].requester_id) + + mock_get_client.assert_called_once_with(self.context, admin=True) + self.assertEqual( + [[uuids.group1, uuids.group2], [uuids.group3]], + req_lvl_params.same_subtree, + ) + def test_get_segment_ids_for_network_no_segment_ext(self): with mock.patch.object( self.api, '_has_segment_extension', return_value=False @@ -6876,6 +7082,92 @@ class TestAPI(TestAPIBase): self.context, port_new_res_req, neutron=None) ) + @mock.patch( + "nova.network.neutron.API.has_extended_resource_request_extension", + new=mock.Mock(return_value=False), + ) + @mock.patch("neutronclient.v2_0.client.Client.show_port") + def test_get_binding_profile_allocation_no_request(self, mock_show_port): + mock_show_port.return_value = { + "port": { + } + } + self.assertIsNone( + self.api.get_binding_profile_allocation( + self.context, uuids.port_uuid, {})) + + @mock.patch( + "nova.network.neutron.API.has_extended_resource_request_extension", + new=mock.Mock(return_value=False), + ) + @mock.patch("neutronclient.v2_0.client.Client.show_port") + def test_get_binding_profile_allocation_legacy_request( + self, mock_show_port + ): + mock_show_port.return_value = { + "port": { + "id": uuids.port_uuid, + "resource_request": { + "resources": { + "CUSTOM_FOO": 123, + } + }, + } + } + self.assertEqual( + uuids.rp, + self.api.get_binding_profile_allocation( + self.context, uuids.port_uuid, + { + uuids.port_uuid: [uuids.rp] + } + ) + ) + + @mock.patch( + "nova.network.neutron.API.has_extended_resource_request_extension", + new=mock.Mock(return_value=True), + ) + @mock.patch("neutronclient.v2_0.client.Client.show_port") + def test_get_binding_profile_allocation_extended_request( + self, mock_show_port + ): + mock_show_port.return_value = { + "port": { + "id": uuids.port_uuid, + "resource_request": { + "request_groups": [ + { + "id": uuids.group1, + "resources": { + "CUSTOM_FOO": 123, + } + }, + { + "id": uuids.group2, + "resources": { + "CUSTOM_BAR": 321, + } + }, + ], + }, + } + } + self.assertEqual( + { + uuids.group1: uuids.rp1, + uuids.group2: uuids.rp2, + }, + self.api.get_binding_profile_allocation( + self.context, uuids.port_uuid, + { + uuids.group1: [uuids.rp1], + uuids.group2: [uuids.rp2], + uuids.non_port_related_group: [uuids.rp3], + } + ) + ) + class TestInstanceHasExtendedResourceRequest(TestAPIBase): def setUp(self):