Merge lp:~avishai-ish-shalom/cloud-init/chef-refactor into lp:~cloud-init-dev/cloud-init/trunk

Proposed by Avishai Ish-Shalom
Status: Rejected
Rejected by: Scott Moser
Proposed branch: lp:~avishai-ish-shalom/cloud-init/chef-refactor
Merge into: lp:~cloud-init-dev/cloud-init/trunk
Diff against target: 359 lines (+212/-45)
3 files modified
cloudinit/config/cc_chef.py (+189/-42)
doc/examples/cloud-config-chef.txt (+17/-2)
templates/chef_client.rb.tmpl (+6/-1)
To merge this branch: bzr merge lp:~avishai-ish-shalom/cloud-init/chef-refactor
Reviewer Review Type Date Requested Status
Scott Moser Needs Fixing
Review via email: mp+164009@code.launchpad.net

Description of the change

Refactored cc_chef, added omnibus and chef-solo support

To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote :

Avishai,
  Some comments. Sorry for taking so long to see this.
  * BIN_PATHS: I would really prefer that we just use PATH to look for executables. If the system is not set up to have PATH set correctly, the user should just use a full path. ie, the right way to get your executable found is to put it in PATH.
  * repo_path': '/var/lib/cloud/chef_repo'
    doesn't this fit better somewhere else ? I"m not entirely opposed to it, but it just seems like there is likely somewhere else to look. Also, if you're using /var/lib/cloud, please get it through 'cloud_dir' in the Paths object (cloud.paths).
  * some tests would be nice.

Thanks for the submission, sorry its taken so long to get around to it.

review: Needs Fixing
Revision history for this message
Avishai Ish-Shalom (avishai-ish-shalom) wrote :

i'll review my patch and get back to you.

On Wed, Jul 17, 2013 at 11:09 PM, Scott Moser <email address hidden> wrote:

> Review: Needs Fixing
>
> Avishai,
> Some comments. Sorry for taking so long to see this.
> * BIN_PATHS: I would really prefer that we just use PATH to look for
> executables. If the system is not set up to have PATH set correctly, the
> user should just use a full path. ie, the right way to get your executable
> found is to put it in PATH.
> * repo_path': '/var/lib/cloud/chef_repo'
> doesn't this fit better somewhere else ? I"m not entirely opposed to
> it, but it just seems like there is likely somewhere else to look. Also,
> if you're using /var/lib/cloud, please get it through 'cloud_dir' in the
> Paths object (cloud.paths).
> * some tests would be nice.
>
> Thanks for the submission, sorry its taken so long to get around to it.
>
> --
>
> https://code.launchpad.net/~avishai-ish-shalom/cloud-init/chef-refactor/+merge/164009
> You are the owner of lp:~avishai-ish-shalom/cloud-init/chef-refactor.
>

Revision history for this message
Scott Moser (smoser) wrote :

Hello,
Thank you for taking the time to contribute to cloud-init. Cloud-init has moved its revision control system to git. As a result, we are marking all bzr merge proposals as 'rejected'. If you would like to re-submit this proposal for review, please do so by following the current HACKING documentation at http://cloudinit.readthedocs.io/en/latest/topics/hacking.html .

Unmerged revisions

818. By Avishai Ish-Shalom

cc_chef: gem install method should install build-essential

817. By Avishai Ish-Shalom

cc_chef: get_cookbooks from tarball or git

816. By Avishai Ish-Shalom

cc_chef: refactored, implemented omnibus, initial chef-solo support

TODO: Add chef-solo get_cookbooks variants

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cloudinit/config/cc_chef.py'
--- cloudinit/config/cc_chef.py 2012-12-12 15:39:43 +0000
+++ cloudinit/config/cc_chef.py 2013-05-15 17:20:33 +0000
@@ -20,10 +20,15 @@
2020
21import json21import json
22import os22import os
23import urllib
2324
24from cloudinit import templater25from cloudinit import templater
25from cloudinit import url_helper26from cloudinit import url_helper
26from cloudinit import util27from cloudinit import util
28from cloudinit.settings import PER_INSTANCE
29from urlparse import urlparse
30
31frequency = PER_INSTANCE
2732
28RUBY_VERSION_DEFAULT = "1.8"33RUBY_VERSION_DEFAULT = "1.8"
2934
@@ -38,20 +43,51 @@
3843
39OMNIBUS_URL = "https://www.opscode.com/chef/install.sh"44OMNIBUS_URL = "https://www.opscode.com/chef/install.sh"
4045
46BIN_PATHS = (
47 "/usr/bin", "/usr/local/bin",
48 "/var/lib/gems/2.0.0/bin",
49 "/var/lib/gems/1.9.1/bin",
50 "/var/lib/gems/1.9/bin",
51 "/var/lib/gems/1.8/bin"
52)
53DEFAULT_ARGS = {
54 'interval': ('-i', 1800),
55 'splay': ('-s', 300),
56 'daemonize': ('-d', True),
57 'fork': ('-f', True)
58}
59DEFAULT_CFG = {
60 'repo_path': '/var/lib/cloud/chef_repo',
61 'repo_source_type': 'tarball'
62}
63JSON_ATTRIB_FILE = '/etc/chef/firstboot.json'
64
4165
42def handle(name, cfg, cloud, log, _args):66def handle(name, cfg, cloud, log, _args):
4367 log.info("Starting cc_chef")
44 # If there isn't a chef key in the configuration don't do anything68 # If there isn't a chef key in the configuration don't do anything
45 if 'chef' not in cfg:69 if 'chef' not in cfg:
46 log.debug(("Skipping module named %s,"70 log.debug(("Skipping module named %s,"
47 " no 'chef' key in configuration"), name)71 " no 'chef' key in configuration"), name)
48 return72 return
49 chef_cfg = cfg['chef']
5073
74 chef_cfg = dict(DEFAULT_CFG.items() + cfg['chef'].items())
75 log.debug("Chef config: %r", chef_cfg)
51 # Ensure the chef directories we use exist76 # Ensure the chef directories we use exist
52 for d in CHEF_DIRS:77 for d in CHEF_DIRS:
53 util.ensure_dir(d)78 util.ensure_dir(d)
5479
80 if 'mode' in chef_cfg:
81 chef_mode = chef_cfg['mode']
82 else:
83 if 'server_url' in chef_cfg and \
84 ('validation_key' in chef_cfg or 'validation_cert' in chef_cfg):
85 chef_mode = "client"
86 else:
87 chef_mode = "solo"
88
89 log.debug("Chef mode is %s", chef_mode)
90
55 # Set the validation key based on the presence of either 'validation_key'91 # Set the validation key based on the presence of either 'validation_key'
56 # or 'validation_cert'. In the case where both exist, 'validation_key'92 # or 'validation_cert'. In the case where both exist, 'validation_key'
57 # takes precedence93 # takes precedence
@@ -61,80 +97,191 @@
61 break97 break
6298
63 # Create the chef config from template99 # Create the chef config from template
64 template_fn = cloud.get_template_filename('chef_client.rb')100 if not write_chef_config(chef_cfg, cloud, chef_mode, log):
65 if template_fn:101 return False
66 iid = str(cloud.datasource.get_instance_id())
67 params = {
68 'server_url': chef_cfg['server_url'],
69 'node_name': util.get_cfg_option_str(chef_cfg, 'node_name', iid),
70 'environment': util.get_cfg_option_str(chef_cfg, 'environment',
71 '_default'),
72 'validation_name': chef_cfg['validation_name']
73 }
74 templater.render_to_file(template_fn, '/etc/chef/client.rb', params)
75 else:
76 log.warn("No template found, not rendering to /etc/chef/client.rb")
77102
78 # set the firstboot json103 # set the firstboot json
79 initial_json = {}104 write_json_attrib_file(chef_cfg)
80 if 'run_list' in chef_cfg:
81 initial_json['run_list'] = chef_cfg['run_list']
82 if 'initial_attributes' in chef_cfg:
83 initial_attributes = chef_cfg['initial_attributes']
84 for k in list(initial_attributes.keys()):
85 initial_json[k] = initial_attributes[k]
86 util.write_file('/etc/chef/firstboot.json', json.dumps(initial_json))
87105
88 # If chef is not installed, we install chef based on 'install_type'106 # If chef is not installed, we install chef based on 'install_type'
107 install_chef(chef_cfg, cloud)
108
109 if chef_mode == 'solo':
110 get_cookbooks(chef_cfg, cloud)
111
112 chef_args = chef_args_from_cfg(chef_cfg, ['-j', JSON_ATTRIB_FILE])
113 if util.get_cfg_option_bool(chef_cfg, 'autostart', default=True):
114 run_chef(log, chef_mode, chef_args)
115
116
117def install_chef(chef_cfg, cloud):
89 if (not os.path.isfile('/usr/bin/chef-client') or118 if (not os.path.isfile('/usr/bin/chef-client') or
90 util.get_cfg_option_bool(chef_cfg, 'force_install', default=False)):119 util.get_cfg_option_bool(chef_cfg, 'force_install', default=False)):
91120
92 install_type = util.get_cfg_option_str(chef_cfg, 'install_type',121 install_type = util.get_cfg_option_str(chef_cfg, 'install_type',
93 'packages')122 'packages')
123 chef_version = util.get_cfg_option_str(chef_cfg, 'version', None)
124
94 if install_type == "gems":125 if install_type == "gems":
95 # this will install and run the chef-client from gems126 # this will install and run the chef-client from gems
96 chef_version = util.get_cfg_option_str(chef_cfg, 'version', None)
97 ruby_version = util.get_cfg_option_str(chef_cfg, 'ruby_version',127 ruby_version = util.get_cfg_option_str(chef_cfg, 'ruby_version',
98 RUBY_VERSION_DEFAULT)128 RUBY_VERSION_DEFAULT)
99 install_chef_from_gems(cloud.distro, ruby_version, chef_version)129 ohai_version = util.get_cfg_option_str(chef_cfg, 'ohai_version', None)
100 # and finally, run chef-client130 install_chef_from_gems(cloud.distro, ruby_version, chef_version, ohai_version)
101 log.debug('Running chef-client')
102 util.subp(['/usr/bin/chef-client',
103 '-d', '-i', '1800', '-s', '20'], capture=False)
104 elif install_type == 'packages':131 elif install_type == 'packages':
105 # this will install and run the chef-client from packages132 # this will install and run the chef-client from packages
106 cloud.distro.install_packages(('chef',))133 cloud.distro.install_packages(('chef',))
107 elif install_type == 'omnibus':134 elif install_type == 'omnibus':
108 url = util.get_cfg_option_str(chef_cfg, "omnibus_url", OMNIBUS_URL)135 url = util.get_cfg_option_str(chef_cfg, "omnibus_url", OMNIBUS_URL)
109 content = url_helper.readurl(url=url, retries=5)136 content = url_helper.readurl(url=url, retries=5).contents
110 with util.tempdir() as tmpd:137 with util.tempdir() as tmpd:
111 # use tmpd over tmpfile to avoid 'Text file busy' on execute138 # use tmpd over tmpfile to avoid 'Text file busy' on execute
112 tmpf = "%s/chef-omnibus-install" % tmpd139 tmpf = "%s/chef-omnibus-install" % tmpd
113 util.write_file(tmpf, content, mode=0700)140 util.write_file(tmpf, content, mode=0700)
114 util.subp([tmpf], capture=False)141 args = []
142 if chef_version:
143 args.append("-v")
144 args.append(chef_version)
145 util.subp([tmpf] + args, capture=False)
115 else:146 else:
116 log.warn("Unknown chef install type %s", install_type)147 raise RuntimeError("Unknown chef install type %s" % install_type)
148
149
150def write_chef_config(chef_cfg, cloud, chef_mode, log):
151 "Write chef config file from template"
152 template_fn = cloud.get_template_filename('chef_client.rb')
153 cfg_filename = "/etc/chef/" + ("client.rb" if chef_mode == "client" else "solo.rb")
154 if template_fn:
155 iid = str(cloud.datasource.get_instance_id())
156 params = {
157 'node_name': util.get_cfg_option_str(chef_cfg, 'node_name', iid),
158 'environment': util.get_cfg_option_str(chef_cfg, 'environment',
159 '_default'),
160 'mode': chef_mode
161 }
162
163 if chef_mode == "client":
164 # server_url is required
165 params['server_url'] = chef_cfg['server_url']
166 params['validation_name'] = chef_cfg.get('validation_name', None)
167 elif chef_mode == "solo":
168 params['repo_path'] = chef_cfg['repo_path']
169
170 templater.render_to_file(template_fn, cfg_filename, params)
171 return True
172 else:
173 log.warn("No template found, not rendering to %s", cfg_filename)
174 return False
175
176
177def write_json_attrib_file(chef_cfg):
178 initial_json = {}
179 if 'run_list' in chef_cfg:
180 initial_json['run_list'] = chef_cfg['run_list']
181 if 'initial_attributes' in chef_cfg:
182 initial_attributes = chef_cfg['initial_attributes']
183 for k in list(initial_attributes.keys()):
184 initial_json[k] = initial_attributes[k]
185 util.write_file(JSON_ATTRIB_FILE, json.dumps(initial_json))
186
187
188def run_chef(log, chef_type, chef_args):
189 chef_bin = "chef-%s" % chef_type
190 chef_exec = None
191 for path in BIN_PATHS:
192 f = os.path.join(path, chef_bin)
193 if os.path.isfile(f) and os.access(f, os.X_OK):
194 chef_exec = f
195 break
196 if chef_exec is None:
197 raise RuntimeError("Couldn't find chef executable for %s" % chef_bin)
198 log.debug("Running %s", chef_exec)
199 util.subp([chef_exec] + chef_args, capture=False)
117200
118201
119def get_ruby_packages(version):202def get_ruby_packages(version):
120 # return a list of packages needed to install ruby at version203 # return a list of packages needed to install ruby at version
121 pkgs = ['ruby%s' % version, 'ruby%s-dev' % version]204 pkgs = ['ruby%s' % version, 'ruby%s-dev' % version, 'build-essential']
122 if version == "1.8":205 if version == "1.8":
123 pkgs.extend(('libopenssl-ruby1.8', 'rubygems1.8'))206 pkgs.extend(('libopenssl-ruby1.8', 'rubygems1.8'))
124 return pkgs207 return pkgs
125208
126209
127def install_chef_from_gems(ruby_version, chef_version, distro):210def install_chef_from_gems(distro, ruby_version, chef_version, ohai_version=None):
128 distro.install_packages(get_ruby_packages(ruby_version))211 distro.install_packages(get_ruby_packages(ruby_version))
212
213 def gem_install(gem, version=None):
214 cmd_args = ['/usr/bin/gem', 'install', gem]
215 if version is not None:
216 cmd_args.append('-v')
217 cmd_args.append(version)
218
219 cmd_args += ['--no-rdoc', '--bindir', '/usr/bin', '-q']
220
221 return util.subp(cmd_args, capture=False)
222
129 if not os.path.exists('/usr/bin/gem'):223 if not os.path.exists('/usr/bin/gem'):
130 util.sym_link('/usr/bin/gem%s' % ruby_version, '/usr/bin/gem')224 util.sym_link('/usr/bin/gem%s' % ruby_version, '/usr/bin/gem')
131 if not os.path.exists('/usr/bin/ruby'):225 if not os.path.exists('/usr/bin/ruby'):
132 util.sym_link('/usr/bin/ruby%s' % ruby_version, '/usr/bin/ruby')226 util.sym_link('/usr/bin/ruby%s' % ruby_version, '/usr/bin/ruby')
133 if chef_version:227 if ohai_version:
134 util.subp(['/usr/bin/gem', 'install', 'chef',228 gem_install('ohai', ohai_version)
135 '-v %s' % chef_version, '--no-ri',229 gem_install('chef', chef_version)
136 '--no-rdoc', '--bindir', '/usr/bin', '-q'], capture=False)230
231
232def chef_args_from_cfg(cfg, extra_args=[]):
233 merged_args = {}
234 for (k, v) in DEFAULT_ARGS.iteritems():
235 merged_args[k] = (v[0], cfg.get(k, v[1]))
236
237 if merged_args['daemonize'][1] is False:
238 del merged_args['interval']
239 del merged_args['splay']
240
241 args = []
242 for (k, (arg, v)) in merged_args.iteritems():
243 if type(v) == bool:
244 if util.get_cfg_option_bool(cfg, k, v):
245 args.append(arg)
246 else:
247 args.append(arg)
248 args.append(str(cfg.get(k, v)))
249
250 return args + extra_args
251
252
253def get_cookbooks(cfg, cloud):
254 repo_source_type = util.get_cfg_option_str(
255 cfg, 'repo_source_type', default='tarball')
256 if repo_source_type == 'git':
257 get_cookbooks_from_git(cfg, cloud)
258 elif repo_source_type == 'tarball':
259 get_cookbooks_from_tarball(cfg)
137 else:260 else:
138 util.subp(['/usr/bin/gem', 'install', 'chef',261 raise RuntimeError("Unknown cookbooks source type %s" % repo_source_type)
139 '--no-ri', '--no-rdoc', '--bindir',262
140 '/usr/bin', '-q'], capture=False)263
264def get_cookbooks_from_git(cfg, cloud):
265 cloud.distro.install_packages('git')
266 enclosing_dir = os.path.dirname(cfg['repo_path'])
267 if not os.path.isdir(enclosing_dir):
268 os.makedirs(enclosing_dir)
269 util.subp(['git', 'clone', '--recurse-submodules', cfg['repo_source'], cfg['repo_path']])
270
271
272def get_cookbooks_from_tarball(cfg):
273 with util.tempdir() as tmpd:
274 filename = os.path.basename(urlparse(cfg['repo_source']).path)
275 tmpfile = os.path.join(tmpd, filename)
276 urllib.urlretrieve(cfg['repo_source'], tmpfile)
277 if not os.path.isdir(cfg['repo_path']):
278 os.makedirs(cfg['repo_path'])
279 util.subp(['tar', '-xf', tmpfile, '-C', cfg['repo_path']])
280
281
282def get_cookbooks_from_berkshelf(cfg):
283 raise NotImplementedError()
284
285
286def get_cookbooks_from_librarian(cfg):
287 raise NotImplementedError()
141288
=== modified file 'doc/examples/cloud-config-chef.txt'
--- doc/examples/cloud-config-chef.txt 2012-12-12 15:39:43 +0000
+++ doc/examples/cloud-config-chef.txt 2013-05-15 17:20:33 +0000
@@ -84,8 +84,23 @@
84 maxclients: 10084 maxclients: 100
85 keepalive: "off"85 keepalive: "off"
8686
87 # if install_type is 'omnibus', change the url to download87 # For chef-solo, we want to download cookbooks. Currently git and tarball can be used
88 omnibus_url: "https://www.opscode.com/chef/install.sh"88 # tarball/git should contain cookbooks, roles and data_bags folders
89 #repo_source_type: git
90 #repo_source: https://github.com/some-org/cookbooks-repo.git
91 #repo_path: /var/lib/cloud/chef_repo
92
93 # if install_type is 'omnibus', change the url to the download script
94 #omnibus_url: "https://www.opscode.com/chef/install.sh"
95
96 # Daemonize and run every 'interval'
97 #daemonize: true
98 #interval: 1800
99 # Random wait before starting the run
100 #splay: 300
101
102 # fork off a worker for every chef run, effective against memory leaks
103 fork: true
89104
90105
91# Capture all subprocess output into a logfile106# Capture all subprocess output into a logfile
92107
=== modified file 'templates/chef_client.rb.tmpl'
--- templates/chef_client.rb.tmpl 2012-07-09 20:45:26 +0000
+++ templates/chef_client.rb.tmpl 2013-05-15 17:20:33 +0000
@@ -11,13 +11,18 @@
11log_level :info11log_level :info
12log_location "/var/log/chef/client.log"12log_location "/var/log/chef/client.log"
13ssl_verify_mode :verify_none13ssl_verify_mode :verify_none
14#if $mode == 'client'
14validation_client_name "$validation_name"15validation_client_name "$validation_name"
15validation_key "/etc/chef/validation.pem"16validation_key "/etc/chef/validation.pem"
16client_key "/etc/chef/client.pem"17client_key "/etc/chef/client.pem"
17chef_server_url "$server_url"18chef_server_url "$server_url"
18environment "$environment"19environment "$environment"
20#else if $mode == 'solo'
21cookbook_path "$repo_path/cookbooks"
22data_bag_path "$repo_path/data_bags"
23role_path "$repo_path/roles"
24#end if
19node_name "$node_name"25node_name "$node_name"
20json_attribs "/etc/chef/firstboot.json"
21file_cache_path "/var/cache/chef"26file_cache_path "/var/cache/chef"
22file_backup_path "/var/backups/chef"27file_backup_path "/var/backups/chef"
23pid_file "/var/run/chef/client.pid"28pid_file "/var/run/chef/client.pid"