launchpad-reviewers team mailing list archive

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

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1032213 in MAAS: "Not all boot purposes have a kernel+initrd arranged"
  https://bugs.launchpad.net/maas/+bug/1032213

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

For the Cobblerless world, we had a setup where maas-import-pxe-files and maas-import-ephemerals were both run individually, both from cron and from the command-line interface.  This wasn't documented yet as we're still in a state of transition away from Cobbler.  In the old situation, either cron or the user would run maas-import-isos and that would run maas-import-ephemerals.

In this branch, at Julian's request, I make maas-import-pxe-files run maas-import-ephemerals.  This fits the name better, makes things clearer, and eliminates some of the hassle.  The cron tab, or the user, just needs to run maas-import-pxe-files (but with one caveat: a variable MAAS_PROVISIONING_SETTINGS must be set, which isn't documented but breaks the script as we've had it for some time).  The NO_COBBLER variable is set to suppress Cobbler-specific parts of the ephemerals script.

To keep maas-import-pxe-files understandable I split up the “main” function a bit further.  You may think the IMPORT_EPHEMERALS variable, which lets us suppress the call to the ephemerals script, is a bit of a cop-out just to make things more testable.  Actually I just copied that from the old maas-import-isos for consistency.  In my tests I went about suppression in a very different way: I created a fake version of the script in a temporary directory (which I added to PATH) that simply returned 0.  And then I realized that all I had to do was set IMPORT_EPHEMERALS=0!


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

=== modified file 'etc/cron.d/maas-import-pxe-files'
--- etc/cron.d/maas-import-pxe-files	2012-07-17 03:41:17 +0000
+++ etc/cron.d/maas-import-pxe-files	2012-08-03 12:43:22 +0000
@@ -1,7 +1,3 @@
 # Runs the MAAS image downloads early every Sunday.
 
-# Installation images:
 11 4 * * 0 root	/usr/sbin/maas-import-pxe-files &> /dev/null
-
-# Ephemeral images:
-44 4 * * 0 root /usr/sbin/maas-import-ephemerals &> /dev/null

=== modified file 'scripts/maas-import-pxe-files'
--- scripts/maas-import-pxe-files	2012-07-23 19:55:05 +0000
+++ scripts/maas-import-pxe-files	2012-08-03 12:43:22 +0000
@@ -41,6 +41,10 @@
 # directory.  A wget command line will work here, but curl will do as well.
 DOWNLOAD=${DOWNLOAD:-wget --no-verbose}
 
+# Whether to download ephemeral images as well: "1" for yes, "0" for no.
+# Default is yes.
+IMPORT_EPHEMERALS=${IMPORT_EPHEMERALS:-1}
+
 
 # Put together a full URL for where the installer files for architecture $1
 # and release $2 can be downloaded.
@@ -97,8 +101,9 @@
 }
 
 
-main() {
-    local arch release
+# Download and install the "install" images.
+import_install_images() {
+    local arch release DOWNLOAD_DIR
 
     DOWNLOAD_DIR=$(mktemp -d)
     echo "Downloading to temporary location $DOWNLOAD_DIR."
@@ -122,4 +127,21 @@
 }
 
 
+# Download and install the ephemeral images.
+import_ephemeral_images() {
+    if test "$IMPORT_EPHEMERALS" != "0"
+    then
+        NO_COBBLER=1
+        export NO_COBBLER
+        maas-import-ephemerals --import
+    fi
+}
+
+
+main() {
+    import_install_images
+    import_ephemeral_images
+}
+
+
 main

=== modified file 'src/provisioningserver/tests/test_maas_import_pxe_files.py'
--- src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-07-25 03:32:36 +0000
+++ src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-08-03 12:43:22 +0000
@@ -102,13 +102,17 @@
         here = os.path.dirname(__file__)
         root = os.path.join(here, os.pardir, os.pardir, os.pardir)
         script = os.path.join(root, "scripts", "maas-import-pxe-files")
-        path = [os.path.join(root, "bin")]
+
+        path = [os.path.join(root, "bin"), os.path.join(root, "scripts")]
         path.extend(os.environ.get("PATH", "").split(os.pathsep))
         env = {
             'ARCHIVE': 'file://%s' % archive_dir,
             # Substitute curl for wget; it accepts file:// URLs.
             'DOWNLOAD': 'curl -O --silent',
             'PATH': os.pathsep.join(path),
+            # Suppress running of maas-import-ephemerals.  It gets too
+            # intimate with the system to test here.
+            'IMPORT_EPHEMERALS': '0',
         }
         env.update(self.config_fixture.environ)
         if arch is not None:
@@ -116,6 +120,7 @@
         if release is not None:
             env['RELEASES'] = release
             env['CURRENT_RELEASE'] = release
+
         with open(os.devnull, 'wb') as dev_null:
             check_call(script, env=env, stdout=dev_null)