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. --- tests/unittests/test_handler/test_handler_chef.py | 84 +++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 tests/unittests/test_handler/test_handler_chef.py (limited to 'tests/unittests/test_handler') 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 From 28d09d73651772ed6c95e67f24e0a04488e79bd5 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 10 Oct 2014 17:44:57 -0700 Subject: Add a few template delete tests --- tests/unittests/test_handler/test_handler_chef.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'tests/unittests/test_handler') diff --git a/tests/unittests/test_handler/test_handler_chef.py b/tests/unittests/test_handler/test_handler_chef.py index 5562d18a..de7ff2da 100644 --- a/tests/unittests/test_handler/test_handler_chef.py +++ b/tests/unittests/test_handler/test_handler_chef.py @@ -82,3 +82,22 @@ class TestChef(t_help.FilesystemMockingTestCase): 'run_list': ['a', 'b', 'c'], 'c': 'd', }, json.loads(c)) + + def test_template_deletes(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', + 'json_attribs': None, + 'show_time': None, + }, + } + cc_chef.handle('chef', cfg, self.fetch_cloud('ubuntu'), LOG, []) + c = util.load_file(cc_chef.CHEF_RB_PATH) + self.assertNotIn('json_attribs', c) + self.assertNotIn('Formatter.show_time', c) -- cgit v1.2.3 From d87e89d9c674bac7e87d483037850a9ee4fc984a Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Sat, 11 Oct 2014 16:59:50 -0700 Subject: More adjustments - Use the generated_by() utility function to give the ruby template a better header comment - Set special parameters after selecting the basic chef parameters. --- cloudinit/config/cc_chef.py | 19 +++++++++++-------- templates/chef_client.rb.tmpl | 3 +-- tests/unittests/test_handler/test_handler_chef.py | 20 +++++++++++++++++++- 3 files changed, 31 insertions(+), 11 deletions(-) (limited to 'tests/unittests/test_handler') diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py index fb825404..999b658d 100644 --- a/cloudinit/config/cc_chef.py +++ b/cloudinit/config/cc_chef.py @@ -79,13 +79,6 @@ def is_installed(): 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... @@ -101,7 +94,17 @@ def get_template_params(iid, chef_cfg, log): 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() + # These ones are overwritten to be exact values... + params.update({ + 'generated_by': util.make_header(), + 'server_url': util.get_cfg_option_str(chef_cfg, 'server_url'), + 'node_name': util.get_cfg_option_str(chef_cfg, 'node_name', + default=iid), + 'environment': util.get_cfg_option_str(chef_cfg, 'environment', + default='_default'), + 'validation_name': util.get_cfg_option_str(chef_cfg, + 'validation_name'), + }) return params diff --git a/templates/chef_client.rb.tmpl b/templates/chef_client.rb.tmpl index 7b9e6298..c4069d22 100644 --- a/templates/chef_client.rb.tmpl +++ b/templates/chef_client.rb.tmpl @@ -9,12 +9,11 @@ you need to add the following to config: validation_name: XYZ server_url: XYZ -#} - +{{generated_by}} {# 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 %} diff --git a/tests/unittests/test_handler/test_handler_chef.py b/tests/unittests/test_handler/test_handler_chef.py index de7ff2da..ef1aa208 100644 --- a/tests/unittests/test_handler/test_handler_chef.py +++ b/tests/unittests/test_handler/test_handler_chef.py @@ -1,5 +1,5 @@ -import os import json +import os from cloudinit.config import cc_chef @@ -38,6 +38,24 @@ class TestChef(t_help.FilesystemMockingTestCase): self.assertFalse(os.path.isdir(d)) def test_basic_config(self): + # This should create a file of the format... + """ + # Created by cloud-init v. 0.7.6 on Sat, 11 Oct 2014 23:57:21 +0000 + log_level :info + ssl_verify_mode :verify_none + log_location "/var/log/chef/client.log" + validation_client_name "bob" + validation_key "/etc/chef/validation.pem" + client_key "/etc/chef/client.pem" + chef_server_url "localhost" + environment "_default" + node_name "iid-datasource-none" + 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" + Chef::Log::Formatter.show_time = true + """ tpl_file = util.load_file('templates/chef_client.rb.tmpl') self.patchUtils(self.tmp) self.patchOS(self.tmp) -- cgit v1.2.3 From 8eedba0d3b9851bb0101407c5a070b7a975efa04 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 17 Oct 2014 12:32:41 -0700 Subject: Expose uses_systemd as a distro function Without this change the tests are currently failing on rhel7 since a location where a hostname file is written no longer exists at that location when systemd is active. To avoid this allow the test to inspect if the distro has systemd enabled and avoid testing the file when systemd is being used so the test passes. We likely need to figure out a better way to test features that no longer exist as files but exist as commands with systemd in general. --- cloudinit/distros/rhel.py | 12 ++++++------ tests/unittests/test_handler/test_handler_set_hostname.py | 9 +++++---- 2 files changed, 11 insertions(+), 10 deletions(-) (limited to 'tests/unittests/test_handler') diff --git a/cloudinit/distros/rhel.py b/cloudinit/distros/rhel.py index e8abf111..1a269e08 100644 --- a/cloudinit/distros/rhel.py +++ b/cloudinit/distros/rhel.py @@ -98,7 +98,7 @@ class Distro(distros.Distro): rhel_util.update_sysconfig_file(self.network_conf_fn, net_cfg) return dev_names - def _dist_uses_systemd(self): + def uses_systemd(self): # Fedora 18 and RHEL 7 were the first adopters in their series (dist, vers) = util.system_info()['dist'][:2] major = (int)(vers.split('.')[0]) @@ -106,7 +106,7 @@ class Distro(distros.Distro): or (dist.startswith('Fedora') and major >= 18)) def apply_locale(self, locale, out_fn=None): - if self._dist_uses_systemd(): + if self.uses_systemd(): if not out_fn: out_fn = self.systemd_locale_conf_fn out_fn = self.systemd_locale_conf_fn @@ -119,7 +119,7 @@ class Distro(distros.Distro): rhel_util.update_sysconfig_file(out_fn, locale_cfg) def _write_hostname(self, hostname, out_fn): - if self._dist_uses_systemd(): + if self.uses_systemd(): util.subp(['hostnamectl', 'set-hostname', str(hostname)]) else: host_cfg = { @@ -135,14 +135,14 @@ class Distro(distros.Distro): return hostname def _read_system_hostname(self): - if self._dist_uses_systemd(): + if self.uses_systemd(): host_fn = self.systemd_hostname_conf_fn else: host_fn = self.hostname_conf_fn return (host_fn, self._read_hostname(host_fn)) def _read_hostname(self, filename, default=None): - if self._dist_uses_systemd(): + if self.uses_systemd(): (out, _err) = util.subp(['hostname']) if len(out): return out @@ -163,7 +163,7 @@ class Distro(distros.Distro): def set_timezone(self, tz): tz_file = self._find_tz_file(tz) - if self._dist_uses_systemd(): + if self.uses_systemd(): # Currently, timedatectl complains if invoked during startup # so for compatibility, create the link manually. util.del_file(self.tz_local_fn) diff --git a/tests/unittests/test_handler/test_handler_set_hostname.py b/tests/unittests/test_handler/test_handler_set_hostname.py index 03004ab9..e1530e30 100644 --- a/tests/unittests/test_handler/test_handler_set_hostname.py +++ b/tests/unittests/test_handler/test_handler_set_hostname.py @@ -37,10 +37,11 @@ class TestHostname(t_help.FilesystemMockingTestCase): self.patchUtils(self.tmp) cc_set_hostname.handle('cc_set_hostname', cfg, cc, LOG, []) - contents = util.load_file("/etc/sysconfig/network") - n_cfg = ConfigObj(StringIO(contents)) - self.assertEquals({'HOSTNAME': 'blah.blah.blah.yahoo.com'}, - dict(n_cfg)) + if not distro.uses_systemd(): + contents = util.load_file("/etc/sysconfig/network") + n_cfg = ConfigObj(StringIO(contents)) + self.assertEquals({'HOSTNAME': 'blah.blah.blah.yahoo.com'}, + dict(n_cfg)) def test_write_hostname_debian(self): cfg = { -- 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 'tests/unittests/test_handler') 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 1db41a6f5283d38dff6de0b0421d51eac869a39c Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Mon, 24 Nov 2014 16:41:21 -0800 Subject: Use assertNotEqual which exists on python2.6 Instead of using assertGreater which is new on python2.7 just use assertNotEqual which does exist on python2.6 to perform the same/similar operation. This makes the unittest not break on python2.6 --- tests/unittests/test_handler/test_handler_debug.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/unittests/test_handler') diff --git a/tests/unittests/test_handler/test_handler_debug.py b/tests/unittests/test_handler/test_handler_debug.py index bd9e29d8..8891ca04 100644 --- a/tests/unittests/test_handler/test_handler_debug.py +++ b/tests/unittests/test_handler/test_handler_debug.py @@ -59,7 +59,7 @@ class TestDebug(t_help.FilesystemMockingTestCase): 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) + self.assertNotEqual(0, len(contents)) for k in cfg.keys(): self.assertIn(k, contents) -- cgit v1.2.3