From 560317c766afc4a4c4c5017ecfc9ce432fe63ea7 Mon Sep 17 00:00:00 2001 From: Artom Lifshitz Date: Fri, 19 Jul 2019 11:35:24 -0400 Subject: [PATCH] libvirt: move checking CONF.my_ip to init_host() Migrations use the libvirt driver's get_host_ip_addr() method to determine the dest_host field of the migration object. get_host_ip_addr() checks whether CONF.my_ip is actually assigned to one of the host's interfaces. It does so by calling get_machine_ips(), which iterates over all of the host's interfaces. If the host has many interfaces, this can take a long time, and introduces needless delays in processing the migration. get_machine_ips() is only used to print a warning, so this patch moves the get_machine_ips() call to a single method in init_host(). This way, a warning is still emitted at compute service startup, and migration progress is not needlessly slowed down. NOTE(artom) While the following paragraph still applies, the poison patch will not be backported. Stubbing out use of netifaces.interfaces() is still a good thing to do, however. This patch also has a chicken and egg problem with the patch on top of it, which poisons use of netifaces.interfaces() in tests. While this patch fixes all the tests that break with that poison, it starts breaking different tests because of the move of get_machine_ips() into init_host(). Therefore, while not directly related to the bug, this patch also preventatively mocks or stubs out any use of get_machine_ips() that will get poisoned with the subsequent patch. Closes-bug: 1837075 Change-Id: I58a4038b04d5a9c28927d914e71609e4deea3d9f (cherry picked from commit 30d8159d4ee51a26a03de1cb134ea64c6c07ffb2) --- nova/tests/unit/virt/libvirt/fakelibvirt.py | 2 ++ nova/tests/unit/virt/libvirt/test_driver.py | 14 ++++++++++++-- nova/tests/unit/virt/test_virt_drivers.py | 1 + nova/virt/libvirt/driver.py | 14 +++++++++----- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py index 03a70fcea857..51918b6d51f9 100644 --- a/nova/tests/unit/virt/libvirt/fakelibvirt.py +++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py @@ -1772,6 +1772,8 @@ class FakeLibvirtFixture(fixtures.Fixture): self.useFixture( fixtures.MockPatch('nova.virt.libvirt.utils.get_fs_info')) + self.useFixture( + fixtures.MockPatch('nova.compute.utils.get_machine_ips')) # libvirt driver needs to call out to the filesystem to get the # parent_ifname for the SRIOV VFs. diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 28f6983e9618..aa4934789e83 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -894,6 +894,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'resolve_driver_format', imagebackend.Image._get_driver_format) + self.stub_out('nova.compute.utils.get_machine_ips', lambda: []) + self.useFixture(fakelibvirt.FakeLibvirtFixture()) self.test_instance = _create_test_instance() self.test_image_meta = { @@ -13801,16 +13803,22 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch.object(libvirt_driver.LOG, 'warning') @mock.patch('nova.compute.utils.get_machine_ips') - def test_get_host_ip_addr_failure(self, mock_ips, mock_log): + def test_check_my_ip(self, mock_ips, mock_log): mock_ips.return_value = ['8.8.8.8', '75.75.75.75'] drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - drvr.get_host_ip_addr() + drvr._check_my_ip() mock_log.assert_called_once_with(u'my_ip address (%(my_ip)s) was ' u'not found on any of the ' u'interfaces: %(ifaces)s', {'ifaces': '8.8.8.8, 75.75.75.75', 'my_ip': mock.ANY}) + def test_init_host_checks_ip(self): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + with mock.patch.object(drvr, '_check_my_ip') as mock_check: + drvr.init_host('fake-host') + mock_check.assert_called_once_with() + def test_conn_event_handler(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) service_mock = mock.MagicMock() @@ -21446,6 +21454,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): self.assertEqual(set([uuids.mdev1]), drvr._get_existing_mdevs_not_assigned()) + @mock.patch('nova.compute.utils.get_machine_ips', + new=mock.Mock(return_value=[])) @mock.patch.object(nova.privsep.libvirt, 'create_mdev') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_mdev_capable_devices') diff --git a/nova/tests/unit/virt/test_virt_drivers.py b/nova/tests/unit/virt/test_virt_drivers.py index 7b40bea1ff68..7a9f8db559ae 100644 --- a/nova/tests/unit/virt/test_virt_drivers.py +++ b/nova/tests/unit/virt/test_virt_drivers.py @@ -900,6 +900,7 @@ class LibvirtConnTestCase(_VirtDriverTestCase, test.TestCase): # will try to execute some commands which hangs tests so let's just # stub out the unplug call to os-vif since we don't care about it. self.stub_out('os_vif.unplug', lambda a, kw: None) + self.stub_out('nova.compute.utils.get_machine_ips', lambda: []) def test_force_hard_reboot(self): self.flags(wait_soft_reboot_seconds=0, group='libvirt') diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 1b1505c09006..343093501338 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -491,6 +491,8 @@ class LibvirtDriver(driver.ComputeDriver): self._check_file_backed_memory_support() + self._check_my_ip() + if (CONF.libvirt.virt_type == 'lxc' and not (CONF.libvirt.uid_maps and CONF.libvirt.gid_maps)): LOG.warning("Running libvirt-lxc without user namespaces is " @@ -643,6 +645,13 @@ class LibvirtDriver(driver.ComputeDriver): 'Running Nova with file_backed_memory requires ' 'ram_allocation_ratio configured to 1.0') + def _check_my_ip(self): + ips = compute_utils.get_machine_ips() + if CONF.my_ip not in ips: + LOG.warning('my_ip address (%(my_ip)s) was not found on ' + 'any of the interfaces: %(ifaces)s', + {'my_ip': CONF.my_ip, 'ifaces': ", ".join(ips)}) + def _prepare_migration_flags(self): migration_flags = 0 @@ -3279,11 +3288,6 @@ class LibvirtDriver(driver.ComputeDriver): return self._get_console_output_file(instance, console_path) def get_host_ip_addr(self): - ips = compute_utils.get_machine_ips() - if CONF.my_ip not in ips: - LOG.warning('my_ip address (%(my_ip)s) was not found on ' - 'any of the interfaces: %(ifaces)s', - {'my_ip': CONF.my_ip, 'ifaces': ", ".join(ips)}) return CONF.my_ip def get_vnc_console(self, context, instance):