Increase the robustness/configurability 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.
- Make a helper function to tell if already installed.
- Have the install routine not run chef after installed but have it instead
  return a result to tell the caller to run the chef program once completed.
- Use the generated_by() utility function to give the ruby template a
  better header comment.
- Set special parameters after selecting the basic chef parameters.
- Allow for the running after install and run arguments to be configured.
- Allow the omnibus url fetching retries to be configurable.
- Move the chef running to its own helper function
- Add module docs

5 files changed, 414 insertions, 65 deletions

diff --git a/ChangeLog b/ChangeLog
index 449f501f..d9e91410 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -8,6 +8,8 @@
    rendered with jinja (LP: #1355343)
  - Only use datafiles and initsys addon outside virtualenvs
  - Fix the digital ocean test case on python 2.6
+ - Increase the usefulness, robustness, configurability of the chef module
+   so that it is more useful, more documented and better for users
 0.7.6:
  - open 0.7.6
  - Enable vendordata on CloudSigma datasource (LP: #1303986)
diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py
index 806deed9..b30d7361 100644
--- a/cloudinit/config/cc_chef.py
+++ b/cloudinit/config/cc_chef.py
@@ -18,6 +18,57 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+"""
+**Summary:** module that configures, starts and installs chef.
+
+**Description:** This module enables chef to be installed (from packages or
+from gems, or from omnibus). Before this occurs chef configurations are
+written to disk (validation.pem, client.pem, firstboot.json, client.rb),
+and needed chef folders/directories are created (/etc/chef and /var/log/chef
+and so-on). Then once installing proceeds correctly if configured chef will
+be started (in daemon mode or in non-daemon mode) and then once that has
+finished (if ran in non-daemon mode this will be when chef finishes
+converging, if ran in daemon mode then no further actions are possible since
+chef will have forked into its own process) then a post run function can
+run that can do finishing activities (such as removing the validation pem
+file).
+
+It can be configured with the following option structure::
+
+    chef:
+       directories: (defaulting to /etc/chef, /var/log/chef, /var/lib/chef,
+                     /var/cache/chef, /var/backups/chef, /var/run/chef)
+       validation_key or validation_cert: (optional string to be written to
+                                           /etc/chef/validation.pem)
+       firstboot_path: (path to write run_list and initial_attributes keys that
+                        should also be present in this configuration, defaults
+                        to /etc/chef/firstboot.json)
+       exec: boolean to run or not run chef (defaults to false, unless
+                                             a gem installed is requested
+                                             where this will then default
+                                             to true)
+
+    chef.rb template keys (if falsey, then will be skipped and not
+                           written to /etc/chef/client.rb)
+
+    chef:
+      client_key:
+      environment:
+      file_backup_path:
+      file_cache_path:
+      json_attribs:
+      log_level:
+      log_location:
+      node_name:
+      pid_file:
+      server_url:
+      show_time:
+      ssl_verify_mode:
+      validation_key:
+      validation_name:
+"""
+
+import itertools
 import json
 import os
 
@@ -27,16 +78,107 @@ from cloudinit import util
 
 RUBY_VERSION_DEFAULT = "1.8"
 
-CHEF_DIRS = [
+CHEF_DIRS = tuple([
     '/etc/chef',
     '/var/log/chef',
     '/var/lib/chef',
     '/var/cache/chef',
     '/var/backups/chef',
     '/var/run/chef',
-]
+])
+REQUIRED_CHEF_DIRS = tuple([
+    '/etc/chef',
+])
 
 OMNIBUS_URL = "https://www.opscode.com/chef/install.sh"
+OMNIBUS_URL_RETRIES = 5
+
+CHEF_VALIDATION_PEM_PATH = '/etc/chef/validation.pem'
+CHEF_FB_PATH = '/etc/chef/firstboot.json'
+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': CHEF_VALIDATION_PEM_PATH,
+    'client_key': "/etc/chef/client.pem",
+    'json_attribs': CHEF_FB_PATH,
+    '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_PATH_KEYS = frozenset([
+    'log_location',
+    'validation_key',
+    'client_key',
+    'file_cache_path',
+    'json_attribs',
+    'file_cache_path',
+    'pid_file',
+])
+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(CHEF_RB_TPL_PATH_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_EXEC_PATH = '/usr/bin/chef-client'
+CHEF_EXEC_DEF_ARGS = tuple(['-d', '-i', '1800', '-s', '20'])
+
+
+def is_installed():
+    if not os.path.isfile(CHEF_EXEC_PATH):
+        return False
+    if not os.access(CHEF_EXEC_PATH, os.X_OK):
+        return False
+    return True
+
+
+def post_run_chef(chef_cfg, log):
+    delete_pem = util.get_cfg_option_bool(chef_cfg,
+                                          'delete_validation_post_exec',
+                                          default=False)
+    if delete_pem and os.path.isfile(CHEF_VALIDATION_PEM_PATH):
+        os.unlink(CHEF_VALIDATION_PEM_PATH)
+
+
+def get_template_params(iid, chef_cfg, log):
+    params = CHEF_RB_TPL_DEFAULTS.copy()
+    # 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)
+    # These ones are overwritten to be exact values...
+    params.update({
+        'generated_by': util.make_header(),
+        'node_name': util.get_cfg_option_str(chef_cfg, 'node_name',
+                                             default=iid),
+        'environment': util.get_cfg_option_str(chef_cfg, 'environment',
+                                               default='_default'),
+        # These two are mandatory...
+        'server_url': chef_cfg['server_url'],
+        'validation_name': chef_cfg['validation_name'],
+    })
+    return params
 
 
 def handle(name, cfg, cloud, log, _args):
@@ -49,7 +191,10 @@ def handle(name, cfg, cloud, log, _args):
     chef_cfg = cfg['chef']
 
     # Ensure the chef directories we use exist
-    for d in CHEF_DIRS:
+    chef_dirs = util.get_cfg_option_list(chef_cfg, 'directories')
+    if not chef_dirs:
+        chef_dirs = list(CHEF_DIRS)
+    for d in itertools.chain(chef_dirs, REQUIRED_CHEF_DIRS):
         util.ensure_dir(d)
 
     # Set the validation key based on the presence of either 'validation_key'
@@ -57,64 +202,108 @@ def handle(name, cfg, cloud, log, _args):
     # takes precedence
     for key in ('validation_key', 'validation_cert'):
         if key in chef_cfg and chef_cfg[key]:
-            util.write_file('/etc/chef/validation.pem', chef_cfg[key])
+            util.write_file(CHEF_VALIDATION_PEM_PATH, chef_cfg[key])
             break
 
     # Create the chef config from template
     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)
+        # Do a best effort attempt to ensure that the template values that
+        # are associated with paths have there parent directory created
+        # before they are used by the chef-client itself.
+        param_paths = set()
+        for (k, v) in params.items():
+            if k in CHEF_RB_TPL_PATH_KEYS and v:
+                param_paths.add(os.path.dirname(v))
+        util.ensure_dirs(param_paths)
+        templater.render_to_file(template_fn, CHEF_RB_PATH, params)
     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))
+        log.warn("No template found, not rendering to %s",
+                 CHEF_RB_PATH)
 
-    # If chef is not installed, we install chef based on 'install_type'
-    if (not os.path.isfile('/usr/bin/chef-client') or
-            util.get_cfg_option_bool(chef_cfg,
-                'force_install', default=False)):
-
-        install_type = util.get_cfg_option_str(chef_cfg, 'install_type',
-                                               'packages')
-        if install_type == "gems":
-            # this will install and run the chef-client from gems
-            chef_version = util.get_cfg_option_str(chef_cfg, 'version', None)
-            ruby_version = util.get_cfg_option_str(chef_cfg, 'ruby_version',
-                                                   RUBY_VERSION_DEFAULT)
-            install_chef_from_gems(cloud.distro, ruby_version, chef_version)
-            # and finally, run chef-client
-            log.debug('Running chef-client')
-            util.subp(['/usr/bin/chef-client',
-                       '-d', '-i', '1800', '-s', '20'], capture=False)
-        elif install_type == 'packages':
-            # this will install and run the chef-client from packages
-            cloud.distro.install_packages(('chef',))
-        elif install_type == 'omnibus':
-            url = util.get_cfg_option_str(chef_cfg, "omnibus_url", OMNIBUS_URL)
-            content = url_helper.readurl(url=url, retries=5)
-            with util.tempdir() as tmpd:
-                # use tmpd over tmpfile to avoid 'Text file busy' on execute
-                tmpf = "%s/chef-omnibus-install" % tmpd
-                util.write_file(tmpf, str(content), mode=0700)
-                util.subp([tmpf], capture=False)
+    # 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:
+        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))
+
+    # Try to install chef, if its not already installed...
+    force_install = util.get_cfg_option_bool(chef_cfg,
+                                             'force_install', default=False)
+    if not is_installed() or force_install:
+        run = install_chef(cloud, chef_cfg, log)
+    elif is_installed():
+        run = util.get_cfg_option_bool(chef_cfg, 'exec', default=False)
+    else:
+        run = False
+    if run:
+        run_chef(chef_cfg, log)
+        post_run_chef(chef_cfg, log)
+
+
+def run_chef(chef_cfg, log):
+    log.debug('Running chef-client')
+    cmd = [CHEF_EXEC_PATH]
+    if 'exec_arguments' in chef_cfg:
+        cmd_args = chef_cfg['exec_arguments']
+        if isinstance(cmd_args, (list, tuple)):
+            cmd.extend(cmd_args)
+        elif isinstance(cmd_args, (str, basestring)):
+            cmd.append(cmd_args)
         else:
-            log.warn("Unknown chef install type %s", install_type)
+            log.warn("Unknown type %s provided for chef"
+                     " 'exec_arguments' expected list, tuple,"
+                     " or string", type(cmd_args))
+            cmd.extend(CHEF_EXEC_DEF_ARGS)
+    else:
+        cmd.extend(CHEF_EXEC_DEF_ARGS)
+    util.subp(cmd, capture=False)
+
+
+def install_chef(cloud, chef_cfg, log):
+    # If chef is not installed, we install chef based on 'install_type'
+    install_type = util.get_cfg_option_str(chef_cfg, 'install_type',
+                                           'packages')
+    run = util.get_cfg_option_bool(chef_cfg, 'exec', default=False)
+    if install_type == "gems":
+        # This will install and run the chef-client from gems
+        chef_version = util.get_cfg_option_str(chef_cfg, 'version', None)
+        ruby_version = util.get_cfg_option_str(chef_cfg, 'ruby_version',
+                                               RUBY_VERSION_DEFAULT)
+        install_chef_from_gems(cloud.distro, ruby_version, chef_version)
+        # Retain backwards compat, by preferring True instead of False
+        # when not provided/overriden...
+        run = util.get_cfg_option_bool(chef_cfg, 'exec', default=True)
+    elif install_type == 'packages':
+        # This will install and run the chef-client from packages
+        cloud.distro.install_packages(('chef',))
+    elif install_type == 'omnibus':
+        # This will install as a omnibus unified package
+        url = util.get_cfg_option_str(chef_cfg, "omnibus_url", OMNIBUS_URL)
+        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
+            util.write_file(tmpf, str(content), mode=0700)
+            util.subp([tmpf], capture=False)
+    else:
+        log.warn("Unknown chef install type '%s'", install_type)
+        run = False
+    return run
 
 
 def get_ruby_packages(version):
@@ -133,9 +322,9 @@ def install_chef_from_gems(ruby_version, chef_version, distro):
         util.sym_link('/usr/bin/ruby%s' % ruby_version, '/usr/bin/ruby')
     if chef_version:
         util.subp(['/usr/bin/gem', 'install', 'chef',
-                  '-v %s' % chef_version, '--no-ri',
-                  '--no-rdoc', '--bindir', '/usr/bin', '-q'], capture=False)
+                   '-v %s' % chef_version, '--no-ri',
+                   '--no-rdoc', '--bindir', '/usr/bin', '-q'], capture=False)
     else:
         util.subp(['/usr/bin/gem', 'install', 'chef',
-                  '--no-ri', '--no-rdoc', '--bindir',
-                  '/usr/bin', '-q'], capture=False)
+                   '--no-ri', '--no-rdoc', '--bindir',
+                   '/usr/bin', '-q'], capture=False)
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 4bb73c11..b71057fb 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(),
diff --git a/templates/chef_client.rb.tmpl b/templates/chef_client.rb.tmpl
index 538850ca..c4069d22 100644
--- a/templates/chef_client.rb.tmpl
+++ b/templates/chef_client.rb.tmpl
@@ -9,17 +9,50 @@ 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
+{{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...
+#}
+{% 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..ef1aa208
--- /dev/null
+++ b/tests/unittests/test_handler/test_handler_chef.py
@@ -0,0 +1,121 @@
+import json
+import os
+
+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):
+        # 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)
+
+        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))
+
+    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)