Merge lp:~jtv/maas/no-bootresources-rewrite into lp:maas

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: 2336
Merged at revision: 2336
Proposed branch: lp:~jtv/maas/no-bootresources-rewrite
Merge into: lp:maas
Diff against target: 525 lines (+10/-436)
4 files modified
src/provisioningserver/boot/tftppath.py (+0/-1)
src/provisioningserver/import_images/tests/test_boot_resources.py (+8/-2)
src/provisioningserver/tests/test_upgrade_cluster.py (+2/-287)
src/provisioningserver/upgrade_cluster.py (+0/-146)
To merge this branch: bzr merge lp:~jtv/maas/no-bootresources-rewrite
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+220046@code.launchpad.net

Commit message

Remove the code which rewrote bootresources.yaml on cluster upgrade.

Description of the change

We're getting rid of bootresources.yaml, and not reconstructing import settings from previous state. The comment about the configure_me setting will still be in the default file, but not for long. There's not much point taking it out at this stage, because it'll just be one more apt conflict when upgrading. Better to ditch the whole thing.

ADDENDUM: I caught the end-to-end test for the import code (test_successful_run) downloading ppc64el boot images. That's because the test setup code assumed that there was only a PXE boot method (which the test runs) and a UEFI one (which it patches out). The addition of the ppc64el boot method broke the assumption.

Jeroen

To post a comment you must log in.
2335. By Jeroen T. Vermeulen

Stop end-to-end import test from downloading actual ppc64el boot images. It seemed to be doing that even in trunk.

2336. By Jeroen T. Vermeulen

Merge trunk.

Revision history for this message
Gavin Panella (allenap) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/provisioningserver/boot/tftppath.py'
2--- src/provisioningserver/boot/tftppath.py 2014-05-14 15:12:48 +0000
3+++ src/provisioningserver/boot/tftppath.py 2014-05-19 15:11:39 +0000
4@@ -14,7 +14,6 @@
5 __metaclass__ = type
6 __all__ = [
7 'compose_image_path',
8- 'drill_down',
9 'list_boot_images',
10 'list_subdirs',
11 'locate_tftp_path',
12
13=== modified file 'src/provisioningserver/import_images/tests/test_boot_resources.py'
14--- src/provisioningserver/import_images/tests/test_boot_resources.py 2014-05-16 12:42:18 +0000
15+++ src/provisioningserver/import_images/tests/test_boot_resources.py 2014-05-19 15:11:39 +0000
16@@ -28,7 +28,7 @@
17 from maastesting.matchers import MockAnyCall
18 from maastesting.testcase import MAASTestCase
19 import mock
20-from provisioningserver.boot.uefi import UEFIBootMethod
21+from provisioningserver.boot import BootMethodRegistry
22 from provisioningserver.config import BootConfig
23 from provisioningserver.import_images import boot_resources
24 from provisioningserver.import_images.boot_image_mapping import (
25@@ -282,7 +282,13 @@
26 # unit test might not put to the test.
27 self.patch_logger()
28 self.patch(boot_resources, 'call_and_check').return_code = 0
29- self.patch(UEFIBootMethod, 'install_bootloader')
30+
31+ # We'll go through installation of a PXE boot loader here, but skip
32+ # all other boot loader types. Testing them all is a job for proper
33+ # unit tests.
34+ for method_name, boot_method in BootMethodRegistry:
35+ if method_name != 'pxe':
36+ self.patch(boot_method, 'install_bootloader')
37
38 args = self.make_working_args()
39 arch = self.arch
40
41=== modified file 'src/provisioningserver/tests/test_upgrade_cluster.py'
42--- src/provisioningserver/tests/test_upgrade_cluster.py 2014-04-25 18:03:38 +0000
43+++ src/provisioningserver/tests/test_upgrade_cluster.py 2014-05-19 15:11:39 +0000
44@@ -15,12 +15,8 @@
45 __all__ = []
46
47 from argparse import ArgumentParser
48-from os import (
49- listdir,
50- makedirs,
51- )
52+from os import listdir
53 import os.path
54-from textwrap import dedent
55
56 from maastesting.factory import factory
57 from maastesting.matchers import (
58@@ -30,17 +26,8 @@
59 from maastesting.testcase import MAASTestCase
60 from mock import Mock
61 from provisioningserver import upgrade_cluster
62-from provisioningserver.config import BootConfig
63 from provisioningserver.import_images import boot_resources
64-from provisioningserver.testing.config import (
65- BootConfigFixture,
66- ConfigFixture,
67- )
68-from testtools.matchers import (
69- DirExists,
70- FileContains,
71- StartsWith,
72- )
73+from testtools.matchers import DirExists
74
75
76 class TestUpgradeCluster(MAASTestCase):
77@@ -84,278 +71,6 @@
78 self.assertEqual(['first', 'middle', 'last'], calls)
79
80
81-class TestGenerateBootResourcesConfig(MAASTestCase):
82- """Tests for the `generate_boot_resources_config` upgrade."""
83-
84- def patch_rewrite_boot_resources_config(self):
85- """Patch `rewrite_boot_resources_config` with a mock."""
86- return self.patch(upgrade_cluster, 'rewrite_boot_resources_config')
87-
88- def patch_boot_config(self, config):
89- """Replace the bootresources config with the given fake."""
90- fixture = BootConfigFixture(config)
91- self.useFixture(fixture)
92- path = fixture.filename
93- self.patch(upgrade_cluster, 'locate_config').return_value = path
94- return path
95-
96- def test_hook_does_nothing_if_configure_me_is_False(self):
97- self.patch_boot_config({'boot': {'configure_me': False}})
98- rewrite_config = self.patch_rewrite_boot_resources_config()
99- upgrade_cluster.generate_boot_resources_config()
100- self.assertThat(rewrite_config, MockNotCalled())
101-
102- def test_hook_does_nothing_if_configure_me_is_missing(self):
103- self.patch_boot_config({'boot': {}})
104- rewrite_config = self.patch_rewrite_boot_resources_config()
105- upgrade_cluster.generate_boot_resources_config()
106- self.assertThat(rewrite_config, MockNotCalled())
107-
108- def test_hook_rewrites_if_configure_me_is_True(self):
109- config_file = self.patch_boot_config({'boot': {'configure_me': True}})
110- rewrite_config = self.patch_rewrite_boot_resources_config()
111- upgrade_cluster.generate_boot_resources_config()
112- self.assertThat(rewrite_config, MockCalledOnceWith(config_file))
113-
114- def test_find_old_imports_returns_empty_if_no_tftproot(self):
115- non_dir = os.path.join(self.make_dir(), factory.make_name('nonesuch'))
116- self.assertEqual(set(), upgrade_cluster.find_old_imports(non_dir))
117-
118- def test_find_old_imports_returns_empty_if_tftproot_is_empty(self):
119- self.assertEqual(
120- set(),
121- upgrade_cluster.find_old_imports(self.make_dir()))
122-
123- def test_find_old_imports_finds_image(self):
124- tftproot = self.make_dir()
125- arch = factory.make_name('arch')
126- subarch = factory.make_name('subarch')
127- release = factory.make_name('release')
128- purpose = factory.make_name('purpose')
129- makedirs(os.path.join(tftproot, arch, subarch, release, purpose))
130- self.assertEqual(
131- {(arch, subarch, release)},
132- upgrade_cluster.find_old_imports(tftproot))
133-
134- def test_generate_selections_returns_None_if_no_images_found(self):
135- self.assertIsNone(upgrade_cluster.generate_selections([]))
136-
137- def test_generate_selections_matches_image(self):
138- arch = factory.make_name('arch')
139- subarch = factory.make_name('subarch')
140- release = factory.make_name('release')
141- self.assertEqual(
142- [
143- {
144- 'release': release,
145- 'arches': [arch],
146- 'subarches': [subarch],
147- },
148- ],
149- upgrade_cluster.generate_selections([(arch, subarch, release)]))
150-
151- def test_generate_selections_sorts_output(self):
152- images = [
153- (
154- factory.make_name('arch'),
155- factory.make_name('subarch'),
156- factory.make_name('release'),
157- )
158- for _ in range(3)
159- ]
160- self.assertEqual(
161- upgrade_cluster.generate_selections(sorted(images)),
162- upgrade_cluster.generate_selections(sorted(images, reverse=True)))
163-
164- def test_generate_updated_config_clears_configure_me_if_no_images(self):
165- config = {'boot': {'configure_me': True, 'sources': []}}
166- self.assertNotIn(
167- 'configure_me',
168- upgrade_cluster.generate_updated_config(config, None)['boot'])
169-
170- def test_generate_updated_config_clears_configure_me_if_has_images(self):
171- image = (
172- factory.make_name('arch'),
173- factory.make_name('subarch'),
174- factory.make_name('release'),
175- )
176- config = {'boot': {'configure_me': True, 'sources': []}}
177- self.assertNotIn(
178- 'configure_me',
179- upgrade_cluster.generate_updated_config(config, [image])['boot'])
180-
181- def test_generate_updated_config_leaves_static_entries_intact(self):
182- storage = factory.make_name('storage')
183- path = factory.make_name('path')
184- keyring = factory.make_name('keyring')
185- config = {
186- 'boot': {
187- 'configure_me': True,
188- 'storage': storage,
189- 'sources': [
190- {
191- 'path': path,
192- 'keyring': keyring,
193- },
194- ],
195- },
196- }
197- # Set configure_me; generate_updated_config expects it.
198- config['boot']['configure_me'] = True
199-
200- result = upgrade_cluster.generate_updated_config(config, [])
201- self.assertEqual(storage, result['boot']['storage'])
202- self.assertEqual(path, result['boot']['sources'][0]['path'])
203- self.assertEqual(keyring, result['boot']['sources'][0]['keyring'])
204-
205- def test_generate_updated_config_updates_sources(self):
206- arch = factory.make_name('arch')
207- subarch = factory.make_name('subarch')
208- release = factory.make_name('release')
209- path1 = factory.make_name('path')
210- path2 = factory.make_name('path')
211- config = {
212- 'boot': {
213- 'configure_me': True,
214- # There are two sources. Both will have their selections set.
215- 'sources': [
216- {'path': path1},
217- {'path': path2}
218- ],
219- },
220- }
221- result = upgrade_cluster.generate_updated_config(
222- config, [(arch, subarch, release)])
223- self.assertEqual(
224- [
225- {
226- 'path': path1,
227- 'selections': [
228- {
229- 'release': release,
230- 'arches': [arch],
231- 'subarches': [subarch],
232- },
233- ],
234- },
235- {
236- 'path': path2,
237- 'selections': [
238- {
239- 'release': release,
240- 'arches': [arch],
241- 'subarches': [subarch],
242- },
243- ],
244- },
245- ],
246- result['boot']['sources'])
247-
248- def test_generate_updated_config_does_not_touch_sources_if_no_images(self):
249- path = factory.make_name('path')
250- arches = [factory.make_name('arch') for _ in range(2)]
251- config = {
252- 'boot': {
253- 'configure_me': True,
254- 'sources': [
255- {
256- 'path': path,
257- 'selections': [{'arches': arches}],
258- },
259- ],
260- },
261- }
262- no_images = set()
263- result = upgrade_cluster.generate_updated_config(config, no_images)
264- self.assertEqual(
265- [
266- {
267- 'path': path,
268- 'selections': [{'arches': arches}],
269- },
270- ],
271- result['boot']['sources'])
272-
273- def test_extract_top_comment_reads_up_to_first_non_comment_text(self):
274- header = dedent("""\
275- # Comment.
276-
277- # Comment after blank line.
278- # Indented comment.
279- """)
280- filename = self.make_file(contents=(header + 'text#'))
281- self.assertEqual(header, upgrade_cluster.extract_top_comment(filename))
282-
283- def test_update_config_file_rewrites_file_in_place(self):
284- old_storage = factory.make_name('old')
285- new_storage = factory.make_name('new')
286- original_file = dedent("""\
287- # Top comment.
288- boot:
289- configure_me: True
290- storage: %s
291- """) % old_storage
292- expected_file = dedent("""\
293- # Top comment.
294- boot:
295- storage: %s
296- """) % new_storage
297- config_file = self.make_file(contents=original_file)
298-
299- upgrade_cluster.update_config_file(
300- config_file, {'boot': {'storage': new_storage}})
301-
302- self.assertThat(config_file, FileContains(expected_file))
303-
304- def test_update_config_file_flushes_config_cache(self):
305- self.patch(BootConfig, 'flush_cache')
306- config_file = self.make_file()
307- upgrade_cluster.update_config_file(config_file, {})
308- self.assertThat(
309- BootConfig.flush_cache, MockCalledOnceWith(config_file))
310-
311- def test_rewrite_boot_resources_config_integrates(self):
312- tftproot = self.make_dir()
313- # Fake pre-existing images in a pre-Simplestreams TFTP directory tree.
314- self.useFixture(ConfigFixture({'tftp': {'root': tftproot}}))
315- arch = factory.make_name('arch')
316- subarch = factory.make_name('subarch')
317- release = factory.make_name('release')
318- purpose = factory.make_name('purpose')
319- makedirs(os.path.join(tftproot, arch, subarch, release, purpose))
320-
321- config_file = self.make_file(contents=dedent("""\
322- # Boot resources configuration file.
323- #
324- # Configuration follows.
325-
326- boot:
327- # This setting will be removed during rewrite.
328- configure_me: True
329-
330- storage: "/var/lib/maas/boot-resources/"
331-
332- sources:
333- - path: "http://maas.ubuntu.com/images/somewhere"
334- keyring: "/usr/share/keyrings/ubuntu-cloudimage-keyring.gpg"
335-
336- selections:
337- - release: "trusty"
338- """))
339-
340- upgrade_cluster.rewrite_boot_resources_config(config_file)
341-
342- self.assertThat(
343- config_file,
344- FileContains(matcher=StartsWith(dedent("""\
345- # Boot resources configuration file.
346- #
347- # Configuration follows.
348-
349- boot:
350- """))))
351-
352-
353 class TestMakeMAASOwnBootResources(MAASTestCase):
354 """Tests for the `make_maas_own_boot_resources` upgrade."""
355
356
357=== modified file 'src/provisioningserver/upgrade_cluster.py'
358--- src/provisioningserver/upgrade_cluster.py 2014-05-13 16:02:46 +0000
359+++ src/provisioningserver/upgrade_cluster.py 2014-05-19 15:11:39 +0000
360@@ -38,157 +38,12 @@
361 from subprocess import check_call
362
363 from provisioningserver.auth import MAAS_USER_GPGHOME
364-from provisioningserver.boot.tftppath import drill_down
365-from provisioningserver.config import (
366- BootConfig,
367- Config,
368- )
369 from provisioningserver.import_images import boot_resources
370-from provisioningserver.utils import (
371- atomic_write,
372- locate_config,
373- read_text_file,
374- )
375-import yaml
376
377
378 logger = getLogger(__name__)
379
380
381-def find_old_imports(tftproot):
382- """List pre-Simplestreams boot images.
383-
384- Supports the `generate_boot_resources_config` upgrade hook. Returns a set
385- of tuples (arch, subarch, release) describing all of the images found.
386- """
387- if not os.path.isdir(tftproot):
388- return set()
389- paths = [[tftproot]]
390- for level in ['arch', 'subarch', 'release', 'purpose']:
391- paths = drill_down(tftproot, paths)
392- return {
393- (arch, subarch, release)
394- for [root, arch, subarch, release, purpose] in paths
395- }
396-
397-
398-def generate_selections(images):
399- """Generate `selections` stanzas to match pre-existing boot images.
400-
401- Supports the `generate_boot_resources_config` upgrade hook.
402-
403- :param images: An iterable of (arch, subarch, release) tuples as returned
404- by `find_old_imports`.
405- :return: A list of dicts, each describing one `selections` stanza for the
406- `bootresources.yaml` file.
407- """
408- if len(images) == 0:
409- # No old images found.
410- return None
411- else:
412- # Return one "selections" stanza per image. This could be cleverer
413- # and combine multiple architectures/subarchitectures, but there would
414- # have to be a clear gain. Simple is good.
415- return [
416- {
417- 'release': release,
418- 'arches': [arch],
419- 'subarches': [subarch],
420- }
421- for arch, subarch, release in sorted(images)
422- ]
423-
424-
425-def generate_updated_config(config, old_images):
426- """Return an updated version of a config dict.
427-
428- Supports the `generate_boot_resources_config` upgrade hook.
429-
430- This clears the `configure_me` flag, and replaces all sources'
431- `selections` stanzas with ones based on the old boot images.
432-
433- :param config: A config dict, as loaded from `bootresources.yaml`.
434- :param old_images: Old-style boot images, as returned by
435- `find_old_imports`. If `None`, the existing `selections` are left
436- unchanged.
437- :return: An updated version of `config` with the above changes.
438- """
439- config = config.copy()
440- # Remove the configure_me item. It's there exactly to tell us that we
441- # haven't done this rewrite yet.
442- del config['boot']['configure_me']
443- if old_images is None:
444- return config
445-
446- # If we found old images, rewrite the selections.
447- if len(old_images) != 0:
448- new_selections = generate_selections(old_images)
449- for source in config['boot']['sources']:
450- source['selections'] = new_selections
451- return config
452-
453-
454-def extract_top_comment(input_file):
455- """Return just the comment at the top of `input_file`.
456-
457- Supports the `generate_boot_resources_config` upgrade hook.
458- """
459- lines = []
460- for line in read_text_file(input_file).splitlines():
461- stripped_line = line.lstrip()
462- if stripped_line != '' and not stripped_line.startswith('#'):
463- # Not an empty line or comment any more. Stop.
464- break
465- lines.append(line)
466- return '\n'.join(lines) + '\n'
467-
468-
469-def update_config_file(config_file, new_config):
470- """Replace configuration data in `config_file` with `new_config`.
471-
472- Supports the `generate_boot_resources_config` upgrade hook.
473-
474- The first part of the config file, up to the first text that isn't a
475- comment, is kept intact. The part after that is overwritten with YAML
476- for the new configuration.
477- """
478- header = extract_top_comment(config_file)
479- data = yaml.safe_dump(new_config, default_flow_style=False)
480- content = (header + data).encode('utf-8')
481- atomic_write(content, config_file, mode=0644)
482- BootConfig.flush_cache(config_file)
483-
484-
485-def rewrite_boot_resources_config(config_file):
486- """Rewrite the `bootresources.yaml` configuration.
487-
488- Supports the `generate_boot_resources_config` upgrade hook.
489- """
490- # Look for images using the old tftp root setting, not the tftp
491- # resource_root setting. The latter points to where the newer,
492- # Simplestreams-based boot images live.
493- # This should be the final use of the old tftp root setting. After this
494- # has run, it serves no more purpose.
495- tftproot = Config.load_from_cache()['tftp']['root']
496- config = BootConfig.load_from_cache(config_file)
497- old_images = find_old_imports(tftproot)
498- new_config = generate_updated_config(config, old_images)
499- update_config_file(config_file, new_config)
500-
501-
502-def generate_boot_resources_config():
503- """Upgrade hook: rewrite `bootresources.yaml` based on boot images.
504-
505- This finds boot images downloaded into the old, pre-Simplestreams tftp
506- root, and writes a boot-resources configuration to import a similar set of
507- images using Simplestreams.
508- """
509- config_file = locate_config('bootresources.yaml')
510- boot_resource_config = BootConfig.load_from_cache(config_file)
511- if boot_resource_config['boot'].get('configure_me', False):
512- rewrite_boot_resources_config(config_file)
513-
514-
515 def make_maas_own_boot_resources():
516 """Upgrade hook: make the `maas` user the owner of the boot resources."""
517 # This reduces the privileges required for importing and managing images.
518@@ -211,7 +66,6 @@
519 # Each hook figures out for itself whether its changes are needed. There is
520 # no record of previous upgrades.
521 UPGRADE_HOOKS = [
522- generate_boot_resources_config,
523 make_maas_own_boot_resources,
524 create_gnupg_home,
525 ]