From 1053ae0b61a6c253b5684096b96ccad7a39343ac Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 10 Feb 2021 16:53:48 +0000 Subject: [PATCH] tests: Add 'nova.virt.libvirt.utils.get_arch' stub to fixture By add this to the 'LibvirtFixture', we ensure virtually every test that validates a libvirt thing will have this stubbed automatically. Change-Id: I03febf6ad7d76c7eec818d3b16a3ef8b26dcd84c Signed-off-by: Stephen Finucane --- nova/tests/functional/libvirt/base.py | 8 - nova/tests/unit/virt/libvirt/fakelibvirt.py | 5 + .../tests/unit/virt/libvirt/test_blockinfo.py | 155 ++++++++++++------ nova/tests/unit/virt/libvirt/test_driver.py | 10 +- 4 files changed, 114 insertions(+), 64 deletions(-) diff --git a/nova/tests/functional/libvirt/base.py b/nova/tests/functional/libvirt/base.py index f24add011e..86ee3a1d71 100644 --- a/nova/tests/functional/libvirt/base.py +++ b/nova/tests/functional/libvirt/base.py @@ -21,7 +21,6 @@ import mock from oslo_utils import units from nova import conf -from nova.objects import fields as obj_fields from nova.tests import fixtures as nova_fixtures from nova.tests.functional import integrated_helpers from nova.tests.unit.virt.libvirt import fake_imagebackend @@ -82,13 +81,6 @@ class ServersTestBase(integrated_helpers._IntegratedTestBase): self.mock_conn = _p.start() self.addCleanup(_p.stop) - # Mock the 'get_arch' function as we may need to provide different host - # architectures during some tests. We default to x86_64 - _a = mock.patch('nova.virt.libvirt.utils.get_arch') - self.mock_arch = _a.start() - self.mock_arch.return_value = obj_fields.Architecture.X86_64 - self.addCleanup(_a.stop) - def _setup_compute_service(self): # NOTE(stephenfin): We don't start the compute service here as we wish # to configure the host capabilities first. We instead start the diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py index f2c8535717..b411de1f44 100644 --- a/nova/tests/unit/virt/libvirt/fakelibvirt.py +++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py @@ -1919,6 +1919,11 @@ class FakeLibvirtFixture(fixtures.Fixture): self.useFixture(fixtures.MockPatch( 'nova.virt.libvirt.host.Host._log_host_capabilities')) + # Ensure tests perform the same on all host architectures + self.useFixture(fixtures.MockPatch( + 'nova.virt.libvirt.utils.get_arch', + return_value=obj_fields.Architecture.X86_64)) + disable_event_thread(self) if self.stub_os_vif: diff --git a/nova/tests/unit/virt/libvirt/test_blockinfo.py b/nova/tests/unit/virt/libvirt/test_blockinfo.py index 33c2b4dfc9..a979368ee9 100644 --- a/nova/tests/unit/virt/libvirt/test_blockinfo.py +++ b/nova/tests/unit/virt/libvirt/test_blockinfo.py @@ -243,9 +243,9 @@ class LibvirtBlockInfoTest(test.NoDBTestCase): } self.assertEqual(expect, mapping) - @mock.patch('nova.virt.libvirt.utils.get_arch', - new=mock.Mock(return_value=obj_fields.Architecture.X86_64)) - def test_get_disk_mapping_rescue_with_config(self): + def _test_get_disk_mapping_rescue_with_config( + self, expect_disk_config_rescue, + ): # A simple disk mapping setup, but in rescue mode with a config drive test_instance_with_config = self.test_instance @@ -258,21 +258,34 @@ class LibvirtBlockInfoTest(test.NoDBTestCase): image_meta, rescue=True) - expect_disk_config_rescue = { - 'bus': 'ide', 'dev': 'hda', 'type': 'cdrom'} - if blockinfo.libvirt_utils.get_arch({}) == 'aarch64': - expect_disk_config_rescue['bus'] = 'scsi' - expect_disk_config_rescue['dev'] = 'sda' expect = { - 'disk.rescue': {'bus': 'virtio', 'dev': 'vda', - 'type': 'disk', 'boot_index': '1'}, + 'disk.rescue': { + 'bus': 'virtio', 'dev': 'vda', + 'type': 'disk', 'boot_index': '1', + }, 'disk': {'bus': 'virtio', 'dev': 'vdb', 'type': 'disk'}, 'disk.config.rescue': expect_disk_config_rescue, - 'root': {'bus': 'virtio', 'dev': 'vda', - 'type': 'disk', 'boot_index': '1'}, - } + 'root': { + 'bus': 'virtio', 'dev': 'vda', + 'type': 'disk', 'boot_index': '1', + }, + } self.assertEqual(expect, mapping) + @mock.patch('nova.virt.libvirt.utils.get_arch', + new=mock.Mock(return_value=obj_fields.Architecture.X86_64)) + def test_get_disk_mapping_rescue_with_config__x86_64(self): + self._test_get_disk_mapping_rescue_with_config({ + 'bus': 'ide', 'dev': 'hda', 'type': 'cdrom', + }) + + @mock.patch('nova.virt.libvirt.utils.get_arch', + new=mock.Mock(return_value=obj_fields.Architecture.AARCH64)) + def test_get_disk_mapping_rescue_with_config__aarch64(self): + self._test_get_disk_mapping_rescue_with_config({ + 'bus': 'scsi', 'dev': 'sda', 'type': 'cdrom', + }) + def _test_get_disk_mapping_stable_rescue( self, rescue_props, expected, block_device_info, with_local=False): instance = objects.Instance(**self.test_instance) @@ -570,7 +583,9 @@ class LibvirtBlockInfoTest(test.NoDBTestCase): } self.assertEqual(expect, mapping) - def test_get_disk_mapping_simple_configdrive(self): + def _test_get_disk_mapping_simple_configdrive( + self, expect_bus, expect_dev, + ): # A simple disk mapping setup, but with configdrive added # It's necessary to check if the architecture is power, because # power doesn't have support to ide, and so libvirt translate @@ -581,34 +596,51 @@ class LibvirtBlockInfoTest(test.NoDBTestCase): instance_ref = objects.Instance(**self.test_instance) image_meta = objects.ImageMeta.from_dict(self.test_image_meta) - mapping = blockinfo.get_disk_mapping("kvm", instance_ref, - "virtio", "ide", - image_meta) + mapping = blockinfo.get_disk_mapping( + "kvm", instance_ref, "virtio", "ide", image_meta) # Pick the first drive letter on the bus that is available # as the config drive. Delete the last device hardcode as # the config drive here. - bus_ppc = ("scsi", "sda") - bus_aarch64 = ("scsi", "sda") - expect_bus = {"ppc": bus_ppc, "ppc64": bus_ppc, - "ppc64le": bus_ppc, "aarch64": bus_aarch64} - - bus, dev = expect_bus.get(blockinfo.libvirt_utils.get_arch({}), - ("ide", "hda")) - expect = { - 'disk': {'bus': 'virtio', 'dev': 'vda', - 'type': 'disk', 'boot_index': '1'}, + 'disk': { + 'bus': 'virtio', 'dev': 'vda', + 'type': 'disk', 'boot_index': '1', + }, 'disk.local': {'bus': 'virtio', 'dev': 'vdb', 'type': 'disk'}, - 'disk.config': {'bus': bus, 'dev': dev, 'type': 'cdrom'}, - 'root': {'bus': 'virtio', 'dev': 'vda', - 'type': 'disk', 'boot_index': '1'} - } - + 'disk.config': { + 'bus': expect_bus, 'dev': expect_dev, 'type': 'cdrom', + }, + 'root': { + 'bus': 'virtio', 'dev': 'vda', + 'type': 'disk', 'boot_index': '1', + }, + } self.assertEqual(expect, mapping) - def test_get_disk_mapping_cdrom_configdrive(self): + @mock.patch( + 'nova.virt.libvirt.utils.get_arch', + return_value=obj_fields.Architecture.X86_64) + def test_get_disk_mapping_simple_configdrive__x86_64(self, mock_get_arch): + self._test_get_disk_mapping_simple_configdrive('ide', 'hda') + mock_get_arch.assert_called() + + @mock.patch( + 'nova.virt.libvirt.utils.get_arch', + return_value=obj_fields.Architecture.PPC64) + def test_get_disk_mapping_simple_configdrive__ppc64(self, mock_get_arch): + self._test_get_disk_mapping_simple_configdrive('scsi', 'sda') + mock_get_arch.assert_called() + + @mock.patch( + 'nova.virt.libvirt.utils.get_arch', + return_value=obj_fields.Architecture.AARCH64) + def test_get_disk_mapping_simple_configdrive__aarch64(self, mock_get_arch): + self._test_get_disk_mapping_simple_configdrive('scsi', 'sda') + mock_get_arch.assert_called() + + def _test_get_disk_mapping_cdrom_configdrive(self, expect_bus, expect_dev): # A simple disk mapping setup, with configdrive added as cdrom # It's necessary to check if the architecture is power, because # power doesn't have support to ide, and so libvirt translate @@ -620,29 +652,47 @@ class LibvirtBlockInfoTest(test.NoDBTestCase): instance_ref = objects.Instance(**self.test_instance) image_meta = objects.ImageMeta.from_dict(self.test_image_meta) - mapping = blockinfo.get_disk_mapping("kvm", instance_ref, - "virtio", "ide", - image_meta) - - bus_ppc = ("scsi", "sda") - bus_aarch64 = ("scsi", "sda") - expect_bus = {"ppc": bus_ppc, "ppc64": bus_ppc, - "ppc64le": bus_ppc, "aarch64": bus_aarch64} - - bus, dev = expect_bus.get(blockinfo.libvirt_utils.get_arch({}), - ("ide", "hda")) + mapping = blockinfo.get_disk_mapping( + "kvm", instance_ref, "virtio", "ide", image_meta) expect = { - 'disk': {'bus': 'virtio', 'dev': 'vda', - 'type': 'disk', 'boot_index': '1'}, + 'disk': { + 'bus': 'virtio', 'dev': 'vda', + 'type': 'disk', 'boot_index': '1', + }, 'disk.local': {'bus': 'virtio', 'dev': 'vdb', 'type': 'disk'}, - 'disk.config': {'bus': bus, 'dev': dev, 'type': 'cdrom'}, - 'root': {'bus': 'virtio', 'dev': 'vda', - 'type': 'disk', 'boot_index': '1'} + 'disk.config': { + 'bus': expect_bus, 'dev': expect_dev, 'type': 'cdrom', + }, + 'root': { + 'bus': 'virtio', 'dev': 'vda', + 'type': 'disk', 'boot_index': '1', + } } self.assertEqual(expect, mapping) + @mock.patch( + 'nova.virt.libvirt.utils.get_arch', + return_value=obj_fields.Architecture.X86_64) + def test_get_disk_mapping_cdrom_configdrive__x86_64(self, mock_get_arch): + self._test_get_disk_mapping_cdrom_configdrive('ide', 'hda') + mock_get_arch.assert_called() + + @mock.patch( + 'nova.virt.libvirt.utils.get_arch', + return_value=obj_fields.Architecture.PPC64) + def test_get_disk_mapping_cdrom_configdrive__ppc64(self, mock_get_arch): + self._test_get_disk_mapping_cdrom_configdrive('scsi', 'sda') + mock_get_arch.assert_called() + + @mock.patch( + 'nova.virt.libvirt.utils.get_arch', + return_value=obj_fields.Architecture.AARCH64) + def test_get_disk_mapping_cdrom_configdrive__aarch64(self, mock_get_arch): + self._test_get_disk_mapping_cdrom_configdrive('scsi', 'sda') + mock_get_arch.assert_called() + def test_get_disk_mapping_disk_configdrive(self): # A simple disk mapping setup, with configdrive added as disk @@ -1402,7 +1452,10 @@ class DefaultDeviceNamesTestCase(test.NoDBTestCase): for patcher in self.patchers: patcher.stop() - def _test_default_device_names(self, eph, swap, bdm): + @mock.patch( + 'nova.virt.libvirt.utils.get_arch', + return_value=obj_fields.Architecture.X86_64) + def _test_default_device_names(self, eph, swap, bdm, mock_get_arch): bdms = eph + swap + bdm bdi = driver.get_block_device_info(self.instance, bdms) blockinfo.default_device_names(self.virt_type, @@ -1411,6 +1464,8 @@ class DefaultDeviceNamesTestCase(test.NoDBTestCase): bdi, self.image_meta) + mock_get_arch.assert_called() + def test_only_block_device_mapping(self): # Test no-op original_bdm = copy.deepcopy(self.block_device_mapping) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 351b4edd4a..efe470c6c3 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -1402,17 +1402,15 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch.object(libvirt_driver.LibvirtDriver, '_register_instance_machine_type', new=mock.Mock()) - @mock.patch.object(fields.Architecture, "from_host", - return_value=fields.Architecture.PPC64) - def test_min_version_ppc_ok(self, mock_arch): + def test_min_version_ppc_ok(self): + self.mock_get_arch.return_value = fields.Architecture.PPC64 drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) drvr.init_host("dummyhost") @mock.patch.object(libvirt_driver.LibvirtDriver, '_register_instance_machine_type', new=mock.Mock()) - @mock.patch.object(fields.Architecture, "from_host", - return_value=fields.Architecture.S390X) - def test_min_version_s390_ok(self, mock_arch): + def test_min_version_s390_ok(self): + self.mock_get_arch.return_value = fields.Architecture.S390X drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) drvr.init_host("dummyhost")