diff options
Diffstat (limited to 'nova/tests')
19 files changed, 2209 insertions, 229 deletions
diff --git a/nova/tests/fixtures/cinder.py b/nova/tests/fixtures/cinder.py index 29889c784a..025a3d8b81 100644 --- a/nova/tests/fixtures/cinder.py +++ b/nova/tests/fixtures/cinder.py @@ -47,6 +47,13 @@ class CinderFixture(fixtures.Fixture): # This represents a bootable image-backed volume to test # boot-from-volume scenarios. IMAGE_BACKED_VOL = '6ca404f3-d844-4169-bb96-bc792f37de98' + + # This represents a bootable image-backed volume to test + # boot-from-volume scenarios with + # os_require_quiesce + # hw_qemu_guest_agent + IMAGE_BACKED_VOL_QUIESCE = '6ca404f3-d844-4169-bb96-bc792f37de26' + # This represents a bootable image-backed volume with required traits # as part of volume image metadata IMAGE_WITH_TRAITS_BACKED_VOL = '6194fc02-c60e-4a01-a8e5-600798208b5f' @@ -157,6 +164,13 @@ class CinderFixture(fixtures.Fixture): 'image_id': '155d900f-4e14-4e4c-a73d-069cbf4541e6' } + if volume_id == self.IMAGE_BACKED_VOL_QUIESCE: + volume['bootable'] = True + volume['volume_image_metadata'] = { + "os_require_quiesce": "True", + "hw_qemu_guest_agent": "True" + } + if volume_id == self.IMAGE_WITH_TRAITS_BACKED_VOL: volume['bootable'] = True volume['volume_image_metadata'] = { @@ -333,6 +347,10 @@ class CinderFixture(fixtures.Fixture): if 'reimage_reserved' not in kwargs: raise exception.InvalidInput('reimage_reserved not specified') + def fake_get_absolute_limits(_self, context): + limits = {'totalSnapshotsUsed': 0, 'maxTotalSnapshots': -1} + return limits + self.test.stub_out( 'nova.volume.cinder.API.attachment_create', fake_attachment_create) self.test.stub_out( @@ -375,6 +393,9 @@ class CinderFixture(fixtures.Fixture): self.test.stub_out( 'nova.volume.cinder.API.reimage_volume', fake_reimage_volume) + self.test.stub_out( + 'nova.volume.cinder.API.get_absolute_limits', + fake_get_absolute_limits) def volume_ids_for_instance(self, instance_uuid): for volume_id, attachments in self.volume_to_attachment.items(): diff --git a/nova/tests/functional/integrated_helpers.py b/nova/tests/functional/integrated_helpers.py index 91d99d7ec8..0f09d0dcb0 100644 --- a/nova/tests/functional/integrated_helpers.py +++ b/nova/tests/functional/integrated_helpers.py @@ -633,6 +633,13 @@ class InstanceHelperMixin: return self._wait_for_state_change(server, 'SHUTOFF') return server + def _snapshot_server(self, server, snapshot_name): + """Create server snapshot.""" + self.api.post_server_action( + server['id'], + {'createImage': {'name': snapshot_name}} + ) + class PlacementHelperMixin: """A helper mixin for interacting with placement.""" diff --git a/nova/tests/functional/libvirt/test_pci_in_placement.py b/nova/tests/functional/libvirt/test_pci_in_placement.py index 32f6cfeca7..609c3c5ec8 100644 --- a/nova/tests/functional/libvirt/test_pci_in_placement.py +++ b/nova/tests/functional/libvirt/test_pci_in_placement.py @@ -1618,3 +1618,127 @@ class PlacementPCIAllocationHealingTests(PlacementPCIReportingTests): self._run_periodics() self.assert_placement_pci_view( "compute1", **compute1_expected_placement_view) + + +class RCAndTraitBasedPCIAliasTests(PlacementPCIReportingTests): + def setUp(self): + super().setUp() + # TODO(gibi): replace this with setting the [scheduler]pci_prefilter + # confing to True once that config is added + self.mock_pci_in_placement_enabled = self.useFixture( + fixtures.MockPatch( + 'nova.objects.request_spec.RequestSpec.' + '_pci_in_placement_enabled', + return_value=True + ) + ).mock + + def test_boot_with_custom_rc_and_traits(self): + # The fake libvirt will emulate on the host: + # * one type-PCI in slot 0 + pci_info = fakelibvirt.HostPCIDevicesInfo( + num_pci=1, num_pfs=0, num_vfs=0) + device_spec = self._to_device_spec_conf( + [ + { + "address": "0000:81:00.0", + "resource_class": "gpu", + "traits": ",".join( + [ + os_traits.HW_GPU_API_VULKAN, + "purple", + "round", + ] + ), + }, + ] + ) + self.flags(group='pci', device_spec=device_spec) + self.start_compute(hostname="compute1", pci_info=pci_info) + + self.assertPCIDeviceCounts("compute1", total=1, free=1) + compute1_expected_placement_view = { + "inventories": { + "0000:81:00.0": {"CUSTOM_GPU": 1}, + }, + "traits": { + "0000:81:00.0": [ + "HW_GPU_API_VULKAN", + "CUSTOM_PURPLE", + "CUSTOM_ROUND", + ], + }, + "usages": { + "0000:81:00.0": {"CUSTOM_GPU": 0}, + }, + "allocations": {}, + } + self.assert_placement_pci_view( + "compute1", **compute1_expected_placement_view) + + pci_alias_wrong_rc = { + "vendor_id": fakelibvirt.PCI_VEND_ID, + "product_id": fakelibvirt.PCI_PROD_ID, + "name": "a-gpu-wrong-rc", + } + pci_alias_wrong_rc_2 = { + "resource_class": os_resource_classes.PGPU, + "name": "a-gpu-wrong-rc-2", + } + pci_alias_asking_for_missing_trait = { + "resource_class": "GPU", + # NOTE(gibi): "big" is missing from device spec + "traits": "purple,big", + "name": "a-gpu-missing-trait", + } + pci_alias_gpu = { + "resource_class": "GPU", + "traits": "HW_GPU_API_VULKAN,PURPLE", + "name": "a-gpu", + } + self.flags( + group="pci", + # FIXME(gibi): make _to_device_spec_conf a general util for both + # device spec and pci alias + alias=self._to_device_spec_conf( + [ + pci_alias_wrong_rc, + pci_alias_wrong_rc_2, + pci_alias_asking_for_missing_trait, + pci_alias_gpu, + ] + ), + ) + + # try to boot with each alias that does not match + for alias in [ + "a-gpu-wrong-rc", + "a-gpu-wrong-rc-2", + "a-gpu-missing-trait", + ]: + extra_spec = {"pci_passthrough:alias": f"{alias}:1"} + flavor_id = self._create_flavor(extra_spec=extra_spec) + server = self._create_server( + flavor_id=flavor_id, networks=[], expected_state='ERROR') + self.assertIn('fault', server) + self.assertIn('No valid host', server['fault']['message']) + + self.assertPCIDeviceCounts("compute1", total=1, free=1) + self.assert_placement_pci_view( + "compute1", **compute1_expected_placement_view) + + # then boot with the matching alias + extra_spec = {"pci_passthrough:alias": "a-gpu:1"} + flavor_id = self._create_flavor(extra_spec=extra_spec) + server = self._create_server( + flavor_id=flavor_id, networks=[]) + + self.assertPCIDeviceCounts("compute1", total=1, free=0) + compute1_expected_placement_view[ + "usages"]["0000:81:00.0"]["CUSTOM_GPU"] = 1 + compute1_expected_placement_view["allocations"][server["id"]] = { + "0000:81:00.0": {"CUSTOM_GPU": 1} + } + self.assert_placement_pci_view( + "compute1", **compute1_expected_placement_view) + self.assert_no_pci_healing("compute1") diff --git a/nova/tests/functional/libvirt/test_pci_sriov_servers.py b/nova/tests/functional/libvirt/test_pci_sriov_servers.py index b32d165e10..99d136f352 100644 --- a/nova/tests/functional/libvirt/test_pci_sriov_servers.py +++ b/nova/tests/functional/libvirt/test_pci_sriov_servers.py @@ -14,6 +14,8 @@ # under the License. import copy +import pprint +import typing as ty from unittest import mock from urllib import parse as urlparse @@ -27,6 +29,7 @@ from oslo_utils.fixture import uuidsentinel as uuids from oslo_utils import units import nova +from nova.compute import pci_placement_translator from nova import context from nova import exception from nova.network import constants @@ -42,6 +45,52 @@ CONF = cfg.CONF LOG = logging.getLogger(__name__) +class PciPlacementHealingFixture(fixtures.Fixture): + """Allow asserting if the pci_placement_translator module needed to + heal PCI allocations. Such healing is only normal during upgrade. After + every compute is upgraded and the scheduling support of PCI tracking in + placement is enabled there should be no need to heal PCI allocations in + the resource tracker. We assert this as we eventually want to remove the + automatic healing logic from the resource tracker. + """ + + def __init__(self): + super().__init__() + # a list of (nodename, result, allocation_before, allocation_after) + # tuples recoding the result of the calls to + # update_provider_tree_for_pci + self.calls = [] + + def setUp(self): + super().setUp() + + orig = pci_placement_translator.update_provider_tree_for_pci + + def wrapped_update( + provider_tree, nodename, pci_tracker, allocations, same_host + ): + alloc_before = copy.deepcopy(allocations) + updated = orig( + provider_tree, nodename, pci_tracker, allocations, same_host) + alloc_after = copy.deepcopy(allocations) + self.calls.append((nodename, updated, alloc_before, alloc_after)) + return updated + + self.useFixture( + fixtures.MonkeyPatch( + "nova.compute.pci_placement_translator." + "update_provider_tree_for_pci", + wrapped_update, + ) + ) + + def last_healing(self, hostname: str) -> ty.Optional[ty.Tuple[dict, dict]]: + for h, updated, before, after in self.calls: + if h == hostname and updated: + return before, after + return None + + class _PCIServersTestBase(base.ServersTestBase): ADDITIONAL_FILTERS = ['NUMATopologyFilter', 'PciPassthroughFilter'] @@ -66,6 +115,9 @@ class _PCIServersTestBase(base.ServersTestBase): '.PciPassthroughFilter.host_passes', side_effect=host_pass_mock)).mock + self.pci_healing_fixture = self.useFixture( + PciPlacementHealingFixture()) + def assertPCIDeviceCounts(self, hostname, total, free): """Ensure $hostname has $total devices, $free of which are free.""" devices = objects.PciDeviceList.get_by_compute_node( @@ -75,6 +127,24 @@ class _PCIServersTestBase(base.ServersTestBase): self.assertEqual(total, len(devices)) self.assertEqual(free, len([d for d in devices if d.is_available()])) + def assert_no_pci_healing(self, hostname): + last_healing = self.pci_healing_fixture.last_healing(hostname) + before = last_healing[0] if last_healing else None + after = last_healing[1] if last_healing else None + self.assertIsNone( + last_healing, + "The resource tracker needed to heal PCI allocation in placement " + "on host %s. This should not happen in normal operation as the " + "scheduler should create the proper allocation instead.\n" + "Allocations before healing:\n %s\n" + "Allocations after healing:\n %s\n" + % ( + hostname, + pprint.pformat(before), + pprint.pformat(after), + ), + ) + def _get_rp_by_name(self, name, rps): for rp in rps: if rp["name"] == name: @@ -809,7 +879,7 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): # start two compute services with differing PCI device inventory source_pci_info = fakelibvirt.HostPCIDevicesInfo( - num_pfs=2, num_vfs=8, numa_node=0) + num_pfs=1, num_vfs=4, numa_node=0) # add an extra PF without VF to be used by direct-physical ports source_pci_info.add_device( dev_type='PF', @@ -862,7 +932,7 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): # our source host should have marked two PCI devices as used, the VF # and the parent PF, while the future destination is currently unused self.assertEqual('test_compute0', server['OS-EXT-SRV-ATTR:host']) - self.assertPCIDeviceCounts('test_compute0', total=11, free=8) + self.assertPCIDeviceCounts('test_compute0', total=6, free=3) self.assertPCIDeviceCounts('test_compute1', total=4, free=4) # the instance should be on host NUMA node 0, since that's where our @@ -886,7 +956,7 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): # TODO(stephenfin): Stop relying on a side-effect of how nova # chooses from multiple PCI devices (apparently the last # matching one) - 'pci_slot': '0000:81:01.4', + 'pci_slot': '0000:81:00.4', 'physical_network': 'physnet4', }, port['binding:profile'], @@ -910,7 +980,7 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): # we should now have transitioned our usage to the destination, freeing # up the source in the process - self.assertPCIDeviceCounts('test_compute0', total=11, free=11) + self.assertPCIDeviceCounts('test_compute0', total=6, free=6) self.assertPCIDeviceCounts('test_compute1', total=4, free=1) # the instance should now be on host NUMA node 1, since that's where @@ -1824,6 +1894,15 @@ class PCIServersTest(_PCIServersTestBase): def setUp(self): super().setUp() self.flags(group="pci", report_in_placement=True) + # TODO(gibi): replace this with setting the [scheduler]pci_prefilter + # confing to True once that config is added + self.mock_pci_in_placement_enabled = self.useFixture( + fixtures.MockPatch( + 'nova.objects.request_spec.RequestSpec.' + '_pci_in_placement_enabled', + return_value=True + ) + ).mock def test_create_server_with_pci_dev_and_numa(self): """Verifies that an instance can be booted with cpu pinning and with an @@ -1839,6 +1918,7 @@ class PCIServersTest(_PCIServersTestBase): "compute1", inventories={"0000:81:00.0": {self.PCI_RC: 1}}, traits={"0000:81:00.0": []}, + usages={"0000:81:00.0": {self.PCI_RC: 0}}, ) # create a flavor @@ -1848,7 +1928,16 @@ class PCIServersTest(_PCIServersTestBase): } flavor_id = self._create_flavor(extra_spec=extra_spec) - self._create_server(flavor_id=flavor_id, networks='none') + server = self._create_server(flavor_id=flavor_id, networks='none') + + self.assert_placement_pci_view( + "compute1", + inventories={"0000:81:00.0": {self.PCI_RC: 1}}, + traits={"0000:81:00.0": []}, + usages={"0000:81:00.0": {self.PCI_RC: 1}}, + allocations={server['id']: {"0000:81:00.0": {self.PCI_RC: 1}}}, + ) + self.assert_no_pci_healing("compute1") def test_create_server_with_pci_dev_and_numa_fails(self): """This test ensures that it is not possible to allocated CPU and @@ -1860,11 +1949,13 @@ class PCIServersTest(_PCIServersTestBase): pci_info = fakelibvirt.HostPCIDevicesInfo(num_pci=1, numa_node=0) self.start_compute(pci_info=pci_info) + compute1_placement_pci_view = { + "inventories": {"0000:81:00.0": {self.PCI_RC: 1}}, + "traits": {"0000:81:00.0": []}, + "usages": {"0000:81:00.0": {self.PCI_RC: 0}}, + } self.assert_placement_pci_view( - "compute1", - inventories={"0000:81:00.0": {self.PCI_RC: 1}}, - traits={"0000:81:00.0": []}, - ) + "compute1", **compute1_placement_pci_view) # boot one instance with no PCI device to "fill up" NUMA node 0 extra_spec = {'hw:cpu_policy': 'dedicated'} @@ -1877,6 +1968,10 @@ class PCIServersTest(_PCIServersTestBase): self._create_server( flavor_id=flavor_id, networks='none', expected_state='ERROR') + self.assert_placement_pci_view( + "compute1", **compute1_placement_pci_view) + self.assert_no_pci_healing("compute1") + def test_live_migrate_server_with_pci(self): """Live migrate an instance with a PCI passthrough device. @@ -1889,26 +1984,41 @@ class PCIServersTest(_PCIServersTestBase): hostname='test_compute0', pci_info=fakelibvirt.HostPCIDevicesInfo(num_pci=1)) + test_compute0_placement_pci_view = { + "inventories": {"0000:81:00.0": {self.PCI_RC: 1}}, + "traits": {"0000:81:00.0": []}, + "usages": {"0000:81:00.0": {self.PCI_RC: 0}}, + "allocations": {}, + } self.assert_placement_pci_view( - "test_compute0", - inventories={"0000:81:00.0": {self.PCI_RC: 1}}, - traits={"0000:81:00.0": []}, - ) + "test_compute0", **test_compute0_placement_pci_view) self.start_compute( hostname='test_compute1', pci_info=fakelibvirt.HostPCIDevicesInfo(num_pci=1)) + test_compute1_placement_pci_view = { + "inventories": {"0000:81:00.0": {self.PCI_RC: 1}}, + "traits": {"0000:81:00.0": []}, + "usages": {"0000:81:00.0": {self.PCI_RC: 0}}, + } self.assert_placement_pci_view( - "test_compute1", - inventories={"0000:81:00.0": {self.PCI_RC: 1}}, - traits={"0000:81:00.0": []}, - ) + "test_compute1", **test_compute1_placement_pci_view) # create a server extra_spec = {'pci_passthrough:alias': f'{self.ALIAS_NAME}:1'} flavor_id = self._create_flavor(extra_spec=extra_spec) - server = self._create_server(flavor_id=flavor_id, networks='none') + server = self._create_server( + flavor_id=flavor_id, networks='none', host="test_compute0") + + test_compute0_placement_pci_view[ + "usages"]["0000:81:00.0"][self.PCI_RC] = 1 + test_compute0_placement_pci_view[ + "allocations"][server['id']] = {"0000:81:00.0": {self.PCI_RC: 1}} + self.assert_placement_pci_view( + "test_compute0", **test_compute0_placement_pci_view) + self.assert_placement_pci_view( + "test_compute1", **test_compute1_placement_pci_view) # now live migrate that server ex = self.assertRaises( @@ -1920,29 +2030,51 @@ class PCIServersTest(_PCIServersTestBase): # this will bubble to the API self.assertEqual(500, ex.response.status_code) self.assertIn('NoValidHost', str(ex)) + self.assert_placement_pci_view( + "test_compute0", **test_compute0_placement_pci_view) + self.assert_placement_pci_view( + "test_compute1", **test_compute1_placement_pci_view) + self.assert_no_pci_healing("test_compute0") + self.assert_no_pci_healing("test_compute1") def test_resize_pci_to_vanilla(self): # Start two computes, one with PCI and one without. self.start_compute( hostname='test_compute0', pci_info=fakelibvirt.HostPCIDevicesInfo(num_pci=1)) + test_compute0_placement_pci_view = { + "inventories": {"0000:81:00.0": {self.PCI_RC: 1}}, + "traits": {"0000:81:00.0": []}, + "usages": {"0000:81:00.0": {self.PCI_RC: 0}}, + "allocations": {}, + } self.assert_placement_pci_view( - "test_compute0", - inventories={"0000:81:00.0": {self.PCI_RC: 1}}, - traits={"0000:81:00.0": []}, - ) + "test_compute0", **test_compute0_placement_pci_view) + self.start_compute(hostname='test_compute1') + test_compute1_placement_pci_view = { + "inventories": {}, + "traits": {}, + "usages": {}, + "allocations": {}, + } self.assert_placement_pci_view( - "test_compute1", - inventories={}, - traits={}, - ) + "test_compute1", **test_compute1_placement_pci_view) # Boot a server with a single PCI device. extra_spec = {'pci_passthrough:alias': f'{self.ALIAS_NAME}:1'} pci_flavor_id = self._create_flavor(extra_spec=extra_spec) server = self._create_server(flavor_id=pci_flavor_id, networks='none') + test_compute0_placement_pci_view[ + "usages"]["0000:81:00.0"][self.PCI_RC] = 1 + test_compute0_placement_pci_view[ + "allocations"][server['id']] = {"0000:81:00.0": {self.PCI_RC: 1}} + self.assert_placement_pci_view( + "test_compute0", **test_compute0_placement_pci_view) + self.assert_placement_pci_view( + "test_compute1", **test_compute1_placement_pci_view) + # Resize it to a flavor without PCI devices. We expect this to work, as # test_compute1 is available. flavor_id = self._create_flavor() @@ -1955,6 +2087,15 @@ class PCIServersTest(_PCIServersTestBase): self._confirm_resize(server) self.assertPCIDeviceCounts('test_compute0', total=1, free=1) self.assertPCIDeviceCounts('test_compute1', total=0, free=0) + test_compute0_placement_pci_view[ + "usages"]["0000:81:00.0"][self.PCI_RC] = 0 + del test_compute0_placement_pci_view["allocations"][server['id']] + self.assert_placement_pci_view( + "test_compute0", **test_compute0_placement_pci_view) + self.assert_placement_pci_view( + "test_compute1", **test_compute1_placement_pci_view) + self.assert_no_pci_healing("test_compute0") + self.assert_no_pci_healing("test_compute1") def _confirm_resize(self, server, host='host1'): # NOTE(sbauza): Unfortunately, _cleanup_resize() in libvirt checks the @@ -1969,6 +2110,10 @@ class PCIServersTest(_PCIServersTestBase): self.flags(host=orig_host) def test_cold_migrate_server_with_pci(self): + # FIXME(gibi): enable this once the allocation candidate filtering + # in hardware.py and the allocation correlation in the PCI claim is + # implemented + self.mock_pci_in_placement_enabled.return_value = False host_devices = {} orig_create = nova.virt.libvirt.guest.Guest.create @@ -1998,17 +2143,41 @@ class PCIServersTest(_PCIServersTestBase): for hostname in ('test_compute0', 'test_compute1'): pci_info = fakelibvirt.HostPCIDevicesInfo(num_pci=2) self.start_compute(hostname=hostname, pci_info=pci_info) - self.assert_placement_pci_view( - hostname, - inventories={ - "0000:81:00.0": {self.PCI_RC: 1}, - "0000:81:01.0": {self.PCI_RC: 1}, - }, - traits={ - "0000:81:00.0": [], - "0000:81:01.0": [], - }, - ) + test_compute0_placement_pci_view = { + "inventories": { + "0000:81:00.0": {self.PCI_RC: 1}, + "0000:81:01.0": {self.PCI_RC: 1}, + }, + "traits": { + "0000:81:00.0": [], + "0000:81:01.0": [], + }, + "usages": { + "0000:81:00.0": {self.PCI_RC: 0}, + "0000:81:01.0": {self.PCI_RC: 0}, + }, + "allocations": {}, + } + self.assert_placement_pci_view( + "test_compute0", **test_compute0_placement_pci_view) + + test_compute1_placement_pci_view = { + "inventories": { + "0000:81:00.0": {self.PCI_RC: 1}, + "0000:81:01.0": {self.PCI_RC: 1}, + }, + "traits": { + "0000:81:00.0": [], + "0000:81:01.0": [], + }, + "usages": { + "0000:81:00.0": {self.PCI_RC: 0}, + "0000:81:01.0": {self.PCI_RC: 0}, + }, + "allocations": {}, + } + self.assert_placement_pci_view( + "test_compute1", **test_compute1_placement_pci_view) # boot an instance with a PCI device on each host extra_spec = { @@ -2029,6 +2198,23 @@ class PCIServersTest(_PCIServersTestBase): for hostname in ('test_compute0', 'test_compute1'): self.assertPCIDeviceCounts(hostname, total=2, free=1) + # FIXME(gibi): This fails as the scheduler allocates different PCI dev + # in placement than what the pci claim allocates on the host. + # test_compute0_placement_pci_view[ + # "usages"]["0000:81:00.0"][self.PCI_RC] = 1 + # test_compute0_placement_pci_view[ + # "allocations"][server_a['id']] = + # {"0000:81:00.0": {self.PCI_RC: 1}} + # self.assert_placement_pci_view( + # "test_compute0", **test_compute0_placement_pci_view) + # test_compute1_placement_pci_view[ + # "usages"]["0000:81:00.0"][self.PCI_RC] = 1 + # test_compute1_placement_pci_view[ + # "allocations"][server_b['id']] = + # {"0000:81:00.0": {self.PCI_RC: 1}} + # self.assert_placement_pci_view( + # "test_compute1", **test_compute1_placement_pci_view) + # TODO(stephenfin): The mock of 'migrate_disk_and_power_off' should # probably be less...dumb with mock.patch( @@ -2046,13 +2232,40 @@ class PCIServersTest(_PCIServersTestBase): server_a['OS-EXT-SRV-ATTR:host'], server_b['OS-EXT-SRV-ATTR:host'], ) self.assertPCIDeviceCounts('test_compute0', total=2, free=1) + # migration_uuid = self.get_migration_uuid_for_instance(server_a['id']) + # test_compute0_placement_pci_view["allocations"][migration_uuid] = ( + # test_compute0_placement_pci_view["allocations"][server_a['id']]) + # del test_compute0_placement_pci_view["allocations"][server_a['id']] + # self.assert_placement_pci_view( + # "test_compute0", **test_compute0_placement_pci_view) + self.assertPCIDeviceCounts('test_compute1', total=2, free=0) + # test_compute1_placement_pci_view[ + # "usages"]["0000:81:01.0"][self.PCI_RC] = 1 + # test_compute1_placement_pci_view[ + # "allocations"][server_a['id']] = + # {"0000:81:01.0": {self.PCI_RC: 1}} + # self.assert_placement_pci_view( + # "test_compute1", **test_compute1_placement_pci_view) # now, confirm the migration and check our counts once again self._confirm_resize(server_a) self.assertPCIDeviceCounts('test_compute0', total=2, free=2) + # test_compute0_placement_pci_view["usages"] = { + # "0000:81:00.0": {self.PCI_RC: 0}, + # "0000:81:01.0": {self.PCI_RC: 0}, + # } + # del test_compute0_placement_pci_view["allocations"][migration_uuid] + # self.assert_placement_pci_view( + # "test_compute0", **test_compute0_placement_pci_view) + self.assertPCIDeviceCounts('test_compute1', total=2, free=0) + # self.assert_placement_pci_view( + # "test_compute1", **test_compute1_placement_pci_view) + # + # self.assert_no_pci_healing("test_compute0") + # self.assert_no_pci_healing("test_compute1") def test_request_two_pci_but_host_has_one(self): # simulate a single type-PCI device on the host diff --git a/nova/tests/functional/regressions/test_bug_1951656.py b/nova/tests/functional/regressions/test_bug_1951656.py new file mode 100644 index 0000000000..d705ff6fe3 --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_1951656.py @@ -0,0 +1,73 @@ +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# 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 oslo_utils import uuidutils + + +from nova.tests.fixtures import libvirt as fakelibvirt +from nova.tests.functional.libvirt import test_vgpu +from nova.virt.libvirt import utils as libvirt_utils + + +class VGPUTestsLibvirt7_7(test_vgpu.VGPUTestBase): + + def _create_mdev(self, physical_device, mdev_type, uuid=None): + # We need to fake the newly created sysfs object by adding a new + # FakeMdevDevice in the existing persisted Connection object so + # when asking to get the existing mdevs, we would see it. + if not uuid: + uuid = uuidutils.generate_uuid() + mdev_name = libvirt_utils.mdev_uuid2name(uuid) + libvirt_parent = self.pci2libvirt_address(physical_device) + + # Libvirt 7.7 now creates mdevs with a parent_addr suffix. + new_mdev_name = '_'.join([mdev_name, libvirt_parent]) + + # Here, we get the right compute thanks by the self.current_host that + # was modified just before + connection = self.computes[ + self._current_host].driver._host.get_connection() + connection.mdev_info.devices.update( + {mdev_name: fakelibvirt.FakeMdevDevice(dev_name=new_mdev_name, + type_id=mdev_type, + parent=libvirt_parent)}) + return uuid + + def setUp(self): + super(VGPUTestsLibvirt7_7, self).setUp() + extra_spec = {"resources:VGPU": "1"} + self.flavor = self._create_flavor(extra_spec=extra_spec) + + # Start compute1 supporting only nvidia-11 + self.flags( + enabled_mdev_types=fakelibvirt.NVIDIA_11_VGPU_TYPE, + group='devices') + + self.compute1 = self.start_compute_with_vgpu('host1') + + def test_create_servers_with_vgpu(self): + + # Create a single instance against a specific compute node. + self._create_server( + image_uuid='155d900f-4e14-4e4c-a73d-069cbf4541e6', + flavor_id=self.flavor, host=self.compute1.host, + networks='auto', expected_state='ACTIVE') + + self.assert_mdev_usage(self.compute1, expected_amount=1) + + self._create_server( + image_uuid='155d900f-4e14-4e4c-a73d-069cbf4541e6', + flavor_id=self.flavor, host=self.compute1.host, + networks='auto', expected_state='ACTIVE') + + self.assert_mdev_usage(self.compute1, expected_amount=2) diff --git a/nova/tests/functional/regressions/test_bug_1980720.py b/nova/tests/functional/regressions/test_bug_1980720.py new file mode 100644 index 0000000000..ad2e6e6ba2 --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_1980720.py @@ -0,0 +1,68 @@ +# Copyright (C) 2022 Red Hat, Inc +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# 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 nova.tests import fixtures as nova_fixtures +from nova.tests.functional.api import client +from nova.tests.functional import integrated_helpers +from nova.tests.functional.libvirt import base +from unittest import mock + + +class LibvirtDriverTests( + base.ServersTestBase, + integrated_helpers.InstanceHelperMixin +): + api_major_version = 'v2.1' + microversion = 'latest' + + def setUp(self): + super(LibvirtDriverTests, self).setUp() + self.cinder = self.useFixture(nova_fixtures.CinderFixture(self)) + self.start_compute() + + def _create_server_with_block_device(self): + server_request = self._build_server( + networks=[], + ) + # removing imageRef is required as we want + # to boot from volume + server_request.pop('imageRef') + server_request['block_device_mapping_v2'] = [{ + 'boot_index': 0, + 'source_type': 'volume', + 'uuid': nova_fixtures.CinderFixture.IMAGE_BACKED_VOL_QUIESCE, + 'destination_type': 'volume'}] + + server = self.api.post_server({ + 'server': server_request, + }) + self._wait_for_state_change(server, 'ACTIVE') + return server + + def test_snapshot_quiesce_fail(self): + server = self._create_server_with_block_device() + with mock.patch.object( + nova_fixtures.libvirt.Domain, 'fsFreeze' + ) as mock_obj: + ex = nova_fixtures.libvirt.libvirtError("Error") + ex.err = (nova_fixtures.libvirt.VIR_ERR_AGENT_UNRESPONSIVE,) + + mock_obj.side_effect = ex + excep = self.assertRaises( + client.OpenStackApiException, + self._snapshot_server, server, "snapshot-1" + ) + self.assertEqual(409, excep.response.status_code) diff --git a/nova/tests/unit/api/openstack/test_wsgi_app.py b/nova/tests/unit/api/openstack/test_wsgi_app.py index 94e2fe5cb1..0eb7011c11 100644 --- a/nova/tests/unit/api/openstack/test_wsgi_app.py +++ b/nova/tests/unit/api/openstack/test_wsgi_app.py @@ -104,3 +104,18 @@ document_root = /tmp 'disable_compute_service_check_for_ffu', True, group='workarounds') wsgi_app._setup_service('myhost', 'api') + + def test__get_config_files_empty_env(self): + env = {} + result = wsgi_app._get_config_files(env) + expected = ['/etc/nova/api-paste.ini', '/etc/nova/nova.conf'] + self.assertEqual(result, expected) + + def test__get_config_files_with_env(self): + env = { + "OS_NOVA_CONFIG_DIR": "/nova", + "OS_NOVA_CONFIG_FILES": "api.conf", + } + result = wsgi_app._get_config_files(env) + expected = ['/nova/api.conf'] + self.assertEqual(result, expected) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 1a4935f482..e521283acc 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -7927,6 +7927,42 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): @mock.patch.object(virt_driver.ComputeDriver, 'failed_spawn_cleanup') @mock.patch.object(virt_driver.ComputeDriver, 'prepare_for_spawn') + @mock.patch.object(virt_driver.ComputeDriver, + 'prepare_networks_before_block_device_mapping') + @mock.patch.object(virt_driver.ComputeDriver, + 'clean_networks_preparation') + def test_failed_prepare_for_spawn(self, mock_clean, mock_prepnet, + mock_prepspawn, mock_failedspawn): + mock_prepspawn.side_effect = exception.ComputeResourcesUnavailable( + reason="asdf") + with mock.patch.object(self.compute, + '_build_networks_for_instance', + return_value=self.network_info + ) as _build_networks_for_instance: + + try: + with self.compute._build_resources(self.context, self.instance, + self.requested_networks, self.security_groups, + self.image, self.block_device_mapping, + self.resource_provider_mapping, self.accel_uuids): + pass + except Exception as e: + self.assertIsInstance(e, + exception.ComputeResourcesUnavailable) + + _build_networks_for_instance.assert_has_calls( + [mock.call(self.context, self.instance, + self.requested_networks, self.security_groups, + self.resource_provider_mapping, + self.network_arqs)]) + + mock_prepnet.assert_not_called() + mock_clean.assert_called_once_with(self.instance, self.network_info) + mock_prepspawn.assert_called_once_with(self.instance) + mock_failedspawn.assert_called_once_with(self.instance) + + @mock.patch.object(virt_driver.ComputeDriver, 'failed_spawn_cleanup') + @mock.patch.object(virt_driver.ComputeDriver, 'prepare_for_spawn') @mock.patch.object(manager.ComputeManager, '_build_networks_for_instance') def test_build_resources_aborts_on_failed_network_alloc(self, mock_build, mock_prepspawn, diff --git a/nova/tests/unit/compute/test_pci_placement_translator.py b/nova/tests/unit/compute/test_pci_placement_translator.py index ee6a0469ac..0592186e54 100644 --- a/nova/tests/unit/compute/test_pci_placement_translator.py +++ b/nova/tests/unit/compute/test_pci_placement_translator.py @@ -12,12 +12,15 @@ # License for the specific language governing permissions and limitations # under the License. import ddt +from oslo_utils.fixture import uuidsentinel as uuids from unittest import mock from nova.compute import pci_placement_translator as ppt +from nova.compute import provider_tree from nova import exception from nova.objects import fields from nova.objects import pci_device +from nova.pci import devspec from nova import test @@ -88,8 +91,8 @@ class TestTranslator(test.NoDBTestCase): ) def test_trait_normalization(self, trait_names, expected_traits): self.assertEqual( - expected_traits | {"COMPUTE_MANAGED_PCI_DEVICE"}, - ppt._get_traits_for_dev({"traits": trait_names}) + expected_traits, + ppt.get_traits(trait_names) ) @ddt.unpack @@ -110,7 +113,9 @@ class TestTranslator(test.NoDBTestCase): def test_resource_class_normalization(self, pci_dev, rc_name, expected_rc): self.assertEqual( expected_rc, - ppt._get_rc_for_dev(pci_dev, {"resource_class": rc_name}) + ppt.get_resource_class( + rc_name, pci_dev.vendor_id, pci_dev.product_id + ), ) def test_dependent_device_pf_then_vf(self): @@ -118,12 +123,16 @@ class TestTranslator(test.NoDBTestCase): "fake-node", instances_under_same_host_resize=[]) pf = pci_device.PciDevice( address="0000:81:00.0", - dev_type=fields.PciDeviceType.SRIOV_PF + dev_type=fields.PciDeviceType.SRIOV_PF, + vendor_id="dead", + product_id="beef", ) vf = pci_device.PciDevice( address="0000:81:00.1", parent_addr=pf.address, - dev_type=fields.PciDeviceType.SRIOV_VF + dev_type=fields.PciDeviceType.SRIOV_VF, + vendor_id="dead", + product_id="beef", ) pv._add_dev(pf, {"resource_class": "foo"}) @@ -146,17 +155,23 @@ class TestTranslator(test.NoDBTestCase): "fake-node", instances_under_same_host_resize=[]) pf = pci_device.PciDevice( address="0000:81:00.0", - dev_type=fields.PciDeviceType.SRIOV_PF + dev_type=fields.PciDeviceType.SRIOV_PF, + vendor_id="dead", + product_id="beef", ) vf = pci_device.PciDevice( address="0000:81:00.1", parent_addr=pf.address, - dev_type=fields.PciDeviceType.SRIOV_VF + dev_type=fields.PciDeviceType.SRIOV_VF, + vendor_id="dead", + product_id="beef", ) vf2 = pci_device.PciDevice( address="0000:81:00.2", parent_addr=pf.address, - dev_type=fields.PciDeviceType.SRIOV_VF + dev_type=fields.PciDeviceType.SRIOV_VF, + vendor_id="dead", + product_id="beef", ) pv._add_dev(vf, {"resource_class": "foo"}) @@ -182,7 +197,10 @@ class TestTranslator(test.NoDBTestCase): pci_device.PciDevice( address="0000:81:00.%d" % f, parent_addr="0000:71:00.0", - dev_type=fields.PciDeviceType.SRIOV_VF) + dev_type=fields.PciDeviceType.SRIOV_VF, + vendor_id="dead", + product_id="beef", + ) for f in range(0, 4) ] @@ -220,3 +238,54 @@ class TestTranslator(test.NoDBTestCase): "CUSTOM_BAR,CUSTOM_BAZ,CUSTOM_FOO for 0000:81:00.0,0000:81:00.1.", str(ex), ) + + def test_translator_maps_pci_device_to_rp(self): + pv = ppt.PlacementView( + "fake-node", instances_under_same_host_resize=[]) + vf = pci_device.PciDevice( + address="0000:81:00.1", + parent_addr="0000:71:00.0", + dev_type=fields.PciDeviceType.SRIOV_VF, + vendor_id="dead", + product_id="beef", + ) + pf = pci_device.PciDevice( + address="0000:72:00.0", + parent_addr=None, + dev_type=fields.PciDeviceType.SRIOV_PF, + vendor_id="dead", + product_id="beef", + ) + pt = provider_tree.ProviderTree() + pt.new_root("fake-node", uuids.compute_rp) + + pv._add_dev(vf, {}) + pv._add_dev(pf, {}) + pv.update_provider_tree(pt) + + self.assertEqual( + pt.data("fake-node_0000:71:00.0").uuid, vf.extra_info["rp_uuid"] + ) + self.assertEqual( + pt.data("fake-node_0000:72:00.0").uuid, pf.extra_info["rp_uuid"] + ) + + def test_update_provider_tree_for_pci_update_pools(self): + pt = provider_tree.ProviderTree() + pt.new_root("fake-node", uuids.compute_rp) + pf = pci_device.PciDevice( + address="0000:72:00.0", + parent_addr=None, + dev_type=fields.PciDeviceType.SRIOV_PF, + vendor_id="dead", + product_id="beef", + status=fields.PciDeviceStatus.AVAILABLE, + ) + pci_tracker = mock.Mock() + pci_tracker.pci_devs = [pf] + pci_tracker.dev_filter.specs = [devspec.PciDeviceSpec({})] + + ppt.update_provider_tree_for_pci(pt, 'fake-node', pci_tracker, {}, []) + + pci_tracker.stats.populate_pools_metadata_from_assigned_devices.\ + assert_called_once_with() diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index b81d7365d2..6258054aa7 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -1580,6 +1580,7 @@ class TestUpdateComputeNode(BaseTestCase): self.rt._update(mock.sentinel.ctx, new_compute) save_mock.assert_called_once_with() + @mock.patch('nova.objects.ComputeNode.save', new=mock.Mock()) @mock.patch( 'nova.pci.stats.PciDeviceStats.has_remote_managed_device_pools', return_value=True) @@ -1773,7 +1774,7 @@ class TestUpdateComputeNode(BaseTestCase): self.assertEqual(4, ufpt_mock.call_count) self.assertEqual(4, mock_sync_disabled.call_count) # The retry is restricted to _update_to_placement - self.assertEqual(1, mock_resource_change.call_count) + self.assertEqual(0, mock_resource_change.call_count) @mock.patch( 'nova.compute.resource_tracker.ResourceTracker.' @@ -2041,6 +2042,10 @@ class TestUpdateComputeNode(BaseTestCase): self.assertIn('Unable to find services table record for nova-compute', mock_log_error.call_args[0][0]) + @mock.patch( + 'nova.compute.resource_tracker.ResourceTracker.' + '_update_to_placement', + new=mock.Mock()) def test_update_compute_node_save_fails_restores_old_resources(self): """Tests the scenario that compute_node.save() fails and the old_resources value for the node is restored to its previous value @@ -2401,7 +2406,10 @@ class TestInstanceClaim(BaseTestCase): vendor_id='0001', product_id='0002', numa_node=0, - tags={'dev_type': 'type-PCI'}, + tags={ + 'dev_type': 'type-PCI', + 'address': '0000:81:00.0' + }, count=0 ) ] diff --git a/nova/tests/unit/db/main/test_migrations.py b/nova/tests/unit/db/main/test_migrations.py index d2c4ef9762..e52deb262a 100644 --- a/nova/tests/unit/db/main/test_migrations.py +++ b/nova/tests/unit/db/main/test_migrations.py @@ -314,6 +314,15 @@ class NovaMigrationsWalk( self.assertIsInstance( table.c.encryption_options.type, sa.types.String) + def _check_960aac0e09ea(self, connection): + self.assertIndexNotExists( + connection, 'console_auth_tokens', + 'console_auth_tokens_token_hash_idx', + ) + self.assertIndexNotExists( + connection, 'instances', 'uuid', + ) + def test_single_base_revision(self): """Ensure we only have a single base revision. diff --git a/nova/tests/unit/objects/test_request_spec.py b/nova/tests/unit/objects/test_request_spec.py index d91015a699..d1bb59868f 100644 --- a/nova/tests/unit/objects/test_request_spec.py +++ b/nova/tests/unit/objects/test_request_spec.py @@ -14,6 +14,7 @@ import collections from unittest import mock +import fixtures from oslo_serialization import jsonutils from oslo_utils.fixture import uuidsentinel as uuids from oslo_utils import uuidutils @@ -430,6 +431,67 @@ class _TestRequestSpecObject(object): self.assertListEqual([rg], spec.requested_resources) self.assertEqual(req_lvl_params, spec.request_level_params) + # TODO(gibi): replace this with setting the config + # [scheduler]pci_in_placement=True once that flag is available + @mock.patch( + 'nova.objects.request_spec.RequestSpec._pci_in_placement_enabled', + new=mock.Mock(return_value=True), + ) + def test_from_components_flavor_based_pci_requests(self): + ctxt = context.RequestContext( + fakes.FAKE_USER_ID, fakes.FAKE_PROJECT_ID + ) + instance = fake_instance.fake_instance_obj(ctxt) + image = { + "id": uuids.image_id, + "properties": {"mappings": []}, + "status": "fake-status", + "location": "far-away", + } + flavor = fake_flavor.fake_flavor_obj(ctxt) + filter_properties = {"fake": "property"} + + qos_port_rg = request_spec.RequestGroup() + req_lvl_params = request_spec.RequestLevelParams() + + pci_requests = objects.InstancePCIRequests( + requests=[ + objects.InstancePCIRequest( + count=1, + alias_name='a-dev', + request_id=uuids.req1, + spec=[{"vendor_id": "1234", "product_id": "fe12"}], + ) + ] + ) + pci_request_group = request_spec.RequestGroup( + requester_id=f"{uuids.req1}-0", + resources={"CUSTOM_PCI_1234_FE12": 1}, + same_provider=True, + ) + + spec = objects.RequestSpec.from_components( + ctxt, + instance.uuid, + image, + flavor, + instance.numa_topology, + pci_requests, + filter_properties, + None, + instance.availability_zone, + port_resource_requests=[qos_port_rg], + request_level_params=req_lvl_params, + ) + + self.assertEqual(2, len(spec.requested_resources)) + self.assertEqual(qos_port_rg, spec.requested_resources[0]) + self.assertEqual( + pci_request_group.obj_to_primitive(), + spec.requested_resources[1].obj_to_primitive(), + ) + self.assertEqual(req_lvl_params, spec.request_level_params) + def test_get_scheduler_hint(self): spec_obj = objects.RequestSpec(scheduler_hints={'foo_single': ['1'], 'foo_mul': ['1', '2']}) @@ -1054,6 +1116,191 @@ class TestRemoteRequestSpecObject(test_objects._RemoteTest, pass +class TestInstancePCIRequestToRequestGroups(test.NoDBTestCase): + def setUp(self): + super().setUp() + # TODO(gibi): replace this with setting the config + # [scheduler]pci_in_placement=True once that flag is available + self.mock_pci_in_placement_enabled = self.useFixture( + fixtures.MockPatch( + "nova.objects.request_spec.RequestSpec." + "_pci_in_placement_enabled", + return_value=True, + ) + ).mock + + def test_pci_reqs_ignored_if_disabled(self): + self.mock_pci_in_placement_enabled.return_value = False + + spec = request_spec.RequestSpec( + requested_resources=[], + pci_requests=objects.InstancePCIRequests( + requests=[ + objects.InstancePCIRequest( + count=1, + request_id=uuids.req1, + spec=[{"vendor_id": "de12", "product_id": "1234"}], + alias_name="a-dev", + ), + ] + ), + ) + + spec._generate_request_groups_from_pci_requests() + + self.assertEqual(0, len(spec.requested_resources)) + + def test_neutron_based_requests_are_ignored(self): + pci_req = objects.InstancePCIRequest( + count=1, + request_id=uuids.req1, + spec=[], + ) + spec = request_spec.RequestSpec( + requested_resources=[], + pci_requests=objects.InstancePCIRequests(requests=[pci_req]), + ) + self.assertEqual( + objects.InstancePCIRequest.NEUTRON_PORT, pci_req.source + ) + + spec._generate_request_groups_from_pci_requests() + + self.assertEqual(0, len(spec.requested_resources)) + + def test_rc_from_product_and_vendor(self): + spec = request_spec.RequestSpec( + requested_resources=[], + pci_requests=objects.InstancePCIRequests( + requests=[ + objects.InstancePCIRequest( + count=1, + request_id=uuids.req1, + spec=[{"vendor_id": "de12", "product_id": "1234"}], + alias_name="a-dev", + ), + objects.InstancePCIRequest( + count=1, + request_id=uuids.req2, + spec=[{"vendor_id": "fff", "product_id": "dead"}], + alias_name="a-dev", + ), + ] + ), + ) + + spec._generate_request_groups_from_pci_requests() + + self.assertEqual(2, len(spec.requested_resources)) + self.assertEqual( + request_spec.RequestGroup( + requester_id=f"{uuids.req1}-0", + resources={"CUSTOM_PCI_DE12_1234": 1}, + use_same_provider=True, + ).obj_to_primitive(), + spec.requested_resources[0].obj_to_primitive(), + ) + self.assertEqual( + request_spec.RequestGroup( + requester_id=f"{uuids.req2}-0", + resources={"CUSTOM_PCI_FFF_DEAD": 1}, + use_same_provider=True, + ).obj_to_primitive(), + spec.requested_resources[1].obj_to_primitive(), + ) + + def test_multi_device_split_to_multiple_groups(self): + spec = request_spec.RequestSpec( + requested_resources=[], + pci_requests=objects.InstancePCIRequests( + requests=[ + objects.InstancePCIRequest( + count=2, + request_id=uuids.req1, + spec=[{"vendor_id": "de12", "product_id": "1234"}], + alias_name="a-dev", + ), + ] + ), + ) + + spec._generate_request_groups_from_pci_requests() + + self.assertEqual(2, len(spec.requested_resources)) + self.assertEqual( + request_spec.RequestGroup( + requester_id=f"{uuids.req1}-0", + resources={"CUSTOM_PCI_DE12_1234": 1}, + use_same_provider=True, + ).obj_to_primitive(), + spec.requested_resources[0].obj_to_primitive(), + ) + self.assertEqual( + request_spec.RequestGroup( + requester_id=f"{uuids.req1}-1", + resources={"CUSTOM_PCI_DE12_1234": 1}, + use_same_provider=True, + ).obj_to_primitive(), + spec.requested_resources[1].obj_to_primitive(), + ) + + def test_with_rc_and_traits_from_the_pci_req_spec(self): + spec = request_spec.RequestSpec( + requested_resources=[], + pci_requests=objects.InstancePCIRequests( + requests=[ + objects.InstancePCIRequest( + count=1, + request_id=uuids.req1, + spec=[ + { + "vendor_id": "de12", + "product_id": "1234", + "resource_class": "gpu", + } + ], + alias_name="a-dev", + ), + objects.InstancePCIRequest( + count=1, + request_id=uuids.req2, + spec=[ + { + "vendor_id": "fff", + "product_id": "dead", + "traits": "foo,bar,CUSTOM_BLUE", + } + ], + alias_name="a-dev", + ), + ] + ), + ) + + spec._generate_request_groups_from_pci_requests() + + self.assertEqual(2, len(spec.requested_resources)) + self.assertEqual( + request_spec.RequestGroup( + requester_id=f"{uuids.req1}-0", + resources={"CUSTOM_GPU": 1}, + use_same_provider=True, + ).obj_to_primitive(), + spec.requested_resources[0].obj_to_primitive(), + ) + # Note that sets would be serialized to tuples by obj_to_primitive in + # random order, so we need to match this spec field by field + expected = request_spec.RequestGroup( + requester_id=f"{uuids.req2}-0", + resources={"CUSTOM_PCI_FFF_DEAD": 1}, + required_traits={"CUSTOM_FOO", "CUSTOM_BAR", "CUSTOM_BLUE"}, + use_same_provider=True, + ) + actual = spec.requested_resources[1] + for field in request_spec.RequestGroup.fields.keys(): + self.assertEqual(getattr(expected, field), getattr(actual, field)) + + class TestRequestGroupObject(test.NoDBTestCase): def setUp(self): super(TestRequestGroupObject, self).setUp() diff --git a/nova/tests/unit/pci/test_request.py b/nova/tests/unit/pci/test_request.py index 7aefbd15fd..4a3f17f6cb 100644 --- a/nova/tests/unit/pci/test_request.py +++ b/nova/tests/unit/pci/test_request.py @@ -187,6 +187,21 @@ class PciRequestTestCase(test.NoDBTestCase): self.assertIn("xxx", aliases) self.assertEqual(policy, aliases["xxx"][0]) + def test_get_alias_from_config_valid_rc_and_traits(self): + fake_alias = jsonutils.dumps({ + "name": "xxx", + "resource_class": "foo", + "traits": "bar,baz", + }) + self.flags(alias=[fake_alias], group='pci') + aliases = request._get_alias_from_config() + self.assertIsNotNone(aliases) + self.assertIn("xxx", aliases) + self.assertEqual( + ("legacy", [{"resource_class": "foo", "traits": "bar,baz"}]), + aliases["xxx"], + ) + def test_get_alias_from_config_conflicting_device_type(self): """Check behavior when device_type conflicts occur.""" fake_alias_a = jsonutils.dumps({ diff --git a/nova/tests/unit/pci/test_stats.py b/nova/tests/unit/pci/test_stats.py index ef8eb2b2b8..d9b5b7bca1 100644 --- a/nova/tests/unit/pci/test_stats.py +++ b/nova/tests/unit/pci/test_stats.py @@ -17,6 +17,7 @@ from unittest import mock from oslo_config import cfg from oslo_serialization import jsonutils +from oslo_utils.fixture import uuidsentinel as uuids from nova import exception from nova import objects @@ -107,17 +108,19 @@ class PciDeviceStatsTestCase(test.NoDBTestCase): self._create_fake_devs() def test_add_device(self): - self.assertEqual(len(self.pci_stats.pools), 3) + self.assertEqual(len(self.pci_stats.pools), 4) self.assertEqual(set([d['vendor_id'] for d in self.pci_stats]), set(['v1', 'v2', 'v3'])) - self.assertEqual(set([d['count'] for d in self.pci_stats]), - set([1, 2])) + self.assertEqual([d['count'] for d in self.pci_stats], [1, 1, 1, 1]) def test_remove_device(self): + self.assertEqual(len(self.pci_stats.pools), 4) self.pci_stats.remove_device(self.fake_dev_2) - self.assertEqual(len(self.pci_stats.pools), 2) - self.assertEqual(self.pci_stats.pools[0]['count'], 2) + self.assertEqual(len(self.pci_stats.pools), 3) + self.assertEqual(self.pci_stats.pools[0]['count'], 1) self.assertEqual(self.pci_stats.pools[0]['vendor_id'], 'v1') + self.assertEqual(self.pci_stats.pools[1]['count'], 1) + self.assertEqual(self.pci_stats.pools[1]['vendor_id'], 'v1') def test_remove_device_exception(self): self.pci_stats.remove_device(self.fake_dev_2) @@ -146,14 +149,13 @@ class PciDeviceStatsTestCase(test.NoDBTestCase): m = self.pci_stats.to_device_pools_obj() new_stats = stats.PciDeviceStats(objects.NUMATopology(), m) - self.assertEqual(len(new_stats.pools), 3) - self.assertEqual(set([d['count'] for d in new_stats]), - set([1, 2])) + self.assertEqual(len(new_stats.pools), 4) + self.assertEqual([d['count'] for d in new_stats], [1, 1, 1, 1]) self.assertEqual(set([d['vendor_id'] for d in new_stats]), set(['v1', 'v2', 'v3'])) def test_apply_requests(self): - self.assertEqual(len(self.pci_stats.pools), 3) + self.assertEqual(len(self.pci_stats.pools), 4) self.pci_stats.apply_requests(pci_requests) self.assertEqual(len(self.pci_stats.pools), 2) self.assertEqual(self.pci_stats.pools[0]['vendor_id'], 'v1') @@ -166,16 +168,14 @@ class PciDeviceStatsTestCase(test.NoDBTestCase): def test_support_requests(self): self.assertTrue(self.pci_stats.support_requests(pci_requests)) - self.assertEqual(len(self.pci_stats.pools), 3) - self.assertEqual(set([d['count'] for d in self.pci_stats]), - set((1, 2))) + self.assertEqual(len(self.pci_stats.pools), 4) + self.assertEqual([d['count'] for d in self.pci_stats], [1, 1, 1, 1]) def test_support_requests_failed(self): self.assertFalse( self.pci_stats.support_requests(pci_requests_multiple)) - self.assertEqual(len(self.pci_stats.pools), 3) - self.assertEqual(set([d['count'] for d in self.pci_stats]), - set([1, 2])) + self.assertEqual(len(self.pci_stats.pools), 4) + self.assertEqual([d['count'] for d in self.pci_stats], [1, 1, 1, 1]) def test_support_requests_numa(self): cells = [ @@ -571,7 +571,7 @@ class PciDeviceStatsWithTagsTestCase(test.NoDBTestCase): 'compute_node_id': 1, 'address': '0000:0e:00.1', 'vendor_id': '15b3', - 'product_id': '1018', + 'product_id': '101c', 'status': 'available', 'request_id': None, 'dev_type': fields.PciDeviceType.SRIOV_VF, @@ -599,35 +599,68 @@ class PciDeviceStatsWithTagsTestCase(test.NoDBTestCase): self.assertEqual(v, pool[k]) def _assertPools(self): + nr_tagged = len(self.pci_tagged_devices) + nr_untagged = len(self.pci_untagged_devices) + nr_remote = len(self.remote_managed_netdevs) + nr_local = len(self.locally_managed_netdevs) + self.assertEqual( + nr_tagged + nr_untagged + nr_remote + nr_local, + len(self.pci_stats.pools), + ) # Pools are ordered based on the number of keys. 'product_id', # 'vendor_id' are always part of the keys. When tags are present, - # they are also part of the keys. In this test class, we have - # 5 pools with the second one having the tag 'physical_network' - # and the value 'physnet1' and multiple pools for testing - # variations of explicit/implicit remote_managed tagging. - self.assertEqual(5, len(self.pci_stats.pools)) - self._assertPoolContent(self.pci_stats.pools[0], '1137', '0072', - len(self.pci_untagged_devices)) - self.assertEqual(self.pci_untagged_devices, - self.pci_stats.pools[0]['devices']) - self._assertPoolContent(self.pci_stats.pools[1], '1137', '0071', - len(self.pci_tagged_devices), - physical_network='physnet1') - self.assertEqual(self.pci_tagged_devices, - self.pci_stats.pools[1]['devices']) - self._assertPoolContent(self.pci_stats.pools[2], '15b3', '101e', - len(self.remote_managed_netdevs), - remote_managed='true') - self.assertEqual(self.remote_managed_netdevs, - self.pci_stats.pools[2]['devices']) - self._assertPoolContent(self.pci_stats.pools[3], '15b3', '101c', 1, - remote_managed='false') - self.assertEqual([self.locally_managed_netdevs[0]], - self.pci_stats.pools[3]['devices']) - self._assertPoolContent(self.pci_stats.pools[4], '15b3', '1018', 1, - remote_managed='false') - self.assertEqual([self.locally_managed_netdevs[1]], - self.pci_stats.pools[4]['devices']) + # they are also part of the keys. + + # 3 pools for the pci_untagged_devices + devs = [] + j = 0 + for i in range(j, j + nr_untagged): + self._assertPoolContent(self.pci_stats.pools[i], '1137', '0072', 1) + devs += self.pci_stats.pools[i]['devices'] + self.assertEqual(self.pci_untagged_devices, devs) + j += nr_untagged + + # 4 pools for the pci_tagged_devices' + devs = [] + for i in range(j, j + nr_tagged): + self._assertPoolContent( + self.pci_stats.pools[i], + "1137", + "0071", + 1, + physical_network="physnet1", + ) + devs += self.pci_stats.pools[i]['devices'] + self.assertEqual(self.pci_tagged_devices, devs) + j += nr_tagged + + # one with remote_managed_netdevs + devs = [] + for i in range(j, j + nr_remote): + self._assertPoolContent( + self.pci_stats.pools[i], + "15b3", + "101e", + 1, + remote_managed="true", + ) + devs += self.pci_stats.pools[i]['devices'] + self.assertEqual(self.remote_managed_netdevs, devs) + j += nr_remote + + # two with locally_managed_netdevs + devs = [] + for i in range(j, j + nr_local): + self._assertPoolContent( + self.pci_stats.pools[i], + "15b3", + "101c", + 1, + remote_managed="false", + ) + devs += self.pci_stats.pools[i]['devices'] + self.assertEqual(self.locally_managed_netdevs, devs) + j += nr_local def test_add_devices(self): self._create_pci_devices() @@ -650,20 +683,30 @@ class PciDeviceStatsWithTagsTestCase(test.NoDBTestCase): PCI_REMOTE_MANAGED_TAG: 'False'}]), objects.InstancePCIRequest(count=1, spec=[{'vendor_id': '15b3', - 'product_id': '1018', + 'product_id': '101c', PCI_REMOTE_MANAGED_TAG: 'False'}])] devs = self.pci_stats.consume_requests(pci_requests) self.assertEqual(5, len(devs)) - self.assertEqual(set(['0071', '0072', '1018', '101e', '101c']), + self.assertEqual(set(['0071', '0072', '101e', '101c']), set([dev.product_id for dev in devs])) - self._assertPoolContent(self.pci_stats.pools[0], '1137', '0072', 2) - self._assertPoolContent(self.pci_stats.pools[1], '1137', '0071', 3, + self._assertPoolContent(self.pci_stats.pools[0], '1137', '0072', 0) + self._assertPoolContent(self.pci_stats.pools[1], '1137', '0072', 1) + self._assertPoolContent(self.pci_stats.pools[2], '1137', '0072', 1) + + self._assertPoolContent(self.pci_stats.pools[3], '1137', '0071', 0, + physical_network='physnet1') + self._assertPoolContent(self.pci_stats.pools[4], '1137', '0071', 1, + physical_network='physnet1') + self._assertPoolContent(self.pci_stats.pools[5], '1137', '0071', 1, physical_network='physnet1') - self._assertPoolContent(self.pci_stats.pools[2], '15b3', '101e', 0, + self._assertPoolContent(self.pci_stats.pools[6], '1137', '0071', 1, + physical_network='physnet1') + + self._assertPoolContent(self.pci_stats.pools[7], '15b3', '101e', 0, remote_managed='true') - self._assertPoolContent(self.pci_stats.pools[3], '15b3', '101c', 0, + self._assertPoolContent(self.pci_stats.pools[8], '15b3', '101c', 0, remote_managed='false') - self._assertPoolContent(self.pci_stats.pools[4], '15b3', '1018', 0, + self._assertPoolContent(self.pci_stats.pools[9], '15b3', '101c', 0, remote_managed='false') def test_add_device_no_devspec(self): @@ -706,30 +749,267 @@ class PciDeviceStatsWithTagsTestCase(test.NoDBTestCase): self.pci_stats.remove_device(dev2) self._assertPools() - def test_update_device(self): - # Update device type of one of the device from type-PCI to + def test_update_device_splits_the_pool(self): + # Update device type of one of the device from type-VF to # type-PF. Verify if the existing pool is updated and a new # pool is created with dev_type type-PF. - self._create_pci_devices() - dev1 = self.pci_tagged_devices.pop() - dev1.dev_type = 'type-PF' - self.pci_stats.update_device(dev1) - self.assertEqual(6, len(self.pci_stats.pools)) - self._assertPoolContent(self.pci_stats.pools[0], '1137', '0072', - len(self.pci_untagged_devices)) - self.assertEqual(self.pci_untagged_devices, - self.pci_stats.pools[0]['devices']) - self._assertPoolContent(self.pci_stats.pools[1], '1137', '0071', - len(self.pci_tagged_devices), - physical_network='physnet1') - self.assertEqual(self.pci_tagged_devices, - self.pci_stats.pools[1]['devices']) - self._assertPoolContent(self.pci_stats.pools[5], '1137', '0071', - 1, - physical_network='physnet1', - remote_managed='false') - self.assertEqual(dev1, - self.pci_stats.pools[5]['devices'][0]) + vfs = [] + for i in range(3): + dev = objects.PciDevice( + compute_node_id=1, + address="0000:0a:00.%d" % i, + vendor_id="1137", + product_id="0071", + status="available", + dev_type="type-VF", + parent_addr="0000:0a:01.0", + numa_node=0 + ) + vfs.append(dev) + self.pci_stats.add_device(dev) + + self.assertEqual(1, len(self.pci_stats.pools)) + self.assertEqual(3, self.pci_stats.pools[0]["count"]) + self.assertEqual(vfs, self.pci_stats.pools[0]["devices"]) + + dev = vfs.pop() + dev.dev_type = 'type-PF' + dev.parent_addr = None + self.pci_stats.update_device(dev) + self.assertEqual(2, len(self.pci_stats.pools)) + self.assertEqual(2, self.pci_stats.pools[0]["count"]) + self.assertEqual(vfs, self.pci_stats.pools[0]["devices"]) + self.assertEqual(1, self.pci_stats.pools[1]["count"]) + self.assertEqual([dev], self.pci_stats.pools[1]["devices"]) + + def test_only_vfs_from_the_same_parent_are_pooled(self): + pf1_vfs = [] + for i in range(2): + dev = objects.PciDevice( + compute_node_id=1, + address="0000:0a:00.%d" % i, + vendor_id="15b3", + product_id="1018", + status="available", + dev_type="type-VF", + parent_addr="0000:0a:01.0", + numa_node=0 + ) + pf1_vfs.append(dev) + self.pci_stats.add_device(dev) + + pf2_vfs = [] + for i in range(2): + dev = objects.PciDevice( + compute_node_id=1, + address="0000:0b:00.%d" % i, + vendor_id="15b3", + product_id="1018", + status="available", + dev_type="type-VF", + parent_addr="0000:0b:01.0", + numa_node=0 + ) + pf2_vfs.append(dev) + self.pci_stats.add_device(dev) + + self.assertEqual(2, len(self.pci_stats.pools)) + self.assertEqual(2, self.pci_stats.pools[0]["count"]) + self.assertEqual(pf1_vfs, self.pci_stats.pools[0]["devices"]) + self.assertEqual(2, len(self.pci_stats.pools)) + self.assertEqual(2, self.pci_stats.pools[1]["count"]) + self.assertEqual(pf2_vfs, self.pci_stats.pools[1]["devices"]) + + +class PciDeviceStatsPlacementSupportTestCase(test.NoDBTestCase): + + def test_device_spec_rc_and_traits_ignored_during_pooling(self): + """Assert that resource_class and traits from the device spec are not + used as discriminator for pool creation. + """ + device_spec = [ + jsonutils.dumps( + { + "resource_class": "foo", + "address": "*:81:00.1", + "traits": "gold", + } + ), + jsonutils.dumps( + { + "resource_class": "baar", + "address": "*:81:00.2", + "traits": "silver", + } + ), + ] + self.flags(device_spec=device_spec, group="pci") + dev_filter = whitelist.Whitelist(device_spec) + pci_stats = stats.PciDeviceStats( + objects.NUMATopology(), + dev_filter=dev_filter) + pci_dev1 = objects.PciDevice( + vendor_id="dead", + product_id="beef", + address="0000:81:00.1", + parent_addr="0000:81:00.0", + numa_node=0, + dev_type="type-VF", + ) + pci_dev2 = objects.PciDevice( + vendor_id="dead", + product_id="beef", + address="0000:81:00.2", + parent_addr="0000:81:00.0", + numa_node=0, + dev_type="type-VF", + ) + # the two device matched by different device_specs with different + # resource_class and traits fields + pci_stats.add_device(pci_dev1) + pci_stats.add_device(pci_dev2) + + # but they are put in the same pool as all the other fields are + # matching + self.assertEqual(1, len(pci_stats.pools)) + self.assertEqual(2, pci_stats.pools[0]["count"]) + + def test_filter_pools_for_spec_ignores_rc_and_traits_in_spec(self): + """Assert that resource_class and traits are ignored in the pci + request spec during matching the request to pools. + """ + pci_stats = stats.PciDeviceStats(objects.NUMATopology()) + pools = [{"vendor_id": "dead", "product_id": "beef"}] + + matching_pools = pci_stats._filter_pools_for_spec( + pools=pools, + request=objects.InstancePCIRequest( + spec=[ + { + "vendor_id": "dead", + "product_id": "beef", + "resource_class": "foo", + "traits": "blue", + } + ] + ), + ) + + self.assertEqual(pools, matching_pools) + + def test_populate_pools_metadata_from_assigned_devices(self): + device_spec = [ + jsonutils.dumps( + { + "address": "0000:81:00.*", + } + ), + ] + self.flags(device_spec=device_spec, group="pci") + dev_filter = whitelist.Whitelist(device_spec) + pci_stats = stats.PciDeviceStats( + objects.NUMATopology(), + dev_filter=dev_filter) + pci_dev1 = objects.PciDevice( + vendor_id="dead", + product_id="beef", + address="0000:81:00.1", + parent_addr="0000:81:00.0", + numa_node=0, + dev_type="type-VF", + ) + pci_dev2 = objects.PciDevice( + vendor_id="dead", + product_id="beef", + address="0000:81:00.2", + parent_addr="0000:81:00.0", + numa_node=0, + dev_type="type-VF", + ) + pci_stats.add_device(pci_dev1) + pci_dev1.extra_info = {'rp_uuid': uuids.rp1} + pci_stats.add_device(pci_dev2) + pci_dev2.extra_info = {'rp_uuid': uuids.rp1} + + self.assertEqual(1, len(pci_stats.pools)) + + pci_stats.populate_pools_metadata_from_assigned_devices() + + self.assertEqual(uuids.rp1, pci_stats.pools[0]['rp_uuid']) + + def test_populate_pools_metadata_from_assigned_devices_device_without_rp( + self + ): + device_spec = [ + jsonutils.dumps( + { + "address": "0000:81:00.*", + } + ), + ] + self.flags(device_spec=device_spec, group="pci") + dev_filter = whitelist.Whitelist(device_spec) + pci_stats = stats.PciDeviceStats( + objects.NUMATopology(), + dev_filter=dev_filter) + pci_dev1 = objects.PciDevice( + vendor_id="dead", + product_id="beef", + address="0000:81:00.1", + parent_addr="0000:81:00.0", + numa_node=0, + dev_type="type-VF", + ) + pci_stats.add_device(pci_dev1) + + self.assertEqual(1, len(pci_stats.pools)) + + pci_stats.populate_pools_metadata_from_assigned_devices() + + self.assertNotIn('rp_uuid', pci_stats.pools[0]) + + def test_populate_pools_metadata_from_assigned_devices_multiple_rp(self): + device_spec = [ + jsonutils.dumps( + { + "address": "0000:81:00.*", + } + ), + ] + self.flags(device_spec=device_spec, group="pci") + dev_filter = whitelist.Whitelist(device_spec) + pci_stats = stats.PciDeviceStats( + objects.NUMATopology(), + dev_filter=dev_filter) + pci_dev1 = objects.PciDevice( + compute_node_id=1, + vendor_id="dead", + product_id="beef", + address="0000:81:00.1", + parent_addr="0000:81:00.0", + numa_node=0, + dev_type="type-VF", + ) + pci_dev2 = objects.PciDevice( + compute_node_id=1, + vendor_id="dead", + product_id="beef", + address="0000:81:00.2", + parent_addr="0000:81:00.0", + numa_node=0, + dev_type="type-VF", + ) + pci_stats.add_device(pci_dev1) + pci_dev1.extra_info = {'rp_uuid': uuids.rp1} + pci_stats.add_device(pci_dev2) + pci_dev2.extra_info = {'rp_uuid': uuids.rp2} + + self.assertEqual(1, len(pci_stats.pools)) + + self.assertRaises( + ValueError, + pci_stats.populate_pools_metadata_from_assigned_devices, + ) class PciDeviceVFPFStatsTestCase(test.NoDBTestCase): diff --git a/nova/tests/unit/scheduler/test_manager.py b/nova/tests/unit/scheduler/test_manager.py index 9356292918..4e7c0dc008 100644 --- a/nova/tests/unit/scheduler/test_manager.py +++ b/nova/tests/unit/scheduler/test_manager.py @@ -26,6 +26,7 @@ from oslo_utils.fixture import uuidsentinel as uuids from nova import context from nova import exception from nova import objects +from nova.scheduler import filters from nova.scheduler import host_manager from nova.scheduler import manager from nova.scheduler import utils as scheduler_utils @@ -396,9 +397,16 @@ class SchedulerManagerTestCase(test.NoDBTestCase): # assertion at the end of the test. spec_obj.obj_reset_changes(recursive=True) - host_state = mock.Mock(spec=host_manager.HostState, host="fake_host", - uuid=uuids.cn1, cell_uuid=uuids.cell, nodename="fake_node", - limits={}, aggregates=[]) + host_state = mock.Mock( + spec=host_manager.HostState, + host="fake_host", + uuid=uuids.cn1, + cell_uuid=uuids.cell, + nodename="fake_node", + limits={}, + aggregates=[], + allocation_candidates=[], + ) all_host_states = [host_state] mock_get_all_states.return_value = all_host_states @@ -459,20 +467,29 @@ class SchedulerManagerTestCase(test.NoDBTestCase): is_public=True, name="small_flavor"), project_id=uuids.project_id, - instance_group=group) - - host_state = mock.Mock(spec=host_manager.HostState, - host="fake_host", nodename="fake_node", uuid=uuids.cn1, - limits={}, cell_uuid=uuids.cell, instances={}, aggregates=[]) + instance_group=group, + requested_resources=[], + ) + + host_state = mock.Mock( + spec=host_manager.HostState, + host="fake_host", + nodename="fake_node", + uuid=uuids.cn1, + limits={}, + cell_uuid=uuids.cell, + instances={}, + aggregates=[], + allocation_candidates=[], + ) all_host_states = [host_state] mock_get_all_states.return_value = all_host_states - mock_get_hosts.return_value = all_host_states + mock_get_hosts.side_effect = lambda spec_obj, hosts, num: list(hosts) instance_uuids = None ctx = mock.Mock() selected_hosts = self.manager._schedule(ctx, spec_obj, - instance_uuids, mock.sentinel.alloc_reqs_by_rp_uuid, - mock.sentinel.provider_summaries) + instance_uuids, None, mock.sentinel.provider_summaries) mock_get_all_states.assert_called_once_with( ctx.elevated.return_value, spec_obj, @@ -510,14 +527,24 @@ class SchedulerManagerTestCase(test.NoDBTestCase): is_public=True, name="small_flavor"), project_id=uuids.project_id, - instance_group=None) - - host_state = mock.Mock(spec=host_manager.HostState, - host="fake_host", nodename="fake_node", uuid=uuids.cn1, - cell_uuid=uuids.cell1, limits={}, aggregates=[]) + instance_group=None, + requested_resources=[], + ) + + host_state = mock.Mock( + spec=host_manager.HostState, + host="fake_host", + nodename="fake_node", + uuid=uuids.cn1, + cell_uuid=uuids.cell1, + limits={}, + aggregates=[], + allocation_candidates=[], + ) all_host_states = [host_state] mock_get_all_states.return_value = all_host_states - mock_get_hosts.return_value = all_host_states + # simulate that every host passes the filtering + mock_get_hosts.side_effect = lambda spec_obj, hosts, num: list(hosts) mock_claim.return_value = True instance_uuids = [uuids.instance] @@ -583,11 +610,16 @@ class SchedulerManagerTestCase(test.NoDBTestCase): project_id=uuids.project_id, instance_group=None) - host_state = mock.Mock(spec=host_manager.HostState, - host=mock.sentinel.host, uuid=uuids.cn1, cell_uuid=uuids.cell1) + host_state = mock.Mock( + spec=host_manager.HostState, + host=mock.sentinel.host, + uuid=uuids.cn1, + cell_uuid=uuids.cell1, + allocations_candidates=[], + ) all_host_states = [host_state] mock_get_all_states.return_value = all_host_states - mock_get_hosts.return_value = all_host_states + mock_get_hosts.side_effect = lambda spec_obj, hosts, num: list(hosts) mock_claim.return_value = False instance_uuids = [uuids.instance] @@ -604,7 +636,7 @@ class SchedulerManagerTestCase(test.NoDBTestCase): mock_get_all_states.assert_called_once_with( ctx.elevated.return_value, spec_obj, mock.sentinel.provider_summaries) - mock_get_hosts.assert_called_once_with(spec_obj, all_host_states, 0) + mock_get_hosts.assert_called_once_with(spec_obj, mock.ANY, 0) mock_claim.assert_called_once_with(ctx.elevated.return_value, self.manager.placement_client, spec_obj, uuids.instance, alloc_reqs_by_rp_uuid[uuids.cn1][0], @@ -635,18 +667,41 @@ class SchedulerManagerTestCase(test.NoDBTestCase): is_public=True, name="small_flavor"), project_id=uuids.project_id, - instance_group=None) - - host_state = mock.Mock(spec=host_manager.HostState, - host="fake_host", nodename="fake_node", uuid=uuids.cn1, - cell_uuid=uuids.cell1, limits={}, updated='fake') + instance_group=None, + requested_resources=[], + ) + + host_state = mock.Mock( + spec=host_manager.HostState, + host="fake_host", + nodename="fake_node", + uuid=uuids.cn1, + cell_uuid=uuids.cell1, + limits={}, + updated="fake", + allocation_candidates=[], + ) all_host_states = [host_state] mock_get_all_states.return_value = all_host_states - mock_get_hosts.side_effect = [ - all_host_states, # first instance: return all the hosts (only one) - [], # second: act as if no more hosts that meet criteria - all_host_states, # the final call when creating alternates - ] + + calls = [] + + def fake_get_sorted_hosts(spec_obj, hosts, num): + c = len(calls) + calls.append(1) + # first instance: return all the hosts (only one) + if c == 0: + return hosts + # second: act as if no more hosts that meet criteria + elif c == 1: + return [] + # the final call when creating alternates + elif c == 2: + return hosts + else: + raise StopIteration() + + mock_get_hosts.side_effect = fake_get_sorted_hosts mock_claim.return_value = True instance_uuids = [uuids.instance1, uuids.instance2] @@ -679,20 +734,44 @@ class SchedulerManagerTestCase(test.NoDBTestCase): swap=0, vcpus=1), project_id=uuids.project_id, - instance_group=None) - - host_state0 = mock.Mock(spec=host_manager.HostState, - host="fake_host0", nodename="fake_node0", uuid=uuids.cn0, - cell_uuid=uuids.cell, limits={}, aggregates=[]) - host_state1 = mock.Mock(spec=host_manager.HostState, - host="fake_host1", nodename="fake_node1", uuid=uuids.cn1, - cell_uuid=uuids.cell, limits={}, aggregates=[]) - host_state2 = mock.Mock(spec=host_manager.HostState, - host="fake_host2", nodename="fake_node2", uuid=uuids.cn2, - cell_uuid=uuids.cell, limits={}, aggregates=[]) + instance_group=None, + requested_resources=[], + ) + + host_state0 = mock.Mock( + spec=host_manager.HostState, + host="fake_host0", + nodename="fake_node0", + uuid=uuids.cn0, + cell_uuid=uuids.cell, + limits={}, + aggregates=[], + allocation_candidates=[], + ) + host_state1 = mock.Mock( + spec=host_manager.HostState, + host="fake_host1", + nodename="fake_node1", + uuid=uuids.cn1, + cell_uuid=uuids.cell, + limits={}, + aggregates=[], + allocation_candidates=[], + ) + host_state2 = mock.Mock( + spec=host_manager.HostState, + host="fake_host2", + nodename="fake_node2", + uuid=uuids.cn2, + cell_uuid=uuids.cell, + limits={}, + aggregates=[], + allocation_candidates=[], + ) all_host_states = [host_state0, host_state1, host_state2] mock_get_all_states.return_value = all_host_states - mock_get_hosts.return_value = all_host_states + # simulate that every host passes the filtering + mock_get_hosts.side_effect = lambda spec_obj, hosts, num: list(hosts) mock_claim.return_value = True instance_uuids = [uuids.instance0] @@ -744,20 +823,44 @@ class SchedulerManagerTestCase(test.NoDBTestCase): swap=0, vcpus=1), project_id=uuids.project_id, - instance_group=None) - - host_state0 = mock.Mock(spec=host_manager.HostState, - host="fake_host0", nodename="fake_node0", uuid=uuids.cn0, - cell_uuid=uuids.cell, limits={}, aggregates=[]) - host_state1 = mock.Mock(spec=host_manager.HostState, - host="fake_host1", nodename="fake_node1", uuid=uuids.cn1, - cell_uuid=uuids.cell, limits={}, aggregates=[]) - host_state2 = mock.Mock(spec=host_manager.HostState, - host="fake_host2", nodename="fake_node2", uuid=uuids.cn2, - cell_uuid=uuids.cell, limits={}, aggregates=[]) + instance_group=None, + requested_resources=[], + ) + + host_state0 = mock.Mock( + spec=host_manager.HostState, + host="fake_host0", + nodename="fake_node0", + uuid=uuids.cn0, + cell_uuid=uuids.cell, + limits={}, + aggregates=[], + allocation_candidates=[], + ) + host_state1 = mock.Mock( + spec=host_manager.HostState, + host="fake_host1", + nodename="fake_node1", + uuid=uuids.cn1, + cell_uuid=uuids.cell, + limits={}, + aggregates=[], + allocation_candidates=[], + ) + host_state2 = mock.Mock( + spec=host_manager.HostState, + host="fake_host2", + nodename="fake_node2", + uuid=uuids.cn2, + cell_uuid=uuids.cell, + limits={}, + aggregates=[], + allocation_candidates=[], + ) all_host_states = [host_state0, host_state1, host_state2] mock_get_all_states.return_value = all_host_states - mock_get_hosts.return_value = all_host_states + # simulate that every host passes the filtering + mock_get_hosts.side_effect = lambda spec_obj, hosts, num: list(hosts) mock_claim.return_value = True instance_uuids = [uuids.instance0] @@ -814,17 +917,36 @@ class SchedulerManagerTestCase(test.NoDBTestCase): is_public=True, name="small_flavor"), project_id=uuids.project_id, - instance_group=ig, instance_uuid=uuids.instance0) + instance_group=ig, + instance_uuid=uuids.instance0, + requested_resources=[], + ) # Reset the RequestSpec changes so they don't interfere with the # assertion at the end of the test. spec_obj.obj_reset_changes(recursive=True) - hs1 = mock.Mock(spec=host_manager.HostState, host='host1', - nodename="node1", limits={}, uuid=uuids.cn1, - cell_uuid=uuids.cell1, instances={}, aggregates=[]) - hs2 = mock.Mock(spec=host_manager.HostState, host='host2', - nodename="node2", limits={}, uuid=uuids.cn2, - cell_uuid=uuids.cell2, instances={}, aggregates=[]) + hs1 = mock.Mock( + spec=host_manager.HostState, + host="host1", + nodename="node1", + limits={}, + uuid=uuids.cn1, + cell_uuid=uuids.cell1, + instances={}, + aggregates=[], + allocation_candidates=[], + ) + hs2 = mock.Mock( + spec=host_manager.HostState, + host="host2", + nodename="node2", + limits={}, + uuid=uuids.cn2, + cell_uuid=uuids.cell2, + instances={}, + aggregates=[], + allocation_candidates=[], + ) all_host_states = [hs1, hs2] mock_get_all_states.return_value = all_host_states mock_claim.return_value = True @@ -838,13 +960,18 @@ class SchedulerManagerTestCase(test.NoDBTestCase): # _get_sorted_hosts() in the two iterations for each instance in # num_instances visited_instances = set([]) + get_sorted_hosts_called_with_host_states = [] def fake_get_sorted_hosts(_spec_obj, host_states, index): # Keep track of which instances are passed to the filters. visited_instances.add(_spec_obj.instance_uuid) if index % 2: - return [hs1, hs2] - return [hs2, hs1] + s = list(host_states) + get_sorted_hosts_called_with_host_states.append(s) + return s + s = list(host_states) + get_sorted_hosts_called_with_host_states.append(s) + return reversed(s) mock_get_hosts.side_effect = fake_get_sorted_hosts instance_uuids = [ getattr(uuids, 'instance%d' % x) for x in range(num_instances) @@ -871,10 +998,14 @@ class SchedulerManagerTestCase(test.NoDBTestCase): # second time, we pass it the hosts that were returned from # _get_sorted_hosts() the first time sorted_host_calls = [ - mock.call(spec_obj, all_host_states, 0), - mock.call(spec_obj, [hs2, hs1], 1), + mock.call(spec_obj, mock.ANY, 0), + mock.call(spec_obj, mock.ANY, 1), ] mock_get_hosts.assert_has_calls(sorted_host_calls) + self.assertEqual( + all_host_states, get_sorted_hosts_called_with_host_states[0]) + self.assertEqual( + [hs1], get_sorted_hosts_called_with_host_states[1]) # The instance group object should have both host1 and host2 in its # instance group hosts list and there should not be any "changes" to @@ -1168,14 +1299,36 @@ class SchedulerManagerTestCase(test.NoDBTestCase): name="small_flavor"), project_id=uuids.project_id, instance_uuid=uuids.instance_id, - instance_group=None) - - host_state = mock.Mock(spec=host_manager.HostState, host="fake_host", - uuid=uuids.cn1, cell_uuid=uuids.cell, nodename="fake_node", - limits={}, updated="Not None") + instance_group=None, + requested_resources=[], + ) + + host_state = mock.Mock( + spec=host_manager.HostState, + host="fake_host", + uuid=uuids.cn1, + cell_uuid=uuids.cell, + nodename="fake_node", + limits={}, + updated="Not None", + allocation_candidates=[], + ) all_host_states = [host_state] mock_get_all_states.return_value = all_host_states - mock_get_hosts.side_effect = [all_host_states, []] + + calls = [] + + def fake_get_sorted_hosts(spec_obj, hosts, num): + c = len(calls) + calls.append(1) + if c == 0: + return list(hosts) + elif c == 1: + return [] + else: + raise StopIteration + + mock_get_hosts.side_effect = fake_get_sorted_hosts instance_uuids = [uuids.inst1, uuids.inst2] fake_allocs_by_rp = {uuids.cn1: [{}]} @@ -1204,7 +1357,7 @@ class SchedulerManagerTestCase(test.NoDBTestCase): alloc_reqs[hs.uuid] = [{}] mock_get_all_hosts.return_value = all_host_states - mock_sorted.return_value = all_host_states + mock_sorted.side_effect = lambda spec_obj, hosts, num: list(hosts) mock_claim.return_value = True total_returned = num_alternates + 1 self.flags(max_attempts=total_returned, group="scheduler") @@ -1212,14 +1365,14 @@ class SchedulerManagerTestCase(test.NoDBTestCase): for num in range(num_instances)] spec_obj = objects.RequestSpec( - num_instances=num_instances, - flavor=objects.Flavor(memory_mb=512, - root_gb=512, - ephemeral_gb=0, - swap=0, - vcpus=1), - project_id=uuids.project_id, - instance_group=None) + num_instances=num_instances, + flavor=objects.Flavor( + memory_mb=512, root_gb=512, ephemeral_gb=0, swap=0, vcpus=1 + ), + project_id=uuids.project_id, + instance_group=None, + requested_resources=[], + ) dests = self.manager._schedule(self.context, spec_obj, instance_uuids, alloc_reqs, None, return_alternates=True) @@ -1270,11 +1423,24 @@ class SchedulerManagerTestCase(test.NoDBTestCase): alloc_reqs[hs.uuid] = [{}] mock_get_all_hosts.return_value = all_host_states + # There are two instances so _get_sorted_hosts is called once per # instance and then once again before picking alternates. - mock_sorted.side_effect = [all_host_states, - list(reversed(all_host_states)), - all_host_states] + calls = [] + + def fake_get_sorted_hosts(spec_obj, hosts, num): + c = len(calls) + calls.append(1) + if c == 0: + return list(hosts) + elif c == 1: + return list(reversed(all_host_states)) + elif c == 2: + return list(hosts) + else: + raise StopIteration() + + mock_sorted.side_effect = fake_get_sorted_hosts mock_claim.return_value = True total_returned = 3 self.flags(max_attempts=total_returned, group="scheduler") @@ -1282,14 +1448,14 @@ class SchedulerManagerTestCase(test.NoDBTestCase): num_instances = len(instance_uuids) spec_obj = objects.RequestSpec( - num_instances=num_instances, - flavor=objects.Flavor(memory_mb=512, - root_gb=512, - ephemeral_gb=0, - swap=0, - vcpus=1), - project_id=uuids.project_id, - instance_group=None) + num_instances=num_instances, + flavor=objects.Flavor( + memory_mb=512, root_gb=512, ephemeral_gb=0, swap=0, vcpus=1 + ), + project_id=uuids.project_id, + instance_group=None, + requested_resources=[], + ) dests = self.manager._schedule(self.context, spec_obj, instance_uuids, alloc_reqs, None, return_alternates=True) @@ -1323,7 +1489,7 @@ class SchedulerManagerTestCase(test.NoDBTestCase): alloc_reqs[hs.uuid] = [{}] mock_get_all_hosts.return_value = all_host_states - mock_sorted.return_value = all_host_states + mock_sorted.side_effect = lambda spec_obj, hosts, num: list(hosts) mock_claim.return_value = True # Set the total returned to more than the number of available hosts self.flags(max_attempts=max_attempts, group="scheduler") @@ -1331,14 +1497,14 @@ class SchedulerManagerTestCase(test.NoDBTestCase): num_instances = len(instance_uuids) spec_obj = objects.RequestSpec( - num_instances=num_instances, - flavor=objects.Flavor(memory_mb=512, - root_gb=512, - ephemeral_gb=0, - swap=0, - vcpus=1), - project_id=uuids.project_id, - instance_group=None) + num_instances=num_instances, + flavor=objects.Flavor( + memory_mb=512, root_gb=512, ephemeral_gb=0, swap=0, vcpus=1 + ), + project_id=uuids.project_id, + instance_group=None, + requested_resources=[], + ) dests = self.manager._schedule(self.context, spec_obj, instance_uuids, alloc_reqs, None, return_alternates=True) @@ -1521,3 +1687,503 @@ class SchedulerManagerTestCase(test.NoDBTestCase): self.manager._discover_hosts_in_cells(mock.sentinel.context) mock_log_warning.assert_not_called() mock_log_debug.assert_called_once_with(msg) + + +class SchedulerManagerAllocationCandidateTestCase(test.NoDBTestCase): + + class ACRecorderFilter(filters.BaseHostFilter): + """A filter that records what allocation candidates it saw on each host + """ + + def __init__(self): + super().__init__() + self.seen_candidates = [] + + def host_passes(self, host_state, filter_properties): + # record what candidate the filter saw for each host + self.seen_candidates.append(list(host_state.allocation_candidates)) + return True + + class DropFirstFilter(filters.BaseHostFilter): + """A filter that removes one candidate and keeps the rest on each + host + """ + + def host_passes(self, host_state, filter_properties): + host_state.allocation_candidates.pop(0) + return bool(host_state.allocation_candidates) + + @mock.patch.object( + host_manager.HostManager, '_init_instance_info', new=mock.Mock()) + @mock.patch.object( + host_manager.HostManager, '_init_aggregates', new=mock.Mock()) + def setUp(self): + super().setUp() + self.context = context.RequestContext('fake_user', 'fake_project') + self.manager = manager.SchedulerManager() + self.manager.host_manager.weighers = [] + self.request_spec = objects.RequestSpec( + ignore_hosts=[], + force_hosts=[], + force_nodes=[], + requested_resources=[], + ) + + @mock.patch("nova.objects.selection.Selection.from_host_state") + @mock.patch( + "nova.scheduler.manager.SchedulerManager._consume_selected_host", + ) + @mock.patch( + "nova.scheduler.utils.claim_resources", + new=mock.Mock(return_value=True), + ) + @mock.patch("nova.scheduler.manager.SchedulerManager._get_all_host_states") + def test_filters_see_allocation_candidates_for_each_host( + self, + mock_get_all_host_states, + mock_consume, + mock_selection_from_host_state, + ): + # have a single filter configured where we can assert that the filter + # see the allocation_candidates of each host + filter = self.ACRecorderFilter() + self.manager.host_manager.enabled_filters = [filter] + + instance_uuids = [uuids.inst1] + + alloc_reqs_by_rp_uuid = {} + # have two hosts with different candidates + host1 = host_manager.HostState("host1", "node1", uuids.cell1) + host1.uuid = uuids.host1 + alloc_reqs_by_rp_uuid[uuids.host1] = [ + mock.sentinel.host1_a_c_1, + mock.sentinel.host1_a_c_2, + ] + host2 = host_manager.HostState("host2", "node2", uuids.cell1) + host2.uuid = uuids.host2 + alloc_reqs_by_rp_uuid[uuids.host2] = [ + mock.sentinel.host2_a_c_1, + ] + mock_get_all_host_states.return_value = iter([host1, host2]) + + self.manager._schedule( + self.context, + self.request_spec, + instance_uuids, + alloc_reqs_by_rp_uuid, + mock.sentinel.provider_summaries, + mock.sentinel.allocation_request_version, + ) + + # we expect that our filter seen the allocation candidate list of + # each host respectively + self.assertEqual( + [ + alloc_reqs_by_rp_uuid[uuids.host1], + alloc_reqs_by_rp_uuid[uuids.host2], + ], + filter.seen_candidates, + ) + + @mock.patch( + "nova.scheduler.manager.SchedulerManager._consume_selected_host", + ) + @mock.patch( + "nova.scheduler.utils.claim_resources", + new=mock.Mock(return_value=True), + ) + @mock.patch("nova.scheduler.manager.SchedulerManager._get_all_host_states") + def test_scheduler_selects_filtered_a_c_from_hosts_state( + self, + mock_get_all_host_states, + mock_consume, + ): + """Assert that if a filter removes an allocation candidate from a host + then even if that host is selected the removed allocation candidate + is not used by the scheduler. + """ + + self.manager.host_manager.enabled_filters = [self.DropFirstFilter()] + + instance_uuids = [uuids.inst1] + alloc_reqs_by_rp_uuid = {} + # have a host with two candidates + host1 = host_manager.HostState("host1", "node1", uuids.cell1) + host1.uuid = uuids.host1 + alloc_reqs_by_rp_uuid[uuids.host1] = [ + "host1-candidate1", + "host1-candidate2", + ] + mock_get_all_host_states.return_value = iter([host1]) + + result = self.manager._schedule( + self.context, + self.request_spec, + instance_uuids, + alloc_reqs_by_rp_uuid, + mock.sentinel.provider_summaries, + 'fake-alloc-req-version', + ) + # we have requested one instance to be scheduled so expect on set + # of selections + self.assertEqual(1, len(result)) + selections = result[0] + # we did not ask for alternatives so a single selection is expected + self.assertEqual(1, len(selections)) + selection = selections[0] + # we expect that candidate2 is used as candidate1 is dropped by + # the filter + self.assertEqual( + "host1-candidate2", + jsonutils.loads(selection.allocation_request) + ) + + @mock.patch("nova.objects.selection.Selection.from_host_state") + @mock.patch( + "nova.scheduler.manager.SchedulerManager._consume_selected_host", + ) + @mock.patch( + "nova.scheduler.utils.claim_resources", + new=mock.Mock(return_value=True), + ) + @mock.patch("nova.scheduler.manager.SchedulerManager._get_all_host_states") + def test_consecutive_filter_sees_filtered_a_c_list( + self, + mock_get_all_host_states, + mock_consume, + mock_selection_from_host_state, + ): + # create two filters + # 1) DropFirstFilter runs first and drops the first candidate from each + # host + # 2) ACRecorderFilter runs next and records what candidates it saw + recorder_filter = self.ACRecorderFilter() + self.manager.host_manager.enabled_filters = [ + self.DropFirstFilter(), + recorder_filter, + ] + + instance_uuids = [uuids.inst1] + alloc_reqs_by_rp_uuid = {} + # have a host with two candidates + host1 = host_manager.HostState("host1", "node1", uuids.cell1) + host1.uuid = uuids.host1 + alloc_reqs_by_rp_uuid[uuids.host1] = [ + "host1-candidate1", + "host1-candidate2", + ] + mock_get_all_host_states.return_value = iter([host1]) + + self.manager._schedule( + self.context, + self.request_spec, + instance_uuids, + alloc_reqs_by_rp_uuid, + mock.sentinel.provider_summaries, + 'fake-alloc-req-version', + ) + + # we expect that the second filter saw one host with one candidate and + # as candidate1 was already filtered out by the run of the first filter + self.assertEqual( + [["host1-candidate2"]], + recorder_filter.seen_candidates + ) + + @mock.patch( + "nova.scheduler.manager.SchedulerManager._consume_selected_host", + ) + @mock.patch( + "nova.scheduler.utils.claim_resources", + new=mock.Mock(return_value=True), + ) + @mock.patch("nova.scheduler.manager.SchedulerManager._get_all_host_states") + def test_filters_removes_all_a_c_host_is_not_selected( + self, + mock_get_all_host_states, + mock_consume, + ): + # use the filter that always drops the first candidate on each host + self.manager.host_manager.enabled_filters = [self.DropFirstFilter()] + + instance_uuids = [uuids.inst1] + alloc_reqs_by_rp_uuid = {} + # have two hosts + # first with a single candidate + host1 = host_manager.HostState("host1", "node1", uuids.cell1) + host1.uuid = uuids.host1 + alloc_reqs_by_rp_uuid[uuids.host1] = [ + "host1-candidate1", + ] + # second with two candidates + host2 = host_manager.HostState("host2", "node2", uuids.cell1) + host2.uuid = uuids.host2 + alloc_reqs_by_rp_uuid[uuids.host2] = [ + "host2-candidate1", + "host2-candidate2", + ] + mock_get_all_host_states.return_value = iter([host1, host2]) + + result = self.manager._schedule( + self.context, + self.request_spec, + instance_uuids, + alloc_reqs_by_rp_uuid, + mock.sentinel.provider_summaries, + 'fake-alloc-req-version', + ) + # we expect that the first host is not selected as the filter + # removed every candidate from the host + # also we expect that on the second host only candidate2 could have + # been selected + # we asked for one instance, so we expect one set of selections + self.assertEqual(1, len(result)) + selections = result[0] + # we did not ask for alternatives so a single selection is expected + self.assertEqual(1, len(selections)) + selection = selections[0] + # we expect that candidate2 is used as candidate1 is dropped by + # the filter + self.assertEqual(uuids.host2, selection.compute_node_uuid) + self.assertEqual( + "host2-candidate2", + jsonutils.loads(selection.allocation_request) + ) + + @mock.patch( + "nova.scheduler.manager.SchedulerManager._consume_selected_host", + ) + @mock.patch( + "nova.scheduler.utils.claim_resources", + new=mock.Mock(return_value=True), + ) + @mock.patch("nova.scheduler.manager.SchedulerManager._get_all_host_states") + def test_consume_selected_host_sees_updated_request_spec( + self, + mock_get_all_host_states, + mock_consume, + ): + # simulate that nothing is filtered out, by not having any filters + self.manager.host_manager.enabled_filters = [] + + # set up the request spec with a request group to be updated + # by the selected candidate + self.request_spec.requested_resources = [ + objects.RequestGroup( + requester_id=uuids.group_req1, provider_uuids=[] + ) + ] + + instance_uuids = [uuids.inst1] + alloc_reqs_by_rp_uuid = {} + # have single host with a single candidate + # first with a single candidate + host1 = host_manager.HostState("host1", "node1", uuids.cell1) + host1.uuid = uuids.host1 + # simulate that placement fulfilled the above RequestGroup from + # a certain child RP of the host. + alloc_reqs_by_rp_uuid[uuids.host1] = [ + { + "mappings": { + "": [uuids.host1], + uuids.group_req1: [uuids.host1_child_rp], + } + } + ] + mock_get_all_host_states.return_value = iter([host1]) + + # make asserts on the request_spec passed to consume + def assert_request_spec_updated_with_selected_candidate( + selected_host, spec_obj, instance_uuid=None + ): + # we expect that the scheduler updated the request_spec based + # the selected candidate before called consume + self.assertEqual( + [uuids.host1_child_rp], + spec_obj.requested_resources[0].provider_uuids, + ) + + mock_consume.side_effect = ( + assert_request_spec_updated_with_selected_candidate) + + self.manager._schedule( + self.context, + self.request_spec, + instance_uuids, + alloc_reqs_by_rp_uuid, + mock.sentinel.provider_summaries, + 'fake-alloc-req-version', + ) + + mock_consume.assert_called_once() + + @mock.patch( + "nova.scheduler.manager.SchedulerManager._consume_selected_host", + ) + @mock.patch( + "nova.scheduler.utils.claim_resources", + return_value=True, + ) + @mock.patch("nova.scheduler.manager.SchedulerManager._get_all_host_states") + def test_get_alternate_hosts_returns_main_selection_with_claimed_a_c( + self, + mock_get_all_host_states, + mock_claim, + mock_consume, + ): + """Assert that the first (a.k.a main) selection returned for an + instance always maps to the allocation candidate, that was claimed by + the scheduler in placement. + """ + # use the filter that always drops the first candidate on each host + self.manager.host_manager.enabled_filters = [self.DropFirstFilter()] + + instance_uuids = [uuids.inst1] + alloc_reqs_by_rp_uuid = {} + # have one host with 3 candidates each fulfilling a request group + # from different child RP + host1 = host_manager.HostState("host1", "node1", uuids.cell1) + host1.uuid = uuids.host1 + alloc_reqs_by_rp_uuid[uuids.host1] = [ + { + "mappings": { + "": [uuids.host1], + uuids.group_req1: [getattr(uuids, f"host1_child{i}")], + } + } for i in [1, 2, 3] + ] + mock_get_all_host_states.return_value = iter([host1]) + + result = self.manager._schedule( + self.context, + self.request_spec, + instance_uuids, + alloc_reqs_by_rp_uuid, + mock.sentinel.provider_summaries, + 'fake-alloc-req-version', + return_alternates=True, + ) + + # we scheduled one instance + self.assertEqual(1, len(result)) + selections = result[0] + # we did not ask for alternatives + self.assertEqual(1, len(selections)) + selection = selections[0] + self.assertEqual(uuids.host1, selection.compute_node_uuid) + # we expect that host1_child2 candidate is selected + expected_a_c = { + "mappings": { + "": [uuids.host1], + uuids.group_req1: [uuids.host1_child2], + } + } + self.assertEqual( + expected_a_c, + jsonutils.loads(selection.allocation_request), + ) + # and we expect that the same candidate was claimed in placement + mock_claim.assert_called_once_with( + mock.ANY, + self.manager.placement_client, + self.request_spec, + uuids.inst1, + expected_a_c, + allocation_request_version="fake-alloc-req-version", + ) + + @mock.patch( + "nova.scheduler.manager.SchedulerManager._consume_selected_host", + ) + @mock.patch( + "nova.scheduler.utils.claim_resources", + return_value=True, + ) + @mock.patch("nova.scheduler.manager.SchedulerManager._get_all_host_states") + def test_get_alternate_hosts_returns_alts_with_filtered_a_c( + self, + mock_get_all_host_states, + mock_claim, + mock_consume, + ): + """Assert that alternate generation also works based on filtered + candidates. + """ + + class RPFilter(filters.BaseHostFilter): + """A filter that only allows candidates with specific RPs""" + + def __init__(self, allowed_rp_uuids): + self.allowed_rp_uuids = allowed_rp_uuids + + def host_passes(self, host_state, filter_properties): + host_state.allocation_candidates = [ + a_c + for a_c in host_state.allocation_candidates + if a_c["mappings"][uuids.group_req1][0] + in self.allowed_rp_uuids + ] + return True + + instance_uuids = [uuids.inst1] + alloc_reqs_by_rp_uuid = {} + # have 3 hosts each with 2 allocation candidates fulfilling a request + # group from a different child RP + hosts = [] + for i in [1, 2, 3]: + host = host_manager.HostState(f"host{i}", f"node{i}", uuids.cell1) + host.uuid = getattr(uuids, f"host{i}") + alloc_reqs_by_rp_uuid[host.uuid] = [ + { + "mappings": { + "": [host.uuid], + uuids.group_req1: [ + getattr(uuids, f"host{i}_child{j}") + ], + } + } + for j in [1, 2] + ] + hosts.append(host) + mock_get_all_host_states.return_value = iter(hosts) + + # configure a filter that only "likes" host1_child2 and host3_child2 + # RPs. This means host2 is totally out and host1 and host3 only have + # one viable candidate + self.manager.host_manager.enabled_filters = [ + RPFilter(allowed_rp_uuids=[uuids.host1_child2, uuids.host3_child2]) + ] + + result = self.manager._schedule( + self.context, + self.request_spec, + instance_uuids, + alloc_reqs_by_rp_uuid, + mock.sentinel.provider_summaries, + 'fake-alloc-req-version', + return_alternates=True, + ) + # we scheduled one instance + self.assertEqual(1, len(result)) + selections = result[0] + # we expect a main selection and a single alternative + # (host1, and host3) on both selection we expect child2 as selected + # candidate + self.assertEqual(2, len(selections)) + main_selection = selections[0] + self.assertEqual(uuids.host1, main_selection.compute_node_uuid) + self.assertEqual( + [uuids.host1_child2], + jsonutils.loads(main_selection.allocation_request)["mappings"][ + uuids.group_req1 + ], + ) + + alt_selection = selections[1] + self.assertEqual(uuids.host3, alt_selection.compute_node_uuid) + self.assertEqual( + [uuids.host3_child2], + jsonutils.loads(alt_selection.allocation_request)["mappings"][ + uuids.group_req1 + ], + ) diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py index 6ac7ca464e..52aa37ac13 100644 --- a/nova/tests/unit/virt/ironic/test_driver.py +++ b/nova/tests/unit/virt/ironic/test_driver.py @@ -935,6 +935,48 @@ class IronicDriverTestCase(test.NoDBTestCase): expected = { 'CUSTOM_IRON_NFV': { 'total': 1, + 'reserved': 1, + 'min_unit': 1, + 'max_unit': 1, + 'step_size': 1, + 'allocation_ratio': 1.0, + }, + } + mock_nfc.assert_called_once_with(mock.sentinel.nodename) + mock_nr.assert_called_once_with(mock_nfc.return_value) + mock_res_used.assert_called_once_with(mock_nfc.return_value) + mock_res_unavail.assert_called_once_with(mock_nfc.return_value) + result = self.ptree.data(mock.sentinel.nodename).inventory + self.assertEqual(expected, result) + + @mock.patch.object(ironic_driver.IronicDriver, + '_node_resources_used', return_value=True) + @mock.patch.object(ironic_driver.IronicDriver, + '_node_resources_unavailable', return_value=False) + @mock.patch.object(ironic_driver.IronicDriver, '_node_resource') + @mock.patch.object(ironic_driver.IronicDriver, '_node_from_cache') + def test_update_provider_tree_with_rc_occupied_workaround(self, + mock_nfc, mock_nr, mock_res_unavail, mock_res_used): + """Ensure that when a node is used, we report the inventory matching + the consumed resources. + """ + self.flags(skip_reserve_in_use_ironic_nodes=True, + group="workarounds") + mock_nr.return_value = { + 'vcpus': 24, + 'vcpus_used': 24, + 'memory_mb': 1024, + 'memory_mb_used': 1024, + 'local_gb': 100, + 'local_gb_used': 100, + 'resource_class': 'iron-nfv', + } + + self.driver.update_provider_tree(self.ptree, mock.sentinel.nodename) + + expected = { + 'CUSTOM_IRON_NFV': { + 'total': 1, 'reserved': 0, 'min_unit': 1, 'max_unit': 1, @@ -945,7 +987,7 @@ class IronicDriverTestCase(test.NoDBTestCase): mock_nfc.assert_called_once_with(mock.sentinel.nodename) mock_nr.assert_called_once_with(mock_nfc.return_value) mock_res_used.assert_called_once_with(mock_nfc.return_value) - self.assertFalse(mock_res_unavail.called) + mock_res_unavail.assert_called_once_with(mock_nfc.return_value) result = self.ptree.data(mock.sentinel.nodename).inventory self.assertEqual(expected, result) @@ -1016,7 +1058,7 @@ class IronicDriverTestCase(test.NoDBTestCase): mock_nfc.assert_called_once_with(mock.sentinel.nodename) mock_nr.assert_called_once_with(mock_nfc.return_value) mock_res_used.assert_called_once_with(mock_nfc.return_value) - self.assertFalse(mock_res_unavail.called) + mock_res_unavail.assert_called_once_with(mock_nfc.return_value) result = self.ptree.data(mock.sentinel.nodename).traits self.assertEqual(set(), result) @@ -1048,7 +1090,7 @@ class IronicDriverTestCase(test.NoDBTestCase): mock_nfc.assert_called_once_with(mock.sentinel.nodename) mock_nr.assert_called_once_with(mock_nfc.return_value) mock_res_used.assert_called_once_with(mock_nfc.return_value) - self.assertFalse(mock_res_unavail.called) + mock_res_unavail.assert_called_once_with(mock_nfc.return_value) result = self.ptree.data(mock.sentinel.nodename).traits self.assertEqual(set(traits), result) @@ -2500,7 +2542,10 @@ class IronicDriverTestCase(test.NoDBTestCase): @mock.patch.object(cw.IronicClientWrapper, 'call') def test_prepare_for_spawn(self, mock_call): - node = ironic_utils.get_test_node(driver='fake') + node = ironic_utils.get_test_node( + driver='fake', instance_uuid=None, + provision_state=ironic_states.AVAILABLE, + power_state=ironic_states.POWER_OFF) self.mock_conn.get_node.return_value = node instance = fake_instance.fake_instance_obj(self.ctx, node=node.uuid) @@ -2532,7 +2577,10 @@ class IronicDriverTestCase(test.NoDBTestCase): instance) def test_prepare_for_spawn_conflict(self): - node = ironic_utils.get_test_node(driver='fake') + node = ironic_utils.get_test_node( + driver='fake', instance_uuid=None, + provision_state=ironic_states.AVAILABLE, + power_state=ironic_states.POWER_OFF) self.mock_conn.get_node.return_value = node self.mock_conn.update_node.side_effect = sdk_exc.ConflictException instance = fake_instance.fake_instance_obj(self.ctx, node=node.id) @@ -2540,6 +2588,18 @@ class IronicDriverTestCase(test.NoDBTestCase): self.driver.prepare_for_spawn, instance) + def test_prepare_for_spawn_not_available(self): + node = ironic_utils.get_test_node( + driver='fake', instance_uuid=None, + provision_state=ironic_states.CLEANWAIT, + power_state=ironic_states.POWER_OFF) + self.mock_conn.get_node.return_value = node + self.mock_conn.update_node.side_effect = sdk_exc.ConflictException + instance = fake_instance.fake_instance_obj(self.ctx, node=node.id) + self.assertRaises(exception.ComputeResourcesUnavailable, + self.driver.prepare_for_spawn, + instance) + @mock.patch.object(ironic_driver.IronicDriver, '_cleanup_deploy') def test_failed_spawn_cleanup(self, mock_cleanup): node = ironic_utils.get_test_node(driver='fake') diff --git a/nova/tests/unit/virt/libvirt/test_config.py b/nova/tests/unit/virt/libvirt/test_config.py index c4c9359dd8..8f840e8859 100644 --- a/nova/tests/unit/virt/libvirt/test_config.py +++ b/nova/tests/unit/virt/libvirt/test_config.py @@ -1591,7 +1591,7 @@ class LibvirtConfigGuestHostdevPCI(LibvirtConfigBaseTest): </hostdev> """ - def test_config_guest_hosdev_pci(self): + def test_config_guest_hostdev_pci(self): hostdev = config.LibvirtConfigGuestHostdevPCI() hostdev.domain = "1234" hostdev.bus = "11" @@ -1600,7 +1600,7 @@ class LibvirtConfigGuestHostdevPCI(LibvirtConfigBaseTest): xml = hostdev.to_xml() self.assertXmlEqual(self.expected, xml) - def test_parse_guest_hosdev_pci(self): + def test_parse_guest_hostdev_pci(self): xmldoc = self.expected obj = config.LibvirtConfigGuestHostdevPCI() obj.parse_str(xmldoc) @@ -1612,7 +1612,7 @@ class LibvirtConfigGuestHostdevPCI(LibvirtConfigBaseTest): self.assertEqual(obj.slot, '0x22') self.assertEqual(obj.function, '0x3') - def test_parse_guest_hosdev_usb(self): + def test_parse_guest_hostdev_usb(self): xmldoc = """<hostdev mode='subsystem' type='usb'> <source startupPolicy='optional'> <vendor id='0x1234'/> @@ -3181,6 +3181,32 @@ class LibvirtConfigNodeDeviceTest(LibvirtConfigBaseTest): config.LibvirtConfigNodeDeviceMdevInformation) self.assertEqual("nvidia-11", obj.mdev_information.type) self.assertEqual(12, obj.mdev_information.iommu_group) + self.assertIsNone(obj.mdev_information.uuid) + + def test_config_mdev_device_uuid(self): + xmlin = """ + <device> + <name>mdev_b2107403_110c_45b0_af87_32cc91597b8a_0000_41_00_0</name> + <path>/sys/devices/pci0000:40/0000:40:03.1/0000:41:00.0/b2107403-110c-45b0-af87-32cc91597b8a</path> + <parent>pci_0000_41_00_0</parent> + <driver> + <name>vfio_mdev</name> + </driver> + <capability type='mdev'> + <type id='nvidia-442'/> + <uuid>b2107403-110c-45b0-af87-32cc91597b8a</uuid> + <iommuGroup number='57'/> + </capability> + </device>""" + + obj = config.LibvirtConfigNodeDevice() + obj.parse_str(xmlin) + self.assertIsInstance(obj.mdev_information, + config.LibvirtConfigNodeDeviceMdevInformation) + self.assertEqual("nvidia-442", obj.mdev_information.type) + self.assertEqual(57, obj.mdev_information.iommu_group) + self.assertEqual("b2107403-110c-45b0-af87-32cc91597b8a", + obj.mdev_information.uuid) def test_config_vdpa_device(self): xmlin = """ diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index fee87d3bb5..1c5f79dc89 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -9244,6 +9244,26 @@ class LibvirtConnTestCase(test.NoDBTestCase, image_meta)) mock_fsthaw.assert_called_once_with() + def test_set_quiesced_agent_connection_fails(self): + # This is require to mock guest host + self.create_fake_libvirt_mock(lookupByUUIDString=self.fake_lookup) + + with mock.patch.object(FakeVirtDomain, "fsFreeze") as mock_fsfreeze: + error = fakelibvirt.make_libvirtError( + fakelibvirt.libvirtError, + "QEMU guest agent is not connected", + error_code=fakelibvirt.VIR_ERR_AGENT_UNRESPONSIVE) + + mock_fsfreeze.side_effect = error + mock_fsfreeze.error_code = error.get_error_code() + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI()) + instance = objects.Instance(**self.test_instance) + image_meta = objects.ImageMeta.from_dict( + {"properties": {"hw_qemu_guest_agent": "yes", }}) + self.assertRaises(exception.InstanceQuiesceFailed, + drvr._set_quiesced, self.context, instance, image_meta, True) + def test_create_snapshot_metadata(self): base = objects.ImageMeta.from_dict( {'disk_format': 'raw'}) diff --git a/nova/tests/unit/virt/test_netutils.py b/nova/tests/unit/virt/test_netutils.py index de3f451351..fa0e16df19 100644 --- a/nova/tests/unit/virt/test_netutils.py +++ b/nova/tests/unit/virt/test_netutils.py @@ -17,6 +17,17 @@ from nova.virt import netutils class TestNetUtilsTestCase(test.NoDBTestCase): + + def _get_fake_instance_nw_info(self, num_networks, dhcp_server, mtu): + network_info = fake_network.fake_get_instance_nw_info(self, + num_networks) + for vif in network_info: + for subnet in vif['network']['subnets']: + subnet['meta']['dhcp_server'] = dhcp_server + vif['network']['meta']['mtu'] = mtu + + return network_info + def test_get_cached_vifs_with_vlan_no_nw_info(self): # Make sure that an empty dictionary will be returned when # nw_info is None @@ -39,3 +50,15 @@ class TestNetUtilsTestCase(test.NoDBTestCase): expected = {'fa:16:3e:d1:28:e4': '2145'} self.assertEqual(expected, netutils.get_cached_vifs_with_vlan(network_info)) + + def test__get_link_mtu(self): + network_info_dhcp = self._get_fake_instance_nw_info( + 1, '192.168.0.100', 9000) + network_info_no_dhcp = self._get_fake_instance_nw_info( + 1, None, 9000) + + for vif in network_info_dhcp: + self.assertIsNone(netutils._get_link_mtu(vif)) + + for vif in network_info_no_dhcp: + self.assertEqual(9000, netutils._get_link_mtu(vif)) |