launchpad-reviewers team mailing list archive

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

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/install-bootloader-using-custom-command into lp:maas with lp:~jtv/maas/install-pxe-bootloader as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/install-bootloader-using-custom-command/+merge/112714

This builds on two preceding branches that provide the python code that the script can now delegate to.  With these changes, all TFTP paths anywhere in MAAS should now be generated from the tftppath module.

You'll note that I deleted the test that checks against the script overwriting an existing bootloader if the new one is identical.  That test still passes.  The reason I removed it is that responsibility for this detail of the script's behaviour has moved into the maas custom command, where it is tested separately.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/install-bootloader-using-custom-command/+merge/112714
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/install-bootloader-using-custom-command into lp:maas.

=== modified file 'scripts/maas-import-pxe-files'
--- scripts/maas-import-pxe-files	2012-06-26 10:40:51 +0000
+++ scripts/maas-import-pxe-files	2012-06-29 07:01:09 +0000
@@ -51,16 +51,16 @@
 }
 
 
-# Download the pre-boot loader, pxelinux.0, for architecture $2 if it exists,
-# and if so, install it into directory $1.  (Not all architectures need this
+# Download the pre-boot loader, pxelinux.0, for architecture $1.  If
+# successful, install it for netboot use.  (Not all architectures need this
 # file, and there's rarely an urgent need for the very latest file, so if
 # the download fails this function just skips it.)
 update_pre_boot_loader() {
-    local dest=$1 arch=$2
+    local arch=$1
     local url=$(compose_installer_download_url $arch $CURRENT_RELEASE)
     if ! $DOWNLOAD $url/pxelinux.0
     then
-        echo "No pre-boot loader for $arch; skipping."
+        echo "Could not download pre-boot loader for $arch; skipping."
         return
     fi
 
@@ -68,13 +68,9 @@
     # version).  Otherwise, leave the filesystem alone.
     if [ -f pxelinux.0 ]
     then
-        if cmp --quiet pxelinux.0 $dest/pxelinux.0
-        then
-            rm -f -- pxelinux.0
-        else
-            echo "Updating pre-boot loader for $arch."
-            mv -- pxelinux.0 $dest/
-        fi
+        # TODO: Pass sub-architecture once we support those.
+        maas install_pxe_bootloader \
+            --arch=$arch --loader='pxelinux.0' --tftproot=$TFTPROOT
     fi
 }
 
@@ -102,7 +98,7 @@
 
 
 main() {
-    local arch release arch_dir
+    local arch release
 
     DOWNLOAD_DIR=$(mktemp -d)
     echo "Downloading to temporary location $DOWNLOAD_DIR."
@@ -114,18 +110,14 @@
 
     for arch in $ARCHES
     do
-        # Replace the "generic" with sub-architecture once we start
-        # supporting those.
-        arch_dir="$TFTPROOT/maas/$arch/generic"
-
-        mkdir -p -- $arch_dir
-        update_pre_boot_loader $arch_dir $arch
+        update_pre_boot_loader $arch
 
         for release in $RELEASES
         do
             update_install_files $arch $release
         done
 
+        # TODO: Pass sub-architecture once we support those.
         maas generate_enlistment_pxe \
             --arch=$arch --release=$CURRENT_RELEASE \
             --tftproot=$TFTPROOT

=== modified file 'src/provisioningserver/tests/test_maas_import_pxe_files.py'
--- src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-06-26 16:09:34 +0000
+++ src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-06-29 07:01:09 +0000
@@ -148,20 +148,6 @@
         expected_contents = read_file(download_path, 'pxelinux.0')
         self.assertThat(tftp_path, FileContains(expected_contents))
 
-    def test_leaves_pre_boot_loader_untouched_if_unchanged(self):
-        # If pxelinux.0 has not changed between script runs, the old
-        # copy stays in place.
-        arch = factory.make_name('arch')
-        release = 'precise'
-        archive = self.make_downloads(arch=arch, release=release)
-        tftproot = self.make_dir()
-        self.call_script(archive, tftproot, arch=arch, release=release)
-        tftp_path = compose_tftp_path(tftproot, arch, 'pxelinux.0')
-        backdate(tftp_path)
-        original_timestamp = get_write_time(tftp_path)
-        self.call_script(archive, tftproot, arch=arch, release=release)
-        self.assertEqual(original_timestamp, get_write_time(tftp_path))
-
     def test_downloads_install_image(self):
         arch = factory.make_name('arch')
         release = 'precise'