From d9d884f783417795a05ca8a1c2bf64c9b55b6370 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Fri, 30 Apr 2021 15:12:04 +0900 Subject: [PATCH] Make default hypervisor hostname compatible with libvirt This change ensures that neutron relies on the same logic as libvirt to generate hypervisor hostname, to fix imcompatible hostname format used in Nova and Neutron for resource provider name in some configuration pattens like the one generated by TripleO. Conflicts: neutron/agent/common/utils.py neutron/tests/unit/agent/common/test_utils.py Closes-Bug: #1926693 Change-Id: Iea2533f4c52935b4ecda9ec22fb619c131febfa1 (cherry picked from commit 577217c52d677ba35ca78b87c06302d506f66ff9) --- neutron/agent/common/utils.py | 29 ++++++- .../ml2/drivers/mech_sriov/agent_common.py | 2 +- neutron/conf/plugins/ml2/drivers/ovs_conf.py | 2 +- neutron/tests/unit/agent/common/test_utils.py | 86 ++++++++++++++++++- .../notes/bug-1926693-55406915708d59ec.yaml | 7 ++ 5 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/bug-1926693-55406915708d59ec.yaml diff --git a/neutron/agent/common/utils.py b/neutron/agent/common/utils.py index d744436912f..6c44a382d37 100644 --- a/neutron/agent/common/utils.py +++ b/neutron/agent/common/utils.py @@ -70,6 +70,33 @@ def is_agent_down(heart_beat_time): cfg.CONF.agent_down_time) +def get_hypervisor_hostname(): + """Get hypervisor hostname + + This logic is implemented following the logic of virGetHostnameImpl + in libvirt. + """ + hypervisor_hostname = socket.gethostname() + if (hypervisor_hostname.startswith('localhost') or + '.' in hypervisor_hostname): + return hypervisor_hostname + + try: + addrinfo = socket.getaddrinfo(host=hypervisor_hostname, + port=None, + family=socket.AF_UNSPEC, + flags=socket.AI_CANONNAME) + # getaddrinfo returns a list of 5-tuples with; + # (family, type, proto, canonname, sockaddr) + if (addrinfo and addrinfo[0][3] and + not addrinfo[0][3].startswith('localhost')): + return addrinfo[0][3] + except (OSError, socket.gaierror): + pass + + return hypervisor_hostname + + def default_rp_hypervisors(hypervisors, device_mappings, default_hypervisor=None): """Fill config option 'resource_provider_hypervisors' with defaults. @@ -84,7 +111,7 @@ def default_rp_hypervisors(hypervisors, device_mappings, format. :param default_hypervisor: Default hypervisor hostname. """ - _default_hypervisor = default_hypervisor or socket.gethostname() + _default_hypervisor = default_hypervisor or get_hypervisor_hostname() rv = {} for _physnet, devices in device_mappings.items(): diff --git a/neutron/conf/plugins/ml2/drivers/mech_sriov/agent_common.py b/neutron/conf/plugins/ml2/drivers/mech_sriov/agent_common.py index a46848a6635..5b549f023e7 100644 --- a/neutron/conf/plugins/ml2/drivers/mech_sriov/agent_common.py +++ b/neutron/conf/plugins/ml2/drivers/mech_sriov/agent_common.py @@ -71,7 +71,7 @@ sriov_nic_opts = [ cfg.StrOpt('resource_provider_default_hypervisor', help=_("The default hypervisor name used to locate the parent " "of the resource provider. If this option is not set, " - "socket.gethostname() is used")), + "canonical name is used")), cfg.DictOpt('resource_provider_inventory_defaults', default={'allocation_ratio': 1.0, 'min_unit': 1, diff --git a/neutron/conf/plugins/ml2/drivers/ovs_conf.py b/neutron/conf/plugins/ml2/drivers/ovs_conf.py index aebad1bb9ad..3d1ff52a92b 100644 --- a/neutron/conf/plugins/ml2/drivers/ovs_conf.py +++ b/neutron/conf/plugins/ml2/drivers/ovs_conf.py @@ -89,7 +89,7 @@ ovs_opts = [ cfg.StrOpt('resource_provider_default_hypervisor', help=_("The default hypervisor name used to locate the parent " "of the resource provider. If this option is not set, " - "socket.gethostname() is used")), + "canonical name is used")), cfg.DictOpt('resource_provider_inventory_defaults', default={'allocation_ratio': 1.0, 'min_unit': 1, diff --git a/neutron/tests/unit/agent/common/test_utils.py b/neutron/tests/unit/agent/common/test_utils.py index 14cbf98754c..5f65a5b4884 100644 --- a/neutron/tests/unit/agent/common/test_utils.py +++ b/neutron/tests/unit/agent/common/test_utils.py @@ -79,13 +79,91 @@ class TestLoadInterfaceDriver(base.BaseTestCase): utils.load_interface_driver(self.conf) +class TestGetHypervisorHostname(base.BaseTestCase): + + @mock.patch('socket.getaddrinfo') + @mock.patch('socket.gethostname') + def test_get_hypervisor_hostname_gethostname_fqdn(self, hostname_mock, + addrinfo_mock): + hostname_mock.return_value = 'host.domain' + self.assertEqual( + 'host.domain', + utils.get_hypervisor_hostname()) + addrinfo_mock.assert_not_called() + + @mock.patch('socket.getaddrinfo') + @mock.patch('socket.gethostname') + def test_get_hypervisor_hostname_gethostname_localhost(self, hostname_mock, + addrinfo_mock): + hostname_mock.return_value = 'localhost' + self.assertEqual( + 'localhost', + utils.get_hypervisor_hostname()) + addrinfo_mock.assert_not_called() + + @mock.patch('socket.getaddrinfo') + @mock.patch('socket.gethostname') + def test_get_hypervisor_hostname_getaddrinfo(self, hostname_mock, + addrinfo_mock): + hostname_mock.return_value = 'host' + addrinfo_mock.return_value = [(None, None, None, 'host.domain', None)] + self.assertEqual( + 'host.domain', + utils.get_hypervisor_hostname()) + addrinfo_mock.assert_called_once_with( + host='host', port=None, family=socket.AF_UNSPEC, + flags=socket.AI_CANONNAME) + + @mock.patch('socket.getaddrinfo') + @mock.patch('socket.gethostname') + def test_get_hypervisor_hostname_getaddrinfo_no_canonname(self, + hostname_mock, + addrinfo_mock): + hostname_mock.return_value = 'host' + addrinfo_mock.return_value = [(None, None, None, '', None)] + self.assertEqual( + 'host', + utils.get_hypervisor_hostname()) + addrinfo_mock.assert_called_once_with( + host='host', port=None, family=socket.AF_UNSPEC, + flags=socket.AI_CANONNAME) + + @mock.patch('socket.getaddrinfo') + @mock.patch('socket.gethostname') + def test_get_hypervisor_hostname_getaddrinfo_localhost(self, hostname_mock, + addrinfo_mock): + hostname_mock.return_value = 'host' + addrinfo_mock.return_value = [(None, None, None, + 'localhost', None)] + self.assertEqual( + 'host', + utils.get_hypervisor_hostname()) + addrinfo_mock.assert_called_once_with( + host='host', port=None, family=socket.AF_UNSPEC, + flags=socket.AI_CANONNAME) + + @mock.patch('socket.getaddrinfo') + @mock.patch('socket.gethostname') + def test_get_hypervisor_hostname_getaddrinfo_fail(self, hostname_mock, + addrinfo_mock): + hostname_mock.return_value = 'host' + addrinfo_mock.side_effect = OSError + self.assertEqual( + 'host', + utils.get_hypervisor_hostname()) + addrinfo_mock.assert_called_once_with( + host='host', port=None, family=socket.AF_UNSPEC, + flags=socket.AI_CANONNAME) + + class TestDefaultRpHypervisors(base.BaseTestCase): - def test_defaults(self): - this_host = socket.gethostname() + @mock.patch.object(utils, 'get_hypervisor_hostname', + return_value='thishost') + def test_defaults(self, hostname_mock): self.assertEqual( - {'eth0': this_host, 'eth1': this_host}, + {'eth0': 'thishost', 'eth1': 'thishost'}, utils.default_rp_hypervisors( hypervisors={}, device_mappings={'physnet0': ['eth0', 'eth1']}, @@ -94,7 +172,7 @@ class TestDefaultRpHypervisors(base.BaseTestCase): ) self.assertEqual( - {'eth0': 'thathost', 'eth1': this_host}, + {'eth0': 'thathost', 'eth1': 'thishost'}, utils.default_rp_hypervisors( hypervisors={'eth0': 'thathost'}, device_mappings={'physnet0': ['eth0', 'eth1']}, diff --git a/releasenotes/notes/bug-1926693-55406915708d59ec.yaml b/releasenotes/notes/bug-1926693-55406915708d59ec.yaml new file mode 100644 index 00000000000..7474caefb37 --- /dev/null +++ b/releasenotes/notes/bug-1926693-55406915708d59ec.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + `1926693 `_ + The logic to detect the hypervisor hostname, which was introduced by + `change 69660 `_, + has been fixed and now returns the result consistent with libvirt.