Merge "Give instance default hostname if hostname is empty" into stable/kilo

nova/compute/api.py

@@ -623,6 +623,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)
@@ -632,7 +633,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
 
@@ -1361,11 +1365,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

@@ -2780,6 +2780,34 @@ class _ComputeAPIUnitTestMixIn(object):
         api = compute_api.API(skip_policy_check=True)
         api.create(self.context, None, None)
 
+    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

@@ -123,6 +123,40 @@ class GenericUtilsTestCase(test.NoDBTestCase):
             self.assertEqual(data, fake_contents)
             self.assertTrue(self.reload_called)
 
+    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

@@ -570,8 +570,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, unicode):
         hostname = hostname.encode('latin-1', 'ignore')
 
@@ -579,8 +599,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)
 
 
 def read_cached_file(filename, cache_info, reload_func=None):