Merge lp:~jtv/maas/bug-1025582-task into lp:maas/trunk

[Jeroen T. Vermeulen]

Have the master worker report its available boot images.

This creates a new task for the master worker, and runs it on a 5-minute schedule. The task scans the TFTP filesystem tree (which is on the same system as the master worker) for boot images, and reports what it finds to the API. In a subsequent branch, the UI will report missing images as a persistent error.

See commit message. Pre-implementation call with Julian.

There's a lot of testing going on, especially for the helpers that drill down into multiple directories at once. It seems a bit weird in places but it was an effective way of getting a flat listing of all boot images' parameters. Any suggestions for tests that I could sensibly drop are welcome.

This branch is to be backported to 1.1.

Jeroen

[lp:~jtv/maas/bug-1025582-task]

999. By Jeroen T. Vermeulen on 2012-09-13

Merge review changes from pre-predecessor branch.

[Gavin Panella (allenap)]

> The task scans the TFTP filesystem tree (which is on the same system
> as the master worker) for boot images, ...

It /may/ be on the same filesystem, but there's no guarantee. The TFTP
filesystem tree needs to be mirrored to each cluster controller. I
guess where there's a worker there's a cluster controller, because
they're bundled as one?

This doesn't take into account future cluster controllers. The region
asks the cluster for a machine matching $constraints and then to put
$release on it; the cluster could decide to use an different,
hardware-tuned, image, instead of the one from cloud images. Though I
don't think this is a requirement right now, we need to bear a very
much more intelligence-at-the-edge approach in mind, imo.

[Julian Edwards (julian-edwards)]

On Thursday 13 September 2012 08:05:30 you wrote:
> > The task scans the TFTP filesystem tree (which is on the same system
> > as the master worker) for boot images, ...
>
> It /may/ be on the same filesystem, but there's no guarantee. The TFTP
> filesystem tree needs to be mirrored to each cluster controller. I
> guess where there's a worker there's a cluster controller, because
> they're bundled as one?

That is our implementation of the cluster controller, yes. So we can make
this guarantee.

> This doesn't take into account future cluster controllers. The region
> asks the cluster for a machine matching $constraints and then to put
> $release on it; the cluster could decide to use an different,
> hardware-tuned, image, instead of the one from cloud images. Though I
> don't think this is a requirement right now, we need to bear a very
> much more intelligence-at-the-edge approach in mind, imo.

It's the direction in which we're heading, yes. The only thing that can know
about this is the cluster (pserv) which is why it's reporting it via the API.

Everything else is an implementation detail.

[Julian Edwards (julian-edwards)]

FWIW I don't like the commit message, it seems like a lot of it should be in the description. It should really just state the change happening (and why). Also... conflicts :(

[lp:~jtv/maas/bug-1025582-task]

1000. By Jeroen T. Vermeulen on 2012-09-14

Merge trunk and conflict resolution from predecessor branch, resolve another conflict.

1001. By Jeroen T. Vermeulen on 2012-09-14

Revert some debugging changes.

[Julian Edwards (julian-edwards)]

Nice testing!

54 >>>>>>> MERGE-SOURCE

Stray merge marker.

277 + [arch, subarch, release, purpose] = path

You don't need the [ and ] here.

Is it always guaranteed to have four elements? Otherwise the unpacking will blow up.

278 + return {
279 + 'architecture': arch,
280 + 'subarchitecture': subarch,
281 + 'release': release,
282 + 'purpose': purpose,
283 + }

How about:

    return dict(
        architecture=arch, subarchitecture=subarch, release=release, purpose=purpose)

I also wonder if a named tuple might be better so you could do something like ``path.architecture``, but I'll leave it up to you.

310 === modified file 'src/provisioningserver/tasks.py'

[lots of imports]

I really think we should keep tasks.py very lean, and import functions from elsewhere that do the actual work. It'll keep things more structured, and then tasks.py becomes an inclusion point, a bit like models/__init__.py.

373 + task_logger.info("Not reporting boot images: don't have API URL yet.")
377 + task_logger.info("Not reporting boot images: don't have API key yet.")

I think these would be better as debugging logs.

review: Approve

[lp:~jtv/maas/bug-1025582-task]

1002. By Jeroen T. Vermeulen on 2012-09-14

Some review changes.

1003. By Jeroen T. Vermeulen on 2012-09-14

Review changes: move task out into its own module (occasioning creation of apiclient.testing), log missing info from server at debug level.

[Jeroen T. Vermeulen (jtv)]

> Nice testing!
>
> 54 >>>>>>> MERGE-SOURCE
>
> Stray merge marker.

I so hope those were separate comments and not sarcasm. :) The marker was just a bit of diff artifact inherited from a conflict in the parent branch.

> 277 + [arch, subarch, release, purpose] = path
>
> You don't need the [ and ] here.

Fixed.

> Is it always guaranteed to have four elements? Otherwise the unpacking will
> blow up.

Yes, it is — but I could have documented it. Now I have.

> 278 + return {
> 279 + 'architecture': arch,
> 280 + 'subarchitecture': subarch,
> 281 + 'release': release,
> 282 + 'purpose': purpose,
> 283 + }
>
> How about:
>
> return dict(
> architecture=arch, subarchitecture=subarch, release=release,
> purpose=purpose)

Done.

> I also wonder if a named tuple might be better so you could do something like
> ``path.architecture``, but I'll leave it up to you.

It's certainly tempting. In this particular case though I'm translating from a list to a dict — I don't think a named tuple would have made that much better.

> 310 === modified file 'src/provisioningserver/tasks.py'
>
> [lots of imports]
>
> I really think we should keep tasks.py very lean, and import functions from
> elsewhere that do the actual work. It'll keep things more structured, and
> then tasks.py becomes an inclusion point, a bit like models/__init__.py.

You're right. I was just too lazy to create a separate module for this — and it didn't quite fit in any that we had.

I've now given this its own module, and reduced the task test to a single integration test. Unfortunately one helper in test_tasks was now needed in too many places; I put it in a brand-new maasclient.testing.

> 373 + task_logger.info("Not reporting boot images: don't have API URL
> yet.")
> 377 + task_logger.info("Not reporting boot images: don't have API key
> yet.")
>
> I think these would be better as debugging logs.

Fair enough. Done.

Jeroen

[Jeroen T. Vermeulen (jtv)]

I've moved some text from the commit message to the description. Still experimenting with this new distribution of responsibilities between the two.

=== modified file 'etc/celeryconfig.py'
--- etc/celeryconfig.py	2012-08-24 08:39:18 +0000
+++ etc/celeryconfig.py	2012-09-14 11:06:18 +0000
@@ -72,4 +72,11 @@
         'task': 'provisioningserver.tasks.upload_dhcp_leases',
         'schedule': timedelta(minutes=1),
     },
+
+    # XXX JeroenVermeulen 2012-09-12, bug=1039366: this task should run
+    # only on the master worker.
+    'report-boot-images': {
+        'task': 'provisioningserver.report_boot_images',
+        'schedule': timedelta(minutes=5),
+    },
 }

=== added directory 'src/apiclient/testing'
=== added file 'src/apiclient/testing/__init__.py'
=== added file 'src/apiclient/testing/credentials.py'
--- src/apiclient/testing/credentials.py	1970-01-01 00:00:00 +0000
+++ src/apiclient/testing/credentials.py	2012-09-14 11:06:18 +0000
@@ -0,0 +1,26 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Testing facilities for API credentials."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = [
+    'make_api_credentials',
+    ]
+
+from maastesting.factory import factory
+
+
+def make_api_credentials():
+    """Create a tuple of fake API credentials."""
+    return (
+        factory.make_name('consumer-key'),
+        factory.make_name('resource-token'),
+        factory.make_name('resource-secret'),
+        )

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-09-14 10:10:43 +0000
+++ src/maasserver/tests/test_api.py	2012-09-14 11:06:18 +0000
@@ -108,6 +108,7 @@
     )
 from provisioningserver.kernel_opts import KernelParameters
 from provisioningserver.omshell import Omshell
+from provisioningserver.pxe import tftppath
 from testresources import FixtureResource
 from testtools.matchers import (
     Contains,
@@ -2698,6 +2699,10 @@
 
 class TestBootImagesAPI(APITestCase):
 
+    resources = (
+        ('celery', FixtureResource(CeleryFixture())),
+        )
+
     def report_images(self, images, client=None):
         if client is None:
             client = self.client
@@ -2751,6 +2756,17 @@
             (httplib.OK, "OK"),
             (response.status_code, response.content))
 
+    def test_worker_calls_report_boot_images(self):
+        refresh_worker(NodeGroup.objects.ensure_master())
+        self.patch(MAASClient, 'post')
+        self.patch(tftppath, 'list_boot_images', Mock(return_value=[]))
+
+        tasks.report_boot_images.delay()
+
+        MAASClient.post.assert_called_once_with(
+            reverse('boot_images_handler').lstrip('/'), 'report_boot_images',
+            images=json.dumps([]))
+
 
 class TestDescribe(AnonAPITestCase):
     """Tests for the `describe` view."""

=== added file 'src/provisioningserver/boot_images.py'
--- src/provisioningserver/boot_images.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/boot_images.py	2012-09-14 11:06:18 +0000
@@ -0,0 +1,68 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Dealing with boot images.
+
+Most of the lower-level logic is in the `tftppath` module, because it must
+correspond closely to the structure of the TFTP filesystem hierarchy.
+"""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = [
+    'report_to_server',
+    ]
+
+import json
+
+from apiclient.maas_client import (
+    MAASClient,
+    MAASDispatcher,
+    MAASOAuth,
+    )
+from provisioningserver.auth import (
+    get_recorded_api_credentials,
+    get_recorded_maas_url,
+    )
+from provisioningserver.config import Config
+from provisioningserver.logging import task_logger
+from provisioningserver.pxe import tftppath
+
+
+def get_cached_knowledge():
+    """Return cached items required to report to the server.
+
+    :return: Tuple of cached items: (maas_url, api_credentials).  Either may
+        be None if the information has not been received from the server yet.
+    """
+    maas_url = get_recorded_maas_url()
+    if maas_url is None:
+        task_logger.debug("Not reporting boot images: don't have API URL yet.")
+    api_credentials = get_recorded_api_credentials()
+    if api_credentials is None:
+        task_logger.debug("Not reporting boot images: don't have API key yet.")
+    return maas_url, api_credentials
+
+
+def submit(maas_url, api_credentials, images):
+    """Submit images to server."""
+    MAASClient(MAASOAuth(*api_credentials), MAASDispatcher(), maas_url).post(
+        'api/1.0/boot-images/', 'report_boot_images',
+        images=json.dumps(images))
+
+
+def report_to_server():
+    """For master worker only: report available netboot images."""
+    maas_url, api_credentials = get_cached_knowledge()
+    if not all([maas_url, api_credentials]):
+        return
+
+    images = tftppath.list_boot_images(
+        Config.load_from_cache()['tftp']['root'])
+
+    submit(maas_url, api_credentials, images)

=== modified file 'src/provisioningserver/pxe/tests/test_tftppath.py'
--- src/provisioningserver/pxe/tests/test_tftppath.py	2012-08-30 10:37:26 +0000
+++ src/provisioningserver/pxe/tests/test_tftppath.py	2012-09-14 11:06:18 +0000
@@ -21,6 +21,11 @@
     compose_bootloader_path,
     compose_config_path,
     compose_image_path,
+    drill_down,
+    extend_path,
+    is_visible_subdir,
+    list_boot_images,
+    list_subdirs,
     locate_tftp_path,
     )
 from provisioningserver.testing.config import ConfigFixture
@@ -38,6 +43,28 @@
         self.config = {"tftp": {"root": self.tftproot}}
         self.useFixture(ConfigFixture(self.config))
 
+    def make_boot_image_params(self):
+        """Create a dict of boot-image parameters, as in list_boot_images."""
+        return {
+            'architecture': factory.make_name('architecture'),
+            'subarchitecture': factory.make_name('subarchitecture'),
+            'release': factory.make_name('release'),
+            'purpose': factory.make_name('purpose'),
+        }
+
+    def make_image_dir(self, image_params, tftproot):
+        """Fake a boot image matching `image_params` under `tftproot`."""
+        image_dir = locate_tftp_path(
+            compose_image_path(
+                arch=image_params['architecture'],
+                subarch=image_params['subarchitecture'],
+                release=image_params['release'],
+                purpose=image_params['purpose']),
+            tftproot)
+        os.makedirs(image_dir)
+        factory.make_file(image_dir, 'linux')
+        factory.make_file(image_dir, 'initrd.gz')
+
     def test_compose_config_path_follows_maas_pxe_directory_layout(self):
         name = factory.make_name('config')
         self.assertEqual(
@@ -85,3 +112,110 @@
     def test_locate_tftp_path_returns_root_when_path_is_None(self):
         self.assertEqual(
             self.tftproot, locate_tftp_path(None, tftproot=self.tftproot))
+
+    def test_list_boot_images_copes_with_empty_directory(self):
+        self.assertItemsEqual([], list_boot_images(self.tftproot))
+
+    def test_list_boot_images_copes_with_unexpected_files(self):
+        os.makedirs(os.path.join(self.tftproot, factory.make_name('empty')))
+        factory.make_file(self.tftproot)
+        self.assertItemsEqual([], list_boot_images(self.tftproot))
+
+    def test_list_boot_images_finds_boot_image(self):
+        image = self.make_boot_image_params()
+        self.make_image_dir(image, self.tftproot)
+        self.assertItemsEqual([image], list_boot_images(self.tftproot))
+
+    def test_list_boot_images_enumerates_boot_images(self):
+        images = [self.make_boot_image_params() for counter in range(3)]
+        for image in images:
+            self.make_image_dir(image, self.tftproot)
+        self.assertItemsEqual(images, list_boot_images(self.tftproot))
+
+    def test_is_visible_subdir_ignores_regular_files(self):
+        plain_file = self.make_file()
+        self.assertFalse(
+            is_visible_subdir(
+                os.path.dirname(plain_file), os.path.basename(plain_file)))
+
+    def test_is_visible_subdir_ignores_hidden_directories(self):
+        base_dir = self.make_dir()
+        hidden_dir = factory.make_name('.')
+        os.makedirs(os.path.join(base_dir, hidden_dir))
+        self.assertFalse(is_visible_subdir(base_dir, hidden_dir))
+
+    def test_is_visible_subdir_recognizes_subdirectory(self):
+        base_dir = self.make_dir()
+        subdir = factory.make_name('subdir')
+        os.makedirs(os.path.join(base_dir, subdir))
+        self.assertTrue(is_visible_subdir(base_dir, subdir))
+
+    def test_list_subdirs_lists_empty_directory(self):
+        self.assertItemsEqual([], list_subdirs(self.make_dir()))
+
+    def test_list_subdirs_lists_subdirs(self):
+        base_dir = self.make_dir()
+        factory.make_file(base_dir, factory.make_name('plain-file'))
+        subdir = factory.make_name('subdir')
+        os.makedirs(os.path.join(base_dir, subdir))
+        self.assertItemsEqual([subdir], list_subdirs(base_dir))
+
+    def test_extend_path_finds_path_extensions(self):
+        base_dir = self.make_dir()
+        subdirs = [
+            factory.make_name('subdir-%d' % counter)
+            for counter in range(3)]
+        for subdir in subdirs:
+            os.makedirs(os.path.join(base_dir, subdir))
+        self.assertItemsEqual(
+            [[os.path.basename(base_dir), subdir] for subdir in subdirs],
+            extend_path(
+                os.path.dirname(base_dir), [os.path.basename(base_dir)]))
+
+    def test_extend_path_builds_on_given_paths(self):
+        base_dir = self.make_dir()
+        lower_dir = factory.make_name('lower')
+        subdir = factory.make_name('sub')
+        os.makedirs(os.path.join(base_dir, lower_dir, subdir))
+        self.assertEqual(
+            [[lower_dir, subdir]],
+            extend_path(base_dir, [lower_dir]))
+
+    def test_extend_path_stops_if_no_subdirs_found(self):
+        self.assertItemsEqual([], extend_path(self.make_dir(), []))
+
+    def test_drill_down_follows_directory_tree(self):
+        base_dir = self.make_dir()
+        lower_dir = factory.make_name('lower')
+        os.makedirs(os.path.join(base_dir, lower_dir))
+        subdirs = [
+            factory.make_name('subdir-%d' % counter)
+            for counter in range(3)]
+        for subdir in subdirs:
+            os.makedirs(os.path.join(base_dir, lower_dir, subdir))
+        self.assertItemsEqual(
+            [[lower_dir, subdir] for subdir in subdirs],
+            drill_down(base_dir, [[lower_dir]]))
+
+    def test_drill_down_ignores_subdir_not_in_path(self):
+        base_dir = self.make_dir()
+        irrelevant_dir = factory.make_name('irrelevant')
+        irrelevant_subdir = factory.make_name('subdir')
+        relevant_dir = factory.make_name('relevant')
+        relevant_subdir = factory.make_name('subdir')
+        os.makedirs(os.path.join(base_dir, irrelevant_dir, irrelevant_subdir))
+        os.makedirs(os.path.join(base_dir, relevant_dir, relevant_subdir))
+        self.assertEqual(
+            [[relevant_dir, relevant_subdir]],
+            drill_down(base_dir, [[relevant_dir]]))
+
+    def test_drill_down_drops_paths_that_do_not_go_deep_enough(self):
+        base_dir = self.make_dir()
+        shallow_dir = factory.make_name('shallow')
+        os.makedirs(os.path.join(base_dir, shallow_dir))
+        deep_dir = factory.make_name('deep')
+        subdir = factory.make_name('sub')
+        os.makedirs(os.path.join(base_dir, deep_dir, subdir))
+        self.assertEqual(
+            [[deep_dir, subdir]],
+            drill_down(base_dir, [[shallow_dir], [deep_dir]]))

=== modified file 'src/provisioningserver/pxe/tftppath.py'
--- src/provisioningserver/pxe/tftppath.py	2012-08-30 10:50:31 +0000
+++ src/provisioningserver/pxe/tftppath.py	2012-09-14 11:06:18 +0000
@@ -14,6 +14,7 @@
     'compose_bootloader_path',
     'compose_config_path',
     'compose_image_path',
+    'list_boot_images',
     'locate_tftp_path',
     ]
 
@@ -83,3 +84,86 @@
     if path is None:
         return tftproot
     return os.path.join(tftproot, path.lstrip('/'))
+
+
+def is_visible_subdir(directory, subdir):
+    """Is `subdir` a non-hidden sub-directory of `directory`?"""
+    if subdir.startswith('.'):
+        return False
+    else:
+        return os.path.isdir(os.path.join(directory, subdir))
+
+
+def list_subdirs(directory):
+    """Return a list of non-hidden directories in `directory`."""
+    return [
+        subdir
+        for subdir in os.listdir(directory)
+            if is_visible_subdir(directory, subdir)]
+
+
+def extend_path(directory, path):
+    """Dig one directory level deeper on `os.path.join(directory, *path)`.
+
+    If `path` is a list of consecutive path elements drilling down from
+    `directory`, return a list of sub-directory paths leading one step
+    further down.
+
+    :param directory: Base directory that `path` is relative to.
+    :param path: A path to a subdirectory of `directory`, represented as
+        a list of path elements relative to `directory`.
+    :return: A list of paths that go one sub-directory level further
+        down from `path`.
+    """
+    return [
+        path + [subdir]
+        for subdir in list_subdirs(os.path.join(directory, *path))]
+
+
+def drill_down(directory, paths):
+    """Find the extensions of `paths` one level deeper into the filesystem.
+
+    :param directory: Base directory that each path in `paths` is relative to.
+    :param paths: A list of "path lists."  Each path list is a list of
+        path elements drilling down into the filesystem from `directory`.
+    :return: A list of paths, each of which drills one level deeper down into
+        the filesystem hierarchy than the originals in `paths`.
+    """
+    return sum([extend_path(directory, path) for path in paths], [])
+
+
+def extract_image_params(path):
+    """Represent a list of TFTP path elements as a boot-image dict.
+
+    The path must consist of a full [architecture, subarchitecture, release,
+    purpose] that identify a kind of boot that we may need an image for.
+    """
+    arch, subarch, release, purpose = path
+    return dict(
+        architecture=arch, subarchitecture=subarch, release=release,
+        purpose=purpose)
+
+
+def list_boot_images(tftproot):
+    """List the available boot images.
+
+    :param tftproot: TFTP root directory.
+    :return: An iterable of dicts, describing boot images as consumed by
+        the report_boot_images API call.
+    """
+    # The sub-directories directly under tftproot, if they contain
+    # images, represent architectures.
+    potential_archs = list_subdirs(tftproot)
+
+    # Starting point for iteration: paths that contain only the
+    # top-level subdirectory of tftproot, i.e. the architecture name.
+    paths = [[subdir] for subdir in potential_archs]
+
+    # Extend paths deeper into the filesystem, through the levels that
+    # represent sub-architecture, release, and purpose.  Any directory
+    # that doesn't extend this deep isn't a boot image.
+    for level in ['subarch', 'release', 'purpose']:
+        paths = drill_down(tftproot, paths)
+
+    # Each path we find this way should be a boot image.
+    return [extract_image_params(path) for path in paths]

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2012-09-10 14:27:00 +0000
+++ src/provisioningserver/tasks.py	2012-09-14 11:06:18 +0000
@@ -32,6 +32,7 @@
 
 from celery.task import task
 from celeryconfig import DHCP_CONFIG_FILE
+from provisioningserver import boot_images
 from provisioningserver.auth import (
     record_api_credentials,
     record_maas_url,
@@ -320,3 +321,14 @@
 def restart_dhcp_server():
     """Restart the DHCP server."""
     check_call(['sudo', 'service', 'isc-dhcp-server', 'restart'])
+
+
+# =====================================================================
+# Boot images-related tasks
+# =====================================================================
+
+
+@task
+def report_boot_images():
+    """For master worker only: report available netboot images."""
+    boot_images.report_to_server()

=== modified file 'src/provisioningserver/tests/test_auth.py'
--- src/provisioningserver/tests/test_auth.py	2012-09-10 14:27:00 +0000
+++ src/provisioningserver/tests/test_auth.py	2012-09-14 11:06:18 +0000
@@ -13,6 +13,7 @@
 __all__ = []
 
 from apiclient.creds import convert_tuple_to_string
+from apiclient.testing.credentials import make_api_credentials
 from maastesting.factory import factory
 from provisioningserver import (
     auth,
@@ -21,25 +22,16 @@
 from provisioningserver.testing.testcase import PservTestCase
 
 
-def make_credentials():
-    """Produce a tuple of API credentials."""
-    return (
-        factory.make_name('consumer-key'),
-        factory.make_name('resource-token'),
-        factory.make_name('resource-secret'),
-        )
-
-
 class TestAuth(PservTestCase):
 
     def test_record_api_credentials_records_credentials_string(self):
-        creds_string = convert_tuple_to_string(make_credentials())
+        creds_string = convert_tuple_to_string(make_api_credentials())
         auth.record_api_credentials(creds_string)
         self.assertEqual(
             creds_string, cache.cache.get(auth.API_CREDENTIALS_CACHE_KEY))
 
     def test_get_recorded_api_credentials_returns_credentials_as_tuple(self):
-        creds = make_credentials()
+        creds = make_api_credentials()
         auth.record_api_credentials(convert_tuple_to_string(creds))
         self.assertEqual(creds, auth.get_recorded_api_credentials())
 

=== added file 'src/provisioningserver/tests/test_boot_images.py'
--- src/provisioningserver/tests/test_boot_images.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/tests/test_boot_images.py	2012-09-14 11:06:18 +0000
@@ -0,0 +1,80 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for reporting of boot images."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+import json
+
+from apiclient.maas_client import MAASClient
+from apiclient.testing.credentials import make_api_credentials
+from maastesting.factory import factory
+from mock import Mock
+from provisioningserver import boot_images
+from provisioningserver.auth import (
+    record_api_credentials,
+    record_maas_url,
+    )
+from provisioningserver.pxe import tftppath
+from provisioningserver.testing.config import ConfigFixture
+from provisioningserver.testing.testcase import PservTestCase
+
+
+class TestBootImagesTasks(PservTestCase):
+
+    def setUp(self):
+        super(TestBootImagesTasks, self).setUp()
+        self.useFixture(ConfigFixture({'tftp': {'root': self.make_dir()}}))
+
+    def make_image_params(self):
+        """Create a dict of parameters describing a boot image."""
+        return {
+            'architecture': factory.make_name('architecture'),
+            'subarchitecture': factory.make_name('subarchitecture'),
+            'release': factory.make_name('release'),
+            'purpose': factory.make_name('purpose'),
+        }
+
+    def set_maas_url(self):
+        record_maas_url(
+            'http://127.0.0.1/%s' % factory.make_name('path'))
+
+    def set_api_credentials(self):
+        record_api_credentials(':'.join(make_api_credentials()))
+
+    def test_sends_boot_images_to_server(self):
+        self.set_maas_url()
+        self.set_api_credentials()
+        image = self.make_image_params()
+        self.patch(tftppath, 'list_boot_images', Mock(return_value=[image]))
+        self.patch(MAASClient, 'post')
+
+        boot_images.report_to_server()
+
+        self.assertItemsEqual(
+            [image],
+            json.loads(MAASClient.post.call_args[1]['images']))
+
+    def test_does_nothing_without_maas_url(self):
+        self.set_api_credentials()
+        self.patch(
+            tftppath, 'list_boot_images',
+            Mock(return_value=self.make_image_params()))
+        boot_images.report_to_server()
+        self.assertItemsEqual([], tftppath.list_boot_images.call_args_list)
+
+    def test_does_nothing_without_credentials(self):
+        self.set_maas_url()
+        self.patch(
+            tftppath, 'list_boot_images',
+            Mock(return_value=self.make_image_params()))
+        boot_images.report_to_server()
+        self.assertItemsEqual([], tftppath.list_boot_images.call_args_list)

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2012-09-12 09:46:16 +0000
+++ src/provisioningserver/tests/test_tasks.py	2012-09-14 11:06:18 +0000
@@ -13,6 +13,7 @@
 __all__ = []
 
 from datetime import datetime
+import json
 import os
 import random
 from subprocess import (
@@ -21,6 +22,8 @@
     )
 
 from apiclient.creds import convert_tuple_to_string
+from apiclient.maas_client import MAASClient
+from apiclient.testing.credentials import make_api_credentials
 from celeryconfig import DHCP_CONFIG_FILE
 from maastesting.celery import CeleryFixture
 from maastesting.factory import factory
@@ -49,6 +52,7 @@
     )
 from provisioningserver.enum import POWER_TYPE
 from provisioningserver.power.poweraction import PowerActionFail
+from provisioningserver.pxe import tftppath
 from provisioningserver.tasks import (
     add_new_dhcp_host_map,
     Omshell,
@@ -56,6 +60,7 @@
     power_on,
     refresh_secrets,
     remove_dhcp_host_map,
+    report_boot_images,
     restart_dhcp_server,
     rndc_command,
     RNDC_COMMAND_MAX_RETRY,
@@ -66,6 +71,7 @@
     write_full_dns_config,
     )
 from provisioningserver.testing import network_infos
+from provisioningserver.testing.config import ConfigFixture
 from provisioningserver.testing.testcase import PservTestCase
 from testresources import FixtureResource
 from testtools.matchers import (
@@ -103,11 +109,7 @@
         self.assertEqual(maas_url, auth.get_recorded_maas_url())
 
     def test_updates_api_credentials(self):
-        credentials = (
-            factory.make_name('key'),
-            factory.make_name('token'),
-            factory.make_name('secret'),
-            )
+        credentials = make_api_credentials()
         refresh_secrets(
             api_credentials=convert_tuple_to_string(credentials))
         self.assertEqual(credentials, auth.get_recorded_api_credentials())
@@ -423,3 +425,33 @@
                     FileExists(),
                     FileExists(),
                 )))
+
+
+class TestBootImagesTasks(PservTestCase):
+
+    resources = (
+        ("celery", FixtureResource(CeleryFixture())),
+        )
+
+    def make_image_params(self):
+        """Create a dict of parameters describing a boot image."""
+        return {
+            'architecture': factory.make_name('architecture'),
+            'subarchitecture': factory.make_name('subarchitecture'),
+            'release': factory.make_name('release'),
+            'purpose': factory.make_name('purpose'),
+        }
+
+    def test_sends_boot_images_to_server(self):
+        self.useFixture(ConfigFixture({'tftp': {'root': self.make_dir()}}))
+        auth.record_maas_url('http://127.0.0.1/%s' % factory.make_name('path'))
+        auth.record_api_credentials(':'.join(make_api_credentials()))
+        image = self.make_image_params()
+        self.patch(tftppath, 'list_boot_images', Mock(return_value=[image]))
+        self.patch(MAASClient, 'post')
+
+        report_boot_images.delay()
+
+        self.assertItemsEqual(
+            [image],
+            json.loads(MAASClient.post.call_args[1]['images']))