Merge ~smoser/cloud-init:feature/pregen-locale into cloud-init:master

[Scott Moser]

locale: Do not re-run locale-gen if provided locale is system default.

If the system configure default in /etc/default/locale is set to the same
value that is provided for cloud-init's "locale" setting, then do not
re-run locale-gen. This allows images built with a locale already
generated to not re-run locale-gen (which can be very heavy).

Also here is a fix to invoke update-locale correctly and remove the
internal writing of /etc/default/locale. We were calling
  update-locale <locale>
This ends up having no affect. The more correct invocation is:
  update-locale LANG=<locale>

Also added some support here should we ever want to change setting
LANG to setting LC_ALL (or any other key).

Lastly, a test change to allow us to use assert_not_called from mock.
Versions of mock in CentOS 6 do not have assert_not_called.

[Scott Moser (smoser)]

This is a update to
 https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/325406
with the suggestions I had to that MP fixed
(added unit tests)

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

FAILED: Continuous integration, rev:9a607ecd6efecd189a5dd3dd1b50aece02b6d281
https://jenkins.ubuntu.com/server/job/cloud-init-ci/42/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:2a44d6f5d36db7c322891f590752e38069b75b08
https://jenkins.ubuntu.com/server/job/cloud-init-ci/47/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:724ef696e776e6937b8e75a41da35607bede7a6a
https://jenkins.ubuntu.com/server/job/cloud-init-ci/50/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:8b220d36d04538e37de3e7a188dffce3ac67c55d
https://jenkins.ubuntu.com/server/job/cloud-init-ci/56/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

chad thanks for review. i'll get these fixes.

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

FAILED: Continuous integration, rev:8faec3079c58a4db6cbac5cf3ecd8e319fe5509b
https://jenkins.ubuntu.com/server/job/cloud-init-ci/73/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:2c7f87273d3f0ada0a22984067dae9e3ea6958e7
https://jenkins.ubuntu.com/server/job/cloud-init-ci/74/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/75/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:9dd9bb47b48c80ea0b1f4a1bf78e44ee81c14c97
https://jenkins.ubuntu.com/server/job/cloud-init-ci/77/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

please re-review. i think i've addressed all comments.

[Chad Smith (chad.smith)]

just tested on a live system and the work looks good. Thanks +1

review: Approve

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

PASSED: Continuous integration, rev:0d69e5a67889edae66be84ce66a173bbdcdd01c2
https://jenkins.ubuntu.com/server/job/cloud-init-ci/93/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py
index d06d46a..abfb81f 100644
--- a/cloudinit/distros/debian.py
+++ b/cloudinit/distros/debian.py
@@ -37,11 +37,11 @@ ENI_HEADER = """# This file is generated from information provided by
 """
 
 NETWORK_CONF_FN = "/etc/network/interfaces.d/50-cloud-init.cfg"
+LOCALE_CONF_FN = "/etc/default/locale"
 
 
 class Distro(distros.Distro):
     hostname_conf_fn = "/etc/hostname"
-    locale_conf_fn = "/etc/default/locale"
     network_conf_fn = {
         "eni": "/etc/network/interfaces.d/50-cloud-init.cfg",
         "netplan": "/etc/netplan/50-cloud-init.yaml"
@@ -64,16 +64,8 @@ class Distro(distros.Distro):
 
     def apply_locale(self, locale, out_fn=None):
         if not out_fn:
-            out_fn = self.locale_conf_fn
-        util.subp(['locale-gen', locale], capture=False)
-        util.subp(['update-locale', locale], capture=False)
-        # "" provides trailing newline during join
-        lines = [
-            util.make_header(),
-            'LANG="%s"' % (locale),
-            "",
-        ]
-        util.write_file(out_fn, "\n".join(lines))
+            out_fn = LOCALE_CONF_FN
+        apply_locale(locale, out_fn)
 
     def install_packages(self, pkglist):
         self.update_package_sources()
@@ -225,4 +217,38 @@ 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.')
+
+    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)
+    util.subp(
+        ['update-locale', '--locale-file=' + sys_path,
+         '%s=%s' % (keyname, locale)], capture=False)
+
 # vi: ts=4 expandtab
diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py
index 6691cf8..08c5c46 100644
--- a/tests/unittests/helpers.py
+++ b/tests/unittests/helpers.py
@@ -376,4 +376,16 @@ except AttributeError:
             return wrapper
         return decorator
 
+
+# older versions of mock do not have the useful 'assert_not_called'
+if not hasattr(mock.Mock, 'assert_not_called'):
+    def __mock_assert_not_called(mmock):
+        if mmock.call_count != 0:
+            msg = ("[citest] Expected '%s' to not have been called. "
+                   "Called %s times." %
+                   (mmock._mock_name or 'mock', mmock.call_count))
+            raise AssertionError(msg)
+    mock.Mock.assert_not_called = __mock_assert_not_called
+
+
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_distros/test_debian.py b/tests/unittests/test_distros/test_debian.py
new file mode 100644
index 0000000..2330ad5
--- /dev/null
+++ b/tests/unittests/test_distros/test_debian.py
@@ -0,0 +1,82 @@
+# 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 util
+
+
[e-mail]("cloudinit.distros.debian.util.subp")
+class TestDebianApplyLocale(CiTestCase):
+    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)
+        m_subp.assert_not_called()
+
+    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)
+        self.assertEqual(
+            [['locale-gen', locale],
+             ['update-locale', '--locale-file=' + 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.assertEqual(
+            [['locale-gen', locale],
+             ['update-locale', '--locale-file=' + 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)
+        self.assertEqual(
+            [['locale-gen', locale],
+             ['update-locale', '--locale-file=' + 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')
+        self.assertEqual(
+            [['locale-gen', locale],
+             ['update-locale', '--locale-file=' + spath,
+              'LC_ALL=%s' % locale]],
+            [p[0][0] for p in m_subp.call_args_list])
+
+    def test_falseish_locale_raises_valueerror(self, m_subp):
+        """locale as None or "" is invalid and should raise ValueError."""
+
+        with self.assertRaises(ValueError) as ctext_m:
+            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("")
+            m_subp.assert_not_called()
+        self.assertEqual(
+            'Failed to provide locale value.', str(ctext_m.exception))