Merge lp:~blake-rouse/maas/tftp-backend-readers-1.5 into lp:maas/1.5
- tftp-backend-readers-1.5
- Merge into 1.5
Proposed by
Blake Rouse
Status: | Merged |
---|---|
Approved by: | Blake Rouse |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2280 |
Proposed branch: | lp:~blake-rouse/maas/tftp-backend-readers-1.5 |
Merge into: | lp:maas/1.5 |
Diff against target: |
633 lines (+117/-100) 10 files modified
src/provisioningserver/boot/__init__.py (+26/-6) src/provisioningserver/boot/powerkvm.py (+3/-3) src/provisioningserver/boot/pxe.py (+7/-4) src/provisioningserver/boot/tests/test_boot.py (+4/-3) src/provisioningserver/boot/tests/test_powerkvm.py (+4/-4) src/provisioningserver/boot/tests/test_pxe.py (+23/-16) src/provisioningserver/boot/tests/test_uefi.py (+14/-10) src/provisioningserver/boot/uefi.py (+7/-4) src/provisioningserver/tests/test_tftp.py (+21/-20) src/provisioningserver/tftp.py (+8/-30) |
To merge this branch: | bzr merge lp:~blake-rouse/maas/tftp-backend-readers-1.5 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Blake Rouse (community) | Approve | ||
Review via email: mp+221230@code.launchpad.net |
Commit message
Change BootMethods to return their own IReader per-request, update method names to reflect new usage.
Description of the change
To post a comment you must log in.
Revision history for this message
Blake Rouse (blake-rouse) : | # |
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/__init__.py' | |||
2 | --- src/provisioningserver/boot/__init__.py 2014-05-23 16:32:07 +0000 | |||
3 | +++ src/provisioningserver/boot/__init__.py 2014-05-28 13:35:49 +0000 | |||
4 | @@ -23,6 +23,7 @@ | |||
5 | 23 | abstractproperty, | 23 | abstractproperty, |
6 | 24 | ) | 24 | ) |
7 | 25 | from errno import ENOENT | 25 | from errno import ENOENT |
8 | 26 | from io import BytesIO | ||
9 | 26 | from os import path | 27 | from os import path |
10 | 27 | 28 | ||
11 | 28 | from provisioningserver.boot.tftppath import compose_image_path | 29 | from provisioningserver.boot.tftppath import compose_image_path |
12 | @@ -30,6 +31,23 @@ | |||
13 | 30 | from provisioningserver.utils import locate_config | 31 | from provisioningserver.utils import locate_config |
14 | 31 | from provisioningserver.utils.registry import Registry | 32 | from provisioningserver.utils.registry import Registry |
15 | 32 | import tempita | 33 | import tempita |
16 | 34 | from tftp.backend import IReader | ||
17 | 35 | from zope.interface import implementer | ||
18 | 36 | |||
19 | 37 | |||
20 | 38 | @implementer(IReader) | ||
21 | 39 | class BytesReader: | ||
22 | 40 | |||
23 | 41 | def __init__(self, data): | ||
24 | 42 | super(BytesReader, self).__init__() | ||
25 | 43 | self.buffer = BytesIO(data) | ||
26 | 44 | self.size = len(data) | ||
27 | 45 | |||
28 | 46 | def read(self, size): | ||
29 | 47 | return self.buffer.read(size) | ||
30 | 48 | |||
31 | 49 | def finish(self): | ||
32 | 50 | self.buffer.close() | ||
33 | 33 | 51 | ||
34 | 34 | 52 | ||
35 | 35 | class BootMethodError(Exception): | 53 | class BootMethodError(Exception): |
36 | @@ -100,22 +118,24 @@ | |||
37 | 100 | """ | 118 | """ |
38 | 101 | 119 | ||
39 | 102 | @abstractmethod | 120 | @abstractmethod |
43 | 103 | def match_config_path(self, path): | 121 | def match_path(self, backend, path): |
44 | 104 | """Checks path for the configuration file that needs to be | 122 | """Checks path for a file the boot method needs to handle. |
42 | 105 | generated. | ||
45 | 106 | 123 | ||
46 | 124 | :param backend: requesting backend | ||
47 | 107 | :param path: requested path | 125 | :param path: requested path |
48 | 108 | :returns: dict of match params from path, None if no match | 126 | :returns: dict of match params from path, None if no match |
49 | 109 | """ | 127 | """ |
50 | 110 | 128 | ||
51 | 111 | @abstractmethod | 129 | @abstractmethod |
54 | 112 | def render_config(self, kernel_params, **extra): | 130 | def get_reader(self, backend, kernel_params, **extra): |
55 | 113 | """Render a configuration file as a unicode string. | 131 | """Gets the reader the backend will use for this combination of |
56 | 132 | boot method, kernel parameters, and extra parameters. | ||
57 | 114 | 133 | ||
58 | 134 | :param backend: requesting backend | ||
59 | 115 | :param kernel_params: An instance of `KernelParameters`. | 135 | :param kernel_params: An instance of `KernelParameters`. |
60 | 116 | :param extra: Allow for other arguments. This is a safety valve; | 136 | :param extra: Allow for other arguments. This is a safety valve; |
61 | 117 | parameters generated in another component (for example, see | 137 | parameters generated in another component (for example, see |
63 | 118 | `TFTPBackend.get_config_reader`) won't cause this to break. | 138 | `TFTPBackend.get_boot_method_reader`) won't cause this to break. |
64 | 119 | """ | 139 | """ |
65 | 120 | 140 | ||
66 | 121 | @abstractmethod | 141 | @abstractmethod |
67 | 122 | 142 | ||
68 | === modified file 'src/provisioningserver/boot/powerkvm.py' | |||
69 | --- src/provisioningserver/boot/powerkvm.py 2014-05-23 16:32:07 +0000 | |||
70 | +++ src/provisioningserver/boot/powerkvm.py 2014-05-28 13:35:49 +0000 | |||
71 | @@ -45,17 +45,17 @@ | |||
72 | 45 | bootloader_path = "bootppc64.bin" | 45 | bootloader_path = "bootppc64.bin" |
73 | 46 | arch_octet = "00:0C" | 46 | arch_octet = "00:0C" |
74 | 47 | 47 | ||
76 | 48 | def match_config_path(self, path): | 48 | def match_path(self, backend, path): |
77 | 49 | """Doesn't need to do anything, as the UEFIBootMethod provides | 49 | """Doesn't need to do anything, as the UEFIBootMethod provides |
78 | 50 | the grub implementation needed. | 50 | the grub implementation needed. |
79 | 51 | """ | 51 | """ |
80 | 52 | return None | 52 | return None |
81 | 53 | 53 | ||
83 | 54 | def render_config(self, kernel_params, **extra): | 54 | def get_reader(self, backend, kernel_params, **extra): |
84 | 55 | """Doesn't need to do anything, as the UEFIBootMethod provides | 55 | """Doesn't need to do anything, as the UEFIBootMethod provides |
85 | 56 | the grub implementation needed. | 56 | the grub implementation needed. |
86 | 57 | """ | 57 | """ |
88 | 58 | return "" | 58 | return None |
89 | 59 | 59 | ||
90 | 60 | def install_bootloader(self, destination): | 60 | def install_bootloader(self, destination): |
91 | 61 | """Installs the required files for PowerKVM booting into the | 61 | """Installs the required files for PowerKVM booting into the |
92 | 62 | 62 | ||
93 | === modified file 'src/provisioningserver/boot/pxe.py' | |||
94 | --- src/provisioningserver/boot/pxe.py 2014-03-28 04:06:27 +0000 | |||
95 | +++ src/provisioningserver/boot/pxe.py 2014-05-28 13:35:49 +0000 | |||
96 | @@ -22,6 +22,7 @@ | |||
97 | 22 | 22 | ||
98 | 23 | from provisioningserver.boot import ( | 23 | from provisioningserver.boot import ( |
99 | 24 | BootMethod, | 24 | BootMethod, |
100 | 25 | BytesReader, | ||
101 | 25 | get_parameters, | 26 | get_parameters, |
102 | 26 | ) | 27 | ) |
103 | 27 | from provisioningserver.boot.install_bootloader import install_bootloader | 28 | from provisioningserver.boot.install_bootloader import install_bootloader |
104 | @@ -78,10 +79,11 @@ | |||
105 | 78 | bootloader_path = "pxelinux.0" | 79 | bootloader_path = "pxelinux.0" |
106 | 79 | arch_octet = "00:00" | 80 | arch_octet = "00:00" |
107 | 80 | 81 | ||
109 | 81 | def match_config_path(self, path): | 82 | def match_path(self, backend, path): |
110 | 82 | """Checks path for the configuration file that needs to be | 83 | """Checks path for the configuration file that needs to be |
111 | 83 | generated. | 84 | generated. |
112 | 84 | 85 | ||
113 | 86 | :param backend: requesting backend | ||
114 | 85 | :param path: requested path | 87 | :param path: requested path |
115 | 86 | :returns: dict of match params from path, None if no match | 88 | :returns: dict of match params from path, None if no match |
116 | 87 | """ | 89 | """ |
117 | @@ -90,19 +92,20 @@ | |||
118 | 90 | return None | 92 | return None |
119 | 91 | return get_parameters(match) | 93 | return get_parameters(match) |
120 | 92 | 94 | ||
122 | 93 | def render_config(self, kernel_params, **extra): | 95 | def get_reader(self, backend, kernel_params, **extra): |
123 | 94 | """Render a configuration file as a unicode string. | 96 | """Render a configuration file as a unicode string. |
124 | 95 | 97 | ||
125 | 98 | :param backend: requesting backend | ||
126 | 96 | :param kernel_params: An instance of `KernelParameters`. | 99 | :param kernel_params: An instance of `KernelParameters`. |
127 | 97 | :param extra: Allow for other arguments. This is a safety valve; | 100 | :param extra: Allow for other arguments. This is a safety valve; |
128 | 98 | parameters generated in another component (for example, see | 101 | parameters generated in another component (for example, see |
130 | 99 | `TFTPBackend.get_config_reader`) won't cause this to break. | 102 | `TFTPBackend.get_boot_method_reader`) won't cause this to break. |
131 | 100 | """ | 103 | """ |
132 | 101 | template = self.get_template( | 104 | template = self.get_template( |
133 | 102 | kernel_params.purpose, kernel_params.arch, | 105 | kernel_params.purpose, kernel_params.arch, |
134 | 103 | kernel_params.subarch) | 106 | kernel_params.subarch) |
135 | 104 | namespace = self.compose_template_namespace(kernel_params) | 107 | namespace = self.compose_template_namespace(kernel_params) |
137 | 105 | return template.substitute(namespace) | 108 | return BytesReader(template.substitute(namespace).encode("utf-8")) |
138 | 106 | 109 | ||
139 | 107 | def install_bootloader(self, destination): | 110 | def install_bootloader(self, destination): |
140 | 108 | """Installs the required files for PXE booting into the | 111 | """Installs the required files for PXE booting into the |
141 | 109 | 112 | ||
142 | === modified file 'src/provisioningserver/boot/tests/test_boot.py' | |||
143 | --- src/provisioningserver/boot/tests/test_boot.py 2014-03-21 19:01:40 +0000 | |||
144 | +++ src/provisioningserver/boot/tests/test_boot.py 2014-05-28 13:35:49 +0000 | |||
145 | @@ -24,6 +24,7 @@ | |||
146 | 24 | from provisioningserver import boot | 24 | from provisioningserver import boot |
147 | 25 | from provisioningserver.boot import ( | 25 | from provisioningserver.boot import ( |
148 | 26 | BootMethod, | 26 | BootMethod, |
149 | 27 | BytesReader, | ||
150 | 27 | gen_template_filenames, | 28 | gen_template_filenames, |
151 | 28 | ) | 29 | ) |
152 | 29 | import tempita | 30 | import tempita |
153 | @@ -36,11 +37,11 @@ | |||
154 | 36 | bootloader_path = "fake.efi" | 37 | bootloader_path = "fake.efi" |
155 | 37 | arch_octet = "00:00" | 38 | arch_octet = "00:00" |
156 | 38 | 39 | ||
158 | 39 | def match_config_path(self, path): | 40 | def match_path(self, backend, path): |
159 | 40 | return {} | 41 | return {} |
160 | 41 | 42 | ||
163 | 42 | def render_config(kernel_params, **extra): | 43 | def get_reader(backend, kernel_params, **extra): |
164 | 43 | return "" | 44 | return BytesReader("") |
165 | 44 | 45 | ||
166 | 45 | def install_bootloader(): | 46 | def install_bootloader(): |
167 | 46 | pass | 47 | pass |
168 | 47 | 48 | ||
169 | === modified file 'src/provisioningserver/boot/tests/test_powerkvm.py' | |||
170 | --- src/provisioningserver/boot/tests/test_powerkvm.py 2014-05-23 16:32:07 +0000 | |||
171 | +++ src/provisioningserver/boot/tests/test_powerkvm.py 2014-05-28 13:35:49 +0000 | |||
172 | @@ -35,17 +35,17 @@ | |||
173 | 35 | class TestPowerKVMBootMethod(MAASTestCase): | 35 | class TestPowerKVMBootMethod(MAASTestCase): |
174 | 36 | """Tests `provisioningserver.boot.powerkvm.PowerKVMBootMethod`.""" | 36 | """Tests `provisioningserver.boot.powerkvm.PowerKVMBootMethod`.""" |
175 | 37 | 37 | ||
177 | 38 | def test_match_config_path_returns_none(self): | 38 | def test_match_path_returns_None(self): |
178 | 39 | method = PowerKVMBootMethod() | 39 | method = PowerKVMBootMethod() |
179 | 40 | paths = [factory.getRandomString() for _ in range(3)] | 40 | paths = [factory.getRandomString() for _ in range(3)] |
180 | 41 | for path in paths: | 41 | for path in paths: |
182 | 42 | self.assertEqual(None, method.match_config_path(path)) | 42 | self.assertEqual(None, method.match_path(None, path)) |
183 | 43 | 43 | ||
185 | 44 | def test_render_config_returns_empty_string(self): | 44 | def test_get_reader_returns_None(self): |
186 | 45 | method = PowerKVMBootMethod() | 45 | method = PowerKVMBootMethod() |
187 | 46 | params = [make_kernel_parameters() for _ in range(3)] | 46 | params = [make_kernel_parameters() for _ in range(3)] |
188 | 47 | for param in params: | 47 | for param in params: |
190 | 48 | self.assertEqual("", method.render_config(params)) | 48 | self.assertEqual(None, method.get_reader(None, params)) |
191 | 49 | 49 | ||
192 | 50 | def test_install_bootloader_get_package_raises_error(self): | 50 | def test_install_bootloader_get_package_raises_error(self): |
193 | 51 | method = PowerKVMBootMethod() | 51 | method = PowerKVMBootMethod() |
194 | 52 | 52 | ||
195 | === modified file 'src/provisioningserver/boot/tests/test_pxe.py' | |||
196 | --- src/provisioningserver/boot/tests/test_pxe.py 2014-03-28 04:31:32 +0000 | |||
197 | +++ src/provisioningserver/boot/tests/test_pxe.py 2014-05-28 13:35:49 +0000 | |||
198 | @@ -20,6 +20,7 @@ | |||
199 | 20 | from maastesting.factory import factory | 20 | from maastesting.factory import factory |
200 | 21 | from maastesting.testcase import MAASTestCase | 21 | from maastesting.testcase import MAASTestCase |
201 | 22 | from provisioningserver import kernel_opts | 22 | from provisioningserver import kernel_opts |
202 | 23 | from provisioningserver.boot import BytesReader | ||
203 | 23 | from provisioningserver.boot.pxe import ( | 24 | from provisioningserver.boot.pxe import ( |
204 | 24 | ARP_HTYPE, | 25 | ARP_HTYPE, |
205 | 25 | PXEBootMethod, | 26 | PXEBootMethod, |
206 | @@ -150,14 +151,15 @@ | |||
207 | 150 | class TestPXEBootMethodRenderConfig(MAASTestCase): | 151 | class TestPXEBootMethodRenderConfig(MAASTestCase): |
208 | 151 | """Tests for `provisioningserver.boot.pxe.PXEBootMethod.render_config`.""" | 152 | """Tests for `provisioningserver.boot.pxe.PXEBootMethod.render_config`.""" |
209 | 152 | 153 | ||
211 | 153 | def test_render_install(self): | 154 | def test_get_reader_install(self): |
212 | 154 | # Given the right configuration options, the PXE configuration is | 155 | # Given the right configuration options, the PXE configuration is |
213 | 155 | # correctly rendered. | 156 | # correctly rendered. |
214 | 156 | method = PXEBootMethod() | 157 | method = PXEBootMethod() |
215 | 157 | params = make_kernel_parameters(self, purpose="install") | 158 | params = make_kernel_parameters(self, purpose="install") |
219 | 158 | output = method.render_config(kernel_params=params) | 159 | output = method.get_reader(backend=None, kernel_params=params) |
220 | 159 | # The output is always a Unicode string. | 160 | # The output is a BytesReader. |
221 | 160 | self.assertThat(output, IsInstance(unicode)) | 161 | self.assertThat(output, IsInstance(BytesReader)) |
222 | 162 | output = output.read(10000) | ||
223 | 161 | # The template has rendered without error. PXELINUX configurations | 163 | # The template has rendered without error. PXELINUX configurations |
224 | 162 | # typically start with a DEFAULT line. | 164 | # typically start with a DEFAULT line. |
225 | 163 | self.assertThat(output, StartsWith("DEFAULT ")) | 165 | self.assertThat(output, StartsWith("DEFAULT ")) |
226 | @@ -177,54 +179,58 @@ | |||
227 | 177 | r'.*^\s+APPEND .+?$', | 179 | r'.*^\s+APPEND .+?$', |
228 | 178 | re.MULTILINE | re.DOTALL))) | 180 | re.MULTILINE | re.DOTALL))) |
229 | 179 | 181 | ||
232 | 180 | def test_render_with_extra_arguments_does_not_affect_output(self): | 182 | def test_get_reader_with_extra_arguments_does_not_affect_output(self): |
233 | 181 | # render_config() allows any keyword arguments as a safety valve. | 183 | # get_reader() allows any keyword arguments as a safety valve. |
234 | 182 | method = PXEBootMethod() | 184 | method = PXEBootMethod() |
235 | 183 | options = { | 185 | options = { |
236 | 186 | "backend": None, | ||
237 | 184 | "kernel_params": make_kernel_parameters(self, purpose="install"), | 187 | "kernel_params": make_kernel_parameters(self, purpose="install"), |
238 | 185 | } | 188 | } |
239 | 186 | # Capture the output before sprinking in some random options. | 189 | # Capture the output before sprinking in some random options. |
241 | 187 | output_before = method.render_config(**options) | 190 | output_before = method.get_reader(**options).read(10000) |
242 | 188 | # Sprinkle some magic in. | 191 | # Sprinkle some magic in. |
243 | 189 | options.update( | 192 | options.update( |
244 | 190 | (factory.make_name("name"), factory.make_name("value")) | 193 | (factory.make_name("name"), factory.make_name("value")) |
245 | 191 | for _ in range(10)) | 194 | for _ in range(10)) |
246 | 192 | # Capture the output after sprinking in some random options. | 195 | # Capture the output after sprinking in some random options. |
248 | 193 | output_after = method.render_config(**options) | 196 | output_after = method.get_reader(**options).read(10000) |
249 | 194 | # The generated template is the same. | 197 | # The generated template is the same. |
250 | 195 | self.assertEqual(output_before, output_after) | 198 | self.assertEqual(output_before, output_after) |
251 | 196 | 199 | ||
253 | 197 | def test_render_config_with_local_purpose(self): | 200 | def test_get_reader_with_local_purpose(self): |
254 | 198 | # If purpose is "local", the config.localboot.template should be | 201 | # If purpose is "local", the config.localboot.template should be |
255 | 199 | # used. | 202 | # used. |
256 | 200 | method = PXEBootMethod() | 203 | method = PXEBootMethod() |
257 | 201 | options = { | 204 | options = { |
258 | 205 | "backend": None, | ||
259 | 202 | "kernel_params": make_kernel_parameters(purpose="local"), | 206 | "kernel_params": make_kernel_parameters(purpose="local"), |
260 | 203 | } | 207 | } |
262 | 204 | output = method.render_config(**options) | 208 | output = method.get_reader(**options).read(10000) |
263 | 205 | self.assertIn("LOCALBOOT 0", output) | 209 | self.assertIn("LOCALBOOT 0", output) |
264 | 206 | 210 | ||
266 | 207 | def test_render_config_with_local_purpose_i386_arch(self): | 211 | def test_get_reader_with_local_purpose_i386_arch(self): |
267 | 208 | # Intel i386 is a special case and needs to use the chain.c32 | 212 | # Intel i386 is a special case and needs to use the chain.c32 |
268 | 209 | # loader as the LOCALBOOT PXE directive is unreliable. | 213 | # loader as the LOCALBOOT PXE directive is unreliable. |
269 | 210 | method = PXEBootMethod() | 214 | method = PXEBootMethod() |
270 | 211 | options = { | 215 | options = { |
271 | 216 | "backend": None, | ||
272 | 212 | "kernel_params": make_kernel_parameters( | 217 | "kernel_params": make_kernel_parameters( |
273 | 213 | arch="i386", purpose="local"), | 218 | arch="i386", purpose="local"), |
274 | 214 | } | 219 | } |
276 | 215 | output = method.render_config(**options) | 220 | output = method.get_reader(**options).read(10000) |
277 | 216 | self.assertIn("chain.c32", output) | 221 | self.assertIn("chain.c32", output) |
278 | 217 | self.assertNotIn("LOCALBOOT", output) | 222 | self.assertNotIn("LOCALBOOT", output) |
279 | 218 | 223 | ||
281 | 219 | def test_render_config_with_local_purpose_amd64_arch(self): | 224 | def test_get_reader_with_local_purpose_amd64_arch(self): |
282 | 220 | # Intel amd64 is a special case and needs to use the chain.c32 | 225 | # Intel amd64 is a special case and needs to use the chain.c32 |
283 | 221 | # loader as the LOCALBOOT PXE directive is unreliable. | 226 | # loader as the LOCALBOOT PXE directive is unreliable. |
284 | 222 | method = PXEBootMethod() | 227 | method = PXEBootMethod() |
285 | 223 | options = { | 228 | options = { |
286 | 229 | "backend": None, | ||
287 | 224 | "kernel_params": make_kernel_parameters( | 230 | "kernel_params": make_kernel_parameters( |
288 | 225 | arch="amd64", purpose="local"), | 231 | arch="amd64", purpose="local"), |
289 | 226 | } | 232 | } |
291 | 227 | output = method.render_config(**options) | 233 | output = method.get_reader(**options).read(10000) |
292 | 228 | self.assertIn("chain.c32", output) | 234 | self.assertIn("chain.c32", output) |
293 | 229 | self.assertNotIn("LOCALBOOT", output) | 235 | self.assertNotIn("LOCALBOOT", output) |
294 | 230 | 236 | ||
295 | @@ -237,17 +243,18 @@ | |||
296 | 237 | ("xinstall", dict(purpose="xinstall")), | 243 | ("xinstall", dict(purpose="xinstall")), |
297 | 238 | ] | 244 | ] |
298 | 239 | 245 | ||
300 | 240 | def test_render_config_scenarios(self): | 246 | def test_get_reader_scenarios(self): |
301 | 241 | # The commissioning config uses an extra PXELINUX module to auto | 247 | # The commissioning config uses an extra PXELINUX module to auto |
302 | 242 | # select between i386 and amd64. | 248 | # select between i386 and amd64. |
303 | 243 | method = PXEBootMethod() | 249 | method = PXEBootMethod() |
304 | 244 | get_ephemeral_name = self.patch(kernel_opts, "get_ephemeral_name") | 250 | get_ephemeral_name = self.patch(kernel_opts, "get_ephemeral_name") |
305 | 245 | get_ephemeral_name.return_value = factory.make_name("ephemeral") | 251 | get_ephemeral_name.return_value = factory.make_name("ephemeral") |
306 | 246 | options = { | 252 | options = { |
307 | 253 | "backend": None, | ||
308 | 247 | "kernel_params": make_kernel_parameters( | 254 | "kernel_params": make_kernel_parameters( |
309 | 248 | testcase=self, subarch="generic", purpose=self.purpose), | 255 | testcase=self, subarch="generic", purpose=self.purpose), |
310 | 249 | } | 256 | } |
312 | 250 | output = method.render_config(**options) | 257 | output = method.get_reader(**options).read(10000) |
313 | 251 | config = parse_pxe_config(output) | 258 | config = parse_pxe_config(output) |
314 | 252 | # The default section is defined. | 259 | # The default section is defined. |
315 | 253 | default_section_label = config.header["DEFAULT"] | 260 | default_section_label = config.header["DEFAULT"] |
316 | 254 | 261 | ||
317 | === modified file 'src/provisioningserver/boot/tests/test_uefi.py' | |||
318 | --- src/provisioningserver/boot/tests/test_uefi.py 2014-05-15 17:39:27 +0000 | |||
319 | +++ src/provisioningserver/boot/tests/test_uefi.py 2014-05-28 13:35:49 +0000 | |||
320 | @@ -18,6 +18,7 @@ | |||
321 | 18 | 18 | ||
322 | 19 | from maastesting.factory import factory | 19 | from maastesting.factory import factory |
323 | 20 | from maastesting.testcase import MAASTestCase | 20 | from maastesting.testcase import MAASTestCase |
324 | 21 | from provisioningserver.boot import BytesReader | ||
325 | 21 | from provisioningserver.boot.tftppath import compose_image_path | 22 | from provisioningserver.boot.tftppath import compose_image_path |
326 | 22 | from provisioningserver.boot.uefi import ( | 23 | from provisioningserver.boot.uefi import ( |
327 | 23 | re_config_file, | 24 | re_config_file, |
328 | @@ -60,14 +61,15 @@ | |||
329 | 60 | class TestRenderUEFIConfig(MAASTestCase): | 61 | class TestRenderUEFIConfig(MAASTestCase): |
330 | 61 | """Tests for `provisioningserver.boot.uefi.UEFIBootMethod`.""" | 62 | """Tests for `provisioningserver.boot.uefi.UEFIBootMethod`.""" |
331 | 62 | 63 | ||
333 | 63 | def test_render(self): | 64 | def test_get_reader(self): |
334 | 64 | # Given the right configuration options, the UEFI configuration is | 65 | # Given the right configuration options, the UEFI configuration is |
335 | 65 | # correctly rendered. | 66 | # correctly rendered. |
336 | 66 | method = UEFIBootMethod() | 67 | method = UEFIBootMethod() |
337 | 67 | params = make_kernel_parameters(purpose="install") | 68 | params = make_kernel_parameters(purpose="install") |
341 | 68 | output = method.render_config(kernel_params=params) | 69 | output = method.get_reader(backend=None, kernel_params=params) |
342 | 69 | # The output is always a Unicode string. | 70 | # The output is a BytesReader. |
343 | 70 | self.assertThat(output, IsInstance(unicode)) | 71 | self.assertThat(output, IsInstance(BytesReader)) |
344 | 72 | output = output.read(10000) | ||
345 | 71 | # The template has rendered without error. UEFI configurations | 73 | # The template has rendered without error. UEFI configurations |
346 | 72 | # typically start with a DEFAULT line. | 74 | # typically start with a DEFAULT line. |
347 | 73 | self.assertThat(output, StartsWith("set default=\"0\"")) | 75 | self.assertThat(output, StartsWith("set default=\"0\"")) |
348 | @@ -85,32 +87,34 @@ | |||
349 | 85 | r'.*^\s+initrd %s/di-initrd$' % re.escape(image_dir), | 87 | r'.*^\s+initrd %s/di-initrd$' % re.escape(image_dir), |
350 | 86 | re.MULTILINE | re.DOTALL))) | 88 | re.MULTILINE | re.DOTALL))) |
351 | 87 | 89 | ||
354 | 88 | def test_render_with_extra_arguments_does_not_affect_output(self): | 90 | def test_get_reader_with_extra_arguments_does_not_affect_output(self): |
355 | 89 | # render_config() allows any keyword arguments as a safety valve. | 91 | # get_reader() allows any keyword arguments as a safety valve. |
356 | 90 | method = UEFIBootMethod() | 92 | method = UEFIBootMethod() |
357 | 91 | options = { | 93 | options = { |
358 | 94 | "backend": None, | ||
359 | 92 | "kernel_params": make_kernel_parameters(purpose="install"), | 95 | "kernel_params": make_kernel_parameters(purpose="install"), |
360 | 93 | } | 96 | } |
361 | 94 | # Capture the output before sprinking in some random options. | 97 | # Capture the output before sprinking in some random options. |
363 | 95 | output_before = method.render_config(**options) | 98 | output_before = method.get_reader(**options).read(10000) |
364 | 96 | # Sprinkle some magic in. | 99 | # Sprinkle some magic in. |
365 | 97 | options.update( | 100 | options.update( |
366 | 98 | (factory.make_name("name"), factory.make_name("value")) | 101 | (factory.make_name("name"), factory.make_name("value")) |
367 | 99 | for _ in range(10)) | 102 | for _ in range(10)) |
368 | 100 | # Capture the output after sprinking in some random options. | 103 | # Capture the output after sprinking in some random options. |
370 | 101 | output_after = method.render_config(**options) | 104 | output_after = method.get_reader(**options).read(10000) |
371 | 102 | # The generated template is the same. | 105 | # The generated template is the same. |
372 | 103 | self.assertEqual(output_before, output_after) | 106 | self.assertEqual(output_before, output_after) |
373 | 104 | 107 | ||
375 | 105 | def test_render_config_with_local_purpose(self): | 108 | def test_get_reader_with_local_purpose(self): |
376 | 106 | # If purpose is "local", the config.localboot.template should be | 109 | # If purpose is "local", the config.localboot.template should be |
377 | 107 | # used. | 110 | # used. |
378 | 108 | method = UEFIBootMethod() | 111 | method = UEFIBootMethod() |
379 | 109 | options = { | 112 | options = { |
380 | 113 | "backend": None, | ||
381 | 110 | "kernel_params": make_kernel_parameters( | 114 | "kernel_params": make_kernel_parameters( |
382 | 111 | purpose="local", arch="amd64"), | 115 | purpose="local", arch="amd64"), |
383 | 112 | } | 116 | } |
385 | 113 | output = method.render_config(**options) | 117 | output = method.get_reader(**options).read(10000) |
386 | 114 | self.assertIn("configfile /efi/ubuntu/grub.cfg", output) | 118 | self.assertIn("configfile /efi/ubuntu/grub.cfg", output) |
387 | 115 | 119 | ||
388 | 116 | 120 | ||
389 | 117 | 121 | ||
390 | === modified file 'src/provisioningserver/boot/uefi.py' | |||
391 | --- src/provisioningserver/boot/uefi.py 2014-03-28 04:06:27 +0000 | |||
392 | +++ src/provisioningserver/boot/uefi.py 2014-05-28 13:35:49 +0000 | |||
393 | @@ -25,6 +25,7 @@ | |||
394 | 25 | from provisioningserver.boot import ( | 25 | from provisioningserver.boot import ( |
395 | 26 | BootMethod, | 26 | BootMethod, |
396 | 27 | BootMethodInstallError, | 27 | BootMethodInstallError, |
397 | 28 | BytesReader, | ||
398 | 28 | get_parameters, | 29 | get_parameters, |
399 | 29 | utils, | 30 | utils, |
400 | 30 | ) | 31 | ) |
401 | @@ -120,10 +121,11 @@ | |||
402 | 120 | bootloader_path = "bootx64.efi" | 121 | bootloader_path = "bootx64.efi" |
403 | 121 | arch_octet = "00:07" # AMD64 EFI | 122 | arch_octet = "00:07" # AMD64 EFI |
404 | 122 | 123 | ||
406 | 123 | def match_config_path(self, path): | 124 | def match_path(self, backend, path): |
407 | 124 | """Checks path for the configuration file that needs to be | 125 | """Checks path for the configuration file that needs to be |
408 | 125 | generated. | 126 | generated. |
409 | 126 | 127 | ||
410 | 128 | :param backend: requesting backend | ||
411 | 127 | :param path: requested path | 129 | :param path: requested path |
412 | 128 | :returns: dict of match params from path, None if no match | 130 | :returns: dict of match params from path, None if no match |
413 | 129 | """ | 131 | """ |
414 | @@ -139,19 +141,20 @@ | |||
415 | 139 | 141 | ||
416 | 140 | return params | 142 | return params |
417 | 141 | 143 | ||
419 | 142 | def render_config(self, kernel_params, **extra): | 144 | def get_reader(self, backend, kernel_params, **extra): |
420 | 143 | """Render a configuration file as a unicode string. | 145 | """Render a configuration file as a unicode string. |
421 | 144 | 146 | ||
422 | 147 | :param backend: requesting backend | ||
423 | 145 | :param kernel_params: An instance of `KernelParameters`. | 148 | :param kernel_params: An instance of `KernelParameters`. |
424 | 146 | :param extra: Allow for other arguments. This is a safety valve; | 149 | :param extra: Allow for other arguments. This is a safety valve; |
425 | 147 | parameters generated in another component (for example, see | 150 | parameters generated in another component (for example, see |
427 | 148 | `TFTPBackend.get_config_reader`) won't cause this to break. | 151 | `TFTPBackend.get_boot_method_reader`) won't cause this to break. |
428 | 149 | """ | 152 | """ |
429 | 150 | template = self.get_template( | 153 | template = self.get_template( |
430 | 151 | kernel_params.purpose, kernel_params.arch, | 154 | kernel_params.purpose, kernel_params.arch, |
431 | 152 | kernel_params.subarch) | 155 | kernel_params.subarch) |
432 | 153 | namespace = self.compose_template_namespace(kernel_params) | 156 | namespace = self.compose_template_namespace(kernel_params) |
434 | 154 | return template.substitute(namespace) | 157 | return BytesReader(template.substitute(namespace).encode("utf-8")) |
435 | 155 | 158 | ||
436 | 156 | def install_bootloader(self, destination): | 159 | def install_bootloader(self, destination): |
437 | 157 | """Installs the required files for UEFI booting into the | 160 | """Installs the required files for UEFI booting into the |
438 | 158 | 161 | ||
439 | === modified file 'src/provisioningserver/tests/test_tftp.py' | |||
440 | --- src/provisioningserver/tests/test_tftp.py 2014-03-28 06:51:59 +0000 | |||
441 | +++ src/provisioningserver/tests/test_tftp.py 2014-05-28 13:35:49 +0000 | |||
442 | @@ -28,11 +28,11 @@ | |||
443 | 28 | from maastesting.testcase import MAASTestCase | 28 | from maastesting.testcase import MAASTestCase |
444 | 29 | import mock | 29 | import mock |
445 | 30 | from provisioningserver import tftp as tftp_module | 30 | from provisioningserver import tftp as tftp_module |
446 | 31 | from provisioningserver.boot import BytesReader | ||
447 | 31 | from provisioningserver.boot.pxe import PXEBootMethod | 32 | from provisioningserver.boot.pxe import PXEBootMethod |
448 | 32 | from provisioningserver.boot.tests.test_pxe import compose_config_path | 33 | from provisioningserver.boot.tests.test_pxe import compose_config_path |
449 | 33 | from provisioningserver.tests.test_kernel_opts import make_kernel_parameters | 34 | from provisioningserver.tests.test_kernel_opts import make_kernel_parameters |
450 | 34 | from provisioningserver.tftp import ( | 35 | from provisioningserver.tftp import ( |
451 | 35 | BytesReader, | ||
452 | 36 | TFTPBackend, | 36 | TFTPBackend, |
453 | 37 | TFTPService, | 37 | TFTPService, |
454 | 38 | ) | 38 | ) |
455 | @@ -127,9 +127,9 @@ | |||
456 | 127 | self.assertEqual(b"", reader.read(1)) | 127 | self.assertEqual(b"", reader.read(1)) |
457 | 128 | 128 | ||
458 | 129 | @inlineCallbacks | 129 | @inlineCallbacks |
462 | 130 | def test_get_reader_config_file(self): | 130 | def test_get_render_file(self): |
463 | 131 | # For paths matching re_config_file, TFTPBackend.get_reader() returns | 131 | # For paths matching PXEBootMethod.match_path, TFTPBackend.get_reader() |
464 | 132 | # a Deferred that will yield a BytesReader. | 132 | # returns a Deferred that will yield a BytesReader. |
465 | 133 | cluster_uuid = factory.getRandomUUID() | 133 | cluster_uuid = factory.getRandomUUID() |
466 | 134 | self.patch(tftp_module, 'get_cluster_uuid').return_value = ( | 134 | self.patch(tftp_module, 'get_cluster_uuid').return_value = ( |
467 | 135 | cluster_uuid) | 135 | cluster_uuid) |
468 | @@ -147,8 +147,8 @@ | |||
469 | 147 | factory.getRandomPort()), | 147 | factory.getRandomPort()), |
470 | 148 | } | 148 | } |
471 | 149 | 149 | ||
474 | 150 | @partial(self.patch, backend, "get_config_reader") | 150 | @partial(self.patch, backend, "get_boot_method_reader") |
475 | 151 | def get_config_reader(boot_method, params): | 151 | def get_boot_method_reader(boot_method, params): |
476 | 152 | params_json = json.dumps(params) | 152 | params_json = json.dumps(params) |
477 | 153 | params_json_reader = BytesReader(params_json) | 153 | params_json_reader = BytesReader(params_json) |
478 | 154 | return succeed(params_json_reader) | 154 | return succeed(params_json_reader) |
479 | @@ -168,9 +168,10 @@ | |||
480 | 168 | self.assertEqual(expected_params, observed_params) | 168 | self.assertEqual(expected_params, observed_params) |
481 | 169 | 169 | ||
482 | 170 | @inlineCallbacks | 170 | @inlineCallbacks |
486 | 171 | def test_get_config_reader_returns_rendered_params(self): | 171 | def test_get_boot_method_reader_returns_rendered_params(self): |
487 | 172 | # get_config_reader() takes a dict() of parameters and returns an | 172 | # get_boot_method_reader() takes a dict() of parameters and returns an |
488 | 173 | # `IReader` of a PXE configuration, rendered by `render_pxe_config`. | 173 | # `IReader` of a PXE configuration, rendered by |
489 | 174 | # `PXEBootMethod.get_reader`. | ||
490 | 174 | backend = TFTPBackend(self.make_dir(), b"http://example.com/") | 175 | backend = TFTPBackend(self.make_dir(), b"http://example.com/") |
491 | 175 | # Fake configuration parameters, as discovered from the file path. | 176 | # Fake configuration parameters, as discovered from the file path. |
492 | 176 | fake_params = {"mac": factory.getRandomMACAddress("-")} | 177 | fake_params = {"mac": factory.getRandomMACAddress("-")} |
493 | @@ -182,15 +183,15 @@ | |||
494 | 182 | get_page_patch = self.patch(backend, "get_page") | 183 | get_page_patch = self.patch(backend, "get_page") |
495 | 183 | get_page_patch.return_value = succeed(fake_get_page_result) | 184 | get_page_patch.return_value = succeed(fake_get_page_result) |
496 | 184 | 185 | ||
498 | 185 | # Stub render_config to return the render parameters. | 186 | # Stub get_reader to return the render parameters. |
499 | 186 | method = PXEBootMethod() | 187 | method = PXEBootMethod() |
503 | 187 | fake_render_result = factory.make_name("render") | 188 | fake_render_result = factory.make_name("render").encode("utf-8") |
504 | 188 | render_patch = self.patch(method, "render_config") | 189 | render_patch = self.patch(method, "get_reader") |
505 | 189 | render_patch.return_value = fake_render_result | 190 | render_patch.return_value = BytesReader(fake_render_result) |
506 | 190 | 191 | ||
507 | 191 | # Get the rendered configuration, which will actually be a JSON dump | 192 | # Get the rendered configuration, which will actually be a JSON dump |
508 | 192 | # of the render-time parameters. | 193 | # of the render-time parameters. |
510 | 193 | reader = yield backend.get_config_reader(method, fake_params) | 194 | reader = yield backend.get_boot_method_reader(method, fake_params) |
511 | 194 | self.addCleanup(reader.finish) | 195 | self.addCleanup(reader.finish) |
512 | 195 | self.assertIsInstance(reader, BytesReader) | 196 | self.assertIsInstance(reader, BytesReader) |
513 | 196 | output = reader.read(10000) | 197 | output = reader.read(10000) |
514 | @@ -198,13 +199,13 @@ | |||
515 | 198 | # The kernel parameters were fetched using `backend.get_page`. | 199 | # The kernel parameters were fetched using `backend.get_page`. |
516 | 199 | self.assertThat(backend.get_page, MockCalledOnceWith(mock.ANY)) | 200 | self.assertThat(backend.get_page, MockCalledOnceWith(mock.ANY)) |
517 | 200 | 201 | ||
519 | 201 | # The result has been rendered by `backend.render_config`. | 202 | # The result has been rendered by `method.get_reader`. |
520 | 202 | self.assertEqual(fake_render_result.encode("utf-8"), output) | 203 | self.assertEqual(fake_render_result.encode("utf-8"), output) |
523 | 203 | self.assertThat(method.render_config, MockCalledOnceWith( | 204 | self.assertThat(method.get_reader, MockCalledOnceWith( |
524 | 204 | kernel_params=fake_kernel_params, **fake_params)) | 205 | backend, kernel_params=fake_kernel_params, **fake_params)) |
525 | 205 | 206 | ||
526 | 206 | @inlineCallbacks | 207 | @inlineCallbacks |
528 | 207 | def test_get_config_reader_substitutes_armhf_in_params(self): | 208 | def test_get_boot_method_render_substitutes_armhf_in_params(self): |
529 | 208 | # get_config_reader() should substitute "arm" for "armhf" in the | 209 | # get_config_reader() should substitute "arm" for "armhf" in the |
530 | 209 | # arch field of the parameters (mapping from pxe to maas | 210 | # arch field of the parameters (mapping from pxe to maas |
531 | 210 | # namespace). | 211 | # namespace). |
532 | @@ -224,8 +225,8 @@ | |||
533 | 224 | factory.getRandomPort()), | 225 | factory.getRandomPort()), |
534 | 225 | } | 226 | } |
535 | 226 | 227 | ||
538 | 227 | @partial(self.patch, backend, "get_config_reader") | 228 | @partial(self.patch, backend, "get_boot_method_reader") |
539 | 228 | def get_config_reader(boot_method, params): | 229 | def get_boot_method_reader(boot_method, params): |
540 | 229 | params_json = json.dumps(params) | 230 | params_json = json.dumps(params) |
541 | 230 | params_json_reader = BytesReader(params_json) | 231 | params_json_reader = BytesReader(params_json) |
542 | 231 | return succeed(params_json_reader) | 232 | return succeed(params_json_reader) |
543 | 232 | 233 | ||
544 | === modified file 'src/provisioningserver/tftp.py' | |||
545 | --- src/provisioningserver/tftp.py 2014-03-28 15:17:34 +0000 | |||
546 | +++ src/provisioningserver/tftp.py 2014-05-28 13:35:49 +0000 | |||
547 | @@ -18,7 +18,6 @@ | |||
548 | 18 | ] | 18 | ] |
549 | 19 | 19 | ||
550 | 20 | import httplib | 20 | import httplib |
551 | 21 | from io import BytesIO | ||
552 | 22 | import json | 21 | import json |
553 | 23 | from urllib import urlencode | 22 | from urllib import urlencode |
554 | 24 | from urlparse import ( | 23 | from urlparse import ( |
555 | @@ -34,10 +33,7 @@ | |||
556 | 34 | deferred, | 33 | deferred, |
557 | 35 | get_all_interface_addresses, | 34 | get_all_interface_addresses, |
558 | 36 | ) | 35 | ) |
563 | 37 | from tftp.backend import ( | 36 | from tftp.backend import FilesystemSynchronousBackend |
560 | 38 | FilesystemSynchronousBackend, | ||
561 | 39 | IReader, | ||
562 | 40 | ) | ||
564 | 41 | from tftp.errors import FileNotFound | 37 | from tftp.errors import FileNotFound |
565 | 42 | from tftp.protocol import TFTP | 38 | from tftp.protocol import TFTP |
566 | 43 | from twisted.application import internet | 39 | from twisted.application import internet |
567 | @@ -45,22 +41,6 @@ | |||
568 | 45 | from twisted.python.context import get | 41 | from twisted.python.context import get |
569 | 46 | from twisted.web.client import getPage | 42 | from twisted.web.client import getPage |
570 | 47 | import twisted.web.error | 43 | import twisted.web.error |
571 | 48 | from zope.interface import implementer | ||
572 | 49 | |||
573 | 50 | |||
574 | 51 | @implementer(IReader) | ||
575 | 52 | class BytesReader: | ||
576 | 53 | |||
577 | 54 | def __init__(self, data): | ||
578 | 55 | super(BytesReader, self).__init__() | ||
579 | 56 | self.buffer = BytesIO(data) | ||
580 | 57 | self.size = len(data) | ||
581 | 58 | |||
582 | 59 | def read(self, size): | ||
583 | 60 | return self.buffer.read(size) | ||
584 | 61 | |||
585 | 62 | def finish(self): | ||
586 | 63 | self.buffer.close() | ||
587 | 64 | 44 | ||
588 | 65 | 45 | ||
589 | 66 | class TFTPBackend(FilesystemSynchronousBackend): | 46 | class TFTPBackend(FilesystemSynchronousBackend): |
590 | @@ -118,7 +98,7 @@ | |||
591 | 118 | def get_boot_method(self, file_name): | 98 | def get_boot_method(self, file_name): |
592 | 119 | """Finds the correct boot method.""" | 99 | """Finds the correct boot method.""" |
593 | 120 | for _, method in BootMethodRegistry: | 100 | for _, method in BootMethodRegistry: |
595 | 121 | params = method.match_config_path(file_name) | 101 | params = method.match_path(self, file_name) |
596 | 122 | if params is not None: | 102 | if params is not None: |
597 | 123 | return method, params | 103 | return method, params |
598 | 124 | return None, None | 104 | return None, None |
599 | @@ -142,21 +122,19 @@ | |||
600 | 142 | return d | 122 | return d |
601 | 143 | 123 | ||
602 | 144 | @deferred | 124 | @deferred |
604 | 145 | def get_config_reader(self, boot_method, params): | 125 | def get_boot_method_reader(self, boot_method, params): |
605 | 146 | """Return an `IReader` for a boot method. | 126 | """Return an `IReader` for a boot method. |
606 | 147 | 127 | ||
607 | 148 | :param boot_method: Boot method that is generating the config | 128 | :param boot_method: Boot method that is generating the config |
608 | 149 | :param params: Parameters so far obtained, typically from the file | 129 | :param params: Parameters so far obtained, typically from the file |
609 | 150 | path requested. | 130 | path requested. |
610 | 151 | """ | 131 | """ |
615 | 152 | def generate_config(kernel_params): | 132 | def generate(kernel_params): |
616 | 153 | config = boot_method.render_config( | 133 | return boot_method.get_reader( |
617 | 154 | kernel_params=kernel_params, **params) | 134 | self, kernel_params=kernel_params, **params) |
614 | 155 | return config.encode("utf-8") | ||
618 | 156 | 135 | ||
619 | 157 | d = self.get_kernel_params(params) | 136 | d = self.get_kernel_params(params) |
622 | 158 | d.addCallback(generate_config) | 137 | d.addCallback(generate) |
621 | 159 | d.addCallback(BytesReader) | ||
623 | 160 | return d | 138 | return d |
624 | 161 | 139 | ||
625 | 162 | @staticmethod | 140 | @staticmethod |
626 | @@ -203,7 +181,7 @@ | |||
627 | 203 | remote_host, remote_port = get("remote", (None, None)) | 181 | remote_host, remote_port = get("remote", (None, None)) |
628 | 204 | params["remote"] = remote_host | 182 | params["remote"] = remote_host |
629 | 205 | params["cluster_uuid"] = get_cluster_uuid() | 183 | params["cluster_uuid"] = get_cluster_uuid() |
631 | 206 | d = self.get_config_reader(boot_method, params) | 184 | d = self.get_boot_method_reader(boot_method, params) |
632 | 207 | d.addErrback(self.get_page_errback, file_name) | 185 | d.addErrback(self.get_page_errback, file_name) |
633 | 208 | return d | 186 | return d |
634 | 209 | 187 |