Merge lp:~jtv/maas/label-level-in-boot-image-dir-tree into lp:~maas-committers/maas/trunk
- label-level-in-boot-image-dir-tree
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Jeroen T. Vermeulen | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 2136 | ||||
Proposed branch: | lp:~jtv/maas/label-level-in-boot-image-dir-tree | ||||
Merge into: | lp:~maas-committers/maas/trunk | ||||
Diff against target: |
696 lines (+120/-96) 15 files modified
src/maasserver/api.py (+9/-12) src/maasserver/api_utils.py (+3/-2) src/provisioningserver/import_images/ephemerals_script.py (+8/-4) src/provisioningserver/import_images/tests/test_ephemerals_script.py (+9/-5) src/provisioningserver/kernel_opts.py (+2/-1) src/provisioningserver/pxe/config.py (+2/-2) src/provisioningserver/pxe/install_image.py (+14/-8) src/provisioningserver/pxe/tests/test_config.py (+2/-2) src/provisioningserver/pxe/tests/test_install_image.py (+29/-21) src/provisioningserver/pxe/tests/test_tftppath.py (+6/-11) src/provisioningserver/pxe/tftppath.py (+8/-7) src/provisioningserver/rpc/cluster.py (+1/-0) src/provisioningserver/rpc/tests/test_clusterservice.py (+18/-14) src/provisioningserver/tasks.py (+2/-1) src/provisioningserver/tests/test_maas_import_pxe_files.py (+7/-6) |
||||
To merge this branch: | bzr merge lp:~jtv/maas/label-level-in-boot-image-dir-tree | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Raphaël Badin (community) | Approve | ||
Review via email: mp+210740@code.launchpad.net |
Commit message
Splice ‘label’ level into the directory hierarchy for boot images.
Description of the change
The new level goes between ‘release’ and ‘purpose’ — so the new directory hierarchy is incompatible with the old one. We still need to provide migration code. The default label is ‘release’ (which can get a bit confusing because of the adjoining ‘release’ parameter).
This is a large change, driven largely by just making the change in one place and following the propagating test failures from there. There may be more places that need the change, such as the import scripts.
Jeroen
Julian Edwards (julian-edwards) wrote : | # |
Agree with rvb, let's QA this *before* landing.
+ :param label: Image label, e.g. "release" or a special hardware-
enablement
+ image.
Just FYI, but Scott thinks that we don't need label for this any more and that
subarch will solve everything related to specific HWE images. While that
might be the case, I don't think it's the right way to do it, subarch is meant
to be a specialisation of some hardware, and QA folks will want to pull in
kernel versions from different point releases for that hardware.
Preview Diff
1 | === modified file 'src/maasserver/api.py' |
2 | --- src/maasserver/api.py 2014-03-14 08:50:51 +0000 |
3 | +++ src/maasserver/api.py 2014-03-18 03:24:23 +0000 |
4 | @@ -2358,9 +2358,8 @@ |
5 | hostname = 'maas-enlist' |
6 | domain = Config.objects.get_config('enlistment_domain') |
7 | |
8 | - try: |
9 | - arch = request.GET['arch'] |
10 | - except KeyError: |
11 | + arch = get_optional_param(request.GET, 'arch') |
12 | + if arch is None: |
13 | if 'mac' in request.GET: |
14 | # Request was pxelinux.cfg/01-<mac>, so attempt fall back |
15 | # to pxelinux.cfg/default-<arch>-<subarch> for arch detection. |
16 | @@ -2376,11 +2375,9 @@ |
17 | else: |
18 | arch = image.architecture |
19 | |
20 | - # Use subarch if supplied; otherwise assume 'generic'. |
21 | - try: |
22 | - subarch = request.GET['subarch'] |
23 | - except KeyError: |
24 | - subarch = 'generic' |
25 | + subarch = get_optional_param(request.GET, 'subarch', 'generic') |
26 | + |
27 | + label = get_optional_param(request.GET, 'label', 'release') |
28 | |
29 | if node is not None: |
30 | # We don't care if the kernel opts is from the global setting or a tag, |
31 | @@ -2395,10 +2392,10 @@ |
32 | cluster_address = get_mandatory_param(request.GET, "local") |
33 | |
34 | params = KernelParameters( |
35 | - arch=arch, subarch=subarch, release=series, purpose=purpose, |
36 | - hostname=hostname, domain=domain, preseed_url=preseed_url, |
37 | - log_host=server_address, fs_host=cluster_address, |
38 | - extra_opts=extra_kernel_opts) |
39 | + arch=arch, subarch=subarch, release=series, label=label, |
40 | + purpose=purpose, hostname=hostname, domain=domain, |
41 | + preseed_url=preseed_url, log_host=server_address, |
42 | + fs_host=cluster_address, extra_opts=extra_kernel_opts) |
43 | |
44 | return HttpResponse( |
45 | json.dumps(params._asdict()), |
46 | |
47 | === modified file 'src/maasserver/api_utils.py' |
48 | --- src/maasserver/api_utils.py 2014-03-12 14:09:13 +0000 |
49 | +++ src/maasserver/api_utils.py 2014-03-18 03:24:23 +0000 |
50 | @@ -1,4 +1,4 @@ |
51 | -# Copyright 2012 Canonical Ltd. This software is licensed under the |
52 | +# Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
53 | # GNU Affero General Public License version 3 (see the file LICENSE). |
54 | |
55 | """Helpers for Piston-based MAAS APIs.""" |
56 | @@ -19,6 +19,7 @@ |
57 | 'get_mandatory_param', |
58 | 'get_oauth_token', |
59 | 'get_optional_list', |
60 | + 'get_optional_param', |
61 | 'get_overridden_query_dict', |
62 | ] |
63 | |
64 | @@ -95,7 +96,7 @@ |
65 | :return: The value of the parameter. |
66 | :raises: ValidationError |
67 | """ |
68 | - value = data.get(key, None) |
69 | + value = data.get(key) |
70 | if value is None: |
71 | return default |
72 | if validator is not None: |
73 | |
74 | === modified file 'src/provisioningserver/import_images/ephemerals_script.py' |
75 | --- src/provisioningserver/import_images/ephemerals_script.py 2014-01-29 10:12:41 +0000 |
76 | +++ src/provisioningserver/import_images/ephemerals_script.py 2014-03-18 03:24:23 +0000 |
77 | @@ -1,4 +1,4 @@ |
78 | -# Copyright 2013 Canonical Ltd. This software is licensed under the |
79 | +# Copyright 2013-2014 Canonical Ltd. This software is licensed under the |
80 | # GNU Affero General Public License version 3 (see the file LICENSE). |
81 | |
82 | """Script code for `maas-import-ephemerals.py`.""" |
83 | @@ -157,7 +157,7 @@ |
84 | return image_dir |
85 | |
86 | |
87 | -def install_image_from_simplestreams(storage_dir, release, arch, |
88 | +def install_image_from_simplestreams(storage_dir, release, label, arch, |
89 | subarch='generic', |
90 | purpose='commissioning', |
91 | alternate_purpose='xinstall', |
92 | @@ -168,6 +168,7 @@ |
93 | `initrd.gz`, `dist-root.tar.gz`). The image that will be installed is |
94 | a directory of symlinks to these files. |
95 | :param release: Release name to install, e.g. "precise". |
96 | + :param label: Image label, e.g. "release" or an alpha or beta version. |
97 | :param arch: Architecture for the image, e.g. "i386". |
98 | :param subarch: Sub-architecture. Defaults to "generic". |
99 | :param purpose: Boot purpose for the image. Defaults to `commissioning`. |
100 | @@ -183,7 +184,8 @@ |
101 | |
102 | try: |
103 | install_image( |
104 | - provision_tmp, release=release, arch=arch, subarch=subarch, |
105 | + provision_tmp, arch=arch, subarch=subarch, |
106 | + release=release, label=label, |
107 | purpose=purpose, alternate_purpose=alternate_purpose) |
108 | finally: |
109 | shutil.rmtree(provision_tmp, ignore_errors=True) |
110 | @@ -280,7 +282,9 @@ |
111 | release = metadata['release'] |
112 | version = metadata['version'] |
113 | version_name = metadata['version_name'] |
114 | - label = metadata['label'] |
115 | + # XXX jtv 2014-03-14: At the time of writing we don't know whether |
116 | + # simplestreams will pass the release label in this way. |
117 | + label = metadata.get('label', 'release') |
118 | target_dir = self._target_dir(metadata) |
119 | name = get_target_name( |
120 | release=release, version=version, arch=arch, |
121 | |
122 | === modified file 'src/provisioningserver/import_images/tests/test_ephemerals_script.py' |
123 | --- src/provisioningserver/import_images/tests/test_ephemerals_script.py 2014-01-05 21:27:58 +0000 |
124 | +++ src/provisioningserver/import_images/tests/test_ephemerals_script.py 2014-03-18 03:24:23 +0000 |
125 | @@ -1,4 +1,4 @@ |
126 | -# Copyright 2013 Canonical Ltd. This software is licensed under the |
127 | +# Copyright 2013-2014 Canonical Ltd. This software is licensed under the |
128 | # GNU Affero General Public License version 3 (see the file LICENSE). |
129 | |
130 | """Tests for `ephemerals_script`.""" |
131 | @@ -323,13 +323,15 @@ |
132 | self.patch_config(tftp_root) |
133 | storage_dir = self.prepare_storage_dir() |
134 | release = factory.make_name('release') |
135 | + label = factory.make_name('label') |
136 | arch = factory.make_name('arch') |
137 | |
138 | install_image_from_simplestreams( |
139 | - storage_dir, release=release, arch=arch) |
140 | + storage_dir, release=release, label=label, arch=arch) |
141 | |
142 | install_dir = locate_tftp_path( |
143 | - compose_image_path(arch, 'generic', release, 'commissioning'), |
144 | + compose_image_path( |
145 | + arch, 'generic', release, label, 'commissioning'), |
146 | tftproot=tftp_root) |
147 | self.assertItemsEqual( |
148 | ['linux', 'initrd.gz', 'root.tar.gz'], |
149 | @@ -345,7 +347,8 @@ |
150 | |
151 | install_image_from_simplestreams( |
152 | storage_dir, release=factory.make_name('release'), |
153 | - arch=factory.make_name('arch'), temp_location=temp_location) |
154 | + label=factory.make_name('label'), arch=factory.make_name('arch'), |
155 | + temp_location=temp_location) |
156 | |
157 | self.assertItemsEqual([], listdir(temp_location)) |
158 | |
159 | @@ -362,7 +365,8 @@ |
160 | DeliberateFailure, |
161 | install_image_from_simplestreams, |
162 | storage_dir, release=factory.make_name('release'), |
163 | - arch=factory.make_name('arch'), temp_location=temp_location) |
164 | + label=factory.make_name('label'), arch=factory.make_name('arch'), |
165 | + temp_location=temp_location) |
166 | |
167 | self.assertItemsEqual([], listdir(temp_location)) |
168 | |
169 | |
170 | === modified file 'src/provisioningserver/kernel_opts.py' |
171 | --- src/provisioningserver/kernel_opts.py 2014-03-17 08:32:52 +0000 |
172 | +++ src/provisioningserver/kernel_opts.py 2014-03-18 03:24:23 +0000 |
173 | @@ -1,4 +1,4 @@ |
174 | -# Copyright 2012, 2013 Canonical Ltd. This software is licensed under the |
175 | +# Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
176 | # GNU Affero General Public License version 3 (see the file LICENSE). |
177 | |
178 | """Generate kernel command-line options for inclusion in PXE configs.""" |
179 | @@ -35,6 +35,7 @@ |
180 | "arch", # Machine architecture, e.g. "i386" |
181 | "subarch", # Machine subarchitecture, e.g. "generic" |
182 | "release", # Ubuntu release, e.g. "precise" |
183 | + "label", # Image label, e.g. "release" |
184 | "purpose", # Boot purpose, e.g. "commissioning" |
185 | "hostname", # Machine hostname, e.g. "coleman" |
186 | "domain", # Machine domain name, e.g. "example.com" |
187 | |
188 | === modified file 'src/provisioningserver/pxe/config.py' |
189 | --- src/provisioningserver/pxe/config.py 2014-01-17 08:45:01 +0000 |
190 | +++ src/provisioningserver/pxe/config.py 2014-03-18 03:24:23 +0000 |
191 | @@ -1,4 +1,4 @@ |
192 | -# Copyright 2012-2013 Canonical Ltd. This software is licensed under the |
193 | +# Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
194 | # GNU Affero General Public License version 3 (see the file LICENSE). |
195 | |
196 | """Generating PXE configuration files. |
197 | @@ -101,7 +101,7 @@ |
198 | def image_dir(params): |
199 | return compose_image_path( |
200 | params.arch, params.subarch, |
201 | - params.release, params.purpose) |
202 | + params.release, params.label, params.purpose) |
203 | |
204 | def initrd_path(params): |
205 | return "%s/initrd.gz" % image_dir(params) |
206 | |
207 | === modified file 'src/provisioningserver/pxe/install_image.py' |
208 | --- src/provisioningserver/pxe/install_image.py 2013-10-07 09:12:40 +0000 |
209 | +++ src/provisioningserver/pxe/install_image.py 2014-03-18 03:24:23 +0000 |
210 | @@ -1,4 +1,4 @@ |
211 | -# Copyright 2012, 2013 Canonical Ltd. This software is licensed under the |
212 | +# Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
213 | # GNU Affero General Public License version 3 (see the file LICENSE). |
214 | |
215 | """Install a netboot image directory for TFTP download.""" |
216 | @@ -33,7 +33,7 @@ |
217 | from twisted.python.filepath import FilePath |
218 | |
219 | |
220 | -def make_destination(tftproot, arch, subarch, release, purpose): |
221 | +def make_destination(tftproot, arch, subarch, release, label, purpose): |
222 | """Locate an image's destination. Create containing directory if needed. |
223 | |
224 | :param tftproot: The root directory served up by the TFTP server, |
225 | @@ -41,13 +41,15 @@ |
226 | :param arch: Main architecture to locate the destination for. |
227 | :param subarch: Sub-architecture of the main architecture. |
228 | :param release: OS release name, e.g. "precise". |
229 | + :param label: Image label, e.g. for a beta release, or "release" for the |
230 | + default images. |
231 | :param purpose: Purpose of this image, e.g. "install". |
232 | :return: Full path describing the image directory's new location and |
233 | name. In other words, a file "linux" in the image directory should |
234 | become os.path.join(make_destination(...), 'linux'). |
235 | """ |
236 | dest = locate_tftp_path( |
237 | - compose_image_path(arch, subarch, release, purpose), |
238 | + compose_image_path(arch, subarch, release, label, purpose), |
239 | tftproot=tftproot) |
240 | parent = os.path.dirname(dest) |
241 | if not os.path.isdir(parent): |
242 | @@ -157,8 +159,8 @@ |
243 | rmtree('%s.old' % old, ignore_errors=True) |
244 | |
245 | |
246 | -def install_image(image_dir, arch, subarch, release, purpose, config_file=None, |
247 | - alternate_purpose=None): |
248 | +def install_image(image_dir, arch, subarch, release, label, purpose, |
249 | + config_file=None, alternate_purpose=None): |
250 | """Install a PXE boot image. |
251 | |
252 | This is the function-call equivalent to a command-line invocation calling |
253 | @@ -166,7 +168,8 @@ |
254 | """ |
255 | config = Config.load_from_cache(config_file) |
256 | tftproot = config["tftp"]["root"] |
257 | - destination = make_destination(tftproot, arch, subarch, release, purpose) |
258 | + destination = make_destination( |
259 | + tftproot, arch, subarch, release, label, purpose) |
260 | if not are_identical_dirs(destination, image_dir): |
261 | # Image has changed. Move the new version into place. |
262 | install_dir(image_dir, destination) |
263 | @@ -174,7 +177,7 @@ |
264 | if alternate_purpose is not None: |
265 | # Symlink the new image directory under the alternate purpose name. |
266 | alternate_destination = make_destination( |
267 | - tftproot, arch, subarch, release, alternate_purpose) |
268 | + tftproot, arch, subarch, release, label, alternate_purpose) |
269 | install_symlink(destination, alternate_destination) |
270 | |
271 | rmtree(image_dir, ignore_errors=True) |
272 | @@ -191,6 +194,9 @@ |
273 | '--release', dest='release', default=None, |
274 | help="Ubuntu release that the image is for.") |
275 | parser.add_argument( |
276 | + '--label', dest='label', default='release', |
277 | + help="Image label, for specific pre-release images."), |
278 | + parser.add_argument( |
279 | '--purpose', dest='purpose', default=None, |
280 | help="Purpose of the image (e.g. 'install' or 'commissioning').") |
281 | parser.add_argument( |
282 | @@ -211,5 +217,5 @@ |
283 | """ |
284 | install_image( |
285 | args.image, arch=args.arch, subarch=args.subarch, release=args.release, |
286 | - purpose=args.purpose, config_file=args.config_file, |
287 | + label=args.label, purpose=args.purpose, config_file=args.config_file, |
288 | alternate_purpose=args.symlink) |
289 | |
290 | === modified file 'src/provisioningserver/pxe/tests/test_config.py' |
291 | --- src/provisioningserver/pxe/tests/test_config.py 2014-03-13 06:17:50 +0000 |
292 | +++ src/provisioningserver/pxe/tests/test_config.py 2014-03-18 03:24:23 +0000 |
293 | @@ -1,4 +1,4 @@ |
294 | -# Copyright 2012-2013 Canonical Ltd. This software is licensed under the |
295 | +# Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
296 | # GNU Affero General Public License version 3 (see the file LICENSE). |
297 | |
298 | """Tests for `provisioningserver.pxe.config`.""" |
299 | @@ -174,7 +174,7 @@ |
300 | # The PXE parameters are all set according to the options. |
301 | image_dir = compose_image_path( |
302 | arch=params.arch, subarch=params.subarch, |
303 | - release=params.release, purpose=params.purpose) |
304 | + release=params.release, label=params.label, purpose=params.purpose) |
305 | self.assertThat( |
306 | output, MatchesAll( |
307 | MatchesRegex( |
308 | |
309 | === modified file 'src/provisioningserver/pxe/tests/test_install_image.py' |
310 | --- src/provisioningserver/pxe/tests/test_install_image.py 2013-10-07 09:12:40 +0000 |
311 | +++ src/provisioningserver/pxe/tests/test_install_image.py 2014-03-18 03:24:23 +0000 |
312 | @@ -1,4 +1,4 @@ |
313 | -# Copyright 2012, 2013 Canonical Ltd. This software is licensed under the |
314 | +# Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
315 | # GNU Affero General Public License version 3 (see the file LICENSE). |
316 | |
317 | """Tests for the install_pxe_image command.""" |
318 | @@ -41,14 +41,15 @@ |
319 | from twisted.python.filepath import FilePath |
320 | |
321 | |
322 | -def make_arch_subarch_release_purpose(): |
323 | - """Create arbitrary architecture/subarchitecture/release names. |
324 | +def make_arch_subarch_release_label_purpose(): |
325 | + """Create arbitrary architecture/subarchitecture/release etc. names. |
326 | |
327 | - :return: A triplet of three identifiers for these respective items. |
328 | + :return: A tuple of architecture, subarchitecture, release, label, and |
329 | + purpose. |
330 | """ |
331 | return tuple( |
332 | factory.make_name(item) |
333 | - for item in ('arch', 'subarch', 'release', 'purpose')) |
334 | + for item in ('arch', 'subarch', 'release', 'label', 'purpose')) |
335 | |
336 | |
337 | class TestInstallPXEImage(MAASTestCase): |
338 | @@ -63,20 +64,21 @@ |
339 | image_dir = os.path.join(download_dir, 'image') |
340 | os.makedirs(image_dir) |
341 | factory.make_file(image_dir, 'kernel') |
342 | - arch, subarch, release, purpose = make_arch_subarch_release_purpose() |
343 | + arch, subarch, release, label, purpose = ( |
344 | + make_arch_subarch_release_label_purpose()) |
345 | |
346 | action = factory.make_name("action") |
347 | script = MainScript(action) |
348 | script.register(action, provisioningserver.pxe.install_image) |
349 | script.execute( |
350 | ("--config-file", config_fixture.filename, action, "--arch", arch, |
351 | - "--subarch", subarch, "--release", release, "--purpose", purpose, |
352 | - "--image", image_dir)) |
353 | + "--subarch", subarch, "--release", release, "--label", label, |
354 | + "--purpose", purpose, "--image", image_dir)) |
355 | |
356 | self.assertThat( |
357 | os.path.join( |
358 | locate_tftp_path( |
359 | - compose_image_path(arch, subarch, release, purpose), |
360 | + compose_image_path(arch, subarch, release, label, purpose), |
361 | tftproot=tftproot), |
362 | 'kernel'), |
363 | FileExists()) |
364 | @@ -88,31 +90,35 @@ |
365 | # (Where the /var/lib/maas/tftp/ part is configurable, so we |
366 | # can test this without overwriting system files). |
367 | tftproot = self.make_dir() |
368 | - arch, subarch, release, purpose = make_arch_subarch_release_purpose() |
369 | + arch, subarch, release, label, purpose = ( |
370 | + make_arch_subarch_release_label_purpose()) |
371 | self.assertEqual( |
372 | - os.path.join(tftproot, arch, subarch, release, purpose), |
373 | - make_destination(tftproot, arch, subarch, release, purpose)) |
374 | + os.path.join(tftproot, arch, subarch, release, label, purpose), |
375 | + make_destination(tftproot, arch, subarch, release, label, purpose)) |
376 | |
377 | def test_make_destination_creates_directory_if_not_present(self): |
378 | tftproot = self.make_dir() |
379 | - arch, subarch, release, purpose = make_arch_subarch_release_purpose() |
380 | + arch, subarch, release, label, purpose = ( |
381 | + make_arch_subarch_release_label_purpose()) |
382 | expected_destination = os.path.dirname(locate_tftp_path( |
383 | - compose_image_path(arch, subarch, release, purpose), |
384 | + compose_image_path(arch, subarch, release, label, purpose), |
385 | tftproot=tftproot)) |
386 | - make_destination(tftproot, arch, subarch, release, purpose) |
387 | + make_destination(tftproot, arch, subarch, release, label, purpose) |
388 | self.assertThat(expected_destination, DirExists()) |
389 | |
390 | def test_make_destination_returns_existing_directory(self): |
391 | tftproot = self.make_dir() |
392 | - arch, subarch, release, purpose = make_arch_subarch_release_purpose() |
393 | + arch, subarch, release, label, purpose = ( |
394 | + make_arch_subarch_release_label_purpose()) |
395 | expected_dest = locate_tftp_path( |
396 | - compose_image_path(arch, subarch, release, purpose), |
397 | + compose_image_path(arch, subarch, release, label, purpose), |
398 | tftproot=tftproot) |
399 | os.makedirs(expected_dest) |
400 | contents = factory.getRandomString() |
401 | testfile = factory.make_name('testfile') |
402 | factory.make_file(expected_dest, contents=contents, name=testfile) |
403 | - dest = make_destination(tftproot, arch, subarch, release, purpose) |
404 | + dest = make_destination( |
405 | + tftproot, arch, subarch, release, label, purpose) |
406 | self.assertThat(os.path.join(dest, testfile), FileContains(contents)) |
407 | |
408 | def test_are_identical_dirs_sees_missing_old_dir_as_different(self): |
409 | @@ -293,19 +299,21 @@ |
410 | arch = factory.make_name('arch') |
411 | subarch = factory.make_name('subarch') |
412 | release = factory.make_name('release') |
413 | + label = factory.make_name('label') |
414 | purpose = factory.make_name('purpose') |
415 | alternate_purpose = factory.make_name('alt') |
416 | |
417 | install_image( |
418 | - downloaded_image, arch, subarch, release, purpose, |
419 | + downloaded_image, arch, subarch, release, label, purpose, |
420 | alternate_purpose=alternate_purpose) |
421 | |
422 | - main_image = os.path.join(tftp_root, arch, subarch, release, purpose) |
423 | + main_image = os.path.join( |
424 | + tftp_root, arch, subarch, release, label, purpose) |
425 | self.assertThat( |
426 | os.path.join(main_image, 'linux'), |
427 | FileContains(kernel_content)) |
428 | alternate_image = os.path.join( |
429 | - tftp_root, arch, subarch, release, alternate_purpose) |
430 | + tftp_root, arch, subarch, release, label, alternate_purpose) |
431 | self.assertTrue(os.path.islink(os.path.join(alternate_image))) |
432 | self.assertThat( |
433 | os.path.join(alternate_image, 'linux'), |
434 | |
435 | === modified file 'src/provisioningserver/pxe/tests/test_tftppath.py' |
436 | --- src/provisioningserver/pxe/tests/test_tftppath.py 2014-03-14 08:50:51 +0000 |
437 | +++ src/provisioningserver/pxe/tests/test_tftppath.py 2014-03-18 03:24:23 +0000 |
438 | @@ -53,6 +53,7 @@ |
439 | arch=image_params['architecture'], |
440 | subarch=image_params['subarchitecture'], |
441 | release=image_params['release'], |
442 | + label=image_params['label'], |
443 | purpose=image_params['purpose']), |
444 | tftproot) |
445 | os.makedirs(image_dir) |
446 | @@ -75,18 +76,20 @@ |
447 | arch = factory.make_name('arch') |
448 | subarch = factory.make_name('subarch') |
449 | release = factory.make_name('release') |
450 | + label = factory.make_name('label') |
451 | purpose = factory.make_name('purpose') |
452 | self.assertEqual( |
453 | - '%s/%s/%s/%s' % (arch, subarch, release, purpose), |
454 | - compose_image_path(arch, subarch, release, purpose)) |
455 | + '%s/%s/%s/%s/%s' % (arch, subarch, release, label, purpose), |
456 | + compose_image_path(arch, subarch, release, label, purpose)) |
457 | |
458 | def test_compose_image_path_does_not_include_tftp_root(self): |
459 | arch = factory.make_name('arch') |
460 | subarch = factory.make_name('subarch') |
461 | release = factory.make_name('release') |
462 | + label = factory.make_name('label') |
463 | purpose = factory.make_name('purpose') |
464 | self.assertThat( |
465 | - compose_image_path(arch, subarch, release, purpose), |
466 | + compose_image_path(arch, subarch, release, label, purpose), |
467 | Not(StartsWith(self.tftproot))) |
468 | |
469 | def test_compose_bootloader_path_follows_maas_pxe_directory_layout(self): |
470 | @@ -117,20 +120,12 @@ |
471 | |
472 | def test_list_boot_images_finds_boot_image(self): |
473 | image = make_boot_image_params() |
474 | - # XXX jtv 2014-03-11 bug=1290822: The directory hierarchy for boot |
475 | - # images doesn't actually support labels yet. Hard-code "release" |
476 | - # as the label for now, to make the test pass during transition. |
477 | - image['label'] = 'release' |
478 | self.make_image_dir(image, self.tftproot) |
479 | self.assertItemsEqual([image], list_boot_images(self.tftproot)) |
480 | |
481 | def test_list_boot_images_enumerates_boot_images(self): |
482 | images = [make_boot_image_params() for counter in range(3)] |
483 | for image in images: |
484 | - # XXX jtv 2014-03-11 bug=1290822: The directory hierarchy for boot |
485 | - # images doesn't actually support labels yet. Hard-code "release" |
486 | - # as the label for now, to make the test pass during transition. |
487 | - image['label'] = 'release' |
488 | self.make_image_dir(image, self.tftproot) |
489 | self.assertItemsEqual(images, list_boot_images(self.tftproot)) |
490 | |
491 | |
492 | === modified file 'src/provisioningserver/pxe/tftppath.py' |
493 | --- src/provisioningserver/pxe/tftppath.py 2014-03-14 09:48:52 +0000 |
494 | +++ src/provisioningserver/pxe/tftppath.py 2014-03-18 03:24:23 +0000 |
495 | @@ -1,4 +1,4 @@ |
496 | -# Copyright 2012 Canonical Ltd. This software is licensed under the |
497 | +# Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
498 | # GNU Affero General Public License version 3 (see the file LICENSE). |
499 | |
500 | """Construct TFTP paths for PXE files.""" |
501 | @@ -60,7 +60,7 @@ |
502 | htype=ARP_HTYPE.ETHERNET, mac=mac) |
503 | |
504 | |
505 | -def compose_image_path(arch, subarch, release, purpose): |
506 | +def compose_image_path(arch, subarch, release, label, purpose): |
507 | """Compose the TFTP path for a PXE kernel/initrd directory. |
508 | |
509 | The path returned is relative to the TFTP root, as it would be |
510 | @@ -69,12 +69,14 @@ |
511 | :param arch: Main machine architecture. |
512 | :param subarch: Sub-architecture, or "generic" if there is none. |
513 | :param release: Operating system release, e.g. "precise". |
514 | + :param label: An image label, e.g. for a beta version, or "release" for |
515 | + the default images. |
516 | :param purpose: Purpose of the image, e.g. "install" or |
517 | "commissioning". |
518 | :return: Path for the corresponding image directory (containing a |
519 | kernel and initrd) as exposed over TFTP. |
520 | """ |
521 | - return '/'.join([arch, subarch, release, purpose]) |
522 | + return '/'.join([arch, subarch, release, label, purpose]) |
523 | |
524 | |
525 | def locate_tftp_path(path, tftproot): |
526 | @@ -146,11 +148,10 @@ |
527 | The path must consist of a full [architecture, subarchitecture, release, |
528 | purpose] that identify a kind of boot that we may need an image for. |
529 | """ |
530 | - arch, subarch, release, purpose = path |
531 | - # XXX: the hard-coded label needs to be fixed in a future change. |
532 | + arch, subarch, release, label, purpose = path |
533 | return dict( |
534 | architecture=arch, subarchitecture=subarch, release=release, |
535 | - purpose=purpose, label="release") |
536 | + label=label, purpose=purpose) |
537 | |
538 | |
539 | def list_boot_images(tftproot): |
540 | @@ -171,7 +172,7 @@ |
541 | # Extend paths deeper into the filesystem, through the levels that |
542 | # represent sub-architecture, release, and purpose. Any directory |
543 | # that doesn't extend this deep isn't a boot image. |
544 | - for level in ['subarch', 'release', 'purpose']: |
545 | + for level in ['subarch', 'release', 'label', 'purpose']: |
546 | paths = drill_down(tftproot, paths) |
547 | |
548 | # Each path we find this way should be a boot image. |
549 | |
550 | === modified file 'src/provisioningserver/rpc/cluster.py' |
551 | --- src/provisioningserver/rpc/cluster.py 2014-03-10 04:22:07 +0000 |
552 | +++ src/provisioningserver/rpc/cluster.py 2014-03-18 03:24:23 +0000 |
553 | @@ -41,6 +41,7 @@ |
554 | [(b"architecture", amp.Unicode()), |
555 | (b"subarchitecture", amp.Unicode()), |
556 | (b"release", amp.Unicode()), |
557 | + (b"label", amp.Unicode()), |
558 | (b"purpose", amp.Unicode())])) |
559 | ] |
560 | errors = [] |
561 | |
562 | === modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py' |
563 | --- src/provisioningserver/rpc/tests/test_clusterservice.py 2014-03-10 04:22:07 +0000 |
564 | +++ src/provisioningserver/rpc/tests/test_clusterservice.py 2014-03-18 03:24:23 +0000 |
565 | @@ -161,6 +161,7 @@ |
566 | |
567 | self.assertEqual({"images": []}, response) |
568 | |
569 | + @inlineCallbacks |
570 | def test_list_boot_images_with_things_to_report(self): |
571 | # tftppath.list_boot_images()'s return value matches the |
572 | # response schema that ListBootImages declares, and is |
573 | @@ -170,11 +171,12 @@ |
574 | archs = "i386", "amd64" |
575 | subarchs = "generic", "special" |
576 | releases = "precise", "trusty" |
577 | + labels = "beta-1", "release" |
578 | purposes = "commission", "install" |
579 | |
580 | # Create a TFTP file tree with a variety of subdirectories. |
581 | tftpdir = self.make_dir() |
582 | - for options in product(archs, subarchs, releases, purposes): |
583 | + for options in product(archs, subarchs, releases, labels, purposes): |
584 | os.makedirs(os.path.join(tftpdir, *options)) |
585 | |
586 | # Ensure that list_boot_images() uses the above TFTP file tree. |
587 | @@ -182,19 +184,21 @@ |
588 | load_from_cache.return_value = {"tftp": {"root": tftpdir}} |
589 | |
590 | expected_images = [ |
591 | - {"architecture": arch, "subarchitecture": subarch, |
592 | - "release": release, "purpose": purpose} |
593 | - for arch, subarch, release, purpose in product( |
594 | - archs, subarchs, releases, purposes) |
595 | - ] |
596 | - |
597 | - d = call_responder(Cluster(), cluster.ListBootImages, {}) |
598 | - |
599 | - def check(response): |
600 | - self.assertThat(response, KeysEqual("images")) |
601 | - self.assertItemsEqual(expected_images, response["images"]) |
602 | - |
603 | - return d.addCallback(check) |
604 | + { |
605 | + "architecture": arch, |
606 | + "subarchitecture": subarch, |
607 | + "release": release, |
608 | + "label": label, |
609 | + "purpose": purpose, |
610 | + } |
611 | + for arch, subarch, release, label, purpose in product( |
612 | + archs, subarchs, releases, labels, purposes) |
613 | + ] |
614 | + |
615 | + response = yield call_responder(Cluster(), cluster.ListBootImages, {}) |
616 | + |
617 | + self.assertThat(response, KeysEqual("images")) |
618 | + self.assertItemsEqual(expected_images, response["images"]) |
619 | |
620 | |
621 | class TestClusterProtocol_DescribePowerTypes(MAASTestCase): |
622 | |
623 | === modified file 'src/provisioningserver/tasks.py' |
624 | --- src/provisioningserver/tasks.py 2014-03-12 14:09:13 +0000 |
625 | +++ src/provisioningserver/tasks.py 2014-03-18 03:24:23 +0000 |
626 | @@ -1,4 +1,4 @@ |
627 | -# Copyright 2012 Canonical Ltd. This software is licensed under the |
628 | +# Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
629 | # GNU Affero General Public License version 3 (see the file LICENSE). |
630 | |
631 | """Provisioning server tasks that are run in Celery workers.""" |
632 | @@ -16,6 +16,7 @@ |
633 | 'power_off', |
634 | 'power_on', |
635 | 'refresh_secrets', |
636 | + 'report_boot_images', |
637 | 'rndc_command', |
638 | 'setup_rndc_configuration', |
639 | 'restart_dhcp_server', |
640 | |
641 | === modified file 'src/provisioningserver/tests/test_maas_import_pxe_files.py' |
642 | --- src/provisioningserver/tests/test_maas_import_pxe_files.py 2014-03-11 12:19:40 +0000 |
643 | +++ src/provisioningserver/tests/test_maas_import_pxe_files.py 2014-03-18 03:24:23 +0000 |
644 | @@ -119,7 +119,7 @@ |
645 | tftppath.compose_bootloader_path(), tftproot) |
646 | |
647 | |
648 | -def compose_tftp_path(tftproot, arch, release, purpose, *path): |
649 | +def compose_tftp_path(tftproot, arch, release, label, purpose, *path): |
650 | """Compose path for MAAS TFTP files for given architecture. |
651 | |
652 | After the TFTP root directory and the architecture, just append any path |
653 | @@ -128,7 +128,8 @@ |
654 | """ |
655 | return os.path.join( |
656 | tftppath.locate_tftp_path( |
657 | - tftppath.compose_image_path(arch[0], arch[1], release, purpose), |
658 | + tftppath.compose_image_path( |
659 | + arch[0], arch[1], release, label, purpose), |
660 | tftproot), |
661 | *path) |
662 | |
663 | @@ -235,7 +236,7 @@ |
664 | archive = self.make_downloads(arch=arch, release=release) |
665 | self.call_script(archive, self.tftproot, arch=arch, release=release) |
666 | tftp_path = compose_tftp_path( |
667 | - self.tftproot, arch, release, 'install', 'linux') |
668 | + self.tftproot, arch, release, 'release', 'install', 'linux') |
669 | _, download_path = compose_download_dir(archive, arch, release) |
670 | expected_contents = read_file( |
671 | download_path, compose_download_kernel_name(arch, release)) |
672 | @@ -245,7 +246,7 @@ |
673 | arch = self.get_arch() |
674 | release = 'precise' |
675 | tftp_path = compose_tftp_path( |
676 | - self.tftproot, arch, release, 'install', 'linux') |
677 | + self.tftproot, arch, release, 'release', 'install', 'linux') |
678 | os.makedirs(os.path.dirname(tftp_path)) |
679 | with open(tftp_path, 'w') as existing_file: |
680 | existing_file.write(factory.getRandomString()) |
681 | @@ -262,7 +263,7 @@ |
682 | archive = self.make_downloads(arch=arch, release=release) |
683 | self.call_script(archive, self.tftproot, arch=arch, release=release) |
684 | tftp_path = compose_tftp_path( |
685 | - self.tftproot, arch, release, 'install', 'linux') |
686 | + self.tftproot, arch, release, 'release', 'install', 'linux') |
687 | backdate(tftp_path) |
688 | original_timestamp = get_write_time(tftp_path) |
689 | self.call_script(archive, self.tftproot, arch=arch, release=release) |
690 | @@ -280,5 +281,5 @@ |
691 | |
692 | # The script does not install the broken image, and does not fail. |
693 | installed_kernel = compose_tftp_path( |
694 | - self.tftproot, arch, release, 'initrd.gz') |
695 | + self.tftproot, arch, release, 'release', 'initrd.gz') |
696 | self.assertThat(installed_kernel, Not(DirExists())) |
Looks good; I'm afraid this will require some serious QA before we can be confident all the places requiring a change have been fixed.
[0]
> There may be more places that need the change, such as the import scripts.
Looks like you've taken care of ephemerals_ script. py but not maas-import- pxe-files… but I see a card on the board named "Change import script to use label in path" so this might be what this card is about.
[1]
This is a fairly fundamental change and we want to keep the daily package is a working state as much as possible; many people rely on the daily package being in a working state to do their QA/testing. Consider working on dependent branches without actually landing this until QA proves that we reached a state where a newly-installed packages imports its images okay (the migration script for existing installations can probably be done as a completely separated step).