From 7e76c57b590c7c2c13f7b1a2a8b5b7d4f2d18396 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Wed, 16 Aug 2017 16:50:07 -0500 Subject: 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 but continues to respect an image which has preconfigured a locale. - Distro object now has a get_locale method which will return a preconfigure locale setting by checking the distros locale system configuration file. If not set or not present, return the default locale of en_US.UTF-8 which retains behavior of all previous cloud-init releases. - Apply locale now handles regenerating locales or system configuration files as needed. - Adjust apply_locale logic to skip locale-regen if the specified LANG value is C.UTF-8,C, or POSIX; they do not require regeneration. - Further add unittests to exercise the default paths for Ubuntu and non-ubuntu paths to validate they get the LANG expected. --- cloudinit/distros/__init__.py | 3 + cloudinit/distros/debian.py | 94 ++++++++++++++++------ cloudinit/sources/__init__.py | 9 ++- tests/unittests/test_distros/test_debian.py | 66 +++++++++------ tests/unittests/test_distros/test_generic.py | 16 ++++ tests/unittests/test_handler/test_handler_debug.py | 11 ++- .../unittests/test_handler/test_handler_locale.py | 48 +++++++++++ 7 files changed, 195 insertions(+), 52 deletions(-) diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index 807b3ea2..b714b9ab 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 abfb81f4..33cc0bf1 100644 --- a/cloudinit/distros/debian.py +++ b/cloudinit/distros/debian.py @@ -61,11 +61,49 @@ class Distro(distros.Distro): # should only happen say once per instance...) self._runner = helpers.Runners(paths) self.osfamily = 'debian' + self.default_locale = 'en_US.UTF-8' + self.system_locale = None - def apply_locale(self, locale, out_fn=None): + def get_locale(self): + """Return the default locale if set, else use default locale""" + + # read system locale value + if not self.system_locale: + self.system_locale = read_system_locale() + + # Return system_locale setting if valid, else use default locale + return (self.system_locale if self.system_locale else + self.default_locale) + + def apply_locale(self, locale, out_fn=None, keyname='LANG'): + """Apply specified locale to system, regenerate if specified locale + differs from system default.""" if not out_fn: out_fn = LOCALE_CONF_FN - apply_locale(locale, out_fn) + + if not locale: + raise ValueError('Failed to provide locale value.') + + # Only call locale regeneration if needed + # Update system locale config with specified locale if needed + distro_locale = self.get_locale() + conf_fn_exists = os.path.exists(out_fn) + sys_locale_unset = False if self.system_locale else True + need_regen = (locale.lower() != distro_locale.lower() or + not conf_fn_exists or sys_locale_unset) + need_conf = not conf_fn_exists or need_regen or sys_locale_unset + + if need_regen: + regenerate_locale(locale, out_fn, keyname=keyname) + else: + LOG.debug( + "System has '%s=%s' requested '%s', skipping regeneration.", + keyname, self.system_locale, locale) + + if need_conf: + update_locale_conf(locale, out_fn, keyname=keyname) + # once we've updated the system config, invalidate cache + self.system_locale = None def install_packages(self, pkglist): self.update_package_sources() @@ -218,37 +256,47 @@ def _maybe_remove_legacy_eth0(path="/etc/network/interfaces.d/eth0.cfg"): LOG.warning(msg) -def apply_locale(locale, sys_path=LOCALE_CONF_FN, keyname='LANG'): - """Apply the locale. - - Run locale-gen for the provided locale and set the default - system variable `keyname` appropriately in the provided `sys_path`. - - If sys_path indicates that `keyname` is already set to `locale` - then no changes will be made and locale-gen not called. - This allows images built with a locale already generated to not re-run - locale-gen which can be very heavy. - """ - if not locale: - raise ValueError('Failed to provide locale value.') - +def read_system_locale(sys_path=LOCALE_CONF_FN, keyname='LANG'): + """Read system default locale setting, if present""" + sys_val = "" if not sys_path: raise ValueError('Invalid path: %s' % sys_path) if os.path.exists(sys_path): locale_content = util.load_file(sys_path) - # if LANG isn't present, regen sys_defaults = util.load_shell_content(locale_content) sys_val = sys_defaults.get(keyname, "") - if sys_val.lower() == locale.lower(): - LOG.debug( - "System has '%s=%s' requested '%s', skipping regeneration.", - keyname, sys_val, locale) - return - util.subp(['locale-gen', locale], capture=False) + return sys_val + + +def update_locale_conf(locale, sys_path, keyname='LANG'): + """Update system locale config""" + LOG.debug('Updating %s with locale setting %s=%s', + sys_path, keyname, locale) util.subp( ['update-locale', '--locale-file=' + sys_path, '%s=%s' % (keyname, locale)], capture=False) + +def regenerate_locale(locale, sys_path, keyname='LANG'): + """ + Run locale-gen for the provided locale and set the default + system variable `keyname` appropriately in the provided `sys_path`. + + """ + # special case for locales which do not require regen + # % locale -a + # C + # C.UTF-8 + # POSIX + if locale.lower() in ['c', 'c.utf-8', 'posix']: + LOG.debug('%s=%s does not require rengeneration', keyname, locale) + return + + # finally, trigger regeneration + LOG.debug('Generating locales for %s', locale) + util.subp(['locale-gen', locale], capture=False) + + # vi: ts=4 expandtab diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 952caf35..9a43fbee 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -44,6 +44,7 @@ class DataSourceNotFoundException(Exception): class DataSource(object): dsmode = DSMODE_NETWORK + default_locale = 'en_US.UTF-8' def __init__(self, sys_cfg, distro, paths, ud_proc=None): self.sys_cfg = sys_cfg @@ -150,7 +151,13 @@ 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 = self.default_locale + try: + locale = self.distro.get_locale() + except NotImplementedError: + 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 2330ad52..72d3aad6 100644 --- a/tests/unittests/test_distros/test_debian.py +++ b/tests/unittests/test_distros/test_debian.py @@ -1,67 +1,85 @@ # This file is part of cloud-init. See LICENSE file for license information. -from ..helpers import (CiTestCase, mock) - -from cloudinit.distros.debian import apply_locale +from cloudinit import distros from cloudinit import util +from ..helpers import (FilesystemMockingTestCase, mock) @mock.patch("cloudinit.distros.debian.util.subp") -class TestDebianApplyLocale(CiTestCase): +class TestDebianApplyLocale(FilesystemMockingTestCase): + + def setUp(self): + super(TestDebianApplyLocale, self).setUp() + self.new_root = self.tmp_dir() + self.patchOS(self.new_root) + self.patchUtils(self.new_root) + self.spath = self.tmp_path('etc/default/locale', self.new_root) + cls = distros.fetch("debian") + self.distro = cls("debian", {}, None) + def test_no_rerun(self, m_subp): """If system has defined locale, no re-run is expected.""" - spath = self.tmp_path("default-locale") m_subp.return_value = (None, None) locale = 'en_US.UTF-8' - util.write_file(spath, 'LANG=%s\n' % locale, omode="w") - apply_locale(locale, sys_path=spath) + util.write_file(self.spath, 'LANG=%s\n' % locale, omode="w") + self.distro.apply_locale(locale, out_fn=self.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""" + m_subp.return_value = (None, None) + locale = 'C.UTF-8' + util.write_file(self.spath, 'LANG=%s\n' % 'en_US.UTF-8', omode="w") + self.distro.apply_locale(locale, out_fn=self.spath) + self.assertEqual( + [['update-locale', '--locale-file=' + self.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") m_subp.return_value = (None, None) locale = 'en_US.UTF-8' - util.write_file(spath, 'LANG=fr_FR.UTF-8', omode="w") - apply_locale(locale, sys_path=spath) + util.write_file(self.spath, 'LANG=fr_FR.UTF-8', omode="w") + self.distro.apply_locale(locale, out_fn=self.spath) self.assertEqual( [['locale-gen', locale], - ['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale]], + ['update-locale', '--locale-file=' + self.spath, + 'LANG=%s' % locale]], [p[0][0] for p in m_subp.call_args_list]) def test_rerun_if_no_file(self, m_subp): """If system has no locale file, locale-gen should be called.""" - spath = self.tmp_path("default-locale") m_subp.return_value = (None, None) locale = 'en_US.UTF-8' - apply_locale(locale, sys_path=spath) + self.distro.apply_locale(locale, out_fn=self.spath) self.assertEqual( [['locale-gen', locale], - ['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale]], + ['update-locale', '--locale-file=' + self.spath, + 'LANG=%s' % locale]], [p[0][0] for p in m_subp.call_args_list]) def test_rerun_on_unset_system_locale(self, m_subp): """If system has unset locale, locale-gen should be called.""" m_subp.return_value = (None, None) - spath = self.tmp_path("default-locale") locale = 'en_US.UTF-8' - util.write_file(spath, 'LANG=', omode="w") - apply_locale(locale, sys_path=spath) + util.write_file(self.spath, 'LANG=', omode="w") + self.distro.apply_locale(locale, out_fn=self.spath) self.assertEqual( [['locale-gen', locale], - ['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale]], + ['update-locale', '--locale-file=' + self.spath, + 'LANG=%s' % locale]], [p[0][0] for p in m_subp.call_args_list]) def test_rerun_on_mismatched_keys(self, m_subp): """If key is LC_ALL and system has only LANG, rerun is expected.""" m_subp.return_value = (None, None) - spath = self.tmp_path("default-locale") locale = 'en_US.UTF-8' - util.write_file(spath, 'LANG=', omode="w") - apply_locale(locale, sys_path=spath, keyname='LC_ALL') + util.write_file(self.spath, 'LANG=', omode="w") + self.distro.apply_locale(locale, out_fn=self.spath, keyname='LC_ALL') self.assertEqual( [['locale-gen', locale], - ['update-locale', '--locale-file=' + spath, + ['update-locale', '--locale-file=' + self.spath, 'LC_ALL=%s' % locale]], [p[0][0] for p in m_subp.call_args_list]) @@ -69,14 +87,14 @@ class TestDebianApplyLocale(CiTestCase): """locale as None or "" is invalid and should raise ValueError.""" with self.assertRaises(ValueError) as ctext_m: - apply_locale(None) + self.distro.apply_locale(None) m_subp.assert_not_called() self.assertEqual( 'Failed to provide locale value.', str(ctext_m.exception)) with self.assertRaises(ValueError) as ctext_m: - apply_locale("") + self.distro.apply_locale("") m_subp.assert_not_called() self.assertEqual( 'Failed to provide locale value.', str(ctext_m.exception)) diff --git a/tests/unittests/test_distros/test_generic.py b/tests/unittests/test_distros/test_generic.py index c9be277e..b355a19e 100644 --- a/tests/unittests/test_distros/test_generic.py +++ b/tests/unittests/test_distros/test_generic.py @@ -228,5 +228,21 @@ class TestGenericDistro(helpers.FilesystemMockingTestCase): os.symlink('/', '/run/systemd/system') self.assertFalse(d.uses_systemd()) + @mock.patch('cloudinit.distros.debian.read_system_locale') + def test_get_locale_ubuntu(self, m_locale): + """Test ubuntu distro returns locale set to C.UTF-8""" + m_locale.return_value = '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_debug.py b/tests/unittests/test_handler/test_handler_debug.py index 929f786e..1873c3e1 100644 --- a/tests/unittests/test_handler/test_handler_debug.py +++ b/tests/unittests/test_handler/test_handler_debug.py @@ -11,7 +11,7 @@ from cloudinit import util from cloudinit.sources import DataSourceNone -from .. import helpers as t_help +from ..helpers import (FilesystemMockingTestCase, mock) import logging import shutil @@ -20,7 +20,8 @@ import tempfile LOG = logging.getLogger(__name__) -class TestDebug(t_help.FilesystemMockingTestCase): +@mock.patch('cloudinit.distros.debian.read_system_locale') +class TestDebug(FilesystemMockingTestCase): def setUp(self): super(TestDebug, self).setUp() self.new_root = tempfile.mkdtemp() @@ -36,7 +37,8 @@ class TestDebug(t_help.FilesystemMockingTestCase): ds.metadata.update(metadata) return cloud.Cloud(ds, paths, {}, d, None) - def test_debug_write(self): + def test_debug_write(self, m_locale): + m_locale.return_value = 'en_US.UTF-8' cfg = { 'abc': '123', 'c': u'\u20a0', @@ -54,7 +56,8 @@ class TestDebug(t_help.FilesystemMockingTestCase): for k in cfg.keys(): self.assertIn(k, contents) - def test_debug_no_write(self): + def test_debug_no_write(self, m_locale): + m_locale.return_value = 'en_US.UTF-8' cfg = { 'abc': '123', 'debug': { diff --git a/tests/unittests/test_handler/test_handler_locale.py b/tests/unittests/test_handler/test_handler_locale.py index cba5cae8..a789db32 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() @@ -60,4 +65,47 @@ class TestLocale(t_help.FilesystemMockingTestCase): else: 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, []) + + if cc.distro.uses_systemd(): + locale_conf = cc.distro.systemd_locale_conf_fn + keyname = 'LANG' + else: + locale_conf = cc.distro.locale_conf_fn + keyname = 'RC_LANG' + + contents = util.load_file(locale_conf, decode=False) + n_cfg = ConfigObj(BytesIO(contents)) + self.assertEqual({keyname: '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 = {'locale': 'C.UTF-8'} + 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 -- cgit v1.2.3