cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Ryan Harper has proposed merging ~raharper/cloud-init:default-lang-c-utf8 into cloud-init:master.

Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/329152

distro: allow distro to specify a default locale
    
Currently the cloud-init default locale (en_US.UTF-8) is set by
the base datasource class.  This patch allows a distro to overide
the fallback value with one that's available in the distro.  For
Debian and Ubuntu, the C.UTF-8 lang is available and built-in to
cloud-images.  Adjust apply_locale logic to skip locale-regen if
the specified LANG value is C.UTF-8, it does not require regeneration.
Further add unittests to exercise the default paths for Ubuntu and
non-ubuntu paths to validate they get the LANG expected.


-- 
Your team cloud-init commiters is requested to review the proposed merge of ~raharper/cloud-init:default-lang-c-utf8 into cloud-init:master.

diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index 1fd48a7..ac4b2a4 100755
--- a/cloudinit/distros/__init__.py
+++ b/cloudinit/distros/__init__.py
@@ -188,6 +188,9 @@ class Distro(object):
     def _get_localhost_ip(self):
         return "127.0.0.1"
 
+    def get_locale(self):
+        raise NotImplementedError()
+
     @abc.abstractmethod
     def _read_hostname(self, filename, default=None):
         raise NotImplementedError()
diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py
index abfb81f..b2904e8 100644
--- a/cloudinit/distros/debian.py
+++ b/cloudinit/distros/debian.py
@@ -127,6 +127,10 @@ class Distro(distros.Distro):
         # Note: http://www.leonardoborda.com/blog/127-0-1-1-ubuntu-debian/
         return "127.0.1.1"
 
+    def get_locale(self):
+        # Debian and Ubuntu include C.UTF-8 Lang support in their images
+        return 'C.UTF-8'
+
     def set_timezone(self, tz):
         distros.set_etc_timezone(tz=tz, tz_file=self._find_tz_file(tz))
 
@@ -237,7 +241,7 @@ def apply_locale(locale, sys_path=LOCALE_CONF_FN, keyname='LANG'):
 
     if os.path.exists(sys_path):
         locale_content = util.load_file(sys_path)
-        # if LANG isn't present, regen
+        # if LANG is set and matches locale, no need to regenerate
         sys_defaults = util.load_shell_content(locale_content)
         sys_val = sys_defaults.get(keyname, "")
         if sys_val.lower() == locale.lower():
@@ -246,9 +250,17 @@ def apply_locale(locale, sys_path=LOCALE_CONF_FN, keyname='LANG'):
                 keyname, sys_val, locale)
             return
 
-    util.subp(['locale-gen', locale], capture=False)
+    # Update system configuration (not found, or not the same)
     util.subp(
         ['update-locale', '--locale-file=' + sys_path,
          '%s=%s' % (keyname, locale)], capture=False)
 
+    # special case for C.UTF-8, it does not need to be regenerated
+    if locale.lower() == 'c.utf-8':
+        return
+
+    # finally, trigger regeneration
+    util.subp(['locale-gen', locale], capture=False)
+
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
index 952caf3..42634e7 100644
--- a/cloudinit/sources/__init__.py
+++ b/cloudinit/sources/__init__.py
@@ -150,7 +150,15 @@ class DataSource(object):
         return None
 
     def get_locale(self):
-        return 'en_US.UTF-8'
+        """Default locale is en_US.UTF-8, but allow distros to override"""
+        locale = 'en_US.UTF-8'
+        try:
+            print('asking distro for locale')
+            locale = self.distro.get_locale()
+        except NotImplementedError:
+            print('not implemented, using cloud-init default')
+            pass
+        return locale
 
     @property
     def availability_zone(self):
diff --git a/tests/unittests/test_distros/test_debian.py b/tests/unittests/test_distros/test_debian.py
index 2330ad5..8096694 100644
--- a/tests/unittests/test_distros/test_debian.py
+++ b/tests/unittests/test_distros/test_debian.py
@@ -17,6 +17,16 @@ class TestDebianApplyLocale(CiTestCase):
         apply_locale(locale, sys_path=spath)
         m_subp.assert_not_called()
 
+    def test_no_regen_on_c_utf8(self, m_subp):
+        """If locale is set to C.UTF8, do not attempt to call locale-gen"""
+        spath = self.tmp_path("default-locale")
+        m_subp.return_value = (None, None)
+        locale = 'C.UTF-8'
+        apply_locale(locale, sys_path=spath)
+        self.assertEqual(
+            [['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale]],
+            [p[0][0] for p in m_subp.call_args_list])
+
     def test_rerun_if_different(self, m_subp):
         """If system has different locale, locale-gen should be called."""
         spath = self.tmp_path("default-locale")
@@ -25,8 +35,8 @@ class TestDebianApplyLocale(CiTestCase):
         util.write_file(spath, 'LANG=fr_FR.UTF-8', omode="w")
         apply_locale(locale, sys_path=spath)
         self.assertEqual(
-            [['locale-gen', locale],
-             ['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale]],
+            [['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale],
+             ['locale-gen', locale]],
             [p[0][0] for p in m_subp.call_args_list])
 
     def test_rerun_if_no_file(self, m_subp):
@@ -36,8 +46,8 @@ class TestDebianApplyLocale(CiTestCase):
         locale = 'en_US.UTF-8'
         apply_locale(locale, sys_path=spath)
         self.assertEqual(
-            [['locale-gen', locale],
-             ['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale]],
+            [['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale],
+             ['locale-gen', locale]],
             [p[0][0] for p in m_subp.call_args_list])
 
     def test_rerun_on_unset_system_locale(self, m_subp):
@@ -48,8 +58,8 @@ class TestDebianApplyLocale(CiTestCase):
         util.write_file(spath, 'LANG=', omode="w")
         apply_locale(locale, sys_path=spath)
         self.assertEqual(
-            [['locale-gen', locale],
-             ['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale]],
+            [['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale],
+             ['locale-gen', locale]],
             [p[0][0] for p in m_subp.call_args_list])
 
     def test_rerun_on_mismatched_keys(self, m_subp):
@@ -60,9 +70,8 @@ class TestDebianApplyLocale(CiTestCase):
         util.write_file(spath, 'LANG=', omode="w")
         apply_locale(locale, sys_path=spath, keyname='LC_ALL')
         self.assertEqual(
-            [['locale-gen', locale],
-             ['update-locale', '--locale-file=' + spath,
-              'LC_ALL=%s' % locale]],
+            [['update-locale', '--locale-file=' + spath, 'LC_ALL=%s' % locale],
+             ['locale-gen', locale]],
             [p[0][0] for p in m_subp.call_args_list])
 
     def test_falseish_locale_raises_valueerror(self, m_subp):
diff --git a/tests/unittests/test_distros/test_generic.py b/tests/unittests/test_distros/test_generic.py
index c9be277..1972a7a 100644
--- a/tests/unittests/test_distros/test_generic.py
+++ b/tests/unittests/test_distros/test_generic.py
@@ -228,5 +228,19 @@ class TestGenericDistro(helpers.FilesystemMockingTestCase):
         os.symlink('/', '/run/systemd/system')
         self.assertFalse(d.uses_systemd())
 
+    def test_get_locale_ubuntu(self):
+        """Test ubuntu distro returns locale set to C.UTF-8"""
+        cls = distros.fetch("ubuntu")
+        d = cls("ubuntu", {}, None)
+        locale = d.get_locale()
+        self.assertEqual('C.UTF-8', locale)
+
+    def test_get_locale_rhel(self):
+        """Test rhel distro returns NotImplementedError exception"""
+        cls = distros.fetch("rhel")
+        d = cls("rhel", {}, None)
+        with self.assertRaises(NotImplementedError):
+            d.get_locale()
+
 
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_handler/test_handler_locale.py b/tests/unittests/test_handler/test_handler_locale.py
index e9a810c..3c860c5 100644
--- a/tests/unittests/test_handler/test_handler_locale.py
+++ b/tests/unittests/test_handler/test_handler_locale.py
@@ -20,6 +20,8 @@ from configobj import ConfigObj
 from six import BytesIO
 
 import logging
+import mock
+import os
 import shutil
 import tempfile
 
@@ -27,6 +29,9 @@ LOG = logging.getLogger(__name__)
 
 
 class TestLocale(t_help.FilesystemMockingTestCase):
+
+    with_logs = True
+
     def setUp(self):
         super(TestLocale, self).setUp()
         self.new_root = tempfile.mkdtemp()
@@ -54,4 +59,40 @@ class TestLocale(t_help.FilesystemMockingTestCase):
         n_cfg = ConfigObj(BytesIO(contents))
         self.assertEqual({'RC_LANG': cfg['locale']}, dict(n_cfg))
 
+    def test_set_locale_sles_default(self):
+        cfg = {}
+        cc = self._get_cloud('sles')
+        cc_locale.handle('cc_locale', cfg, cc, LOG, [])
+
+        contents = util.load_file('/etc/sysconfig/language', decode=False)
+        n_cfg = ConfigObj(BytesIO(contents))
+        self.assertEqual({'RC_LANG': 'en_US.UTF-8'}, dict(n_cfg))
+
+    def test_locale_update_config_if_different_than_default(self):
+        """Test cc_locale writes updates conf if different than default"""
+        locale_conf = os.path.join(self.new_root, "etc/default/locale")
+        util.write_file(locale_conf, 'LANG="en_US.UTF-8"\n')
+        cfg = {}
+        cc = self._get_cloud('ubuntu')
+        with mock.patch('cloudinit.distros.debian.util.subp') as m_subp:
+            with mock.patch('cloudinit.distros.debian.LOCALE_CONF_FN',
+                            locale_conf):
+                cc_locale.handle('cc_locale', cfg, cc, LOG, [])
+                m_subp.assert_called_with(['update-locale',
+                                           '--locale-file=%s' % locale_conf,
+                                           'LANG=C.UTF-8'], capture=False)
+
+    def test_locale_rhel_defaults_en_us_utf8(self):
+        """Test cc_locale gets en_US.UTF-8 from distro get_locale fallback"""
+        cfg = {}
+        cc = self._get_cloud('rhel')
+        update_sysconfig = 'cloudinit.distros.rhel_util.update_sysconfig_file'
+        with mock.patch.object(cc.distro, 'uses_systemd') as m_use_sd:
+            m_use_sd.return_value = True
+            with mock.patch(update_sysconfig) as m_update_syscfg:
+                cc_locale.handle('cc_locale', cfg, cc, LOG, [])
+                m_update_syscfg.assert_called_with('/etc/locale.conf',
+                                                   {'LANG': 'en_US.UTF-8'})
+
+
 # vi: ts=4 expandtab