Merge lp:~blake-rouse/maas/tftp-backend-readers into lp:~maas-committers/maas/trunk
- tftp-backend-readers
- Merge into trunk
Proposed by
Blake Rouse
Status: | Merged |
---|---|
Approved by: | Blake Rouse |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2378 |
Proposed branch: | lp:~blake-rouse/maas/tftp-backend-readers |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
635 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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | Approve | ||
Review via email: mp+221138@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
BootMethods now return an IReader to the tftp backend, instead of a string. This allows the boot methods, to respond with configuration files and binary files. This change is will be used by the WindowsBootMethod and the PowerNVBootMethod.
Re-factored the BootMethod class method's to reflect their new usage.
To post a comment you must log in.
Revision history for this message
Blake Rouse (blake-rouse) wrote : | # |
Yes, a later branch will use backend.
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 14:12:26 +0000 | |||
3 | +++ src/provisioningserver/boot/__init__.py 2014-05-27 19:49:46 +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 14:12:26 +0000 | |||
70 | +++ src/provisioningserver/boot/powerkvm.py 2014-05-27 19:49:46 +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-27 19:49:46 +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-27 19:49:46 +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 14:12:26 +0000 | |||
171 | +++ src/provisioningserver/boot/tests/test_powerkvm.py 2014-05-27 19:49:46 +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-04-24 14:09:58 +0000 | |||
197 | +++ src/provisioningserver/boot/tests/test_pxe.py 2014-05-27 19:49:46 +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,7 +243,7 @@ | |||
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 | @@ -245,11 +251,12 @@ | |||
305 | 245 | get_ephemeral_name.return_value = factory.make_name("ephemeral") | 251 | get_ephemeral_name.return_value = factory.make_name("ephemeral") |
306 | 246 | osystem = factory.make_name('osystem') | 252 | osystem = factory.make_name('osystem') |
307 | 247 | options = { | 253 | options = { |
308 | 254 | "backend": None, | ||
309 | 248 | "kernel_params": make_kernel_parameters( | 255 | "kernel_params": make_kernel_parameters( |
310 | 249 | testcase=self, osystem=osystem, subarch="generic", | 256 | testcase=self, osystem=osystem, subarch="generic", |
311 | 250 | purpose=self.purpose), | 257 | purpose=self.purpose), |
312 | 251 | } | 258 | } |
314 | 252 | output = method.render_config(**options) | 259 | output = method.get_reader(**options).read(10000) |
315 | 253 | config = parse_pxe_config(output) | 260 | config = parse_pxe_config(output) |
316 | 254 | # The default section is defined. | 261 | # The default section is defined. |
317 | 255 | default_section_label = config.header["DEFAULT"] | 262 | default_section_label = config.header["DEFAULT"] |
318 | 256 | 263 | ||
319 | === modified file 'src/provisioningserver/boot/tests/test_uefi.py' | |||
320 | --- src/provisioningserver/boot/tests/test_uefi.py 2014-05-15 17:27:53 +0000 | |||
321 | +++ src/provisioningserver/boot/tests/test_uefi.py 2014-05-27 19:49:46 +0000 | |||
322 | @@ -18,6 +18,7 @@ | |||
323 | 18 | 18 | ||
324 | 19 | from maastesting.factory import factory | 19 | from maastesting.factory import factory |
325 | 20 | from maastesting.testcase import MAASTestCase | 20 | from maastesting.testcase import MAASTestCase |
326 | 21 | from provisioningserver.boot import BytesReader | ||
327 | 21 | from provisioningserver.boot.tftppath import compose_image_path | 22 | from provisioningserver.boot.tftppath import compose_image_path |
328 | 22 | from provisioningserver.boot.uefi import ( | 23 | from provisioningserver.boot.uefi import ( |
329 | 23 | re_config_file, | 24 | re_config_file, |
330 | @@ -60,14 +61,15 @@ | |||
331 | 60 | class TestRenderUEFIConfig(MAASTestCase): | 61 | class TestRenderUEFIConfig(MAASTestCase): |
332 | 61 | """Tests for `provisioningserver.boot.uefi.UEFIBootMethod`.""" | 62 | """Tests for `provisioningserver.boot.uefi.UEFIBootMethod`.""" |
333 | 62 | 63 | ||
335 | 63 | def test_render(self): | 64 | def test_get_reader(self): |
336 | 64 | # Given the right configuration options, the UEFI configuration is | 65 | # Given the right configuration options, the UEFI configuration is |
337 | 65 | # correctly rendered. | 66 | # correctly rendered. |
338 | 66 | method = UEFIBootMethod() | 67 | method = UEFIBootMethod() |
339 | 67 | params = make_kernel_parameters(purpose="install") | 68 | params = make_kernel_parameters(purpose="install") |
343 | 68 | output = method.render_config(kernel_params=params) | 69 | output = method.get_reader(backend=None, kernel_params=params) |
344 | 69 | # The output is always a Unicode string. | 70 | # The output is a BytesReader. |
345 | 70 | self.assertThat(output, IsInstance(unicode)) | 71 | self.assertThat(output, IsInstance(BytesReader)) |
346 | 72 | output = output.read(10000) | ||
347 | 71 | # The template has rendered without error. UEFI configurations | 73 | # The template has rendered without error. UEFI configurations |
348 | 72 | # typically start with a DEFAULT line. | 74 | # typically start with a DEFAULT line. |
349 | 73 | self.assertThat(output, StartsWith("set default=\"0\"")) | 75 | self.assertThat(output, StartsWith("set default=\"0\"")) |
350 | @@ -85,32 +87,34 @@ | |||
351 | 85 | r'.*^\s+initrd %s/di-initrd$' % re.escape(image_dir), | 87 | r'.*^\s+initrd %s/di-initrd$' % re.escape(image_dir), |
352 | 86 | re.MULTILINE | re.DOTALL))) | 88 | re.MULTILINE | re.DOTALL))) |
353 | 87 | 89 | ||
356 | 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): |
357 | 89 | # render_config() allows any keyword arguments as a safety valve. | 91 | # get_reader() allows any keyword arguments as a safety valve. |
358 | 90 | method = UEFIBootMethod() | 92 | method = UEFIBootMethod() |
359 | 91 | options = { | 93 | options = { |
360 | 94 | "backend": None, | ||
361 | 92 | "kernel_params": make_kernel_parameters(purpose="install"), | 95 | "kernel_params": make_kernel_parameters(purpose="install"), |
362 | 93 | } | 96 | } |
363 | 94 | # Capture the output before sprinking in some random options. | 97 | # Capture the output before sprinking in some random options. |
365 | 95 | output_before = method.render_config(**options) | 98 | output_before = method.get_reader(**options).read(10000) |
366 | 96 | # Sprinkle some magic in. | 99 | # Sprinkle some magic in. |
367 | 97 | options.update( | 100 | options.update( |
368 | 98 | (factory.make_name("name"), factory.make_name("value")) | 101 | (factory.make_name("name"), factory.make_name("value")) |
369 | 99 | for _ in range(10)) | 102 | for _ in range(10)) |
370 | 100 | # Capture the output after sprinking in some random options. | 103 | # Capture the output after sprinking in some random options. |
372 | 101 | output_after = method.render_config(**options) | 104 | output_after = method.get_reader(**options).read(10000) |
373 | 102 | # The generated template is the same. | 105 | # The generated template is the same. |
374 | 103 | self.assertEqual(output_before, output_after) | 106 | self.assertEqual(output_before, output_after) |
375 | 104 | 107 | ||
377 | 105 | def test_render_config_with_local_purpose(self): | 108 | def test_get_reader_with_local_purpose(self): |
378 | 106 | # If purpose is "local", the config.localboot.template should be | 109 | # If purpose is "local", the config.localboot.template should be |
379 | 107 | # used. | 110 | # used. |
380 | 108 | method = UEFIBootMethod() | 111 | method = UEFIBootMethod() |
381 | 109 | options = { | 112 | options = { |
382 | 113 | "backend": None, | ||
383 | 110 | "kernel_params": make_kernel_parameters( | 114 | "kernel_params": make_kernel_parameters( |
384 | 111 | purpose="local", arch="amd64"), | 115 | purpose="local", arch="amd64"), |
385 | 112 | } | 116 | } |
387 | 113 | output = method.render_config(**options) | 117 | output = method.get_reader(**options).read(10000) |
388 | 114 | self.assertIn("configfile /efi/ubuntu/grub.cfg", output) | 118 | self.assertIn("configfile /efi/ubuntu/grub.cfg", output) |
389 | 115 | 119 | ||
390 | 116 | 120 | ||
391 | 117 | 121 | ||
392 | === modified file 'src/provisioningserver/boot/uefi.py' | |||
393 | --- src/provisioningserver/boot/uefi.py 2014-03-28 04:06:27 +0000 | |||
394 | +++ src/provisioningserver/boot/uefi.py 2014-05-27 19:49:46 +0000 | |||
395 | @@ -25,6 +25,7 @@ | |||
396 | 25 | from provisioningserver.boot import ( | 25 | from provisioningserver.boot import ( |
397 | 26 | BootMethod, | 26 | BootMethod, |
398 | 27 | BootMethodInstallError, | 27 | BootMethodInstallError, |
399 | 28 | BytesReader, | ||
400 | 28 | get_parameters, | 29 | get_parameters, |
401 | 29 | utils, | 30 | utils, |
402 | 30 | ) | 31 | ) |
403 | @@ -120,10 +121,11 @@ | |||
404 | 120 | bootloader_path = "bootx64.efi" | 121 | bootloader_path = "bootx64.efi" |
405 | 121 | arch_octet = "00:07" # AMD64 EFI | 122 | arch_octet = "00:07" # AMD64 EFI |
406 | 122 | 123 | ||
408 | 123 | def match_config_path(self, path): | 124 | def match_path(self, backend, path): |
409 | 124 | """Checks path for the configuration file that needs to be | 125 | """Checks path for the configuration file that needs to be |
410 | 125 | generated. | 126 | generated. |
411 | 126 | 127 | ||
412 | 128 | :param backend: requesting backend | ||
413 | 127 | :param path: requested path | 129 | :param path: requested path |
414 | 128 | :returns: dict of match params from path, None if no match | 130 | :returns: dict of match params from path, None if no match |
415 | 129 | """ | 131 | """ |
416 | @@ -139,19 +141,20 @@ | |||
417 | 139 | 141 | ||
418 | 140 | return params | 142 | return params |
419 | 141 | 143 | ||
421 | 142 | def render_config(self, kernel_params, **extra): | 144 | def get_reader(self, backend, kernel_params, **extra): |
422 | 143 | """Render a configuration file as a unicode string. | 145 | """Render a configuration file as a unicode string. |
423 | 144 | 146 | ||
424 | 147 | :param backend: requesting backend | ||
425 | 145 | :param kernel_params: An instance of `KernelParameters`. | 148 | :param kernel_params: An instance of `KernelParameters`. |
426 | 146 | :param extra: Allow for other arguments. This is a safety valve; | 149 | :param extra: Allow for other arguments. This is a safety valve; |
427 | 147 | parameters generated in another component (for example, see | 150 | parameters generated in another component (for example, see |
429 | 148 | `TFTPBackend.get_config_reader`) won't cause this to break. | 151 | `TFTPBackend.get_boot_method_reader`) won't cause this to break. |
430 | 149 | """ | 152 | """ |
431 | 150 | template = self.get_template( | 153 | template = self.get_template( |
432 | 151 | kernel_params.purpose, kernel_params.arch, | 154 | kernel_params.purpose, kernel_params.arch, |
433 | 152 | kernel_params.subarch) | 155 | kernel_params.subarch) |
434 | 153 | namespace = self.compose_template_namespace(kernel_params) | 156 | namespace = self.compose_template_namespace(kernel_params) |
436 | 154 | return template.substitute(namespace) | 157 | return BytesReader(template.substitute(namespace).encode("utf-8")) |
437 | 155 | 158 | ||
438 | 156 | def install_bootloader(self, destination): | 159 | def install_bootloader(self, destination): |
439 | 157 | """Installs the required files for UEFI booting into the | 160 | """Installs the required files for UEFI booting into the |
440 | 158 | 161 | ||
441 | === modified file 'src/provisioningserver/tests/test_tftp.py' | |||
442 | --- src/provisioningserver/tests/test_tftp.py 2014-03-28 06:51:59 +0000 | |||
443 | +++ src/provisioningserver/tests/test_tftp.py 2014-05-27 19:49:46 +0000 | |||
444 | @@ -28,11 +28,11 @@ | |||
445 | 28 | from maastesting.testcase import MAASTestCase | 28 | from maastesting.testcase import MAASTestCase |
446 | 29 | import mock | 29 | import mock |
447 | 30 | from provisioningserver import tftp as tftp_module | 30 | from provisioningserver import tftp as tftp_module |
448 | 31 | from provisioningserver.boot import BytesReader | ||
449 | 31 | from provisioningserver.boot.pxe import PXEBootMethod | 32 | from provisioningserver.boot.pxe import PXEBootMethod |
450 | 32 | from provisioningserver.boot.tests.test_pxe import compose_config_path | 33 | from provisioningserver.boot.tests.test_pxe import compose_config_path |
451 | 33 | from provisioningserver.tests.test_kernel_opts import make_kernel_parameters | 34 | from provisioningserver.tests.test_kernel_opts import make_kernel_parameters |
452 | 34 | from provisioningserver.tftp import ( | 35 | from provisioningserver.tftp import ( |
453 | 35 | BytesReader, | ||
454 | 36 | TFTPBackend, | 36 | TFTPBackend, |
455 | 37 | TFTPService, | 37 | TFTPService, |
456 | 38 | ) | 38 | ) |
457 | @@ -127,9 +127,9 @@ | |||
458 | 127 | self.assertEqual(b"", reader.read(1)) | 127 | self.assertEqual(b"", reader.read(1)) |
459 | 128 | 128 | ||
460 | 129 | @inlineCallbacks | 129 | @inlineCallbacks |
464 | 130 | def test_get_reader_config_file(self): | 130 | def test_get_render_file(self): |
465 | 131 | # For paths matching re_config_file, TFTPBackend.get_reader() returns | 131 | # For paths matching PXEBootMethod.match_path, TFTPBackend.get_reader() |
466 | 132 | # a Deferred that will yield a BytesReader. | 132 | # returns a Deferred that will yield a BytesReader. |
467 | 133 | cluster_uuid = factory.getRandomUUID() | 133 | cluster_uuid = factory.getRandomUUID() |
468 | 134 | self.patch(tftp_module, 'get_cluster_uuid').return_value = ( | 134 | self.patch(tftp_module, 'get_cluster_uuid').return_value = ( |
469 | 135 | cluster_uuid) | 135 | cluster_uuid) |
470 | @@ -147,8 +147,8 @@ | |||
471 | 147 | factory.getRandomPort()), | 147 | factory.getRandomPort()), |
472 | 148 | } | 148 | } |
473 | 149 | 149 | ||
476 | 150 | @partial(self.patch, backend, "get_config_reader") | 150 | @partial(self.patch, backend, "get_boot_method_reader") |
477 | 151 | def get_config_reader(boot_method, params): | 151 | def get_boot_method_reader(boot_method, params): |
478 | 152 | params_json = json.dumps(params) | 152 | params_json = json.dumps(params) |
479 | 153 | params_json_reader = BytesReader(params_json) | 153 | params_json_reader = BytesReader(params_json) |
480 | 154 | return succeed(params_json_reader) | 154 | return succeed(params_json_reader) |
481 | @@ -168,9 +168,10 @@ | |||
482 | 168 | self.assertEqual(expected_params, observed_params) | 168 | self.assertEqual(expected_params, observed_params) |
483 | 169 | 169 | ||
484 | 170 | @inlineCallbacks | 170 | @inlineCallbacks |
488 | 171 | def test_get_config_reader_returns_rendered_params(self): | 171 | def test_get_boot_method_reader_returns_rendered_params(self): |
489 | 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 |
490 | 173 | # `IReader` of a PXE configuration, rendered by `render_pxe_config`. | 173 | # `IReader` of a PXE configuration, rendered by |
491 | 174 | # `PXEBootMethod.get_reader`. | ||
492 | 174 | backend = TFTPBackend(self.make_dir(), b"http://example.com/") | 175 | backend = TFTPBackend(self.make_dir(), b"http://example.com/") |
493 | 175 | # Fake configuration parameters, as discovered from the file path. | 176 | # Fake configuration parameters, as discovered from the file path. |
494 | 176 | fake_params = {"mac": factory.getRandomMACAddress("-")} | 177 | fake_params = {"mac": factory.getRandomMACAddress("-")} |
495 | @@ -182,15 +183,15 @@ | |||
496 | 182 | get_page_patch = self.patch(backend, "get_page") | 183 | get_page_patch = self.patch(backend, "get_page") |
497 | 183 | get_page_patch.return_value = succeed(fake_get_page_result) | 184 | get_page_patch.return_value = succeed(fake_get_page_result) |
498 | 184 | 185 | ||
500 | 185 | # Stub render_config to return the render parameters. | 186 | # Stub get_reader to return the render parameters. |
501 | 186 | method = PXEBootMethod() | 187 | method = PXEBootMethod() |
505 | 187 | fake_render_result = factory.make_name("render") | 188 | fake_render_result = factory.make_name("render").encode("utf-8") |
506 | 188 | render_patch = self.patch(method, "render_config") | 189 | render_patch = self.patch(method, "get_reader") |
507 | 189 | render_patch.return_value = fake_render_result | 190 | render_patch.return_value = BytesReader(fake_render_result) |
508 | 190 | 191 | ||
509 | 191 | # Get the rendered configuration, which will actually be a JSON dump | 192 | # Get the rendered configuration, which will actually be a JSON dump |
510 | 192 | # of the render-time parameters. | 193 | # of the render-time parameters. |
512 | 193 | reader = yield backend.get_config_reader(method, fake_params) | 194 | reader = yield backend.get_boot_method_reader(method, fake_params) |
513 | 194 | self.addCleanup(reader.finish) | 195 | self.addCleanup(reader.finish) |
514 | 195 | self.assertIsInstance(reader, BytesReader) | 196 | self.assertIsInstance(reader, BytesReader) |
515 | 196 | output = reader.read(10000) | 197 | output = reader.read(10000) |
516 | @@ -198,13 +199,13 @@ | |||
517 | 198 | # The kernel parameters were fetched using `backend.get_page`. | 199 | # The kernel parameters were fetched using `backend.get_page`. |
518 | 199 | self.assertThat(backend.get_page, MockCalledOnceWith(mock.ANY)) | 200 | self.assertThat(backend.get_page, MockCalledOnceWith(mock.ANY)) |
519 | 200 | 201 | ||
521 | 201 | # The result has been rendered by `backend.render_config`. | 202 | # The result has been rendered by `method.get_reader`. |
522 | 202 | self.assertEqual(fake_render_result.encode("utf-8"), output) | 203 | self.assertEqual(fake_render_result.encode("utf-8"), output) |
525 | 203 | self.assertThat(method.render_config, MockCalledOnceWith( | 204 | self.assertThat(method.get_reader, MockCalledOnceWith( |
526 | 204 | kernel_params=fake_kernel_params, **fake_params)) | 205 | backend, kernel_params=fake_kernel_params, **fake_params)) |
527 | 205 | 206 | ||
528 | 206 | @inlineCallbacks | 207 | @inlineCallbacks |
530 | 207 | def test_get_config_reader_substitutes_armhf_in_params(self): | 208 | def test_get_boot_method_render_substitutes_armhf_in_params(self): |
531 | 208 | # get_config_reader() should substitute "arm" for "armhf" in the | 209 | # get_config_reader() should substitute "arm" for "armhf" in the |
532 | 209 | # arch field of the parameters (mapping from pxe to maas | 210 | # arch field of the parameters (mapping from pxe to maas |
533 | 210 | # namespace). | 211 | # namespace). |
534 | @@ -224,8 +225,8 @@ | |||
535 | 224 | factory.getRandomPort()), | 225 | factory.getRandomPort()), |
536 | 225 | } | 226 | } |
537 | 226 | 227 | ||
540 | 227 | @partial(self.patch, backend, "get_config_reader") | 228 | @partial(self.patch, backend, "get_boot_method_reader") |
541 | 228 | def get_config_reader(boot_method, params): | 229 | def get_boot_method_reader(boot_method, params): |
542 | 229 | params_json = json.dumps(params) | 230 | params_json = json.dumps(params) |
543 | 230 | params_json_reader = BytesReader(params_json) | 231 | params_json_reader = BytesReader(params_json) |
544 | 231 | return succeed(params_json_reader) | 232 | return succeed(params_json_reader) |
545 | 232 | 233 | ||
546 | === modified file 'src/provisioningserver/tftp.py' | |||
547 | --- src/provisioningserver/tftp.py 2014-03-28 15:17:34 +0000 | |||
548 | +++ src/provisioningserver/tftp.py 2014-05-27 19:49:46 +0000 | |||
549 | @@ -18,7 +18,6 @@ | |||
550 | 18 | ] | 18 | ] |
551 | 19 | 19 | ||
552 | 20 | import httplib | 20 | import httplib |
553 | 21 | from io import BytesIO | ||
554 | 22 | import json | 21 | import json |
555 | 23 | from urllib import urlencode | 22 | from urllib import urlencode |
556 | 24 | from urlparse import ( | 23 | from urlparse import ( |
557 | @@ -34,10 +33,7 @@ | |||
558 | 34 | deferred, | 33 | deferred, |
559 | 35 | get_all_interface_addresses, | 34 | get_all_interface_addresses, |
560 | 36 | ) | 35 | ) |
565 | 37 | from tftp.backend import ( | 36 | from tftp.backend import FilesystemSynchronousBackend |
562 | 38 | FilesystemSynchronousBackend, | ||
563 | 39 | IReader, | ||
564 | 40 | ) | ||
566 | 41 | from tftp.errors import FileNotFound | 37 | from tftp.errors import FileNotFound |
567 | 42 | from tftp.protocol import TFTP | 38 | from tftp.protocol import TFTP |
568 | 43 | from twisted.application import internet | 39 | from twisted.application import internet |
569 | @@ -45,22 +41,6 @@ | |||
570 | 45 | from twisted.python.context import get | 41 | from twisted.python.context import get |
571 | 46 | from twisted.web.client import getPage | 42 | from twisted.web.client import getPage |
572 | 47 | import twisted.web.error | 43 | import twisted.web.error |
573 | 48 | from zope.interface import implementer | ||
574 | 49 | |||
575 | 50 | |||
576 | 51 | @implementer(IReader) | ||
577 | 52 | class BytesReader: | ||
578 | 53 | |||
579 | 54 | def __init__(self, data): | ||
580 | 55 | super(BytesReader, self).__init__() | ||
581 | 56 | self.buffer = BytesIO(data) | ||
582 | 57 | self.size = len(data) | ||
583 | 58 | |||
584 | 59 | def read(self, size): | ||
585 | 60 | return self.buffer.read(size) | ||
586 | 61 | |||
587 | 62 | def finish(self): | ||
588 | 63 | self.buffer.close() | ||
589 | 64 | 44 | ||
590 | 65 | 45 | ||
591 | 66 | class TFTPBackend(FilesystemSynchronousBackend): | 46 | class TFTPBackend(FilesystemSynchronousBackend): |
592 | @@ -118,7 +98,7 @@ | |||
593 | 118 | def get_boot_method(self, file_name): | 98 | def get_boot_method(self, file_name): |
594 | 119 | """Finds the correct boot method.""" | 99 | """Finds the correct boot method.""" |
595 | 120 | for _, method in BootMethodRegistry: | 100 | for _, method in BootMethodRegistry: |
597 | 121 | params = method.match_config_path(file_name) | 101 | params = method.match_path(self, file_name) |
598 | 122 | if params is not None: | 102 | if params is not None: |
599 | 123 | return method, params | 103 | return method, params |
600 | 124 | return None, None | 104 | return None, None |
601 | @@ -142,21 +122,19 @@ | |||
602 | 142 | return d | 122 | return d |
603 | 143 | 123 | ||
604 | 144 | @deferred | 124 | @deferred |
606 | 145 | def get_config_reader(self, boot_method, params): | 125 | def get_boot_method_reader(self, boot_method, params): |
607 | 146 | """Return an `IReader` for a boot method. | 126 | """Return an `IReader` for a boot method. |
608 | 147 | 127 | ||
609 | 148 | :param boot_method: Boot method that is generating the config | 128 | :param boot_method: Boot method that is generating the config |
610 | 149 | :param params: Parameters so far obtained, typically from the file | 129 | :param params: Parameters so far obtained, typically from the file |
611 | 150 | path requested. | 130 | path requested. |
612 | 151 | """ | 131 | """ |
617 | 152 | def generate_config(kernel_params): | 132 | def generate(kernel_params): |
618 | 153 | config = boot_method.render_config( | 133 | return boot_method.get_reader( |
619 | 154 | kernel_params=kernel_params, **params) | 134 | self, kernel_params=kernel_params, **params) |
616 | 155 | return config.encode("utf-8") | ||
620 | 156 | 135 | ||
621 | 157 | d = self.get_kernel_params(params) | 136 | d = self.get_kernel_params(params) |
624 | 158 | d.addCallback(generate_config) | 137 | d.addCallback(generate) |
623 | 159 | d.addCallback(BytesReader) | ||
625 | 160 | return d | 138 | return d |
626 | 161 | 139 | ||
627 | 162 | @staticmethod | 140 | @staticmethod |
628 | @@ -203,7 +181,7 @@ | |||
629 | 203 | remote_host, remote_port = get("remote", (None, None)) | 181 | remote_host, remote_port = get("remote", (None, None)) |
630 | 204 | params["remote"] = remote_host | 182 | params["remote"] = remote_host |
631 | 205 | params["cluster_uuid"] = get_cluster_uuid() | 183 | params["cluster_uuid"] = get_cluster_uuid() |
633 | 206 | d = self.get_config_reader(boot_method, params) | 184 | d = self.get_boot_method_reader(boot_method, params) |
634 | 207 | d.addErrback(self.get_page_errback, file_name) | 185 | d.addErrback(self.get_page_errback, file_name) |
635 | 208 | return d | 186 | return d |
636 | 209 | 187 |
Looks good.