Merge "Make default hypervisor hostname compatible with libvirt"

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.