launchpad-reviewers team mailing list archive

[Robie Basak <robie.basak@xxxxxxxxxxxxx>]

Robie Basak has proposed merging lp:~racb/maas/import-pxe-files-subarch into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~racb/maas/import-pxe-files-subarch/+merge/124885
-- 
https://code.launchpad.net/~racb/maas/import-pxe-files-subarch/+merge/124885
Your team MAAS Maintainers is requested to review the proposed merge of lp:~racb/maas/import-pxe-files-subarch into lp:maas.

=== modified file 'etc/maas/import_pxe_files'
--- etc/maas/import_pxe_files	2012-08-15 12:54:49 +0000
+++ etc/maas/import_pxe_files	2012-09-19 14:56:28 +0000
@@ -14,6 +14,6 @@
 
 #RELEASES="oneiric precise"
 RELEASES="precise"
-#ARCHES="amd64 i386"
+#ARCHES="amd64/generic i386/generic"
 #LOCALE="en_US"
 #IMPORT_EPHEMERALS=1

=== modified file 'scripts/maas-import-pxe-files'
--- scripts/maas-import-pxe-files	2012-08-31 19:59:31 +0000
+++ scripts/maas-import-pxe-files	2012-09-19 14:56:28 +0000
@@ -20,8 +20,9 @@
 local_settings="$(pwd)/$settings"
 [ -r $local_settings ] && . $local_settings
 
-# Download location for Ubuntu releases.
-ARCHIVE=${ARCHIVE:-http://archive.ubuntu.com/ubuntu/}
+# Download locations for Ubuntu releases.
+MAIN_ARCHIVE=${ARCHIVE:-http://archive.ubuntu.com/ubuntu/}
+PORTS_ARCHIVE=${ARCHIVE:-http://ports.ubuntu.com/ubuntu-ports/}
 
 # Ubuntu releases that are to be downloaded.
 SUPPORTED_RELEASES=$(distro-info --supported)
@@ -31,7 +32,10 @@
 STABLE_RELEASE=$(distro-info --stable)
 
 # Supported architectures.
-ARCHES=${ARCHES:-amd64 i386}
+# armhf/highbank is also supported by this script, but cannot be enabled here
+# until maas-import-ephemerals also supports it or IMPORT_EPHEMERALS is set to
+# 0.
+ARCHES=${ARCHES:-amd64/generic i386/generic}
 
 # Command line to download a resource at a given URL into the current
 # directory.  A wget command line will work here, but curl will do as well.
@@ -60,10 +64,62 @@
 # and release $2 can be downloaded.
 compose_installer_download_url() {
     local arch=$1 release=$2
-    local installer_url="$ARCHIVE/dists/$release/main/installer-$arch"
-    echo "$installer_url/current/images/netboot/ubuntu-installer/$arch/"
-}
-
+
+    case $arch in
+        amd64/*|i386/*)
+            local installer_url="$MAIN_ARCHIVE/dists/$release/main/installer-${arch%%/*}"
+            echo "$installer_url/current/images/netboot/ubuntu-installer/${arch%%/*}/"
+            ;;
+        armhf/*)
+            # No ARM server installers were available in precise, so always go for -updates for now
+            # A better general fix is LP: #1052397
+            local installer_url="$PORTS_ARCHIVE/dists/$release-updates/main/installer-${arch%%/*}"
+            echo "$installer_url/current/images/${arch#*/}/netboot/"
+            ;;
+        *)
+            echo "Unknown architecture: $arch" >&2
+            exit 1
+            ;;
+    esac
+}
+
+# Return a list of files for architecture $1 and release $2 that need to be
+# downloaded
+compose_installer_download_files() {
+    local arch=$1 release=$2
+
+    case $arch in
+        amd64/*|i386/*)
+            echo "linux initrd.gz"
+            ;;
+        armhf/highbank)
+            echo "vmlinuz initrd.gz"
+            ;;
+        *)
+            echo "Unknown architecture: $arch" >&2
+            exit 1
+            ;;
+    esac
+}
+
+# Rename downloaded files for architecture $1 and release $2 into the form that
+# MAAS expects them
+rename_installer_download_files() {
+    local arch=$1 release=$2
+
+    case $arch in
+        amd64/*|i386/*)
+            # do nothing
+            ;;
+        armhf/highbank)
+            mv vmlinuz linux
+            ;;
+        *)
+            echo "Unknown architecture: $arch" >&2
+            exit 1
+            ;;
+    esac
+}
 
 # Copy the pre-boot loader pxelinux.0, and modules we need, from the
 # installed syslinux version.  Install it into the TFTP tree for
@@ -81,7 +137,7 @@
 # architecture $1 and install it into the TFTP directory hierarchy.
 update_install_files() {
     local arch=$1 release=$2
-    local files="initrd.gz linux"
+    local files=$(compose_installer_download_files $arch $release)
     local url=$(compose_installer_download_url $arch $release)
     local file
 
@@ -91,10 +147,12 @@
     do
         $DOWNLOAD $url/$file
     done
+    rename_installer_download_files $arch $release
     popd >/dev/null
 
     maas-provision install-pxe-image \
-        --arch=$arch --release=$release --purpose="install" \
+        --arch=${arch%%/*} --subarch=${arch#*/} \
+        --release=$release --purpose="install" \
         --image="install"
 }
 

=== modified file 'src/provisioningserver/tests/test_maas_import_pxe_files.py'
--- src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-09-13 06:53:34 +0000
+++ src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-09-19 14:56:28 +0000
@@ -47,9 +47,45 @@
     :return: Full absolute path to the directory holding the requisite files
         for this archive, arch, and release.
     """
-    return os.path.join(
-        archive, 'dists', release, 'main', 'installer-%s' % arch, 'current',
-        'images', 'netboot', 'ubuntu-installer', arch)
+    if arch[0] in ['amd64', 'i386']:
+        return os.path.join(
+            archive, 'dists', release, 'main', 'installer-%s' % arch[0],
+            'current', 'images', 'netboot', 'ubuntu-installer', arch[0])
+    elif arch[0] == 'armhf':
+        return os.path.join(
+            archive, 'dists', '%s-updates' % release, 'main',
+            'installer-%s' % arch[0], 'current', 'images', arch[1], 'netboot')
+    else:
+        raise NotImplementedError('Unknown architecture: %r' % arch)
+
+
+def compose_download_kernel_name(arch, release):
+    """Name the kernel file expected to be found in a netboot archive.
+
+    :param archive: Archive directory (corresponding to the script's ARCHIVE
+        setting, except here it's a filesystem path not a URL).
+    :param arch: Architecture.
+    :return: the kernel name string, eg. "vmlinuz" or "linux" as appropriate
+    """
+    if arch[0] in ['amd64', 'i386']:
+        return 'linux'
+    elif arch[0] == 'armhf':
+        return 'vmlinuz'
+    else:
+        raise NotImplementedError('Unknown architecture: %r' % arch)
+
+
+def compose_download_filenames(arch, release):
+    """Name files expected to be found in the directory of a netboot archive.
+
+    :param archive: Archive directory (corresponding to the script's ARCHIVE
+        setting, except here it's a filesystem path not a URL).
+    :param arch: Architecture.
+    :return: list of names to be found in the netboot directory for this
+        particular arch and release. Eg: ['vmlinuz', 'initrd.gz'] or
+        ['linux', 'initrd.gz'] as appropriate.
+    """
+    return [compose_download_kernel_name(arch, release), 'initrd.gz']
 
 
 def compose_tftp_bootloader_path(tftproot):
@@ -67,13 +103,19 @@
     """
     return os.path.join(
         tftppath.locate_tftp_path(
-            tftppath.compose_image_path(arch, "generic", release, purpose),
+            tftppath.compose_image_path(arch[0], arch[1], release, purpose),
             tftproot),
         *path)
 
 
 class TestImportPXEFiles(TestCase):
 
+    scenarios = (
+        ("i386/generic", dict(arch=("i386", "generic"))),
+        ("amd64/generic", dict(arch=("amd64", "generic"))),
+        ("armhf/highbank", dict(arch=("armhf", "highbank"))),
+        )
+
     def setUp(self):
         super(TestImportPXEFiles, self).setUp()
         self.tftproot = self.make_dir()
@@ -88,12 +130,10 @@
         """
         if release is None:
             release = factory.make_name('release')
-        if arch is None:
-            arch = factory.make_name('arch')
         archive = self.make_dir()
         download = compose_download_dir(archive, arch, release)
         os.makedirs(download)
-        for filename in ['initrd.gz', 'linux']:
+        for filename in compose_download_filenames(arch, release):
             factory.make_file(download, filename)
         return archive
 
@@ -123,7 +163,7 @@
         }
         env.update(self.config_fixture.environ)
         if arch is not None:
-            env['ARCHES'] = arch
+            env['ARCHES'] = '/'.join(arch)
         if release is not None:
             env['RELEASES'] = release
 
@@ -131,58 +171,61 @@
             check_call(script, env=env, stdout=dev_null)
 
     def test_procures_pre_boot_loader(self):
-        arch = factory.make_name('arch')
         release = 'precise'
-        archive = self.make_downloads(arch=arch, release=release)
-        self.call_script(archive, self.tftproot, arch=arch, release=release)
+        archive = self.make_downloads(arch=self.arch, release=release)
+        self.call_script(
+            archive, self.tftproot, arch=self.arch, release=release)
         tftp_path = compose_tftp_bootloader_path(self.tftproot)
         expected_contents = read_file('/usr/lib/syslinux', 'pxelinux.0')
         self.assertThat(tftp_path, FileContains(expected_contents))
 
     def test_updates_pre_boot_loader(self):
-        arch = factory.make_name('arch')
         release = 'precise'
         tftp_path = compose_tftp_bootloader_path(self.tftproot)
         with open(tftp_path, 'w') as existing_file:
             existing_file.write(factory.getRandomString())
-        archive = self.make_downloads(arch=arch, release=release)
-        self.call_script(archive, self.tftproot, arch=arch, release=release)
+        archive = self.make_downloads(arch=self.arch, release=release)
+        self.call_script(
+            archive, self.tftproot, arch=self.arch, release=release)
         expected_contents = read_file('/usr/lib/syslinux', 'pxelinux.0')
         self.assertThat(tftp_path, FileContains(expected_contents))
 
     def test_procures_install_image(self):
-        arch = factory.make_name('arch')
         release = 'precise'
-        archive = self.make_downloads(arch=arch, release=release)
-        self.call_script(archive, self.tftproot, arch=arch, release=release)
+        archive = self.make_downloads(arch=self.arch, release=release)
+        self.call_script(
+            archive, self.tftproot, arch=self.arch, release=release)
         tftp_path = compose_tftp_path(
-            self.tftproot, arch, release, 'install', 'linux')
-        download_path = compose_download_dir(archive, arch, release)
-        expected_contents = read_file(download_path, 'linux')
+            self.tftproot, self.arch, release, 'install', 'linux')
+        download_path = compose_download_dir(archive, self.arch, release)
+        expected_contents = read_file(download_path,
+            compose_download_kernel_name(self.arch, release))
         self.assertThat(tftp_path, FileContains(expected_contents))
 
     def test_updates_install_image(self):
-        arch = factory.make_name('arch')
         release = 'precise'
         tftp_path = compose_tftp_path(
-            self.tftproot, arch, release, 'install', 'linux')
+            self.tftproot, self.arch, release, 'install', 'linux')
         os.makedirs(os.path.dirname(tftp_path))
         with open(tftp_path, 'w') as existing_file:
             existing_file.write(factory.getRandomString())
-        archive = self.make_downloads(arch=arch, release=release)
-        self.call_script(archive, self.tftproot, arch=arch, release=release)
-        download_path = compose_download_dir(archive, arch, release)
-        expected_contents = read_file(download_path, 'linux')
+        archive = self.make_downloads(arch=self.arch, release=release)
+        self.call_script(
+            archive, self.tftproot, arch=self.arch, release=release)
+        download_path = compose_download_dir(archive, self.arch, release)
+        expected_contents = read_file(download_path,
+            compose_download_kernel_name(self.arch, release))
         self.assertThat(tftp_path, FileContains(expected_contents))
 
     def test_leaves_install_image_untouched_if_unchanged(self):
-        arch = factory.make_name('arch')
         release = 'precise'
-        archive = self.make_downloads(arch=arch, release=release)
-        self.call_script(archive, self.tftproot, arch=arch, release=release)
+        archive = self.make_downloads(arch=self.arch, release=release)
+        self.call_script(
+            archive, self.tftproot, arch=self.arch, release=release)
         tftp_path = compose_tftp_path(
-            self.tftproot, arch, release, 'install', 'linux')
+            self.tftproot, self.arch, release, 'install', 'linux')
         backdate(tftp_path)
         original_timestamp = get_write_time(tftp_path)
-        self.call_script(archive, self.tftproot, arch=arch, release=release)
+        self.call_script(
+            archive, self.tftproot, arch=self.arch, release=release)
         self.assertEqual(original_timestamp, get_write_time(tftp_path))