From 40a400e42603aa1b80d9f623bc779799b370c091 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 21 Sep 2016 15:45:45 -0400 Subject: subp: add 'update_env' argument In order for a caller to use 'env' argument of subp, they will realistically do: env = os.environ.copy() env['FOO'] = 'BZR' subp(cmd, env=env) This shortens that to be: subp(cmd, update_env={'FOO': 'BZR'}) Add tests, and update growpart tests to use mock when playing with os.environ. --- cloudinit/util.py | 9 ++++++- .../test_handler/test_handler_growpart.py | 4 ++- tests/unittests/test_util.py | 30 ++++++++++++++++++++-- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/cloudinit/util.py b/cloudinit/util.py index 6c5cf741..05cb587c 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1762,7 +1762,7 @@ def delete_dir_contents(dirname): def subp(args, data=None, rcs=None, env=None, capture=True, shell=False, - logstring=False, decode="replace", target=None): + logstring=False, decode="replace", target=None, update_env=None): # not supported in cloud-init (yet), for now kept in the call signature # to ease maintaining code shared between cloud-init and curtin @@ -1773,6 +1773,13 @@ def subp(args, data=None, rcs=None, env=None, capture=True, shell=False, rcs = [0] devnull_fp = None + + if update_env: + if env is None: + env = os.environ + env = env.copy() + env.update(update_env) + try: if target_path(target) != "/": args = ['chroot', target] + list(args) diff --git a/tests/unittests/test_handler/test_handler_growpart.py b/tests/unittests/test_handler/test_handler_growpart.py index e653488a..e28067de 100644 --- a/tests/unittests/test_handler/test_handler_growpart.py +++ b/tests/unittests/test_handler/test_handler_growpart.py @@ -81,11 +81,11 @@ class TestConfig(TestCase): self.cloud = cloud.Cloud(None, self.paths, None, None, None) self.log = logging.getLogger("TestConfig") self.args = [] - os.environ = {} self.cloud_init = None self.handle = cc_growpart.handle + @mock.patch.dict("os.environ", clear=True) def test_no_resizers_auto_is_fine(self): with mock.patch.object( util, 'subp', @@ -98,6 +98,7 @@ class TestConfig(TestCase): mockobj.assert_called_once_with( ['growpart', '--help'], env={'LANG': 'C'}) + @mock.patch.dict("os.environ", clear=True) def test_no_resizers_mode_growpart_is_exception(self): with mock.patch.object( util, 'subp', @@ -110,6 +111,7 @@ class TestConfig(TestCase): mockobj.assert_called_once_with( ['growpart', '--help'], env={'LANG': 'C'}) + @mock.patch.dict("os.environ", clear=True) def test_mode_auto_prefers_growpart(self): with mock.patch.object( util, 'subp', diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index d2031f59..30f603cb 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -223,8 +223,10 @@ class TestKeyValStrings(helpers.TestCase): class TestGetCmdline(helpers.TestCase): def test_cmdline_reads_debug_env(self): - os.environ['DEBUG_PROC_CMDLINE'] = 'abcd 123' - self.assertEqual(os.environ['DEBUG_PROC_CMDLINE'], util.get_cmdline()) + with mock.patch.dict("os.environ", + values={'DEBUG_PROC_CMDLINE': 'abcd 123'}): + ret = util.get_cmdline() + self.assertEqual("abcd 123", ret) class TestLoadYaml(helpers.TestCase): @@ -516,6 +518,7 @@ class TestSubp(helpers.TestCase): utf8_invalid = b'ab\xaadef' utf8_valid = b'start \xc3\xa9 end' utf8_valid_2 = b'd\xc3\xa9j\xc8\xa7' + printenv = ['bash', '-c', 'for n in "$@"; do echo "$n=${!n}"; done', '--'] def printf_cmd(self, *args): # bash's printf supports \xaa. So does /usr/bin/printf @@ -566,6 +569,29 @@ class TestSubp(helpers.TestCase): self.assertEqual(err, data) self.assertEqual(out, b'') + def test_subp_reads_env(self): + with mock.patch.dict("os.environ", values={'FOO': 'BAR'}): + out, err = util.subp(self.printenv + ['FOO'], capture=True) + self.assertEqual('FOO=BAR', out.splitlines()[0]) + + def test_subp_env_and_update_env(self): + out, err = util.subp( + self.printenv + ['FOO', 'HOME', 'K1', 'K2'], capture=True, + env={'FOO': 'BAR'}, + update_env={'HOME': '/myhome', 'K2': 'V2'}) + self.assertEqual( + ['FOO=BAR', 'HOME=/myhome', 'K1=', 'K2=V2'], out.splitlines()) + + def test_subp_update_env(self): + extra = {'FOO': 'BAR', 'HOME': '/root', 'K1': 'V1'} + with mock.patch.dict("os.environ", values=extra): + out, err = util.subp( + self.printenv + ['FOO', 'HOME', 'K1', 'K2'], capture=True, + update_env={'HOME': '/myhome', 'K2': 'V2'}) + + self.assertEqual( + ['FOO=BAR', 'HOME=/myhome', 'K1=V1', 'K2=V2'], out.splitlines()) + def test_returns_none_if_no_capture(self): (out, err) = util.subp(self.stdin2out, data=b'', capture=False) self.assertEqual(err, None) -- cgit v1.2.3 From 30d0adb71c9adadf437b2a1c69529ad9f44e75a8 Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Thu, 25 Aug 2016 16:43:46 -0700 Subject: Allow ephemeral drive to be unpartitioned If device has no partition table, the first line of output from `sgdisk -p ` will be "Creating new GPT entries.", instead of something like "Disk /dev/sdb: 266338304 sectors, 127.0 GiB". Also, protect against localized output by adjusting subp calls that parse sgdisk output to set LANG=C. --- cloudinit/config/cc_disk_setup.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/cloudinit/config/cc_disk_setup.py b/cloudinit/config/cc_disk_setup.py index b642f1f8..39a23688 100644 --- a/cloudinit/config/cc_disk_setup.py +++ b/cloudinit/config/cc_disk_setup.py @@ -33,6 +33,8 @@ BLKID_CMD = util.which("blkid") BLKDEV_CMD = util.which("blockdev") WIPEFS_CMD = util.which("wipefs") +LANG_C_ENV = {'LANG': 'C'} + LOG = logging.getLogger(__name__) @@ -355,8 +357,11 @@ def get_mbr_hdd_size(device): def get_gpt_hdd_size(device): - out, _ = util.subp([SGDISK_CMD, '-p', device]) - return out.splitlines()[0].split()[2] + out, _ = util.subp([SGDISK_CMD, '-p', device], update_env=LANG_C_ENV) + for line in out.splitlines(): + if line.startswith("Disk"): + return line.split()[2] + raise Exception("Failed to get %s size from sgdisk" % (device)) def get_hdd_size(table_type, device): @@ -408,7 +413,7 @@ def check_partition_mbr_layout(device, layout): def check_partition_gpt_layout(device, layout): prt_cmd = [SGDISK_CMD, '-p', device] try: - out, _err = util.subp(prt_cmd) + out, _err = util.subp(prt_cmd, update_env=LANG_C_ENV) except Exception as e: raise Exception("Error running partition command on %s\n%s" % ( device, e)) -- cgit v1.2.3 From 1b71b474c0fc06e67aab8676268fd83d99091910 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Tue, 20 Sep 2016 14:13:25 -0700 Subject: systemd: Ensure that cloud-init-local happens before NetworkManager --- systemd/cloud-init-local.service | 1 + 1 file changed, 1 insertion(+) diff --git a/systemd/cloud-init-local.service b/systemd/cloud-init-local.service index bc2db60e..55834ba4 100644 --- a/systemd/cloud-init-local.service +++ b/systemd/cloud-init-local.service @@ -5,6 +5,7 @@ Wants=local-fs.target Wants=network-pre.target After=local-fs.target Before=basic.target +Before=NetworkManager.service Before=network-pre.target Before=shutdown.target Conflicts=shutdown.target -- cgit v1.2.3 From 0439d8a17d181a2546f2f7cb2d71a04bbb13b186 Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Thu, 15 Sep 2016 12:05:15 -0400 Subject: Decode unicode types in decode_binary The test in decode_binary for six.text_type was incorrect as that includes unicode type in Python 2 which should actually be decoded. When the type is string_types we now properly check only for basestring and str in Python 2 and Python 3 respectively and return the given blob without making an attempt to decode. --- cloudinit/util.py | 2 +- tests/unittests/test_util.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/cloudinit/util.py b/cloudinit/util.py index 05cb587c..eb3e5899 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -154,7 +154,7 @@ def target_path(target, path=None): def decode_binary(blob, encoding='utf-8'): # Converts a binary type into a text type using given encoding. - if isinstance(blob, six.text_type): + if isinstance(blob, six.string_types): return blob return blob.decode(encoding) diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 30f603cb..fc6b9d40 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -603,4 +603,12 @@ class TestSubp(helpers.TestCase): self.assertEqual("/target/my/path/", util.target_path("/target/", "///my/path/")) + +class TestEncode(helpers.TestCase): + """Test the encoding functions""" + def test_decode_binary_plain_text_with_hex(self): + blob = 'BOOTABLE_FLAG=\x80init=/bin/systemd' + text = util.decode_binary(blob) + self.assertEqual(text, blob) + # vi: ts=4 expandtab -- cgit v1.2.3