From 3cf6667c503a5c27c85c2129e4495cf8bf53d66c Mon Sep 17 00:00:00 2001 From: melanie witt Date: Thu, 20 Feb 2025 20:44:04 +0000 Subject: [PATCH] Reproducer for bug 2098892 Change I60d6f04d374e9ede5895a43b7a75e955b0fea3c5 added tpool.Proxy wrapping to the listDevices() and listAllDevices() methods but introduced a regression for listDevices() that led to an error in update_available_resource(): TypeError: virNodeDeviceLookupByName() argument 2 must be str or None, not Proxy The error was not caught by unit or functional testing because those test environments intentionally do not import the libvirt Python module -- so mocked code in the LibvirtFixture runs instead. Also, the update_available_resource() method has a 'except Exception:' at the end which logs an error but does not re-raise. So it would not cause a functional test to fail. This adds a functional test to reproduce the bug and adds a keyword arg to the test _run_periodics() method to specify whether it should raise an exception if an error is logged. Related-Bug: #2098892 Change-Id: I3a3dda57f2181b24bd6589ac7bb8160014ab2396 --- nova/test.py | 11 +- .../regressions/test_bug_2098892.py | 119 ++++++++++++++++++ 2 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 nova/tests/functional/regressions/test_bug_2098892.py diff --git a/nova/test.py b/nova/test.py index faf02ea4532f..0ddd928904c2 100644 --- a/nova/test.py +++ b/nova/test.py @@ -496,7 +496,7 @@ class TestCase(base.BaseTestCase): self.computes[host] = compute return compute - def _run_periodics(self): + def _run_periodics(self, raise_on_error=False): """Run the update_available_resource task on every compute manager This runs periodics on the computes in an undefined order; some child @@ -511,6 +511,15 @@ class TestCase(base.BaseTestCase): with context.target_cell( ctx, self.host_mappings[host].cell_mapping) as cctxt: compute.manager.update_available_resource(cctxt) + + if raise_on_error: + if 'Traceback (most recent call last' in self.stdlog.logger.output: + # Get the last line of the traceback, for example: + # TypeError: virNodeDeviceLookupByName() argument 2 must be + # str or None, not Proxy + last_tb_line = self.stdlog.logger.output.splitlines()[-1] + raise TestingException(last_tb_line) + LOG.info('Finished with periodics') def restart_compute_service(self, compute, keep_hypervisor_state=True): diff --git a/nova/tests/functional/regressions/test_bug_2098892.py b/nova/tests/functional/regressions/test_bug_2098892.py new file mode 100644 index 000000000000..909a77bc92cd --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_2098892.py @@ -0,0 +1,119 @@ +# 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 import test +from nova.tests.fixtures import libvirt as fakelibvirt +from nova.tests.functional.libvirt import test_vgpu + + +class VGPUTestsListDevices(test_vgpu.VGPUTestBase): + """Regression test for bug 2098892. + + Test that nodeDeviceLookupByName() is called with valid types to prevent: + + File "/usr/lib64/python3.9/site-packages/libvirt.py", line 5201, in + nodeDeviceLookupByName ret = + libvirtmod.virNodeDeviceLookupByName(self._o, name) + TypeError: virNodeDeviceLookupByName() argument 2 must be str or None, + not Proxy + + in the future. This test relies on the LibvirtFixture checking for the + correct types in its nodeDeviceLookupByName() method and raising TypeError + if they are invalid. + + We don't test this by importing the libvirt module because the libvirt + module is forbidden to be imported into our test environment. It is + excluded from test-requirements.txt and we also use the + ImportModulePoisonFixture in nova/test.py to prevent use of modules such as + libvirt. + """ + + def setUp(self): + super().setUp() + + # Start compute supporting only nvidia-11 + self.flags( + enabled_mdev_types=fakelibvirt.NVIDIA_11_VGPU_TYPE, + group='devices') + + self.start_compute_with_vgpu('host1') + + def fake_nodeDeviceLookupByName(self, name): + # See bug https://bugs.launchpad.net/nova/+bug/2098892 + # We don't test this by importing the libvirt module because the + # libvirt module is forbidden to be imported into our test + # environment. It is excluded from test-requirements.txt and we + # also use the ImportModulePoisonFixture in nova/test.py to prevent + # use of modules such as libvirt. + if not isinstance(name, str) and name is not None: + raise TypeError( + 'virNodeDeviceLookupByName() argument 2 must be str or ' + f'None, not {type(name)}') + + # FIXME(melwitt): We need to patch this only for this test because if + # we add it to the LibvirtFixture right away, it will cause the + # following additional tests to fail: + # + # nova.tests.functional.libvirt.test_reshape.VGPUReshapeTests + # test_create_servers_with_vgpu + # + # nova.tests.functional.libvirt.test_vgpu.DifferentMdevClassesTests + # test_create_servers_with_different_mdev_classes + # test_resize_servers_with_mlx5 + # + # nova.tests.functional.libvirt.test_vgpu.VGPULimitMultipleTypesTests + # test_create_servers_with_vgpu + # + # nova.tests.functional.libvirt.test_vgpu.VGPULiveMigrationTests + # test_live_migrate_server + # test_live_migration_fails_on_old_source + # test_live_migration_fails_due_to_non_supported_mdev_types + # test_live_migration_fails_on_old_destination + # + # nova.tests.functional.libvirt. + # test_vgpu.VGPULiveMigrationTestsLMFailed + # test_live_migrate_server + # test_live_migration_fails_on_old_source + # test_live_migration_fails_due_to_non_supported_mdev_types + # test_live_migration_fails_on_old_destination + # + # nova.tests.functional.libvirt.test_vgpu.VGPUMultipleTypesTests + # test_create_servers_with_specific_type + # test_create_servers_with_vgpu + # + # nova.tests.functional.libvirt.test_vgpu.VGPUTests + # test_multiple_instance_create + # test_create_servers_with_vgpu + # test_create_server_with_two_vgpus_isolated + # test_resize_servers_with_vgpu + # + # nova.tests.functional.libvirt.test_vgpu.VGPUTestsLibvirt7_3 + # test_create_servers_with_vgpu + # test_create_server_with_two_vgpus_isolated + # test_resize_servers_with_vgpu + # test_multiple_instance_create + # + # nova.tests.functional.regressions. + # test_bug_1951656.VGPUTestsLibvirt7_7 + # test_create_servers_with_vgpu + self.stub_out( + 'nova.tests.fixtures.libvirt.Connection.nodeDeviceLookupByName', + fake_nodeDeviceLookupByName) + + def test_update_available_resource(self): + # We only want to verify no errors were logged by + # update_available_resource (logging under the 'except Exception:'). + # FIXME(melwitt): This currently will log an error and traceback + # because of the bug. Update this when the bug is fixed. + e = self.assertRaises( + test.TestingException, self._run_periodics, raise_on_error=True) + self.assertIn('TypeError', str(e))