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.

Closes-Bug: #1926693
Change-Id: Iea2533f4c52935b4ecda9ec22fb619c131febfa1

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:
+        pass
+
+    return hypervisor_hostname
+
+
 # TODO(bence romsics): rehome this to neutron_lib.placement.utils
 def default_rp_hypervisors(hypervisors, device_mappings,
                            default_hypervisor=None):
@@ -85,7 +112,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():

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,

neutron/conf/plugins/ml2/drivers/ovs_conf.py

@@ -90,7 +90,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,

neutron/tests/unit/agent/common/test_utils.py

@@ -78,14 +78,92 @@ 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)
+
+
 # TODO(bence romsics): rehome this to neutron_lib
 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']},

releasenotes/notes/bug-1926693-55406915708d59ec.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    `1926693 <https://bugs.launchpad.net/neutron/+bug/1926693>`_
+    The logic to detect the hypervisor hostname, which was introduced by
+    `change 69660 <https://review.opendev.org/c/openstack/neutron/+/696600>`_,
+    has been fixed and now returns the result consistent with libvirt.