← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/maas/import-to-tftp into lp:maas

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/import-to-tftp into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/import-to-tftp/+merge/110451

See the “PXE boot requirements” document for the reasoning and conventions behind this branch.  The branch adds a script that downloads the files that MAAS needs to provide over TFTP for a node to boot into the installer.

(Only the kernel and initrd are needed for this use-case; the minimal netbooted system will download the installer from an Ubuntu archive and start executing it.)

In a Cobblerless world this script replaces most, but not quite all, of what the existing maas-import-isos script does.  There's still the preseeds and kernel parameters to be dealt with, but I think those will need to be done with knowledge from the database.  My guess right now is that it can be done separately from the downloads.

There is still one problem with this script: a race condition.  Updates are skipped when there are no changes, and the files are replaced as near-atomically as possible, but that doesn't help downloads that are ongoing at the time.  Even if tftpd keeps file handles open for ongoing downloads (not too likely for a stateless UDP-based protocol!) so that each file will produce a consistent byte stream, the download is still 2 files, so a node could still see a mismatch between the kernel and the initrd.  We agreed to tolerate this for now, but I'll file a bug.

For testing purposes I had to introduce a test dependency on curl.  Amazingly, wget does not support the “file://” URLs I needed for my test.  So for testing (and testing only) I substitute curl.  The script is written such that it should work with either.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/import-to-tftp/+merge/110451
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/import-to-tftp into lp:maas.
=== modified file 'HACKING.txt'
--- HACKING.txt	2012-06-08 12:27:18 +0000
+++ HACKING.txt	2012-06-15 09:34:23 +0000
@@ -44,16 +44,16 @@
         python-formencode python-oauth python-oops python-oops-datedir-repo \
         python-twisted python-oops-wsgi python-oops-twisted \
         python-psycopg2 python-yaml python-convoy python-django-south \
-        python-avahi python-dbus python-celery python-tempita
+        python-avahi python-dbus python-celery python-tempita distro-info
 
 Additionally, you need to install the following python libraries
 for development convenience::
 
     $ sudo apt-get install python-sphinx python-lxml python-pocket-lint
 
-If you intend to run the test suite, you also need xvfb and firefox::
+If you intend to run the test suite, you also need a few other things:
 
-    $ sudo apt-get install xvfb firefox avahi-utils
+    $ sudo apt-get install xvfb firefox avahi-utils curl
 
 All other development dependencies are pulled automatically from `PyPI`_
 when buildout runs.

=== added file 'scripts/maas-import-pxe-files'
--- scripts/maas-import-pxe-files	1970-01-01 00:00:00 +0000
+++ scripts/maas-import-pxe-files	2012-06-15 09:34:23 +0000
@@ -0,0 +1,191 @@
+#!/usr/bin/env bash
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+#
+# Download static files needed for net-booting nodes through TFTP:
+# pre-boot loader, kernels, and initrd images.
+#
+# This script downloads the required files into the TFTP home directory
+# (by default, /var/lib/tftpboot).  Run it with the necessarily privileges
+# to write them there.
+
+# Exit immediately if a command exits with a non-zero status.
+set -o errexit
+# Treat unset variables as an error when substituting.
+set -o nounset
+
+# Load settings if available.
+settings="/etc/maas/import_isos"
+[ -r $settings ] && . $settings
+local_settings="$(pwd)/$settings"
+[ -r $local_settings ] && . $local_settings
+
+# Download location for Ubuntu releases.
+ARCHIVE=${ARCHIVE:-http://archive.ubuntu.com/ubuntu/dists/}
+
+# Ubuntu releases that are to be downloaded.
+SUPPORTED_RELEASES=$(distro-info --supported)
+RELEASES=${RELEASES:-$SUPPORTED_RELEASES}
+
+# The current Ubuntu release.
+STABLE_RELEASE=$(distro-info --stable)
+CURRENT_RELEASE=${CURRENT_RELEASE:-$STABLE_RELEASE}
+
+# Supported architectures.
+ARCHES=${ARCHES:-amd64 i386}
+
+# TFTP root directory.  (Don't let the "root" vs. "boot" confuse you.)
+TFTPROOT=${TFTPROOT:-/var/lib/tftpboot/}
+
+# 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.
+DOWNLOAD=${DOWNLOAD:-wget --no-verbose}
+
+
+# Put together a full URL for where the installer files for architecture $1
+# and release $2 can be downloaded.
+compose_installer_download_url() {
+    local arch=$1 release=$2
+    local images_url="$ARCHIVE/$release/main/installer-$arch/current/images"
+    echo "$images_url/netboot/ubuntu-installer/$arch/"
+}
+
+
+# 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
+# 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 url=$(compose_installer_download_url $arch $CURRENT_RELEASE)
+    if ! $DOWNLOAD $url/pxelinux.0
+    then
+        echo "No pre-boot loader for $arch; skipping."
+        return
+    fi
+
+    # If the file has changed, move it into place (replacing any previous
+    # version).  Otherwise, leave the filesystem alone.
+    if [ -f pxelinux.0 ]
+    then
+	if [ -f $dest/pxelinux.0 ] && cmp pxelinux.0 $dest/pxelinux.0
+	then
+	    rm -f -- pxelinux.0
+	else
+	    echo "Updating pre-boot loader for $arch."
+	    mv -- pxelinux.0 $dest/
+	fi
+    fi
+}
+
+
+# Move local directory $1 into directory $2, so that it becomes $2/$1.
+# If a directory of the same name already existed in $2, remove it along
+# with all its contents.
+# This will use "$2/$1.new" and "$2/$1.old" as temporary locations, which
+# will also be deleted.
+install_dir() {
+    local name=$1 dest=$2
+    # Use the "old"/"new" directories to maximize speed and minimize
+    # inconvenience: ensure that the new downloads are on the same
+    # partition, then move the old ones out of the way and immediately
+    # replace them with the new, and finally remove the old ones outside
+    # of the critical path.
+    # This will not resolve races with ongoing downloads by booting nodes.
+    # It merely minimizes this script's side of the race window.
+    rm -rf -- $dest/$name.old $dest/$name.new
+    # Put new downloads next to the old.  If any file copying is needed
+    # because directories are on different partitions, it happens here.
+    mv -- $name $dest/$name.new
+
+    # Start of the critical window.
+
+    # Move existing directory (if any) out of the way.
+    [ -d $dest/$name ] && mv -- $dest/$name $dest/$name.old
+
+    # Move new directory into place.
+    mv -- $dest/$name.new $dest/$name
+
+    # End of the critical window.
+
+    # Clean up the old directory (if any).
+    rm -rf -- $dest/$name.old
+}
+
+
+# Compare given file names between directories $1 and $2.  Print "yes" if
+# they are all identical, or "no" if any of them are different or missing.
+identical_files() {
+    local lhs=$1 rhs=$2
+    local file
+    shift 2
+    for file in $*
+    do
+        if ! cmp --quiet $lhs/$file $rhs/$file
+        then
+            echo "no"
+            return
+        fi
+    done
+    echo "yes"
+}
+
+
+# Download kernel/initrd for installing Ubuntu release $3 for
+# architecture $2 into directory $1/install.
+update_install_files() {
+    local dest=$1 arch=$2 release=$3
+    local files="initrd.gz linux"
+    local url=$(compose_installer_download_url $arch $release)
+    local file
+
+    mkdir "install"
+    pushd "install" >/dev/null
+    for file in $files
+    do
+        $DOWNLOAD $url/$file
+    done
+    popd >/dev/null
+
+    # TODO: Prevent this change from upsetting any currently booting nodes.
+    if [ $(identical_files "install" $dest $files) != 'yes' ]
+    then
+        echo "Updating files for $release-$arch."
+        install_dir "install" $dest
+    fi
+}
+
+
+main() {
+    local arch release arch_dir rel_dir
+
+    DOWNLOAD_DIR=$(mktemp -d)
+    echo "Downloading to temporary location $DOWNLOAD_DIR."
+    cd -- $DOWNLOAD_DIR
+
+    # All files we create here are public.  The TFTP user will need to be
+    # able to read them.
+    umask a+r
+
+    for arch in $ARCHES
+    do
+	# Replace the "base" with sub-architecture once we start supporting
+	# those.
+        arch_dir="$TFTPROOT/maas/$arch/base"
+
+	mkdir -p -- $arch_dir
+	update_pre_boot_loader $arch_dir $arch
+
+	for release in $RELEASES
+	do
+            rel_dir="$arch_dir/$release"
+            mkdir -p -- $rel_dir
+	    update_install_files $rel_dir $arch $release
+	done
+    done
+
+    rm -rf -- $DOWNLOAD_DIR
+}
+
+
+main

=== added file 'src/maas/tests/test_maas_import_pxe_files.py'
--- src/maas/tests/test_maas_import_pxe_files.py	1970-01-01 00:00:00 +0000
+++ src/maas/tests/test_maas_import_pxe_files.py	2012-06-15 09:34:23 +0000
@@ -0,0 +1,195 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the maas-import-pxe-files script."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+import os
+from stat import ST_MTIME
+from subprocess import check_call
+
+from maastesting.factory import factory
+from maastesting.testcase import TestCase
+from testtools.matchers import (
+    FileContains,
+    FileExists,
+    Not,
+    )
+
+
+def make_name(prefix, separator='-'):
+    """Return an arbitrary name, with the given prefix."""
+    return separator.join([prefix, factory.getRandomString(4)])
+
+
+def read_file(path, name):
+    """Return the contents of the file at `path/name`."""
+    with open(os.path.join(path, name)) as infile:
+        return infile.read()
+
+
+def get_write_time(path):
+    """Return last modification time of file at `path`."""
+    return os.stat(path)[ST_MTIME]
+
+
+def compose_download_dir(archive, arch, release):
+    """Locate a bootloader, initrd, and kernel in an 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.
+    :param release: Ubuntu release name.
+    :return: Full absolute path to the directory holding the requisite files
+        for this archive, arch, and release.
+    """
+    return os.path.join(
+        archive, release, 'main', 'installer-%s' % arch, 'current',
+        'images', 'netboot', 'ubuntu-installer', arch)
+
+
+def compose_tftp_path(tftproot, arch, *path):
+    """Compose path for MAAS TFTP files for given architecture.
+
+    After the TFTP root directory and the architecture, just append any path
+    components you want to drill deeper, e.g. the release name to get to the
+    files for a specific release.
+    """
+    return os.path.join(tftproot, 'maas', arch, 'base', *path)
+
+
+class TestImportPXEFiles(TestCase):
+
+    def make_downloads(self, release=None, arch=None):
+        """Set up a directory with an image for "download" by the script.
+
+        Returns an "ARCHIVE" URL for the download.
+        """
+        if release is None:
+            release = make_name('release')
+        if arch is None:
+            arch = make_name('arch')
+        archive = self.make_dir()
+        download = compose_download_dir(archive, arch, release)
+        os.makedirs(download)
+        for filename in ['initrd.gz', 'linux', 'pxelinux.0']:
+            factory.make_file(download, filename)
+        return archive
+
+    def call_script(self, archive_dir, tftproot, arch=None, release=None):
+        """Call the maas-download-pxe-files script with given settings.
+
+        The ARCHIVE URL and TFTPROOT path must be set, or the script will try
+        to download from the Ubuntu server and store into the system's real
+        TFTP root directory, respectively.  Both bad ideas.
+        """
+        script = './scripts/maas-import-pxe-files'
+        env = {
+            'ARCHIVE': 'file://%s' % archive_dir,
+            # Substitute curl for wget; it accepts file:// URLs.
+            'DOWNLOAD': 'curl -O --silent',
+            'TFTPROOT': tftproot,
+        }
+        if arch is not None:
+            env['ARCHES'] = arch
+        if release is not None:
+            env['RELEASES'] = release
+            env['CURRENT_RELEASE'] = release
+        with open('/dev/null', 'w') as dev_null:
+            check_call(script, env=env, stdout=dev_null)
+
+    def test_downloads_pre_boot_loader(self):
+        arch = 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')
+        download_path = compose_download_dir(archive, arch, release)
+        expected_contents = read_file(download_path, 'pxelinux.0')
+        self.assertThat(tftp_path, FileContains(expected_contents))
+
+    def test_ignores_missing_pre_boot_loader(self):
+        arch = make_name('arch')
+        release = 'precise'
+        archive = self.make_downloads(arch=arch, release=release)
+        download_path = compose_download_dir(archive, arch, release)
+        os.remove(os.path.join(download_path, 'pxelinux.0'))
+        tftproot = self.make_dir()
+        self.call_script(archive, tftproot, arch=arch, release=release)
+        tftp_path = compose_tftp_path(tftproot, arch, 'pxelinux.0')
+        self.assertThat(tftp_path, Not(FileExists()))
+
+    def test_updates_pre_boot_loader(self):
+        arch = make_name('arch')
+        release = 'precise'
+        tftproot = self.make_dir()
+        tftp_path = compose_tftp_path(tftproot, arch, 'pxelinux.0')
+        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, tftproot, arch=arch, release=release)
+        download_path = compose_download_dir(archive, arch, release)
+        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 = 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')
+        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 = 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, release, 'install', 'linux')
+        download_path = compose_download_dir(archive, arch, release)
+        expected_contents = read_file(download_path, 'linux')
+        self.assertThat(tftp_path, FileContains(expected_contents))
+
+    def test_updates_install_image(self):
+        arch = make_name('arch')
+        release = 'precise'
+        tftproot = self.make_dir()
+        tftp_path = compose_tftp_path(
+            tftproot, 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, tftproot, arch=arch, release=release)
+        download_path = compose_download_dir(archive, arch, release)
+        expected_contents = read_file(download_path, 'linux')
+        self.assertThat(tftp_path, FileContains(expected_contents))
+
+    def test_leaves_install_image_untouched_if_unchanged(self):
+        arch = 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, release, 'install', 'linux')
+        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))