cloud-init-dev team mailing list archive

[Joshua Harlow <harlowja@xxxxxxxxx>]

Joshua Harlow has proposed merging lp:~harlowja/cloud-init/better-chef-module into lp:cloud-init.

Requested reviews:
  cloud init development team (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~harlowja/cloud-init/better-chef-module/+merge/238040


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.
-- 
https://code.launchpad.net/~harlowja/cloud-init/better-chef-module/+merge/238040
Your team cloud init development team is requested to review the proposed merge of lp:~harlowja/cloud-init/better-chef-module into lp:cloud-init.

=== modified file 'cloudinit/config/cc_chef.py'
--- cloudinit/config/cc_chef.py	2014-08-26 18:50:11 +0000
+++ cloudinit/config/cc_chef.py	2014-10-11 00:35:26 +0000
@@ -18,6 +18,8 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+from datetime import datetime
+
 import json
 import os
 
@@ -38,6 +40,61 @@
 
 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 @@
     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 @@
     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 /etc/chef/client.rb")
+        log.warn("No template found, not rendering to %s",
+                 CHEF_RB_PATH)
 
-    # 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))
+    # 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))
 
     # If chef is not installed, we install chef based on 'install_type'
     if (not os.path.isfile('/usr/bin/chef-client') or

=== modified file 'cloudinit/util.py'
--- cloudinit/util.py	2014-09-30 20:24:54 +0000
+++ cloudinit/util.py	2014-10-11 00:35:26 +0000
@@ -2007,3 +2007,4 @@
         raise ValueError("'%s': cannot be negative" % size_in)
 
     return int(num * mpliers[mplier])
+

=== modified file 'templates/chef_client.rb.tmpl'
--- templates/chef_client.rb.tmpl	2014-03-05 23:05:59 +0000
+++ templates/chef_client.rb.tmpl	2014-10-11 00:35:26 +0000
@@ -9,17 +9,51 @@
     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 %}

=== added file 'tests/unittests/test_handler/test_handler_chef.py'
--- tests/unittests/test_handler/test_handler_chef.py	1970-01-01 00:00:00 +0000
+++ tests/unittests/test_handler/test_handler_chef.py	2014-10-11 00:35:26 +0000
@@ -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))