Give instance default hostname if hostname is empty

Instance hostname is generated by displayname, currently, displayname
can be '----', and other Non-ASCII character such as Chinese characters,
sanitize_hostname will return empty in such cases.

This patch adds a helper function _default_host_name to give an instance
default name if sanitize_hostname returns empty hostname. And,
sanitize_hostname will truncated to 63 if the host name is too long.

Also Window, Linux, and Dnsmasq have different limitation:

Windows: 255 (net_bios limit to 15, but window will truncate it)
Linux: 64
Dnsmasq: 63

Due to nova-network will leverage dnsmasq to set hostname, so we chose
63.

Besides, added more test cases to cover sanitize_hostname.

DocImpact
Change-Id: I443b51e0576cf657d96e498d6700bfc34d987301
Closes-Bug: #1495388
Co-authored-by: Alex Xu <hejie.xu@intel.com>

nova/compute/api.py

@@ -636,6 +636,7 @@ class API(base.Base):
             'name': instance.display_name,
             'count': index + 1,
         }
+        original_name = instance.display_name
         try:
             new_name = (CONF.multi_instance_display_name_template %
                         params)
@@ -645,7 +646,10 @@ class API(base.Base):
             new_name = instance.display_name
         instance.display_name = new_name
         if not instance.get('hostname', None):
-            instance.hostname = utils.sanitize_hostname(new_name)
+            if utils.sanitize_hostname(original_name) == "":
+                instance.hostname = self._default_host_name(instance.uuid)
+            else:
+                instance.hostname = utils.sanitize_hostname(new_name)
         instance.save()
         return instance
 
@@ -1399,11 +1403,16 @@ class API(base.Base):
             # Otherwise, it will be built after the template based
             # display_name.
             hostname = display_name
-            instance.hostname = utils.sanitize_hostname(hostname)
+            default_hostname = self._default_host_name(instance.uuid)
+            instance.hostname = utils.sanitize_hostname(hostname,
+                                                        default_hostname)
 
     def _default_display_name(self, instance_uuid):
         return "Server %s" % instance_uuid
 
+    def _default_host_name(self, instance_uuid):
+        return "Server-%s" % instance_uuid
+
     def _populate_instance_for_create(self, context, instance, image,
                                       index, security_groups, instance_type):
         """Build the beginning of a new instance."""

nova/tests/unit/compute/test_compute_api.py

@@ -2965,6 +2965,34 @@ class _ComputeAPIUnitTestMixIn(object):
         filters = mock_get.call_args_list[0][0][1]
         self.assertEqual({'project_id': 'foo'}, filters)
 
+    def test_populate_instance_names_host_name(self):
+        params = dict(display_name="vm1")
+        instance = self._create_instance_obj(params=params)
+        self.compute_api._populate_instance_names(instance, 1)
+        self.assertEqual('vm1', instance.hostname)
+
+    def test_populate_instance_names_host_name_is_empty(self):
+        params = dict(display_name=u'\u865a\u62df\u673a\u662f\u4e2d\u6587')
+        instance = self._create_instance_obj(params=params)
+        self.compute_api._populate_instance_names(instance, 1)
+        self.assertEqual('Server-%s' % instance.uuid, instance.hostname)
+
+    def test_populate_instance_names_host_name_multi(self):
+        params = dict(display_name="vm")
+        instance = self._create_instance_obj(params=params)
+        with mock.patch.object(instance, 'save'):
+            self.compute_api._apply_instance_name_template(self.context,
+                                                           instance, 1)
+            self.assertEqual('vm-2', instance.hostname)
+
+    def test_populate_instance_names_host_name_is_empty_multi(self):
+        params = dict(display_name=u'\u865a\u62df\u673a\u662f\u4e2d\u6587')
+        instance = self._create_instance_obj(params=params)
+        with mock.patch.object(instance, 'save'):
+            self.compute_api._apply_instance_name_template(self.context,
+                                                           instance, 1)
+            self.assertEqual('Server-%s' % instance.uuid, instance.hostname)
+
 
 class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
     def setUp(self):

nova/tests/unit/test_utils.py

@@ -94,6 +94,40 @@ class GenericUtilsTestCase(test.NoDBTestCase):
         hostname = "<}\x1fh\x10e\x08l\x02l\x05o\x12!{>"
         self.assertEqual("hello", utils.sanitize_hostname(hostname))
 
+    def test_hostname_has_default(self):
+        hostname = u"\u7684hello"
+        defaultname = "Server-1"
+        self.assertEqual("hello", utils.sanitize_hostname(hostname,
+                                                          defaultname))
+
+    def test_hostname_empty_has_default(self):
+        hostname = u"\u7684"
+        defaultname = "Server-1"
+        self.assertEqual(defaultname, utils.sanitize_hostname(hostname,
+                                                              defaultname))
+
+    def test_hostname_empty_has_default_too_long(self):
+        hostname = u"\u7684"
+        defaultname = "a" * 64
+        self.assertEqual("a" * 63, utils.sanitize_hostname(hostname,
+                                                           defaultname))
+
+    def test_hostname_empty_no_default(self):
+        hostname = u"\u7684"
+        self.assertEqual("", utils.sanitize_hostname(hostname))
+
+    def test_hostname_empty_minus_period(self):
+        hostname = "---..."
+        self.assertEqual("", utils.sanitize_hostname(hostname))
+
+    def test_hostname_with_space(self):
+        hostname = " a b c "
+        self.assertEqual("a-b-c", utils.sanitize_hostname(hostname))
+
+    def test_hostname_too_long(self):
+        hostname = "a" * 64
+        self.assertEqual(63, len(utils.sanitize_hostname(hostname)))
+
     def test_generate_password(self):
         password = utils.generate_password()
         self.assertTrue([c for c in password if c in '0123456789'])

nova/utils.py

@@ -771,8 +771,28 @@ def make_dev_path(dev, partition=None, base='/dev'):
     return path
 
 
-def sanitize_hostname(hostname):
-    """Return a hostname which conforms to RFC-952 and RFC-1123 specs."""
+def sanitize_hostname(hostname, default_name=None):
+    """Return a hostname which conforms to RFC-952 and RFC-1123 specs except
+       the length of hostname.
+
+       Window, Linux, and Dnsmasq has different limitation:
+
+       Windows: 255 (net_bios limits to 15, but window will truncate it)
+       Linux: 64
+       Dnsmasq: 63
+
+       Due to nova-network will leverage dnsmasq to set hostname, so we chose
+       63.
+
+       """
+
+    def truncate_hostname(name):
+        if len(name) > 63:
+            LOG.warning(_LW("Hostname %(hostname)s is longer than 63, "
+                            "truncate it to %(truncated_name)s"),
+                            {'hostname': name, 'truncated_name': name[:63]})
+        return name[:63]
+
     if isinstance(hostname, six.text_type):
         # Remove characters outside the Unicode range U+0000-U+00FF
         hostname = hostname.encode('latin-1', 'ignore')
@@ -783,8 +803,12 @@ def sanitize_hostname(hostname):
     hostname = re.sub('[^\w.-]+', '', hostname)
     hostname = hostname.lower()
     hostname = hostname.strip('.-')
+    # NOTE(eliqiao): set hostname to default_display_name to avoid
+    # empty hostname
+    if hostname == "" and default_name is not None:
+        return truncate_hostname(default_name)
 
-    return hostname
+    return truncate_hostname(hostname)
 
 
 @contextlib.contextmanager