From db45b1eca8a3db8f1b5153c58b138711ed69c388 Mon Sep 17 00:00:00 2001 From: Eli Qiao Date: Thu, 17 Sep 2015 13:57:24 +0800 Subject: [PATCH] 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 Closes-Bug: #1495388 Co-authored-by: Alex Xu (cherry picked from commit bc6f30de953303604625e84ad2345cfb595170d2) Conflicts: nova/tests/unit/compute/test_compute_api.py nova/tests/unit/test_utils.py nova/utils.py Change-Id: I443b51e0576cf657d96e498d6700bfc34d987301 --- nova/compute/api.py | 13 ++++++-- nova/tests/unit/compute/test_compute_api.py | 28 +++++++++++++++++ nova/tests/unit/test_utils.py | 34 +++++++++++++++++++++ nova/utils.py | 30 ++++++++++++++++-- 4 files changed, 100 insertions(+), 5 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 5937895eb10a..969668aae935 100644 --- a/nova/compute/api.py +++ b/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.""" diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 5b4c2b3e4ace..fc613e9cff38 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/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): diff --git a/nova/tests/unit/test_utils.py b/nova/tests/unit/test_utils.py index 521c7b0e7776..e322aa0b06b5 100644 --- a/nova/tests/unit/test_utils.py +++ b/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']) diff --git a/nova/utils.py b/nova/utils.py index 870ebe228912..39daaedad73c 100644 --- a/nova/utils.py +++ b/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):