Merge ~chad.smith/cloud-init:chef_omnibus_version into cloud-init:master
- Git
- lp:~chad.smith/cloud-init
- chef_omnibus_version
- Merge into master
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Chad Smith | ||||
Approved revision: | accd83c97fa3b3b928af2a7955febad410471fc2 | ||||
Merged at revision: | cf10a2ff2e2f666d9370f38297a5a105e809ea3c | ||||
Proposed branch: | ~chad.smith/cloud-init:chef_omnibus_version | ||||
Merge into: | cloud-init:master | ||||
Diff against target: |
266 lines (+138/-24) 4 files modified
cloudinit/config/cc_chef.py (+33/-12) cloudinit/util.py (+25/-0) doc/examples/cloud-config-chef.txt (+4/-0) tests/unittests/test_handler/test_handler_chef.py (+76/-12) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Chad Smith | Approve | ||
Server Team CI bot | continuous-integration | Approve | |
Review via email: mp+330712@code.launchpad.net |
Commit message
Description of the change
Add option to pin chef omnibus install version
Most users of chef will want to pin the version that is installed.
Typically new versions of chef have to be evaluated for breakage etc.
This change proposes a new optional `omnibus_version` field to the
chef configuration. This changes also adds an reference to the new
field to the chef example.
LP: #1462693
Chad Smith (chad.smith) wrote : | # |
- 6cfd206... by Chad Smith
-
tempdir moved to temp_utils in master. fix per rebase
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:3180af3df20
https:/
Executed test runs:
SUCCESS: Checkout
FAILED: Unit & Style Tests
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:6cfd2060387
https:/
Executed test runs:
SUCCESS: Checkout
FAILED: Unit & Style Tests
Click here to trigger a rebuild:
https:/
- f74d44e... by Chad Smith
-
rework unit tests to handle py2 py3 string/bytes differences and mock readurl to avoid test_install_
chef_from_ omnibus_ retries_ url to avoid executing time-consuming retries inside readurl
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:f74d44e7be6
https:/
Executed test runs:
SUCCESS: Checkout
FAILED: Unit & Style Tests
Click here to trigger a rebuild:
https:/
- accd83c... by Chad Smith
-
lint param name is now omnibus_version
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:accd83c97fa
https:/
Executed test runs:
SUCCESS: Checkout
SUCCESS: Unit & Style Tests
SUCCESS: Ubuntu LTS: Build
SUCCESS: Ubuntu LTS: Integration
SUCCESS: MAAS Compatability Testing
IN_PROGRESS: Declarative: Post Actions
Click here to trigger a rebuild:
https:/
Chad Smith (chad.smith) : | # |
Preview Diff
1 | diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py | |||
2 | index c192dd3..46abedd 100644 | |||
3 | --- a/cloudinit/config/cc_chef.py | |||
4 | +++ b/cloudinit/config/cc_chef.py | |||
5 | @@ -58,6 +58,9 @@ file). | |||
6 | 58 | log_level: | 58 | log_level: |
7 | 59 | log_location: | 59 | log_location: |
8 | 60 | node_name: | 60 | node_name: |
9 | 61 | omnibus_url: | ||
10 | 62 | omnibus_url_retries: | ||
11 | 63 | omnibus_version: | ||
12 | 61 | pid_file: | 64 | pid_file: |
13 | 62 | server_url: | 65 | server_url: |
14 | 63 | show_time: | 66 | show_time: |
15 | @@ -71,7 +74,6 @@ import itertools | |||
16 | 71 | import json | 74 | import json |
17 | 72 | import os | 75 | import os |
18 | 73 | 76 | ||
19 | 74 | from cloudinit import temp_utils | ||
20 | 75 | from cloudinit import templater | 77 | from cloudinit import templater |
21 | 76 | from cloudinit import url_helper | 78 | from cloudinit import url_helper |
22 | 77 | from cloudinit import util | 79 | from cloudinit import util |
23 | @@ -280,6 +282,31 @@ def run_chef(chef_cfg, log): | |||
24 | 280 | util.subp(cmd, capture=False) | 282 | util.subp(cmd, capture=False) |
25 | 281 | 283 | ||
26 | 282 | 284 | ||
27 | 285 | def install_chef_from_omnibus(url=None, retries=None, omnibus_version=None): | ||
28 | 286 | """Install an omnibus unified package from url. | ||
29 | 287 | |||
30 | 288 | @param url: URL where blob of chef content may be downloaded. Defaults to | ||
31 | 289 | OMNIBUS_URL. | ||
32 | 290 | @param retries: Number of retries to perform when attempting to read url. | ||
33 | 291 | Defaults to OMNIBUS_URL_RETRIES | ||
34 | 292 | @param omnibus_version: Optional version string to require for omnibus | ||
35 | 293 | install. | ||
36 | 294 | """ | ||
37 | 295 | if url is None: | ||
38 | 296 | url = OMNIBUS_URL | ||
39 | 297 | if retries is None: | ||
40 | 298 | retries = OMNIBUS_URL_RETRIES | ||
41 | 299 | |||
42 | 300 | if omnibus_version is None: | ||
43 | 301 | args = [] | ||
44 | 302 | else: | ||
45 | 303 | args = ['-v', omnibus_version] | ||
46 | 304 | content = url_helper.readurl(url=url, retries=retries).contents | ||
47 | 305 | return util.subp_blob_in_tempfile( | ||
48 | 306 | blob=content, args=args, | ||
49 | 307 | basename='chef-omnibus-install', capture=False) | ||
50 | 308 | |||
51 | 309 | |||
52 | 283 | def install_chef(cloud, chef_cfg, log): | 310 | def install_chef(cloud, chef_cfg, log): |
53 | 284 | # If chef is not installed, we install chef based on 'install_type' | 311 | # If chef is not installed, we install chef based on 'install_type' |
54 | 285 | install_type = util.get_cfg_option_str(chef_cfg, 'install_type', | 312 | install_type = util.get_cfg_option_str(chef_cfg, 'install_type', |
55 | @@ -298,17 +325,11 @@ def install_chef(cloud, chef_cfg, log): | |||
56 | 298 | # This will install and run the chef-client from packages | 325 | # This will install and run the chef-client from packages |
57 | 299 | cloud.distro.install_packages(('chef',)) | 326 | cloud.distro.install_packages(('chef',)) |
58 | 300 | elif install_type == 'omnibus': | 327 | elif install_type == 'omnibus': |
70 | 301 | # This will install as a omnibus unified package | 328 | omnibus_version = util.get_cfg_option_str(chef_cfg, "omnibus_version") |
71 | 302 | url = util.get_cfg_option_str(chef_cfg, "omnibus_url", OMNIBUS_URL) | 329 | install_chef_from_omnibus( |
72 | 303 | retries = max(0, util.get_cfg_option_int(chef_cfg, | 330 | url=util.get_cfg_option_str(chef_cfg, "omnibus_url"), |
73 | 304 | "omnibus_url_retries", | 331 | retries=util.get_cfg_option_int(chef_cfg, "omnibus_url_retries"), |
74 | 305 | default=OMNIBUS_URL_RETRIES)) | 332 | omnibus_version=omnibus_version) |
64 | 306 | content = url_helper.readurl(url=url, retries=retries).contents | ||
65 | 307 | with temp_utils.tempdir() as tmpd: | ||
66 | 308 | # Use tmpdir over tmpfile to avoid 'text file busy' on execute | ||
67 | 309 | tmpf = "%s/chef-omnibus-install" % tmpd | ||
68 | 310 | util.write_file(tmpf, content, mode=0o700) | ||
69 | 311 | util.subp([tmpf], capture=False) | ||
75 | 312 | else: | 333 | else: |
76 | 313 | log.warn("Unknown chef install type '%s'", install_type) | 334 | log.warn("Unknown chef install type '%s'", install_type) |
77 | 314 | run = False | 335 | run = False |
78 | diff --git a/cloudinit/util.py b/cloudinit/util.py | |||
79 | index ae5cda8..7e9d94f 100644 | |||
80 | --- a/cloudinit/util.py | |||
81 | +++ b/cloudinit/util.py | |||
82 | @@ -1742,6 +1742,31 @@ def delete_dir_contents(dirname): | |||
83 | 1742 | del_file(node_fullpath) | 1742 | del_file(node_fullpath) |
84 | 1743 | 1743 | ||
85 | 1744 | 1744 | ||
86 | 1745 | def subp_blob_in_tempfile(blob, *args, **kwargs): | ||
87 | 1746 | """Write blob to a tempfile, and call subp with args, kwargs. Then cleanup. | ||
88 | 1747 | |||
89 | 1748 | 'basename' as a kwarg allows providing the basename for the file. | ||
90 | 1749 | The 'args' argument to subp will be updated with the full path to the | ||
91 | 1750 | filename as the first argument. | ||
92 | 1751 | """ | ||
93 | 1752 | basename = kwargs.pop('basename', "subp_blob") | ||
94 | 1753 | |||
95 | 1754 | if len(args) == 0 and 'args' not in kwargs: | ||
96 | 1755 | args = [tuple()] | ||
97 | 1756 | |||
98 | 1757 | # Use tmpdir over tmpfile to avoid 'text file busy' on execute | ||
99 | 1758 | with temp_utils.tempdir() as tmpd: | ||
100 | 1759 | tmpf = os.path.join(tmpd, basename) | ||
101 | 1760 | if 'args' in kwargs: | ||
102 | 1761 | kwargs['args'] = [tmpf] + list(kwargs['args']) | ||
103 | 1762 | else: | ||
104 | 1763 | args = list(args) | ||
105 | 1764 | args[0] = [tmpf] + args[0] | ||
106 | 1765 | |||
107 | 1766 | write_file(tmpf, blob, mode=0o700) | ||
108 | 1767 | return subp(*args, **kwargs) | ||
109 | 1768 | |||
110 | 1769 | |||
111 | 1745 | def subp(args, data=None, rcs=None, env=None, capture=True, shell=False, | 1770 | def subp(args, data=None, rcs=None, env=None, capture=True, shell=False, |
112 | 1746 | logstring=False, decode="replace", target=None, update_env=None): | 1771 | logstring=False, decode="replace", target=None, update_env=None): |
113 | 1747 | 1772 | ||
114 | diff --git a/doc/examples/cloud-config-chef.txt b/doc/examples/cloud-config-chef.txt | |||
115 | index 9d23581..58d5fdc 100644 | |||
116 | --- a/doc/examples/cloud-config-chef.txt | |||
117 | +++ b/doc/examples/cloud-config-chef.txt | |||
118 | @@ -94,6 +94,10 @@ chef: | |||
119 | 94 | # if install_type is 'omnibus', change the url to download | 94 | # if install_type is 'omnibus', change the url to download |
120 | 95 | omnibus_url: "https://www.chef.io/chef/install.sh" | 95 | omnibus_url: "https://www.chef.io/chef/install.sh" |
121 | 96 | 96 | ||
122 | 97 | # if install_type is 'omnibus', pass pinned version string | ||
123 | 98 | # to the install script | ||
124 | 99 | omnibus_version: "12.3.0" | ||
125 | 100 | |||
126 | 97 | 101 | ||
127 | 98 | # Capture all subprocess output into a logfile | 102 | # Capture all subprocess output into a logfile |
128 | 99 | # Useful for troubleshooting cloud-init issues | 103 | # Useful for troubleshooting cloud-init issues |
129 | diff --git a/tests/unittests/test_handler/test_handler_chef.py b/tests/unittests/test_handler/test_handler_chef.py | |||
130 | index e5785cf..0136a93 100644 | |||
131 | --- a/tests/unittests/test_handler/test_handler_chef.py | |||
132 | +++ b/tests/unittests/test_handler/test_handler_chef.py | |||
133 | @@ -1,11 +1,10 @@ | |||
134 | 1 | # This file is part of cloud-init. See LICENSE file for license information. | 1 | # This file is part of cloud-init. See LICENSE file for license information. |
135 | 2 | 2 | ||
136 | 3 | import httpretty | ||
137 | 3 | import json | 4 | import json |
138 | 4 | import logging | 5 | import logging |
139 | 5 | import os | 6 | import os |
140 | 6 | import shutil | ||
141 | 7 | import six | 7 | import six |
142 | 8 | import tempfile | ||
143 | 9 | 8 | ||
144 | 10 | from cloudinit import cloud | 9 | from cloudinit import cloud |
145 | 11 | from cloudinit.config import cc_chef | 10 | from cloudinit.config import cc_chef |
146 | @@ -14,18 +13,83 @@ from cloudinit import helpers | |||
147 | 14 | from cloudinit.sources import DataSourceNone | 13 | from cloudinit.sources import DataSourceNone |
148 | 15 | from cloudinit import util | 14 | from cloudinit import util |
149 | 16 | 15 | ||
151 | 17 | from cloudinit.tests import helpers as t_help | 16 | from cloudinit.tests.helpers import ( |
152 | 17 | CiTestCase, FilesystemMockingTestCase, mock, skipIf) | ||
153 | 18 | 18 | ||
154 | 19 | LOG = logging.getLogger(__name__) | 19 | LOG = logging.getLogger(__name__) |
155 | 20 | 20 | ||
156 | 21 | CLIENT_TEMPL = os.path.sep.join(["templates", "chef_client.rb.tmpl"]) | 21 | CLIENT_TEMPL = os.path.sep.join(["templates", "chef_client.rb.tmpl"]) |
157 | 22 | 22 | ||
158 | 23 | 23 | ||
160 | 24 | class TestChef(t_help.FilesystemMockingTestCase): | 24 | class TestInstallChefOmnibus(CiTestCase): |
161 | 25 | |||
162 | 26 | def setUp(self): | ||
163 | 27 | self.new_root = self.tmp_dir() | ||
164 | 28 | |||
165 | 29 | @httpretty.activate | ||
166 | 30 | def test_install_chef_from_omnibus_runs_chef_url_content(self): | ||
167 | 31 | """install_chef_from_omnibus runs downloaded OMNIBUS_URL as script.""" | ||
168 | 32 | chef_outfile = self.tmp_path('chef.out', self.new_root) | ||
169 | 33 | response = '#!/bin/bash\necho "Hi Mom" > {0}'.format(chef_outfile) | ||
170 | 34 | httpretty.register_uri( | ||
171 | 35 | httpretty.GET, cc_chef.OMNIBUS_URL, body=response, status=200) | ||
172 | 36 | cc_chef.install_chef_from_omnibus() | ||
173 | 37 | self.assertEqual('Hi Mom\n', util.load_file(chef_outfile)) | ||
174 | 38 | |||
175 | 39 | @mock.patch('cloudinit.config.cc_chef.url_helper.readurl') | ||
176 | 40 | @mock.patch('cloudinit.config.cc_chef.util.subp_blob_in_tempfile') | ||
177 | 41 | def test_install_chef_from_omnibus_retries_url(self, m_subp_blob, m_rdurl): | ||
178 | 42 | """install_chef_from_omnibus retries OMNIBUS_URL upon failure.""" | ||
179 | 43 | |||
180 | 44 | class FakeURLResponse(object): | ||
181 | 45 | contents = '#!/bin/bash\necho "Hi Mom" > {0}/chef.out'.format( | ||
182 | 46 | self.new_root) | ||
183 | 47 | |||
184 | 48 | m_rdurl.return_value = FakeURLResponse() | ||
185 | 49 | |||
186 | 50 | cc_chef.install_chef_from_omnibus() | ||
187 | 51 | expected_kwargs = {'retries': cc_chef.OMNIBUS_URL_RETRIES, | ||
188 | 52 | 'url': cc_chef.OMNIBUS_URL} | ||
189 | 53 | self.assertItemsEqual(expected_kwargs, m_rdurl.call_args_list[0][1]) | ||
190 | 54 | cc_chef.install_chef_from_omnibus(retries=10) | ||
191 | 55 | expected_kwargs = {'retries': 10, | ||
192 | 56 | 'url': cc_chef.OMNIBUS_URL} | ||
193 | 57 | self.assertItemsEqual(expected_kwargs, m_rdurl.call_args_list[1][1]) | ||
194 | 58 | expected_subp_kwargs = { | ||
195 | 59 | 'args': ['-v', '2.0'], | ||
196 | 60 | 'basename': 'chef-omnibus-install', | ||
197 | 61 | 'blob': m_rdurl.return_value.contents, | ||
198 | 62 | 'capture': False | ||
199 | 63 | } | ||
200 | 64 | self.assertItemsEqual( | ||
201 | 65 | expected_subp_kwargs, | ||
202 | 66 | m_subp_blob.call_args_list[0][1]) | ||
203 | 67 | |||
204 | 68 | @httpretty.activate | ||
205 | 69 | @mock.patch('cloudinit.config.cc_chef.util.subp_blob_in_tempfile') | ||
206 | 70 | def test_install_chef_from_omnibus_has_omnibus_version(self, m_subp_blob): | ||
207 | 71 | """install_chef_from_omnibus provides version arg to OMNIBUS_URL.""" | ||
208 | 72 | chef_outfile = self.tmp_path('chef.out', self.new_root) | ||
209 | 73 | response = '#!/bin/bash\necho "Hi Mom" > {0}'.format(chef_outfile) | ||
210 | 74 | httpretty.register_uri( | ||
211 | 75 | httpretty.GET, cc_chef.OMNIBUS_URL, body=response) | ||
212 | 76 | cc_chef.install_chef_from_omnibus(omnibus_version='2.0') | ||
213 | 77 | |||
214 | 78 | called_kwargs = m_subp_blob.call_args_list[0][1] | ||
215 | 79 | expected_kwargs = { | ||
216 | 80 | 'args': ['-v', '2.0'], | ||
217 | 81 | 'basename': 'chef-omnibus-install', | ||
218 | 82 | 'blob': response, | ||
219 | 83 | 'capture': False | ||
220 | 84 | } | ||
221 | 85 | self.assertItemsEqual(expected_kwargs, called_kwargs) | ||
222 | 86 | |||
223 | 87 | |||
224 | 88 | class TestChef(FilesystemMockingTestCase): | ||
225 | 89 | |||
226 | 25 | def setUp(self): | 90 | def setUp(self): |
227 | 26 | super(TestChef, self).setUp() | 91 | super(TestChef, self).setUp() |
230 | 27 | self.tmp = tempfile.mkdtemp() | 92 | self.tmp = self.tmp_dir() |
229 | 28 | self.addCleanup(shutil.rmtree, self.tmp) | ||
231 | 29 | 93 | ||
232 | 30 | def fetch_cloud(self, distro_kind): | 94 | def fetch_cloud(self, distro_kind): |
233 | 31 | cls = distros.fetch(distro_kind) | 95 | cls = distros.fetch(distro_kind) |
234 | @@ -43,8 +107,8 @@ class TestChef(t_help.FilesystemMockingTestCase): | |||
235 | 43 | for d in cc_chef.CHEF_DIRS: | 107 | for d in cc_chef.CHEF_DIRS: |
236 | 44 | self.assertFalse(os.path.isdir(d)) | 108 | self.assertFalse(os.path.isdir(d)) |
237 | 45 | 109 | ||
240 | 46 | @t_help.skipIf(not os.path.isfile(CLIENT_TEMPL), | 110 | @skipIf(not os.path.isfile(CLIENT_TEMPL), |
241 | 47 | CLIENT_TEMPL + " is not available") | 111 | CLIENT_TEMPL + " is not available") |
242 | 48 | def test_basic_config(self): | 112 | def test_basic_config(self): |
243 | 49 | """ | 113 | """ |
244 | 50 | test basic config looks sane | 114 | test basic config looks sane |
245 | @@ -122,8 +186,8 @@ class TestChef(t_help.FilesystemMockingTestCase): | |||
246 | 122 | 'c': 'd', | 186 | 'c': 'd', |
247 | 123 | }, json.loads(c)) | 187 | }, json.loads(c)) |
248 | 124 | 188 | ||
251 | 125 | @t_help.skipIf(not os.path.isfile(CLIENT_TEMPL), | 189 | @skipIf(not os.path.isfile(CLIENT_TEMPL), |
252 | 126 | CLIENT_TEMPL + " is not available") | 190 | CLIENT_TEMPL + " is not available") |
253 | 127 | def test_template_deletes(self): | 191 | def test_template_deletes(self): |
254 | 128 | tpl_file = util.load_file('templates/chef_client.rb.tmpl') | 192 | tpl_file = util.load_file('templates/chef_client.rb.tmpl') |
255 | 129 | self.patchUtils(self.tmp) | 193 | self.patchUtils(self.tmp) |
256 | @@ -143,8 +207,8 @@ class TestChef(t_help.FilesystemMockingTestCase): | |||
257 | 143 | self.assertNotIn('json_attribs', c) | 207 | self.assertNotIn('json_attribs', c) |
258 | 144 | self.assertNotIn('Formatter.show_time', c) | 208 | self.assertNotIn('Formatter.show_time', c) |
259 | 145 | 209 | ||
262 | 146 | @t_help.skipIf(not os.path.isfile(CLIENT_TEMPL), | 210 | @skipIf(not os.path.isfile(CLIENT_TEMPL), |
263 | 147 | CLIENT_TEMPL + " is not available") | 211 | CLIENT_TEMPL + " is not available") |
264 | 148 | def test_validation_cert_and_validation_key(self): | 212 | def test_validation_cert_and_validation_key(self): |
265 | 149 | # test validation_cert content is written to validation_key path | 213 | # test validation_cert content is written to validation_key path |
266 | 150 | tpl_file = util.load_file('templates/chef_client.rb.tmpl') | 214 | tpl_file = util.load_file('templates/chef_client.rb.tmpl') |
Pulled the original content from https:/ /code.launchpad .net/~papodaca/ cloud-init/ +git/cloud- init/+merge/ 328943 and addresses review comments as well as added unit tests.