Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
please re-review. i think i've addressed all comments.
Diff comments:
> diff --git a/tests/unittests/test_distros/test_debian.py
> b/tests/unittests/test_distros/test_debian.py
> new file mode 100644
> index 000..bd7721c
> --- /dev/null
> +++ b/tests/unittests/test_distros/test_debian.py
> @@ -0,0 +1,70 @@
> +# 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
> +
> +
> +class TestDebianApplyLocale(CiTestCase):
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +def test_no_rerun(self, m_subp):
to be clear, i'd like to avoid adding such a test here.
> +"""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()
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +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])
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +def test_rerun_if_no_file(self, m_subp):
> +"""If system has different locale, locale-gen should be called."""
ack done.
> +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])
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +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])
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
ack. done.
> +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])
--
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532
Your team cloud-init commiters is requested to review the proposed merge of
~smoser/cloud-init:feature/pregen-locale into cloud-init:master.
___
Mailing list: https://launchpad.net/~cloud-init-dev
Post to : cloud-init-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help : https://help.launchpad.net/ListHelp
[Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
The proposal to merge ~smoser/cloud-init:feature/pregen-locale into cloud-init:master has been updated. Commit Message changed to: 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 This ends up having no affect. The more correct invocation is: update-locale LANG= 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. For more details, see: https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 -- Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
Review: Approve continuous-integration 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 -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
Review: Approve continuous-integration 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 -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
Review: Needs Fixing continuous-integration 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 -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
Review: Needs Fixing continuous-integration 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 -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
chad thanks for review. i'll get these fixes. -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
Diff comments:
> diff --git a/tests/unittests/test_distros/test_debian.py
> b/tests/unittests/test_distros/test_debian.py
> new file mode 100644
> index 000..bd7721c
> --- /dev/null
> +++ b/tests/unittests/test_distros/test_debian.py
> @@ -0,0 +1,70 @@
> +# 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
> +
> +
> +class TestDebianApplyLocale(CiTestCase):
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +def test_no_rerun(self, m_subp):
Can you add a couple tests for the exceptions
with self.assertRaises(ValueError) as context_manager:
apply_locale(None)
self.assertEqual('Failed to provide locale valiue',
str(context.manager.exception))
...
and with self.assertRaises(ValueError) as context_manager:
apply_locale('en_US.UTF-8', 'IDONTEXIST')
> +"""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()
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +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])
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +def test_rerun_if_no_file(self, m_subp):
> +"""If system has different locale, locale-gen should be called."""
docstring doesn't map to this unit test, should talk about abent locale file.
> +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])
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +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])
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +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])
--
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532
Your team cloud-init commiters is requested to review the proposed merge of
~smoser/cloud-init:feature/pregen-locale into cloud-init:master.
___
Mailing list: https://launchpad.net/~cloud-init-dev
Post to : cloud-init-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
Diff comments:
> diff --git a/tests/unittests/test_distros/test_debian.py
> b/tests/unittests/test_distros/test_debian.py
> new file mode 100644
> index 000..bd7721c
> --- /dev/null
> +++ b/tests/unittests/test_distros/test_debian.py
> @@ -0,0 +1,70 @@
> +# 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
> +
> +
> +class TestDebianApplyLocale(CiTestCase):
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +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()
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +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])
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +def test_rerun_if_no_file(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'
> +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])
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
> +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])
> +
> +@mock.patch("cloudinit.distros.debian.util.subp")
please pul this common mock up to as a class decorator, since you need it on
all tests
> +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])
--
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532
Your team cloud-init commiters is requested to review the proposed merge of
~smoser/cloud-init:feature/pregen-locale into cloud-init:master.
___
Mailing list: https://launchpad.net/~cloud-init-dev
Post to : cloud-init-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
Review: Approve continuous-integration 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 -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
Review: Approve continuous-integration 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 -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
[Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
The proposal to merge ~smoser/cloud-init:feature/pregen-locale into cloud-init:master has been updated. Commit Message changed to: 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 This ends up having no affect. The more correct invocation is: update-locale LANG= Also added some support here should we ever want to change setting LANG to setting LC_ALL (or any other key). For more details, see: https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 -- Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
Review: Approve continuous-integration 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 -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
Review: Needs Fixing continuous-integration 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 -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
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) -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/pregen-locale into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
[Cloud-init-dev] [Merge] ~smoser/cloud-init:feature/pregen-locale into cloud-init:master
Scott Moser has proposed merging ~smoser/cloud-init:feature/pregen-locale into
cloud-init:master.
Requested reviews:
cloud-init commiters (cloud-init-dev)
For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532
--
Your team cloud-init commiters is requested to review the proposed merge of
~smoser/cloud-init:feature/pregen-locale into cloud-init:master.
diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py
index d06d46a..c6c24dc 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,37 @@ 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 'path'.
+
+If sys_path indicates that this value is already the default
+('keyname=locale') then no changes will be made and locale-gen not called.
+"""
+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
+print("system = %s locale=%s" % (sys_val, locale))
+
+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/test_distros/test_debian.py b/tests/unittests/test_distros/test_debian.py
new file mode 100644
index 000..8e47e0d
--- /dev/null
+++ b/tests/unittests/test_distros/test_debian.py
@@ -0,0 +1,72 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+import os
+
+from ..helpers import (CiTestCase, mock)
+
+from cloudinit.distros.debian import apply_locale
+from cloudinit import util
+
+
+class TestDebianApplyLocale(CiTestCase):
+@mock.patch("cloudinit.distros.debian.util.subp")
+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()
+
+@mock.patch("cloudinit.distros.debian.util.subp")
+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])
+
+@mock.patch("cloudinit.distros.debian.util.subp")
+def test_rerun_if_no_file(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'
+apply_locale(locale, sys_path=spath)
+
