← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] lp:~harlowja/cloud-init/better-chef-module into lp:cloud-init

 


Diff comments:

> === 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-12 00:01:55 +0000
> @@ -18,6 +18,7 @@
>  #    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 +39,74 @@
>  
>  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'
> +CHEF_EXEC_PATH = '/usr/bin/chef-client'
> +CHEF_EXEC_CMD = tuple([CHEF_EXEC_PATH, '-d', '-i', '1800', '-s', '20'])

Fixed, allowed new 'exec_arguments' to allow people to override this, so a user can pass in ['-j', 'path'], if not provided this just does the default.

> +
> +
> +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 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(),
> +        '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
> +
>  
>  def handle(name, cfg, cloud, log, _args):
>  
> @@ -49,7 +118,7 @@
>      chef_cfg = cfg['chef']
>  
>      # Ensure the chef directories we use exist
> -    for d in CHEF_DIRS:
> +    for d in list(chef_cfg.get('directories', CHEF_DIRS)):
>          util.ensure_dir(d)
>  
>      # Set the validation key based on the presence of either 'validation_key'
> @@ -64,57 +133,64 @@
>      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)
> -    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))
> -
> +        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:
> +        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)
> +        if run:
> +            log.debug('Running chef-client')
> +            util.subp(CHEF_EXEC_CMD, capture=False)
> +
> +
> +def install_chef(cloud, chef_cfg, log):
>      # 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)
> -        else:
> -            log.warn("Unknown chef install type %s", install_type)
> +    install_type = util.get_cfg_option_str(chef_cfg, 'install_type',
> +                                           'packages')
> +    run_after = 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)
> +        run_after = 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)
> +        content = url_helper.readurl(url=url, retries=5)
> +        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)

I'd assume packages do there own daemon management, wouldn't then, using init.d, systemd or other... For the omnibus method this seems ok, anyway made it configurable (defaulting to the old behavior in a recent commit).

> +    else:
> +        log.warn("Unknown chef install type '%s'", install_type)
> +    return run_after
>  
>  
>  def get_ruby_packages(version):
> @@ -133,9 +209,9 @@
>          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)
> 
> === 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-12 00:01:55 +0000
> @@ -9,17 +9,50 @@
>      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 %}
> 
> === 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-12 00:01:55 +0000
> @@ -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)
> 


-- 
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.


References