launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-1030884 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1030884/+merge/117826

This fixes bug 1030884, but not in the way that the bug prescribes.  The bug basically says “you must download menu.c32 and put it in the right place.”  But that just states what to do rather than what the problem is, so a lot of the work on this bug consisted of reconstructing its rationale and context.

The menu.c32 file is a module that implements a nicer boot menu on the nodes, compared to the default command-line interface.  This is what syslinux/pxelinux call the “menu,” even though the boot CLI offers the same flexibility without it.  The menu involved a whole can of worms:
 - Where would we download menu.c32?  It's not available with the kernels and initrds.
 - Can we instead use the one that syslinux-common already installs?
 - It's a binary though.  Does ARM need this file?
 - What is the right place, exactly?  Syslinux and PXELinux differ on this point.
 - Our PXE configs only give the node one action to perform anyway.  Why a menu in the first place?

Discussed with Robie and Scott.  Apparently our use-case for a boot menu is to have the enlistment boot, in a small-scale x86/amd64 setup, provide a user-friendly option to boot from local disk instead of enlisting.  Which apparently would also involve some logic involving chain.c32 which we don't have yet either, so we're not yet ready to support that.  Things would be different again for ARM, so we'd need separate templates per architecture (with i386 and amd64 being duplicates).  Not very nice, and certainly not something to hold up the critical path for.

So instead, for now, we opted to do without the menu — which seems to be entirely possible, and avoids the need for menu.c32.  It's just a change to the config template.

The new template gives you an arbitrary 20 seconds to back out of the default boot.  I termed it “boot under MAAS direction.”  In the future we can add an alternate boot choice but crucially, we can do this with or without a boot menu and menu.c32.  The boot menu and the local-boot option are essentially different features that interact only in the PXE config template.

Jeroen
-- 
https://code.launchpad.net/~jtv/maas/bug-1030884/+merge/117826
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/bug-1030884 into lp:maas.

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-07-31 13:27:05 +0000
+++ src/maasserver/tests/test_api.py	2012-08-02 03:20:32 +0000
@@ -2281,7 +2281,7 @@
                 (
                     Equals(httplib.OK),
                     Equals("text/plain; charset=utf-8"),
-                    StartsWith('DEFAULT menu'),
+                    StartsWith('DEFAULT execute'),
                 )),
             response)
 

=== modified file 'src/provisioningserver/pxe/config.template'
--- src/provisioningserver/pxe/config.template	2012-07-30 20:57:19 +0000
+++ src/provisioningserver/pxe/config.template	2012-08-02 03:20:32 +0000
@@ -1,14 +1,10 @@
-DEFAULT menu
-PROMPT 0
-MENU TITLE {{title}}
-TIMEOUT 1
-ONTIMEOUT execute
+DEFAULT execute
+TIMEOUT 20
+SAY Booting under MAAS direction in 20 seconds.
 
 LABEL execute
+        SAY Booting under MAAS direction...
         KERNEL {{kernel}}
         INITRD {{initrd}}
-        MENU LABEL Continue
         APPEND {{append}}
         IPAPPEND 2
-
-MENU end

=== modified file 'src/provisioningserver/pxe/tests/test_config.py'
--- src/provisioningserver/pxe/tests/test_config.py	2012-07-31 10:15:07 +0000
+++ src/provisioningserver/pxe/tests/test_config.py	2012-08-02 03:20:32 +0000
@@ -53,9 +53,6 @@
         self.assertThat(
             output, MatchesAll(
                 MatchesRegex(
-                    r'.*^MENU TITLE %s$' % re.escape(options["title"]),
-                    re.MULTILINE | re.DOTALL),
-                MatchesRegex(
                     r'.*^\s+KERNEL %s/kernel$' % re.escape(image_dir),
                     re.MULTILINE | re.DOTALL),
                 MatchesRegex(