Merge lp:~jtv/maas/bug-1238376 into lp:maas/trunk

Proposed by Jeroen T. Vermeulen on 2013-10-11
Status: Merged
Approved by: Jeroen T. Vermeulen on 2013-10-11
Approved revision: 1703
Merged at revision: 1701
Proposed branch: lp:~jtv/maas/bug-1238376
Merge into: lp:maas/trunk
Diff against target: 243 lines (+51/-109)
5 files modified
etc/maas/import_ephemerals (+14/-0)
etc/maas/pserv.yaml (+26/-3)
setup.py (+1/-0)
src/provisioningserver/import_images/ephemerals_script.py (+1/-24)
src/provisioningserver/import_images/tests/test_ephemerals_script.py (+9/-82)
To merge this branch: bzr merge lp:~jtv/maas/bug-1238376
Reviewer Review Type Date Requested Status
Gavin Panella (community) 2013-10-11 Approve on 2013-10-11
Review via email: mp+190656@code.launchpad.net

Commit message

Keep the legacy ephemerals import script config for now.

Description of the change

This took a while to run through its paces. As per bug 1238376, users are bemused with the import_pxe_files config no longer being included as configuration for the ephemerals import script. Therefore in this branch I change it as follows:

1. No longer rewrite pserv.yaml with the combined config, and no longer move the import_ephemerals config out of the way. I kept the generic infrastructure for doing these things, but not the specific code.

2. Strip the now-inert items out of the import_ephemerals config, and add a notice about the file being obsolete. This means that upgrading users will get a notice about the change, giving them a fair chance of learning about the change. And it means that any further changes they make to either import_pxe_files or import_ephemerals will still be respected.

3. Document the new options in pserv.yaml. These options will still be respected, unless the legacy shell scripts set different values. It'd be nice to have this the other way around, but the config has no real difference between letting a setting empty to use the default, and deliberately setting it to its default value. At least with the legacy config we can tell whether a value has been set or not.

A packaging branch will follow, to add the new /etc/maas/import_ephemerals back into the package.

Jeroen

To post a comment you must log in.
Gavin Panella (allenap) wrote :

Sad, but probably the right thing to do. Undoing badness takes time, and this is proof.

review: Approve
lp:~jtv/maas/bug-1238376 updated on 2013-10-11
1703. By Jeroen T. Vermeulen on 2013-10-11

Setting not really needed for test.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'etc/maas/import_ephemerals'
2--- etc/maas/import_ephemerals 1970-01-01 00:00:00 +0000
3+++ etc/maas/import_ephemerals 2013-10-11 13:46:22 +0000
4@@ -0,0 +1,14 @@
5+# Legacy configuration file for the maas-import-ephemerals script.
6+#
7+# This file is obsolete, but the script will still read it for compatibility.
8+# Configure /etc/maas/pserv.yaml by preference.
9+
10+## Include settings from import_pxe_files.
11+[ ! -f /etc/maas/import_pxe_files ] || . /etc/maas/import_pxe_files
12+
13+# These options can be defined here, although for future compatibility they
14+# should be set in pserv.yaml instead:
15+
16+#DATA_DIR="/var/lib/maas/ephemeral"
17+#RELEASES="precise"
18+#ARCHES="amd64/generic i386/generic"
19
20=== modified file 'etc/maas/pserv.yaml'
21--- etc/maas/pserv.yaml 2013-10-09 04:33:07 +0000
22+++ etc/maas/pserv.yaml 2013-10-11 13:46:22 +0000
23@@ -40,6 +40,29 @@
24
25 ## Boot configuration.
26 boot:
27- # ephemeral:
28- ## Directory containing ephemeral boot images, etc.
29- # images_directory: /var/lib/maas/ephemeral
30+ ## CPU architectures for which boot images should be downloaded from the
31+ ## server, e.g. ['i386/generic', 'amd64/generic']. MAAS needs these images
32+ ## in order to boot nodes up.
33+ ##
34+ ## Leave this option out to download images for all architectures.
35+ #
36+ # architectures:
37+
38+ ## Settings for ephemeral boot images. These images are used when
39+ ## commissioning nodes, and during fast-path installation.
40+ #
41+ ephemeral:
42+
43+ ## Directory where ephemeral boot images and related state should be
44+ ## stored.
45+ #
46+ # images_directory: /var/lib/maas/ephemeral
47+ images_directory: /var/lib/maas/ephemeral
48+
49+ ## Releases for which ephemeral images should be downloaded.
50+ ## These images are quite large (about a quarter GB each), so you may want
51+ ## to restrict these separately even if you do want the regular install
52+ ## images for all releases. Leave this out to download all currently
53+ ## supported releases.
54+ #
55+ # releases:
56
57=== modified file 'setup.py'
58--- setup.py 2013-10-09 07:58:23 +0000
59+++ setup.py 2013-10-11 13:46:22 +0000
60@@ -69,6 +69,7 @@
61 'etc/txlongpoll.yaml',
62 'contrib/maas_local_celeryconfig.py',
63 'contrib/maas_local_celeryconfig_cluster.py',
64+ 'etc/maas/import_ephemerals',
65 'etc/maas/import_pxe_files',
66 'contrib/maas-http.conf',
67 'contrib/maas-cluster-http.conf',
68
69=== modified file 'src/provisioningserver/import_images/ephemerals_script.py'
70--- src/provisioningserver/import_images/ephemerals_script.py 2013-10-11 08:30:04 +0000
71+++ src/provisioningserver/import_images/ephemerals_script.py 2013-10-11 13:46:22 +0000
72@@ -33,9 +33,7 @@
73
74 from provisioningserver.config import Config
75 from provisioningserver.import_images.config import (
76- EPHEMERALS_LEGACY_CONFIG,
77 merge_legacy_ephemerals_config,
78- retire_legacy_config,
79 )
80 from provisioningserver.import_images.tgt import (
81 clean_up_info_file,
82@@ -361,8 +359,7 @@
83 except IOError as e:
84 if e.errno != errno.ENOENT:
85 raise
86- # Plod on with defaults, in case this is just a --help run. If it's
87- # not, main() will fail anyway.
88+ # Plod on with defaults. There may be a legacy shell-script config.
89 config = Config.get_defaults()
90 # Merge legacy settings into our copy of the config.
91 merge_legacy_ephemerals_config(config)
92@@ -406,31 +403,11 @@
93 return parser
94
95
96-def convert_legacy_config():
97- """If appropriate, update config based on legacy shell-script config."""
98- config = Config.load_from_cache()
99- changed = merge_legacy_ephemerals_config(config)
100- if not changed:
101- return
102-
103- logger.info(
104- "Updating configuration %s from legacy shell-script config %s.",
105- Config.DEFAULT_FILENAME, EPHEMERALS_LEGACY_CONFIG)
106- # Backup the old config.
107- Config.create_backup('legacy-script-migration')
108- # Save the new config.
109- Config.save(config)
110- # Now move the legacy config out of the way, so that we don't convert it
111- # a second time.
112- retire_legacy_config()
113-
114-
115 def main(args):
116 """Import ephemeral images.
117
118 :param args: Command-line arguments, in parsed form.
119 """
120- convert_legacy_config()
121
122 def verify_signature(content, path):
123 """Policy callback for Simplestreams: verify index signature."""
124
125=== modified file 'src/provisioningserver/import_images/tests/test_ephemerals_script.py'
126--- src/provisioningserver/import_images/tests/test_ephemerals_script.py 2013-10-09 11:35:14 +0000
127+++ src/provisioningserver/import_images/tests/test_ephemerals_script.py 2013-10-11 13:46:22 +0000
128@@ -27,10 +27,6 @@
129
130 from fixtures import EnvironmentVariableFixture
131 from maastesting.factory import factory
132-from maastesting.utils import (
133- age_file,
134- get_write_time,
135- )
136 from provisioningserver.config import Config
137 from provisioningserver.import_images import (
138 config as config_module,
139@@ -38,7 +34,6 @@
140 )
141 from provisioningserver.import_images.ephemerals_script import (
142 compose_filter,
143- convert_legacy_config,
144 copy_file_by_glob,
145 create_symlinked_image_dir,
146 extract_image_tarball,
147@@ -54,8 +49,6 @@
148 from provisioningserver.utils import read_text_file
149 from testtools.matchers import (
150 FileContains,
151- FileExists,
152- Not,
153 StartsWith,
154 )
155
156@@ -471,78 +464,12 @@
157 original_boot_config,
158 Config.load_from_cache()['boot'])
159
160-
161-class TestConvertLegacyConfig(PservTestCase):
162-
163- def test_converts_legacy_config_if_old_config_available(self):
164- self.useFixture(ConfigFixture({'boot': {'ephemeral': {}}}))
165- data_dir = self.make_dir()
166- arches = [factory.make_name('arch')]
167- releases = [factory.make_name('rel')]
168- legacy_config = make_legacy_config(data_dir, arches, releases)
169- legacy_file = install_legacy_config(self, legacy_config)
170- initial_config = file(Config.DEFAULT_FILENAME).read()
171-
172- convert_legacy_config()
173-
174- self.assertEqual(
175- {
176- 'architectures': arches,
177- 'ephemeral': {
178- 'images_directory': data_dir,
179- 'releases': releases,
180- },
181- },
182- Config.load()['boot'])
183- # Legacy config file has been deleted.
184- self.assertThat(legacy_file, Not(FileExists()))
185- # A backup of the legacy config file has been kept.
186- self.assertThat(legacy_file + '.obsolete', FileContains(legacy_config))
187- # A copy of the initial config (i.e. the pserv config before
188- # migration) has been kept.
189- backup_name = Config._get_backup_name('legacy-script-migration')
190- self.assertThat(backup_name, FileContains(initial_config))
191-
192- def test_does_nothing_without_old_config(self):
193- data_dir = self.make_dir()
194- arches = [factory.make_name('arch')]
195- releases = [factory.make_name('rel')]
196- config = self.useFixture(ConfigFixture(
197- {
198- 'boot': {
199- 'architectures': arches,
200- 'ephemeral': {
201- 'images_directory': data_dir,
202- 'releases': releases,
203- }
204- }
205- }))
206- self.patch(config_module, 'parse_legacy_config').return_value = {}
207- age_file(config.filename, 600)
208- config_last_written = get_write_time(config.filename)
209-
210- convert_legacy_config()
211-
212- self.assertEqual(config_last_written, get_write_time(config.filename))
213- self.assertEqual(
214- {
215- 'architectures': arches,
216- 'ephemeral': {
217- 'images_directory': data_dir,
218- 'releases': releases,
219- },
220- },
221- Config.load()['boot'])
222-
223- def test_is_idempotent(self):
224- self.useFixture(ConfigFixture({'boot': {'ephemeral': {}}}))
225- legacy_config = make_legacy_config()
226- legacy_file = install_legacy_config(self, legacy_config)
227- convert_legacy_config()
228- converted_config = Config.load()['boot']
229-
230- convert_legacy_config()
231-
232- self.assertEqual(converted_config, Config.load()['boot'])
233- self.assertThat(legacy_file, Not(FileExists()))
234- self.assertThat(legacy_file + '.obsolete', FileContains(legacy_config))
235+ def test_uses_legacy_config(self):
236+ data_dir = self.make_dir()
237+ self.useFixture(ConfigFixture({}))
238+ install_legacy_config(self, make_legacy_config(data_dir=data_dir))
239+
240+ parser = make_arg_parser(factory.getRandomString())
241+
242+ args = parser.parse_args('')
243+ self.assertEqual(data_dir, args.output)