Merge lp:~ltrager/maas/rootfs_over_http into lp:maas/trunk

Proposed by Lee Trager on 2017-06-14
Status: Merged
Approved by: Blake Rouse on 2017-06-22
Approved revision: 6093
Merged at revision: 6106
Proposed branch: lp:~ltrager/maas/rootfs_over_http
Merge into: lp:maas/trunk
Diff against target: 479 lines (+98/-180)
6 files modified
src/maasserver/forms/settings.py (+9/-1)
src/maasserver/models/config.py (+3/-2)
src/maasserver/rpc/boot.py (+1/-0)
src/provisioningserver/kernel_opts.py (+44/-67)
src/provisioningserver/rpc/region.py (+1/-0)
src/provisioningserver/tests/test_kernel_opts.py (+40/-110)
To merge this branch: bzr merge lp:~ltrager/maas/rootfs_over_http
Reviewer Review Type Date Requested Status
Blake Rouse Approve on 2017-06-22
Andres Rodriguez Needs Fixing on 2017-06-21
Mike Pontillo 2017-06-14 Approve on 2017-06-21
Review via email: mp+325655@code.launchpad.net

Commit message

When booting the ephemeral environment, fetch the rootfs over HTTP instead of using iSCSI.

To post a comment you must log in.
Mike Pontillo (mpontillo) wrote :

This looks like a nice improvement! A few questions below (mainly around IPv6 support; please add specific tests for that, too).

Somewhat related question: do we ever present a hostname in the fs_host, or is it always an IP address? (I assume it's just an IP address, and I think that's okay.)

We should also make sure we don't present an IPv6 link-local address as the fs_host (if we aren't doing that already).

review: Needs Information
Lee Trager (ltrager) wrote :

Thanks for the review. fs_host was being used by the ISCSI code so I assume it properly works with IPv6. I agree we need to do some IPv6 testing with this but I don't have access to an IPv6 enabled environment right now.

Mike Pontillo (mpontillo) wrote :

Thanks for the replies. In my view, only one thing needs to be done for this to land: add a separate IPv6 test case and ensure that IPv6 addresses have brackets surrounding them, while IPv4 addresses do not.

review: Needs Fixing
lp:~ltrager/maas/rootfs_over_http updated on 2017-06-21
6087. By Lee Trager on 2017-06-16

Merge trunk

6088. By Lee Trager on 2017-06-20

Merge trunk

6089. By Lee Trager on 2017-06-21

Make sure IPv6 address is properly encapsulated when given as a boot argument

Lee Trager (ltrager) wrote :

Thanks for the review. I've updated the branch to ensure that [] are wrapped around IPv6 addresses.

Mike Pontillo (mpontillo) wrote :

Thanks for the fixes. Ship it!

review: Approve
Andres Rodriguez (andreserl) wrote :

To follow on the discussion:

This needs to have a feature flag.

review: Needs Fixing
lp:~ltrager/maas/rootfs_over_http updated on 2017-06-21
6090. By Lee Trager on 2017-06-21

Add feature flag for using iSCSI or not

6091. By Lee Trager on 2017-06-21

s/use_iscsi/http_boot/g

6092. By Lee Trager on 2017-06-21

Fix lint

6093. By Lee Trager on 2017-06-21

Merge trunk

Lee Trager (ltrager) wrote :

Booting over iSCSI is now the default. To enable booting over HTTP use the MAAS configuration option http_boot.

Blake Rouse (blake-rouse) wrote :

This is okay as a global setting allow it to be changed over the API, which is nice. But this needs to be undocumented and removed before release. So this option should really be short lived.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/forms/settings.py'
2--- src/maasserver/forms/settings.py 2017-06-15 14:27:01 +0000
3+++ src/maasserver/forms/settings.py 2017-06-21 22:04:06 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2013-2016 Canonical Ltd. This software is licensed under the
6+# Copyright 2013-2017 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 """Configuration items definition and utilities."""
10@@ -538,6 +538,14 @@
11 'min_value': 1,
12 },
13 },
14+ 'http_boot': {
15+ 'default': False,
16+ 'form': forms.BooleanField,
17+ 'form_kwargs': {
18+ 'label': "When true all ephemeral environments boot over HTTP.",
19+ 'required': False
20+ }
21+ },
22 }
23
24
25
26=== modified file 'src/maasserver/models/config.py'
27--- src/maasserver/models/config.py 2017-05-29 13:44:04 +0000
28+++ src/maasserver/models/config.py 2017-06-21 22:04:06 +0000
29@@ -1,4 +1,4 @@
30-# Copyright 2012-2016 Canonical Ltd. This software is licensed under the
31+# Copyright 2012-2017 Canonical Ltd. This software is licensed under the
32 # GNU Affero General Public License version 3 (see the file LICENSE).
33
34 """Configuration items."""
35@@ -101,7 +101,8 @@
36 'max_node_testing_results': 10,
37 'max_node_installation_results': 1,
38 # Notifications.
39- 'subnet_ip_exhaustion_threshold_count': 16
40+ 'subnet_ip_exhaustion_threshold_count': 16,
41+ 'http_boot': False,
42 }
43
44
45
46=== modified file 'src/maasserver/rpc/boot.py'
47--- src/maasserver/rpc/boot.py 2017-06-01 06:16:00 +0000
48+++ src/maasserver/rpc/boot.py 2017-06-21 22:04:06 +0000
49@@ -326,6 +326,7 @@
50 "fs_host": local_ip,
51 "log_host": server_host,
52 "extra_opts": '' if extra_kernel_opts is None else extra_kernel_opts,
53+ "http_boot": Config.objects.get_config('http_boot'),
54 }
55 if machine is not None:
56 params["system_id"] = machine.system_id
57
58=== modified file 'src/provisioningserver/kernel_opts.py'
59--- src/provisioningserver/kernel_opts.py 2017-04-12 22:00:36 +0000
60+++ src/provisioningserver/kernel_opts.py 2017-06-21 22:04:06 +0000
61@@ -1,4 +1,4 @@
62-# Copyright 2012-2015 Canonical Ltd. This software is licensed under the
63+# Copyright 2012-2017 Canonical Ltd. This software is licensed under the
64 # GNU Affero General Public License version 3 (see the file LICENSE).
65
66 """Generate kernel command-line options for inclusion in PXE configs."""
67@@ -6,7 +6,6 @@
68 __all__ = [
69 'compose_kernel_command_line',
70 'KernelParameters',
71- 'prefix_target_name',
72 ]
73
74 from collections import namedtuple
75@@ -43,6 +42,7 @@
76 "fs_host", # Host/IP on which ephemeral filesystems are hosted.
77 "extra_opts", # String of extra options to supply, will be appended
78 # verbatim to the kernel command line
79+ "http_boot", # Whether or not to boot over HTTP.
80 ))
81
82
83@@ -52,21 +52,6 @@
84 __call__ = KernelParametersBase._replace
85
86
87-def compose_preseed_opt(preseed_url):
88- """Compose a kernel option for preseed URL.
89-
90- :param preseed_url: The URL from which a preseed can be fetched.
91- """
92- # See https://help.ubuntu.com/12.04/installation-guide
93- # /i386/preseed-using.html#preseed-auto
94- return "auto url=%s" % preseed_url
95-
96-
97-def compose_locale_opt():
98- locale = 'en_US'
99- return "locale=%s" % locale
100-
101-
102 def compose_logging_opts(log_host):
103 return [
104 'log_host=%s' % log_host,
105@@ -101,19 +86,6 @@
106 )
107
108
109-def compose_hostname_opts(params):
110- """Return list of hostname/domain options based on `params`.
111-
112- The domain is omitted if `params` does not include it.
113- """
114- options = [
115- 'hostname=%s' % params.hostname,
116- ]
117- if params.domain is not None:
118- options.append('domain=%s' % params.domain)
119- return options
120-
121-
122 def prefix_target_name(name):
123 """Prefix an ISCSI target name with the standard target-name prefix."""
124 return "%s:%s" % (ISCSI_TARGET_NAME_PREFIX, name)
125@@ -121,8 +93,18 @@
126
127 def compose_purpose_opts(params):
128 """Return the list of the purpose-specific kernel options."""
129- if params.purpose in ["commissioning", "xinstall", "enlist"]:
130- # These are kernel parameters read by the ephemeral environment.
131+ if params.http_boot:
132+ kernel_params = [
133+ "root=squash:http://%s:5248/images/%s/%s/%s/%s/%s/squashfs" % (
134+ (
135+ '[%s]' % params.fs_host
136+ if IPAddress(params.fs_host).version == 6
137+ else params.fs_host
138+ ),
139+ params.osystem, params.arch, params.subarch, params.release,
140+ params.label),
141+ ]
142+ else:
143 tname = prefix_target_name(
144 get_ephemeral_name(
145 params.osystem, params.arch, params.subarch,
146@@ -133,43 +115,38 @@
147 "iscsi_target_ip=%s" % params.fs_host,
148 "iscsi_target_port=3260",
149 "iscsi_initiator=%s" % params.hostname,
150- # Read by cloud-initramfs-dyn-netconf initramfs-tools networking
151- # configuration in the initramfs. Choose IPv4 or IPv6 based on the
152- # family of fs_host. If BOOTIF is set, IPv6 config uses that
153- # exclusively.
154- (
155- "ip=::::%s:BOOTIF" % params.hostname
156- if IPAddress(params.fs_host).version == 4 else "ip=off"
157- ),
158- (
159- "ip6=dhcp"
160- if IPAddress(params.fs_host).version == 6 else "ip6=off"
161- ),
162 # kernel / udev name iscsi devices with this path
163- "ro root=/dev/disk/by-path/ip-%s:%s-iscsi-%s-lun-1" % (
164+ "root=/dev/disk/by-path/ip-%s:%s-iscsi-%s-lun-1" % (
165 params.fs_host, "3260", tname),
166- # Read by overlayroot package.
167- "overlayroot=tmpfs",
168- # LP:1533822 - Disable reading overlay data from disk.
169- "overlayroot_cfgdisk=disabled",
170- # Select the MAAS datasource by default.
171- "cc:{'datasource_list': ['MAAS']}end_cc",
172- # Read by cloud-init.
173- "cloud-config-url=%s" % params.preseed_url,
174- # Disable apparmor in the ephemeral environment. This addresses
175- # MAAS bug LP: #1677336 due to LP: #1408106
176- "apparmor=0",
177- ]
178- return kernel_params
179- else:
180- # These are options used by the Debian Installer.
181- return [
182- "netcfg/choose_interface=auto",
183- # Use the text installer, display only critical messages.
184- "text priority=critical",
185- compose_preseed_opt(params.preseed_url),
186- compose_locale_opt(),
187- ] + compose_hostname_opts(params)
188+ ]
189+
190+ kernel_params += [
191+ "ro",
192+ # Read by cloud-initramfs-dyn-netconf initramfs-tools networking
193+ # configuration in the initramfs. Choose IPv4 or IPv6 based on the
194+ # family of fs_host. If BOOTIF is set, IPv6 config uses that
195+ # exclusively.
196+ (
197+ "ip=::::%s:BOOTIF" % params.hostname
198+ if IPAddress(params.fs_host).version == 4 else "ip=off"
199+ ),
200+ (
201+ "ip6=dhcp"
202+ if IPAddress(params.fs_host).version == 6 else "ip6=off"
203+ ),
204+ # Read by overlayroot package.
205+ "overlayroot=tmpfs",
206+ # LP:1533822 - Disable reading overlay data from disk.
207+ "overlayroot_cfgdisk=disabled",
208+ # Select the MAAS datasource by default.
209+ "cc:{'datasource_list': ['MAAS']}end_cc",
210+ # Read by cloud-init.
211+ "cloud-config-url=%s" % params.preseed_url,
212+ # Disable apparmor in the ephemeral environment. This addresses
213+ # MAAS bug LP: #1677336 due to LP: #1408106
214+ "apparmor=0",
215+ ]
216+ return kernel_params
217
218
219 def compose_arch_opts(params):
220
221=== modified file 'src/provisioningserver/rpc/region.py'
222--- src/provisioningserver/rpc/region.py 2017-01-28 00:51:47 +0000
223+++ src/provisioningserver/rpc/region.py 2017-06-21 22:04:06 +0000
224@@ -138,6 +138,7 @@
225 (b"log_host", amp.Unicode()),
226 (b"extra_opts", amp.Unicode()),
227 (b"system_id", amp.Unicode(optional=True)),
228+ (b"http_boot", amp.Boolean(optional=True)),
229 ]
230 errors = {
231 BootConfigNoResponse: b"BootConfigNoResponse",
232
233=== modified file 'src/provisioningserver/tests/test_kernel_opts.py'
234--- src/provisioningserver/tests/test_kernel_opts.py 2017-04-12 22:00:36 +0000
235+++ src/provisioningserver/tests/test_kernel_opts.py 2017-06-21 22:04:06 +0000
236@@ -21,14 +21,10 @@
237 from provisioningserver.kernel_opts import (
238 compose_arch_opts,
239 compose_kernel_command_line,
240- compose_preseed_opt,
241 CURTIN_KERNEL_CMDLINE_NAME,
242 get_curtin_kernel_cmdline_sep,
243- get_ephemeral_name,
244 get_last_directory,
245- ISCSI_TARGET_NAME_PREFIX,
246 KernelParameters,
247- prefix_target_name,
248 )
249 from testtools.matchers import (
250 Contains,
251@@ -51,6 +47,8 @@
252 {field: factory.make_name(field)
253 for field in KernelParameters._fields
254 if field not in parms})
255+ if not isinstance(parms['http_boot'], bool):
256+ parms['http_boot'] = True
257 params = KernelParameters(**parms)
258
259 if testcase is not None:
260@@ -86,23 +84,6 @@
261 self.assertTrue(callable(params))
262 self.assertIs(params._replace.__func__, params.__call__.__func__)
263
264- def test_prefix_target_name_adds_prefix(self):
265- prefix = factory.make_name('prefix')
266- target = factory.make_name('tgt')
267- self.patch(kernel_opts, 'ISCSI_TARGET_NAME_PREFIX', prefix)
268-
269- self.assertEqual(
270- '%s:%s' % (prefix, target),
271- prefix_target_name(target))
272-
273- def test_prefix_target_name_produces_exactly_one_separating_colon(self):
274- target = factory.make_name('tgt')
275-
276- full_name = prefix_target_name(target)
277-
278- self.assertIn(':' + target, full_name)
279- self.assertNotIn('::' + target, full_name)
280-
281
282 class TestGetCurtinKernelCmdlineSepTest(MAASTestCase):
283
284@@ -138,59 +119,6 @@
285 cmdline = compose_kernel_command_line(params)
286 self.assertThat(cmdline, Contains("overlayroot_cfgdisk=disabled"))
287
288- def test_compose_kernel_command_line_includes_preseed_url(self):
289- params = self.make_kernel_parameters()
290- self.assertIn(
291- "auto url=%s" % params.preseed_url,
292- compose_kernel_command_line(params))
293-
294- def test_install_compose_kernel_command_line_includes_name_domain(self):
295- params = self.make_kernel_parameters(purpose="install")
296- self.assertThat(
297- compose_kernel_command_line(params),
298- ContainsAll([
299- "hostname=%s" % params.hostname,
300- "domain=%s" % params.domain,
301- ]))
302-
303- def test_install_compose_kernel_command_line_omits_domain_if_omitted(self):
304- params = self.make_kernel_parameters(purpose="install", domain=None)
305- kernel_command_line = compose_kernel_command_line(params)
306- self.assertIn("hostname=%s" % params.hostname, kernel_command_line)
307- self.assertNotIn('domain=', kernel_command_line)
308-
309- def test_install_compose_kernel_command_line_includes_locale(self):
310- params = self.make_kernel_parameters(purpose="install")
311- locale = "en_US"
312- self.assertIn(
313- "locale=%s" % locale,
314- compose_kernel_command_line(params))
315-
316- def test_install_compose_kernel_command_line_includes_log_settings(self):
317- params = self.make_kernel_parameters(purpose="install")
318- # Port 514 (UDP) is syslog.
319- log_port = "514"
320- self.assertThat(
321- compose_kernel_command_line(params),
322- ContainsAll([
323- "log_host=%s" % params.log_host,
324- "log_port=%s" % log_port,
325- ]))
326-
327- def test_install_compose_kernel_command_line_includes_di_settings(self):
328- params = self.make_kernel_parameters(purpose="install")
329- self.assertThat(
330- compose_kernel_command_line(params),
331- Contains("text priority=critical"))
332-
333- def test_install_compose_kernel_command_line_inc_purpose_opts(self):
334- # The result of compose_kernel_command_line includes the purpose
335- # options for a non "commissioning" node.
336- params = self.make_kernel_parameters(purpose="install")
337- self.assertIn(
338- "netcfg/choose_interface=auto",
339- compose_kernel_command_line(params))
340-
341 def test_xinstall_compose_kernel_command_line_inc_purpose_opts4(self):
342 # The result of compose_kernel_command_line includes the purpose
343 # options for a non "xinstall" node.
344@@ -200,8 +128,7 @@
345 self.assertThat(
346 cmdline,
347 ContainsAll([
348- "root=/dev/disk/by-path/ip-",
349- "iscsi_initiator=",
350+ "root=squash:http://",
351 "overlayroot=tmpfs",
352 "ip6=off",
353 "ip=::::%s:BOOTIF" % params.hostname]))
354@@ -215,8 +142,7 @@
355 self.assertThat(
356 cmdline,
357 ContainsAll([
358- "root=/dev/disk/by-path/ip-",
359- "iscsi_initiator=",
360+ "root=squash:http://",
361 "overlayroot=tmpfs",
362 "ip=off",
363 "ip6=dhcp"]))
364@@ -242,8 +168,7 @@
365 self.assertThat(
366 cmdline,
367 ContainsAll([
368- "root=/dev/disk/by-path/ip-",
369- "iscsi_initiator=",
370+ "root=squash:http://",
371 "overlayroot=tmpfs",
372 "ip6=off",
373 "ip=::::%s:BOOTIF" % params.hostname]))
374@@ -257,8 +182,7 @@
375 self.assertThat(
376 cmdline,
377 ContainsAll([
378- "root=/dev/disk/by-path/ip-",
379- "iscsi_initiator=",
380+ "root=squash:http://",
381 "overlayroot=tmpfs",
382 "ip=off",
383 "ip6=dhcp"]))
384@@ -284,8 +208,7 @@
385 self.assertThat(
386 cmdline,
387 ContainsAll([
388- "root=/dev/disk/by-path/ip-",
389- "iscsi_initiator=",
390+ "root=squash:http://",
391 "overlayroot=tmpfs",
392 "ip6=off",
393 "ip=::::%s:BOOTIF" % params.hostname]))
394@@ -299,8 +222,7 @@
395 self.assertThat(
396 cmdline,
397 ContainsAll([
398- "root=/dev/disk/by-path/ip-",
399- "iscsi_initiator=",
400+ "root=squash:http://",
401 "overlayroot=tmpfs",
402 "ip=off",
403 "ip6=dhcp"]))
404@@ -378,39 +300,17 @@
405 # The result of compose_kernel_command_line includes the purpose
406 # options for a "xinstall" node.
407 params = self.make_kernel_parameters(purpose="xinstall")
408- ephemeral_name = get_ephemeral_name(
409- params.osystem, params.arch, params.subarch,
410- params.release, params.label)
411 self.assertThat(
412 compose_kernel_command_line(params),
413- ContainsAll([
414- "iscsi_target_name=%s:%s" % (
415- ISCSI_TARGET_NAME_PREFIX, ephemeral_name),
416- "iscsi_target_port=3260",
417- "iscsi_target_ip=%s" % params.fs_host,
418- ]))
419+ ContainsAll(["root=squash:http://"]))
420
421 def test_compose_kernel_command_line_inc_purpose_opts_comm_node(self):
422 # The result of compose_kernel_command_line includes the purpose
423 # options for a "commissioning" node.
424 params = self.make_kernel_parameters(purpose="commissioning")
425- ephemeral_name = get_ephemeral_name(
426- params.osystem, params.arch, params.subarch,
427- params.release, params.label)
428 self.assertThat(
429 compose_kernel_command_line(params),
430- ContainsAll([
431- "iscsi_target_name=%s:%s" % (
432- ISCSI_TARGET_NAME_PREFIX, ephemeral_name),
433- "iscsi_target_port=3260",
434- "iscsi_target_ip=%s" % params.fs_host,
435- ]))
436-
437- def test_compose_preseed_kernel_opt_returns_kernel_option(self):
438- dummy_preseed_url = factory.make_name("url")
439- self.assertEqual(
440- "auto url=%s" % dummy_preseed_url,
441- compose_preseed_opt(dummy_preseed_url))
442+ ContainsAll(["root=squash:http://"]))
443
444 def test_compose_kernel_command_line_inc_arm_specific_option(self):
445 params = self.make_kernel_parameters(arch="armhf", subarch="highbank")
446@@ -432,3 +332,33 @@
447 arch=factory.make_name("arch"),
448 subarch=factory.make_name("subarch"))
449 self.assertEqual([], compose_arch_opts(params))
450+
451+ def test_compose_rootfs_over_http_ipv4(self):
452+ params = make_kernel_parameters(fs_host=factory.make_ipv4_address())
453+ self.assertThat(
454+ compose_kernel_command_line(params),
455+ ContainsAll([
456+ "ro",
457+ "root=squash:http://%s:5248/images/%s/%s/%s/%s/%s/squashfs" % (
458+ params.fs_host, params.osystem, params.arch,
459+ params.subarch, params.release, params.label)]))
460+
461+ def test_compose_rootfs_over_http_ipv6(self):
462+ params = make_kernel_parameters(fs_host=factory.make_ipv6_address())
463+ self.assertThat(
464+ compose_kernel_command_line(params),
465+ ContainsAll([
466+ "ro",
467+ "root=squash:http://[%s]:5248/images/%s/%s/%s/%s/%s/squashfs" %
468+ (
469+ params.fs_host, params.osystem, params.arch,
470+ params.subarch, params.release, params.label)]))
471+
472+ def test_compose_rootfs_over_iscsi(self):
473+ params = make_kernel_parameters(
474+ fs_host=factory.make_ipv6_address(), http_boot=False)
475+ self.assertThat(
476+ compose_kernel_command_line(params),
477+ ContainsAll([
478+ "root=/dev/disk/by-path/ip-",
479+ "iscsi_initiator="]))