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.