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/config/cc_chef.py | 97 ++++++++++++---- cloudinit/util.py | 135 +++++----------------- templates/chef_client.rb.tmpl | 52 +++++++-- tests/unittests/test_handler/test_handler_chef.py | 84 ++++++++++++++ 4 files changed, 230 insertions(+), 138 deletions(-) create mode 100644 tests/unittests/test_handler/test_handler_chef.py diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py index 806deed9..691a51bc 100644 --- a/cloudinit/config/cc_chef.py +++ b/cloudinit/config/cc_chef.py @@ -18,6 +18,8 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +from datetime import datetime + import json import os @@ -38,6 +40,61 @@ CHEF_DIRS = [ OMNIBUS_URL = "https://www.opscode.com/chef/install.sh" +CHEF_RB_TPL_DEFAULTS = { + # These are ruby symbols... + 'ssl_verify_mode': ':verify_none', + 'log_level': ':info', + # These are not symbols... + 'log_location': '/var/log/chef/client.log', + 'validation_key': "/etc/chef/validation.pem", + 'client_key': "/etc/chef/client.pem", + 'json_attribs': "/etc/chef/firstboot.json", + 'file_cache_path': "/var/cache/chef", + 'file_backup_path': "/var/backups/chef", + 'pid_file': "/var/run/chef/client.pid", + 'show_time': True, +} +CHEF_RB_TPL_BOOL_KEYS = frozenset(['show_time']) +CHEF_RB_TPL_KEYS = list(CHEF_RB_TPL_DEFAULTS.keys()) +CHEF_RB_TPL_KEYS.extend(CHEF_RB_TPL_BOOL_KEYS) +CHEF_RB_TPL_KEYS.extend([ + 'server_url', + 'node_name', + 'environment', + 'validation_name', +]) +CHEF_RB_TPL_KEYS = frozenset(CHEF_RB_TPL_KEYS) +CHEF_RB_PATH = '/etc/chef/client.rb' +CHEF_FB_PATH = '/etc/chef/firstboot.json' + + +def get_template_params(iid, chef_cfg, log): + params = CHEF_RB_TPL_DEFAULTS.copy() + params.update({ + 'server_url': chef_cfg['server_url'], + 'node_name': util.get_cfg_option_str(chef_cfg, 'node_name', iid), + 'environment': util.get_cfg_option_str(chef_cfg, 'environment', + '_default'), + 'validation_name': chef_cfg['validation_name'], + }) + # Allow users to overwrite any of the keys they want (if they so choose), + # when a value is None, then the value will be set to None and no boolean + # or string version will be populated... + for (k, v) in chef_cfg.items(): + if k not in CHEF_RB_TPL_KEYS: + log.debug("Skipping unknown chef template key '%s'", k) + continue + if v is None: + params[k] = None + else: + # This will make the value a boolean or string... + if k in CHEF_RB_TPL_BOOL_KEYS: + params[k] = util.get_cfg_option_bool(chef_cfg, k) + else: + params[k] = util.get_cfg_option_str(chef_cfg, k) + params['generated_on'] = datetime.now().isoformat() + return params + def handle(name, cfg, cloud, log, _args): @@ -49,7 +106,7 @@ def handle(name, cfg, cloud, log, _args): chef_cfg = cfg['chef'] # Ensure the chef directories we use exist - for d in CHEF_DIRS: + for d in chef_cfg.get('directories', CHEF_DIRS): util.ensure_dir(d) # Set the validation key based on the presence of either 'validation_key' @@ -64,26 +121,26 @@ def handle(name, cfg, cloud, log, _args): template_fn = cloud.get_template_filename('chef_client.rb') if template_fn: iid = str(cloud.datasource.get_instance_id()) - params = { - 'server_url': chef_cfg['server_url'], - 'node_name': util.get_cfg_option_str(chef_cfg, 'node_name', iid), - 'environment': util.get_cfg_option_str(chef_cfg, 'environment', - '_default'), - 'validation_name': chef_cfg['validation_name'] - } - templater.render_to_file(template_fn, '/etc/chef/client.rb', params) + params = get_template_params(iid, chef_cfg, log) + templater.render_to_file(template_fn, CHEF_RB_PATH, params) + else: + log.warn("No template found, not rendering to %s", + CHEF_RB_PATH) + + # Set the firstboot json + fb_filename = util.get_cfg_option_str(chef_cfg, 'firstboot_path', + default=CHEF_FB_PATH) + if not fb_filename: + log.info("First boot path empty, not writing first boot json file") else: - log.warn("No template found, not rendering to /etc/chef/client.rb") - - # set the firstboot json - initial_json = {} - if 'run_list' in chef_cfg: - initial_json['run_list'] = chef_cfg['run_list'] - if 'initial_attributes' in chef_cfg: - initial_attributes = chef_cfg['initial_attributes'] - for k in list(initial_attributes.keys()): - initial_json[k] = initial_attributes[k] - util.write_file('/etc/chef/firstboot.json', json.dumps(initial_json)) + initial_json = {} + if 'run_list' in chef_cfg: + initial_json['run_list'] = chef_cfg['run_list'] + if 'initial_attributes' in chef_cfg: + initial_attributes = chef_cfg['initial_attributes'] + for k in list(initial_attributes.keys()): + initial_json[k] = initial_attributes[k] + util.write_file(fb_filename, json.dumps(initial_json)) # If chef is not installed, we install chef based on 'install_type' if (not os.path.isfile('/usr/bin/chef-client') or 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]) diff --git a/templates/chef_client.rb.tmpl b/templates/chef_client.rb.tmpl index 538850ca..7b9e6298 100644 --- a/templates/chef_client.rb.tmpl +++ b/templates/chef_client.rb.tmpl @@ -9,17 +9,51 @@ you need to add the following to config: validation_name: XYZ server_url: XYZ -#} -log_level :info -log_location "/var/log/chef/client.log" -ssl_verify_mode :verify_none + +{# +The reason these are not in quotes is because they are ruby +symbols that will be placed inside here, and not actual strings... +#} +# This is a generated file, created on {{generated_on}}. +{% if log_level %} +log_level {{log_level}} +{% endif %} +{% if ssl_verify_mode %} +ssl_verify_mode {{ssl_verify_mode}} +{% endif %} +{% if log_location %} +log_location "{{log_location}}" +{% endif %} +{% if validation_name %} validation_client_name "{{validation_name}}" -validation_key "/etc/chef/validation.pem" -client_key "/etc/chef/client.pem" +{% endif %} +{% if validation_key %} +validation_key "{{validation_key}}" +{% endif %} +{% if client_key %} +client_key "{{client_key}}" +{% endif %} +{% if server_url %} chef_server_url "{{server_url}}" +{% endif %} +{% if environment %} environment "{{environment}}" +{% endif %} +{% if node_name %} node_name "{{node_name}}" -json_attribs "/etc/chef/firstboot.json" -file_cache_path "/var/cache/chef" -file_backup_path "/var/backups/chef" -pid_file "/var/run/chef/client.pid" +{% endif %} +{% if json_attribs %} +json_attribs "{{json_attribs}}" +{% endif %} +{% if file_cache_path %} +file_cache_path "{{file_cache_path}}" +{% endif %} +{% if file_backup_path %} +file_backup_path "{{file_backup_path}}" +{% endif %} +{% if pid_file %} +pid_file "{{pid_file}}" +{% endif %} +{% if show_time %} Chef::Log::Formatter.show_time = true +{% endif %} diff --git a/tests/unittests/test_handler/test_handler_chef.py b/tests/unittests/test_handler/test_handler_chef.py new file mode 100644 index 00000000..5562d18a --- /dev/null +++ b/tests/unittests/test_handler/test_handler_chef.py @@ -0,0 +1,84 @@ +import os +import json + +from cloudinit.config import cc_chef + +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 TestChef(t_help.FilesystemMockingTestCase): + def setUp(self): + super(TestChef, self).setUp() + self.tmp = self.makeDir(prefix="unittest_") + + def fetch_cloud(self, distro_kind): + cls = distros.fetch(distro_kind) + paths = helpers.Paths({}) + distro = cls(distro_kind, {}, paths) + ds = DataSourceNone.DataSourceNone({}, distro, paths, None) + return cloud.Cloud(ds, paths, {}, distro, None) + + def test_no_config(self): + self.patchUtils(self.tmp) + self.patchOS(self.tmp) + + cfg = {} + cc_chef.handle('chef', cfg, self.fetch_cloud('ubuntu'), LOG, []) + for d in cc_chef.CHEF_DIRS: + self.assertFalse(os.path.isdir(d)) + + def test_basic_config(self): + tpl_file = util.load_file('templates/chef_client.rb.tmpl') + self.patchUtils(self.tmp) + self.patchOS(self.tmp) + + util.write_file('/etc/cloud/templates/chef_client.rb.tmpl', tpl_file) + cfg = { + 'chef': { + 'server_url': 'localhost', + 'validation_name': 'bob', + }, + } + cc_chef.handle('chef', cfg, self.fetch_cloud('ubuntu'), LOG, []) + for d in cc_chef.CHEF_DIRS: + self.assertTrue(os.path.isdir(d)) + c = util.load_file(cc_chef.CHEF_RB_PATH) + for k, v in cfg['chef'].items(): + self.assertIn(v, c) + for k, v in cc_chef.CHEF_RB_TPL_DEFAULTS.items(): + if isinstance(v, basestring): + self.assertIn(v, c) + c = util.load_file(cc_chef.CHEF_FB_PATH) + self.assertEqual({}, json.loads(c)) + + def test_firstboot_json(self): + self.patchUtils(self.tmp) + self.patchOS(self.tmp) + + cfg = { + 'chef': { + 'server_url': 'localhost', + 'validation_name': 'bob', + 'run_list': ['a', 'b', 'c'], + 'initial_attributes': { + 'c': 'd', + } + }, + } + cc_chef.handle('chef', cfg, self.fetch_cloud('ubuntu'), LOG, []) + c = util.load_file(cc_chef.CHEF_FB_PATH) + self.assertEqual( + { + 'run_list': ['a', 'b', 'c'], + 'c': 'd', + }, json.loads(c)) -- cgit v1.2.3