From 3cb8ecc229999dbe524ff2ba4c4bd693e3c66058 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 10 Oct 2014 17:27:56 -0700 Subject: Increase the robustness of the chef module Add the following adjustments to the chef template and module - Make it so that the chef directories can be provided (defaults to the existing directories) - Make the params much more configurable, and if a parameter is provided in the chef configuration it will override existing template parameters. - Make the template skip lines if the values are None in the configuration so that template lines can be removed if/when this is desirable. - Allow the firstboot json path to be configurable (defaults to the existing location). - Adds a basic set of tests to ensure that good things are happening. --- cloudinit/util.py | 135 +++++++++++------------------------------------------- 1 file changed, 26 insertions(+), 109 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index f236d0bf..bdb0f268 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -191,11 +191,11 @@ def ExtendedTemporaryFile(**kwargs): return fh -def fork_cb(child_cb, *args, **kwargs): +def fork_cb(child_cb, *args): fid = os.fork() if fid == 0: try: - child_cb(*args, **kwargs) + child_cb(*args) os._exit(0) except: logexc(LOG, "Failed forking and calling callback %s", @@ -1297,7 +1297,7 @@ def unmounter(umount): yield umount finally: if umount: - umount_cmd = ["umount", umount] + umount_cmd = ["umount", '-l', umount] subp(umount_cmd) @@ -1346,70 +1346,37 @@ def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True): Mount the device, call method 'callback' passing the directory in which it was mounted, then unmount. Return whatever 'callback' returned. If data != None, also pass data to callback. - - mtype is a filesystem type. it may be a list, string (a single fsname) - or a list of fsnames. """ - - if isinstance(mtype, str): - mtypes = [mtype] - elif isinstance(mtype, (list, tuple)): - mtypes = list(mtype) - elif mtype is None: - mtypes = None - - # clean up 'mtype' input a bit based on platform. - platsys = platform.system().lower() - if platsys == "linux": - if mtypes is None: - mtypes = ["auto"] - elif platsys.endswith("bsd"): - if mtypes is None: - mtypes = ['ufs', 'cd9660', 'vfat'] - for index, mtype in enumerate(mtypes): - if mtype == "iso9660": - mtypes[index] = "cd9660" - else: - # we cannot do a smart "auto", so just call 'mount' once with no -t - mtypes = [''] - mounted = mounts() with tempdir() as tmpd: umount = False if device in mounted: mountpoint = mounted[device]['mountpoint'] else: - for mtype in mtypes: - mountpoint = None - try: - mountcmd = ['mount'] - mountopts = [] - if rw: - mountopts.append('rw') - else: - mountopts.append('ro') - if sync: - # This seems like the safe approach to do - # (ie where this is on by default) - mountopts.append("sync") - if mountopts: - mountcmd.extend(["-o", ",".join(mountopts)]) - if mtype: - mountcmd.extend(['-t', mtype]) - mountcmd.append(device) - mountcmd.append(tmpd) - subp(mountcmd) - umount = tmpd # This forces it to be unmounted (when set) - mountpoint = tmpd - break - except (IOError, OSError) as exc: - LOG.debug("Failed mount of '%s' as '%s': %s", - device, mtype, exc) - pass - if not mountpoint: - raise MountFailedError("Failed mounting %s to %s due to: %s" % + try: + mountcmd = ['mount'] + mountopts = [] + if rw: + mountopts.append('rw') + else: + mountopts.append('ro') + if sync: + # This seems like the safe approach to do + # (ie where this is on by default) + mountopts.append("sync") + if mountopts: + mountcmd.extend(["-o", ",".join(mountopts)]) + if mtype: + mountcmd.extend(['-t', mtype]) + mountcmd.append(device) + mountcmd.append(tmpd) + subp(mountcmd) + umount = tmpd # This forces it to be unmounted (when set) + mountpoint = tmpd + except (IOError, OSError) as exc: + raise MountFailedError(("Failed mounting %s " + "to %s due to: %s") % (device, tmpd, exc)) - # Be nice and ensure it ends with a slash if not mountpoint.endswith("/"): mountpoint += "/" @@ -1957,53 +1924,3 @@ def pathprefix2dict(base, required=None, optional=None, delim=os.path.sep): raise ValueError("Missing required files: %s", ','.join(missing)) return ret - - -def read_meminfo(meminfo="/proc/meminfo", raw=False): - # read a /proc/meminfo style file and return - # a dict with 'total', 'free', and 'available' - mpliers = {'kB': 2**10, 'mB': 2 ** 20, 'B': 1, 'gB': 2 ** 30} - kmap = {'MemTotal:': 'total', 'MemFree:': 'free', - 'MemAvailable:': 'available'} - ret = {} - for line in load_file(meminfo).splitlines(): - try: - key, value, unit = line.split() - except ValueError: - key, value = line.split() - unit = 'B' - if raw: - ret[key] = int(value) * mpliers[unit] - elif key in kmap: - ret[kmap[key]] = int(value) * mpliers[unit] - - return ret - - -def human2bytes(size): - """Convert human string or integer to size in bytes - 10M => 10485760 - .5G => 536870912 - """ - size_in = size - if size.endswith("B"): - size = size[:-1] - - mpliers = {'B': 1, 'K': 2 ** 10, 'M': 2 ** 20, 'G': 2 ** 30, 'T': 2 ** 40} - - num = size - mplier = 'B' - for m in mpliers: - if size.endswith(m): - mplier = m - num = size[0:-len(m)] - - try: - num = float(num) - except ValueError: - raise ValueError("'%s' is not valid input." % size_in) - - if num < 0: - raise ValueError("'%s': cannot be negative" % size_in) - - return int(num * mpliers[mplier]) -- cgit v1.2.3 From e8f9d27c6d43ef368a4047ae5818018a20e11f62 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 10 Oct 2014 17:34:32 -0700 Subject: Undo changes to the util file, not sure why that happened... --- cloudinit/util.py | 136 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 110 insertions(+), 26 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index bdb0f268..58f7455c 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -191,11 +191,11 @@ def ExtendedTemporaryFile(**kwargs): return fh -def fork_cb(child_cb, *args): +def fork_cb(child_cb, *args, **kwargs): fid = os.fork() if fid == 0: try: - child_cb(*args) + child_cb(*args, **kwargs) os._exit(0) except: logexc(LOG, "Failed forking and calling callback %s", @@ -1297,7 +1297,7 @@ def unmounter(umount): yield umount finally: if umount: - umount_cmd = ["umount", '-l', umount] + umount_cmd = ["umount", umount] subp(umount_cmd) @@ -1346,37 +1346,70 @@ def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True): Mount the device, call method 'callback' passing the directory in which it was mounted, then unmount. Return whatever 'callback' returned. If data != None, also pass data to callback. + + mtype is a filesystem type. it may be a list, string (a single fsname) + or a list of fsnames. """ + + if isinstance(mtype, str): + mtypes = [mtype] + elif isinstance(mtype, (list, tuple)): + mtypes = list(mtype) + elif mtype is None: + mtypes = None + + # clean up 'mtype' input a bit based on platform. + platsys = platform.system().lower() + if platsys == "linux": + if mtypes is None: + mtypes = ["auto"] + elif platsys.endswith("bsd"): + if mtypes is None: + mtypes = ['ufs', 'cd9660', 'vfat'] + for index, mtype in enumerate(mtypes): + if mtype == "iso9660": + mtypes[index] = "cd9660" + else: + # we cannot do a smart "auto", so just call 'mount' once with no -t + mtypes = [''] + mounted = mounts() with tempdir() as tmpd: umount = False if device in mounted: mountpoint = mounted[device]['mountpoint'] else: - try: - mountcmd = ['mount'] - mountopts = [] - if rw: - mountopts.append('rw') - else: - mountopts.append('ro') - if sync: - # This seems like the safe approach to do - # (ie where this is on by default) - mountopts.append("sync") - if mountopts: - mountcmd.extend(["-o", ",".join(mountopts)]) - if mtype: - mountcmd.extend(['-t', mtype]) - mountcmd.append(device) - mountcmd.append(tmpd) - subp(mountcmd) - umount = tmpd # This forces it to be unmounted (when set) - mountpoint = tmpd - except (IOError, OSError) as exc: - raise MountFailedError(("Failed mounting %s " - "to %s due to: %s") % + for mtype in mtypes: + mountpoint = None + try: + mountcmd = ['mount'] + mountopts = [] + if rw: + mountopts.append('rw') + else: + mountopts.append('ro') + if sync: + # This seems like the safe approach to do + # (ie where this is on by default) + mountopts.append("sync") + if mountopts: + mountcmd.extend(["-o", ",".join(mountopts)]) + if mtype: + mountcmd.extend(['-t', mtype]) + mountcmd.append(device) + mountcmd.append(tmpd) + subp(mountcmd) + umount = tmpd # This forces it to be unmounted (when set) + mountpoint = tmpd + break + except (IOError, OSError) as exc: + LOG.debug("Failed mount of '%s' as '%s': %s", + device, mtype, exc) + pass + if not mountpoint: + raise MountFailedError("Failed mounting %s to %s due to: %s" % (device, tmpd, exc)) + # Be nice and ensure it ends with a slash if not mountpoint.endswith("/"): mountpoint += "/" @@ -1924,3 +1957,54 @@ def pathprefix2dict(base, required=None, optional=None, delim=os.path.sep): raise ValueError("Missing required files: %s", ','.join(missing)) return ret + + +def read_meminfo(meminfo="/proc/meminfo", raw=False): + # read a /proc/meminfo style file and return + # a dict with 'total', 'free', and 'available' + mpliers = {'kB': 2**10, 'mB': 2 ** 20, 'B': 1, 'gB': 2 ** 30} + kmap = {'MemTotal:': 'total', 'MemFree:': 'free', + 'MemAvailable:': 'available'} + ret = {} + for line in load_file(meminfo).splitlines(): + try: + key, value, unit = line.split() + except ValueError: + key, value = line.split() + unit = 'B' + if raw: + ret[key] = int(value) * mpliers[unit] + elif key in kmap: + ret[kmap[key]] = int(value) * mpliers[unit] + + return ret + + +def human2bytes(size): + """Convert human string or integer to size in bytes + 10M => 10485760 + .5G => 536870912 + """ + size_in = size + if size.endswith("B"): + size = size[:-1] + + mpliers = {'B': 1, 'K': 2 ** 10, 'M': 2 ** 20, 'G': 2 ** 30, 'T': 2 ** 40} + + num = size + mplier = 'B' + for m in mpliers: + if size.endswith(m): + mplier = m + num = size[0:-len(m)] + + try: + num = float(num) + except ValueError: + raise ValueError("'%s' is not valid input." % size_in) + + if num < 0: + raise ValueError("'%s': cannot be negative" % size_in) + + return int(num * mpliers[mplier]) + -- cgit v1.2.3 From b8417c1af4a147240ec5919c2378fcc3e97078f7 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 10 Oct 2014 17:40:10 -0700 Subject: Fix newline added at end of file --- cloudinit/util.py | 1 - 1 file changed, 1 deletion(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index 58f7455c..f236d0bf 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2007,4 +2007,3 @@ def human2bytes(size): raise ValueError("'%s': cannot be negative" % size_in) return int(num * mpliers[mplier]) - -- cgit v1.2.3 From 4994f7cb475713be523f96e077a76f801e6d1db5 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Sat, 11 Oct 2014 18:23:20 -0700 Subject: Allow the omnibus url fetching retries to be configurable --- cloudinit/config/cc_chef.py | 6 +++++- cloudinit/util.py | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py index e503371d..205f4b49 100644 --- a/cloudinit/config/cc_chef.py +++ b/cloudinit/config/cc_chef.py @@ -37,6 +37,7 @@ CHEF_DIRS = [ ] OMNIBUS_URL = "https://www.opscode.com/chef/install.sh" +OMNIBUS_URL_RETRIES = 5 CHEF_RB_TPL_DEFAULTS = { # These are ruby symbols... @@ -199,7 +200,10 @@ def install_chef(cloud, chef_cfg, log): elif install_type == 'omnibus': # This will install as a omnibus unified package url = util.get_cfg_option_str(chef_cfg, "omnibus_url", OMNIBUS_URL) - content = url_helper.readurl(url=url, retries=5) + retries = max(0, util.get_cfg_option_int(chef_cfg, + "omnibus_url_retries", + default=OMNIBUS_URL_RETRIES)) + content = url_helper.readurl(url=url, retries=retries) with util.tempdir() as tmpd: # Use tmpdir over tmpfile to avoid 'text file busy' on execute tmpf = "%s/chef-omnibus-install" % tmpd diff --git a/cloudinit/util.py b/cloudinit/util.py index f236d0bf..71221e09 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -399,6 +399,10 @@ def get_cfg_option_str(yobj, key, default=None): return val +def get_cfg_option_int(yobj, key, default=0): + return int(get_cfg_option_str(yobj, key, default=default)) + + def system_info(): return { 'platform': platform.platform(), -- cgit v1.2.3 From 2d9d3811b4b1d4ea078a0bba6cf5e067339c14f3 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Sat, 18 Oct 2014 09:27:47 -0700 Subject: Pretty up the debug module Previously the usage of the yaml_dumps module was causing any python unicode key and value to show up as: 'item': !!python/unicode "some string" This was not very pretty... Fix this by using safe_dumps (which is also a good thing to use and allow_unicode=True). Also create a tiny helper function in the cc_debug module that does not include the yaml start and end footers (since this module has its own footers and headers). Also includes a basic sanity test for this module. --- cloudinit/config/cc_debug.py | 18 +++-- cloudinit/util.py | 16 ++--- tests/unittests/test_handler/test_handler_debug.py | 78 ++++++++++++++++++++++ 3 files changed, 99 insertions(+), 13 deletions(-) create mode 100644 tests/unittests/test_handler/test_handler_debug.py (limited to 'cloudinit/util.py') diff --git a/cloudinit/config/cc_debug.py b/cloudinit/config/cc_debug.py index 7219b0f8..2705035b 100644 --- a/cloudinit/config/cc_debug.py +++ b/cloudinit/config/cc_debug.py @@ -14,11 +14,14 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from cloudinit import type_utils -from cloudinit import util import copy from StringIO import StringIO +from cloudinit import type_utils +from cloudinit import util + +SKIP_KEYS = frozenset(['log_cfgs']) + def _make_header(text): header = StringIO() @@ -31,6 +34,11 @@ def _make_header(text): return header.getvalue() +def _dumps(obj): + text = util.yaml_dumps(obj, explicit_start=False, explicit_end=False) + return text.rstrip() + + def handle(name, cfg, cloud, log, args): verbose = util.get_cfg_by_path(cfg, ('debug', 'verbose'), default=True) if args: @@ -46,7 +54,7 @@ def handle(name, cfg, cloud, log, args): return # Clean out some keys that we just don't care about showing... dump_cfg = copy.deepcopy(cfg) - for k in ['log_cfgs']: + for k in SKIP_KEYS: dump_cfg.pop(k, None) all_keys = list(dump_cfg.keys()) for k in all_keys: @@ -55,10 +63,10 @@ def handle(name, cfg, cloud, log, args): # Now dump it... to_print = StringIO() to_print.write(_make_header("Config")) - to_print.write(util.yaml_dumps(dump_cfg)) + to_print.write(_dumps(dump_cfg)) to_print.write("\n") to_print.write(_make_header("MetaData")) - to_print.write(util.yaml_dumps(cloud.datasource.metadata)) + to_print.write(_dumps(cloud.datasource.metadata)) to_print.write("\n") to_print.write(_make_header("Misc")) to_print.write("Datasource: %s\n" % diff --git a/cloudinit/util.py b/cloudinit/util.py index f236d0bf..13084374 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1270,14 +1270,14 @@ def read_write_cmdline_url(target_fn): logexc(LOG, "Failed writing url content to %s", target_fn) -def yaml_dumps(obj): - formatted = yaml.dump(obj, - line_break="\n", - indent=4, - explicit_start=True, - explicit_end=True, - default_flow_style=False) - return formatted +def yaml_dumps(obj, explicit_start=True, explicit_end=True): + return yaml.safe_dump(obj, + line_break="\n", + indent=4, + explicit_start=explicit_start, + explicit_end=explicit_end, + default_flow_style=False, + allow_unicode=True) def ensure_dir(path, mode=None): diff --git a/tests/unittests/test_handler/test_handler_debug.py b/tests/unittests/test_handler/test_handler_debug.py new file mode 100644 index 00000000..bd9e29d8 --- /dev/null +++ b/tests/unittests/test_handler/test_handler_debug.py @@ -0,0 +1,78 @@ +# vi: ts=4 expandtab +# +# Copyright (C) 2014 Yahoo! Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from cloudinit.config import cc_debug + +from cloudinit import cloud +from cloudinit import distros +from cloudinit import helpers +from cloudinit import util + +from cloudinit.sources import DataSourceNone + +from .. import helpers as t_help + +import logging + +LOG = logging.getLogger(__name__) + + +class TestDebug(t_help.FilesystemMockingTestCase): + def setUp(self): + super(TestDebug, self).setUp() + self.new_root = self.makeDir(prefix="unittest_") + + def _get_cloud(self, distro, metadata=None): + self.patchUtils(self.new_root) + paths = helpers.Paths({}) + cls = distros.fetch(distro) + d = cls(distro, {}, paths) + ds = DataSourceNone.DataSourceNone({}, d, paths) + if metadata: + ds.metadata.update(metadata) + return cloud.Cloud(ds, paths, {}, d, None) + + def test_debug_write(self): + cfg = { + 'abc': '123', + 'c': u'\u20a0', + 'debug': { + 'verbose': True, + # Does not actually write here due to mocking... + 'output': '/var/log/cloud-init-debug.log', + }, + } + cc = self._get_cloud('ubuntu') + cc_debug.handle('cc_debug', cfg, cc, LOG, []) + contents = util.load_file('/var/log/cloud-init-debug.log') + # Some basic sanity tests... + self.assertGreater(len(contents), 0) + for k in cfg.keys(): + self.assertIn(k, contents) + + def test_debug_no_write(self): + cfg = { + 'abc': '123', + 'debug': { + 'verbose': False, + # Does not actually write here due to mocking... + 'output': '/var/log/cloud-init-debug.log', + }, + } + cc = self._get_cloud('ubuntu') + cc_debug.handle('cc_debug', cfg, cc, LOG, []) + self.assertRaises(IOError, + util.load_file, '/var/log/cloud-init-debug.log') -- cgit v1.2.3 From d3efeef731470b1f840ab57c7fabc38954799a15 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 29 Oct 2014 15:39:04 -0400 Subject: fix bad logic resulting in failure to honor 'output' config. This busted logic causes 'output' to not be paid any attention to, and thus output is not written to /var/log/cloud-init-output.log. LP: #1387340 --- cloudinit/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index f236d0bf..4bb73c11 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1146,7 +1146,7 @@ def chownbyname(fname, user=None, group=None): # this returns the specific 'mode' entry, cleanly formatted, with value def get_output_cfg(cfg, mode): ret = [None, None] - if cfg or 'output' not in cfg: + if not cfg or 'output' not in cfg: return ret outcfg = cfg['output'] -- cgit v1.2.3