Merge ~raharper/cloud-init:default-lang-c-utf8 into cloud-init:master

[Ryan Harper]

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.

[Ryan Harper (raharper)]

Current Ubuntu (and Debian) images already include the C.UTF-8 locale. Updating the default in cloud-init (for Ubuntu and Debian) means we can realize a non-zero speed up during boot.

Building cloud-init from this branch, injecting it into an artful image and comparing time spent in locale-gen vs. existing artful image with older cloud-init (which will locale-gen en_US.UTF-8) there's a non-zero improvement

# existing artful image
% lxc file pull a3/var/log/cloud-init.log - | PYTHONPATH=`pwd` python3 -m cloudinit.analyze blame -i - | grep locale
     01.07200s (modules-config/config-locale)

# modified artful image (updated cloud-init, LANG=C.UTF-8 in /etc/default/locale)
% lxc file pull a1/var/log/cloud-init.log - | PYTHONPATH=`pwd` python3 -m cloudinit.analyze blame -i - | grep locale
     00.00200s (modules-config/config-locale)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:8fe47a4abc15c59579c5a7805ff409c4ec140563
https://jenkins.ubuntu.com/server/job/cloud-init-ci/159/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/159/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:aaec147b763cefe35e9d9f246d502f27de59edd7
https://jenkins.ubuntu.com/server/job/cloud-init-ci/161/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/161/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

Looks good, just some inline nits and questions

[Scott Moser (smoser)]

The distro can already override the default locale by simply providing /etc/cloud/cloud.cfg.d/ entries with 'locale: C.UTF-8'. So I don't know that there is a reason to have the added complexity of looking this up in the distro object. Arguably we should support configuring locale under "system_info" as it is OS configuration similar to "default_user".

The bigger issue with this is that it changes the default locale for any instance with cloud-initnit from en_US.UTF-8 to C.UTF_8. That is a behavior change that we'd then have to revert in Ubuntu stable releases. Admittedly, I'm not sure of the real-world affect of that change other than having 'locale' mention it, but I think that change was generally not intended or thought out.

The one other comment I have is that it'd be best if we chose to not regenerate any locale that was already generated. Currently what we do is simply not generate (or regenerate) the locale if it is configured to be the system default locale.

review: Needs Information

[Ryan Harper (raharper)]

On Fri, Aug 18, 2017 at 3:57 PM, Scott Moser <email address hidden> wrote:

> Review: Needs Information
>
> The distro can already override the default locale by simply providing
> /etc/cloud/cloud.cfg.d/ entries with 'locale: C.UTF-8'. So I don't know
> that there is a reason to have the added complexity of looking this up in
> the distro object. Arguably we should support configuring locale under
> "system_info" as it is OS configuration similar to "default_user".
>

In general, I think default locale is something only the distro can
provide; I don;t think it's something that should be in the datasource (the
default value)
A distro can tell cloud-init what the current settings are:

(read LANG from env)

See what's available:

locale -a

And possible generate new locales

(locale-gen)

If anything, it would be better to remove the "Default" value from
Datasource base-class and leave it to the distro (to determine default).

>
> The bigger issue with this is that it changes the default locale for any
> instance with cloud-initnit from en_US.UTF-8 to C.UTF_8. That is a
> behavior change that we'd then have to revert in Ubuntu stable releases.
> Admittedly, I'm not sure of the real-world affect of that change other than
> having 'locale' mention it, but I think that change was generally not
> intended or thought out.
>

Yes, that's true; I don't think we can change this in previous stable
releases.

>
>
> The one other comment I have is that it'd be best if we chose to not
> regenerate any locale that was already generated. Currently what we do is
> simply not generate (or regenerate) the locale if it is configured to be
> the system default locale.
>

Yes, I was hoping to avoid more exec'ing but with some thought, I probably
makes sense to

1) determine the current LANG setting
2) check if there's a system configured LANG setting (/etc/default/locale)
3) check what's available (locale -a)

compare that to what's being requested

And only locale-gen if the requested locale isn't already available (1, 2)

If the requested locale is available (whether we regen or not) we should
update
the system_conf file with the requested locale.

Does that make sense?

>
> --
> https://code.launchpad.net/~raharper/cloud-init/+git/
> cloud-init/+merge/329152
> You are the owner of ~raharper/cloud-init:default-lang-c-utf8.
>

[Scott Moser (smoser)]

> On Fri, Aug 18, 2017 at 3:57 PM, Scott Moser <email address hidden> wrote:
>
> > Review: Needs Information
> >
> > The distro can already override the default locale by simply providing
> > /etc/cloud/cloud.cfg.d/ entries with 'locale: C.UTF-8'. So I don't know
> > that there is a reason to have the added complexity of looking this up in
> > the distro object. Arguably we should support configuring locale under
> > "system_info" as it is OS configuration similar to "default_user".
> >
>
> In general, I think default locale is something only the distro can
> provide; I don;t think it's something that should be in the datasource (the
> default value)

I agree that the datasource seems an odd place for this setting.
One example of it not being all that odd, though is if it came from the
metadata service. Ie, it isn't that far fetched at all to consider
a China based cloud-provider to want to default to having a Chinese locale.

We're not doing anything dynamic with the locale currently, though.
So for now I have to agree.

> A distro can tell cloud-init what the current settings are:
>
> (read LANG from env)

Well, not really. LANG is just the LANG of the user running cloud-init.
Most of the time that would be the LANG that is set in the init system.
That definitely could contain 'LANG', but to my knowledge usually doesnt.
From my desktop:
  $ cat /etc/default/locale
  LANG="en_US.UTF-8"

  $ sudo cat /proc/1/environ | tr '\0' '\n'
  HOME=/
  init=/sbin/init
  recovery=
  TERM=linux
  drop_caps=
  BOOT_IMAGE=/boot/vmlinuz-4.12.0-11-generic
  PATH=/sbin:/usr/sbin:/bin:/usr/bin
  PWD=/
  rootmnt=/root

Thats the environment that cloud-init would inherit when running in normal
boot, it does not have a LANG set.

> If anything, it would be better to remove the "Default" value from
> Datasource base-class and leave it to the distro (to determine default).

Yes, you've convinced me of this. But your 'get_locale' method that
you've added to distro should read /etc/default/locale on Debian/Ubuntu.
That way we read the "correct" place for such a definition and the
image-builder does not have to repeat themselves or patch cloud-init
to get that change done.

Basically, get_locale should do

   if //etc/default/locale has value:
       return that
   else
       return 'en_US.UTF-8'

That would make cloud-init respect the wishes of the image builder
and also allow the cloud-images team to make the change if they so
choose.
The only issue I see with this is that they wont' necessarily think that
through, and a bug will be opened against cloud-init for behavioral change...

> > The bigger issue with this is that it changes the default locale for any
> > instance with cloud-initnit from en_US.UTF-8 to C.UTF_8. That is a
> > behavior change that we'd then have to revert in Ubuntu stable releases.
> > Admittedly, I'm not sure of the real-world affect of that change other than
> > having 'locale' mention it, but I think that change was generally not
> > intended or thought out.
> >
> Yes, that's true; I don't think we can change this in previous stable
> releases.

I'm willing to push that burden off onto to the cloud-images team if they
agree.

> ...

[Scott Moser (smoser)]

I moved to work-in-progress.
Please feel free to respond and/or disagree with above and then move it back to 'needs review'.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:cf1c4106545b6acceded30ae9f3f65910277335e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/229/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/229/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

Changeset looks good to me assuming (as you said) we can rely on image creators to properly write /etc/default/locale +1.

review: Approve

[Scott Moser (smoser)]

some comments in line.

[Ryan Harper (raharper)]

Thanks for review, will fix up.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:29ab5e4724b755c1fe2346cc1e9a48bdcc216386
https://jenkins.ubuntu.com/server/job/cloud-init-ci/236/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    FAILED: Ubuntu LTS: Build

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/236/rebuild

review: Needs Fixing (continuous-integration)

[Scott Moser (smoser)]

your tests failed due to some bad test in master.
i fixed that with 300e4516f78dbb0a9533749aa84f7e366b023d04
and then fixed your copy of that bad test too.

 http://paste.ubuntu.com/25435526/

I'm 'approving' and merging with my change above.

review: Approve

diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index 807b3ea..b714b9a 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..33cc0bf 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 952caf3..9a43fbe 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 2330ad5..72d3aad 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 c9be277..b355a19 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 929f786..1873c3e 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):
[e-mail]('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 aaf6c76..4e7ce64 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