Merge lp:~gmb/maas/fix-commissioning-page-distro-list-bug-1312844 into lp:~maas-committers/maas/trunk

[Graham Binns]

The "deploy" section of the settings page will now no longer show operating systems and releases for which MAAS doesn't have boot resources. Previously, the page listed options that couldn't be used due to a lack of boot resources.

The logic that I've added / tweaked works as follows:

OSes and releases will appear if:
 - There are boot images for the OS / release
 - The OS and release are supported according the OperatingSystemRegistry.

[MAAS Lander (maas-lander)]

Voting does not meet specified criteria. Required: Approve >= 1, Disapprove == 0. Got: 1 Pending.

[Gavin Panella (allenap)]

Looks good, but I have several comments.

Also, there probably ought to be a test to demonstrate that DeployForm has fields that reflect the current state of the system, rather than that as import time.

Rapheël mentioned using callables for parts of Django's form machinery. Did you investigate that?

[Gavin Panella (allenap)]

review: Approve

[Graham Binns (gmb)]

Thanks for the review and for sanity-checking me :)

On 4 June 2014 15:19, Gavin Panella <email address hidden> wrote:
> Looks good, but I have several comments.
>
> Also, there probably ought to be a test to demonstrate that DeployForm has fields that reflect the current state of the system, rather than that as import time.

Good point. I'll work on that.

> Rapheël mentioned using callables for parts of Django's form machinery. Did you investigate that?

Yeah, doesn't work (there's an open ticket for it in the Django Trac).
Well, it does for `initial` values, but not for `choices`. Because,
why would you want something that useful.

> Diff comments:
>
>> === modified file 'src/maasserver/forms.py'
>> --- src/maasserver/forms.py 2014-05-29 18:20:58 +0000
>> +++ src/maasserver/forms.py 2014-06-04 09:49:20 +0000
>> @@ -230,7 +230,10 @@
>> for nodegroup in NodeGroup.objects.all():
>> osystems = osystems.union(
>> BootImage.objects.get_usable_osystems(nodegroup))
>> - osystems = [OperatingSystemRegistry[osystem] for osystem in osystems]
>> + osystems = [
>> + OperatingSystemRegistry[osystem] for osystem in osystems
>> + if osystem in OperatingSystemRegistry
>
> I suspect this needs to be changed to query the cluster. All information on what OS/release combos could be supported are in the simplestream that describes boot images. The information on what OS/release combos that *can be supported right now* is in the list of boot resources downloaded. Both of those originate in the cluster, though the latter is also cached in BootImage.
>
> This doesn't need to be changed in this branch, but there ought to be a plan to address it.

Right. I'll file a bug once this is landed.

>> + ]
>> return sorted(osystems, key=lambda osystem: osystem.title)
>>
>>
>> @@ -1023,9 +1026,33 @@
>>
>> class DeployForm(ConfigForm):
>> """Settings page, Deploy section."""
>> +
>> default_osystem = get_config_field('default_osystem')
>> default_distro_series = get_config_field('default_distro_series')
>>
>> + def __init__(self, *args, **kwargs):
>> + super(DeployForm, self).__init__(*args, **kwargs)
>> + # There's a race condition of some descrption that causes an
>> + # error when walking the BootImage table at import time (as
>> + # get_usable{os|release}_choices ultimately do). Setting the
>> + # choices here is less expensive than trying to find the
>> + # underlying race, which might be a Django thing.
>> + usable_oses = list_all_usable_osystems()
>> + os_choices = list_osystem_choices(usable_oses)
>> + self.fields['default_osystem'].choices = os_choices
>> + self.fields['default_osystem'].error_messages = {
>> + 'invalid_choice': compose_invalid_choice_text(
>> + 'osystem', os_choices)
>> + }
>> +
>> + release_choices = list_release_choices(
>> + list_all_usable_releases(usable_oses))
>> + self.fields['default_distro_series'].choices = release_choices
>> + self.fields['default_distro_series'].error_messages = {
>> + 'invalid_choice':...

[MAAS Lander (maas-lander)]

The attempt to merge lp:~gmb/maas/fix-commissioning-page-distro-list-bug-1312844 into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Get:2 http://security.ubuntu.com trusty-security Release [58.5 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [58.5 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [17.0 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:6 http://security.ubuntu.com trusty-security/universe Sources [4,212 B]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [53.3 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [17.9 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Ign http://security.ubuntu.com trusty-security/main Translation-en_US
Ign http://security.ubuntu.com trusty-security/universe Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [52.4 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [29.7 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [136 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [79.5 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en_US
Fetched 509 kB in 0s (1,897 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make postgresql python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-formencode python-httpli...

[Julian Edwards (julian-edwards)]

On 05/06/14 00:19, Gavin Panella wrote:
> I suspect this needs to be changed to query the cluster. All information on what OS/release combos could be supported are in the simplestream that describes boot images. The information on what OS/release combos that *can be supported right now* is in the list of boot resources downloaded. Both of those originate in the cluster, though the latter is also cached in BootImage.
>
> This doesn't need to be changed in this branch, but there ought to be a plan to address it.

I have previously raised this with the original author (Blason!) and we
deferred it since the branch at the time was urgent.

So I would check with those guys to see if there's any plan in place. I
suspect not :)

But this is top of my tech-debt list for sure.

[Raphaël Badin (rvb)]

I've added a couple of inline comments… please wait before landing this, I need to check something first…

[Raphaël Badin (rvb)]

The OperatingSystem class has moved apparently, you need to do something like: http://paste.ubuntu.com/7593412/.

[Raphaël Badin (rvb)]

There is a problem with moving the fields's attributes into DeployForm's init method.

These fields (default_osystem and default_distro_series) are also used by the API *without* the DeployForm. This means that, with the change from this branch, you can't set default_osystem and default_distro_series through the API, because the list of choices for these fields is empty.

There is a (generic) test for this in src/maasserver/tests/test_api.py:MAASAPITest.test_admin_set_config. It's generic in the sense that it is not testing default_osystem or default_distro_series specifically but it's a good starting point if you want to recreate the problem I'm describing.

The code path I'm describing here is:
- API:set_config
- src/maasserver/api.py:MaasHandler.set_config()
- src/maasserver/forms_settings.py:get_config_form() (Creates an ad-hoc form with just one field, built from CONFIG_ITEMS)
- src/maasserver/forms_settings.py:get_config_field()

review: Needs Fixing

[Raphaël Badin (rvb)]

Looks good but I've got a couple of suggestions (see inline comments).

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~gmb/maas/fix-commissioning-page-distro-list-bug-1312844 into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Get:2 http://security.ubuntu.com trusty-security Release [58.5 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [58.5 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [17.1 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:6 http://security.ubuntu.com trusty-security/universe Sources [4,212 B]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [55.0 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [17.9 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Ign http://security.ubuntu.com trusty-security/main Translation-en_US
Ign http://security.ubuntu.com trusty-security/universe Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [57.8 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [30.1 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [142 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [80.4 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en_US
Fetched 524 kB in 0s (1,814 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make postgresql python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-formencode python-httpli...

[MAAS Lander (maas-lander)]

The attempt to merge lp:~gmb/maas/fix-commissioning-page-distro-list-bug-1312844 into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Get:2 http://security.ubuntu.com trusty-security Release [58.5 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [58.5 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [17.1 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:6 http://security.ubuntu.com trusty-security/universe Sources [4,212 B]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [55.0 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [17.9 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Ign http://security.ubuntu.com trusty-security/main Translation-en_US
Ign http://security.ubuntu.com trusty-security/universe Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [57.8 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [30.1 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [142 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [80.4 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en_US
Fetched 524 kB in 0s (1,808 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make postgresql python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-formencode python-httpli...

[Graham Binns (gmb)]

On 11 June 2014 07:52, MaaS Lander <email address hidden> wrote:
> FAIL: maasserver.tests.test_forms.TestDeployForm.test_uses_live_data

I can't get this to fail locally, annoyingly… Will
run-once-until-failure -it for a bit.

[Raphaël Badin (rvb)]

I also think it would be worth having an end-to-end API test to make sure one of the config settings you're changing here can still be set using the API. Don't need to test them all since the testing at the form-level covers that.

[MAAS Lander (maas-lander)]

The attempt to merge lp:~gmb/maas/fix-commissioning-page-distro-list-bug-1312844 into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Get:2 http://security.ubuntu.com trusty-security Release [58.5 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [58.5 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [17.1 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:6 http://security.ubuntu.com trusty-security/universe Sources [4,212 B]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [55.0 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [17.9 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Ign http://security.ubuntu.com trusty-security/main Translation-en_US
Ign http://security.ubuntu.com trusty-security/universe Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [57.8 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [30.1 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [142 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [80.4 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en_US
Fetched 524 kB in 0s (1,913 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make postgresql python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-formencode python-httpli...

[MAAS Lander (maas-lander)]

The attempt to merge lp:~gmb/maas/fix-commissioning-page-distro-list-bug-1312844 into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Get:2 http://security.ubuntu.com trusty-security Release [58.5 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [58.5 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [17.1 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:6 http://security.ubuntu.com trusty-security/universe Sources [4,212 B]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [55.0 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [17.9 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Ign http://security.ubuntu.com trusty-security/main Translation-en_US
Ign http://security.ubuntu.com trusty-security/universe Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [57.8 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [30.1 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [142 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [80.4 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en_US
Fetched 524 kB in 0s (1,742 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make postgresql python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-formencode python-httpli...

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2014-06-11 04:38:41 +0000
+++ src/maasserver/forms.py	2014-06-11 09:53:56 +0000
@@ -112,6 +112,14 @@
 from maasserver.utils import strip_domain
 from maasserver.utils.forms import compose_invalid_choice_text
 from maasserver.utils.network import make_network
+from maasserver.utils.os import (
+    get_distro_series_inital,
+    get_release_requires_key,
+    list_all_usable_osystems,
+    list_all_usable_releases,
+    list_osystem_choices,
+    list_release_choices,
+    )
 from metadataserver.fields import Bin
 from metadataserver.models import CommissioningScript
 from provisioningserver.drivers.osystem import OperatingSystemRegistry
@@ -215,99 +223,6 @@
         return all_architectures[0]
 
 
-def list_all_usable_osystems():
-    """Return all operating systems that can be used for nodes.
-
-    These are the operating systems for which any nodegroup has the boot images
-    required to boot the node.
-    """
-    # The Node edit form offers all usable operating systems as options for the
-    # osystem field.  Not all of these may be available in the node's
-    # nodegroup, but to represent that accurately in the UI would depend on
-    # the currently selected nodegroup.  Narrowing the options down further
-    # would have to happen browser-side.
-    osystems = set()
-    for nodegroup in NodeGroup.objects.all():
-        osystems = osystems.union(
-            BootImage.objects.get_usable_osystems(nodegroup))
-    osystems = [OperatingSystemRegistry[osystem] for osystem in osystems]
-    return sorted(osystems, key=lambda osystem: osystem.title)
-
-
-def list_osystem_choices(osystems):
-    """Return Django "choices" list for `osystem`."""
-    choices = [('', 'Default OS')]
-    choices += [
-        (osystem.name, osystem.title)
-        for osystem in osystems
-        ]
-    return choices
-
-
-def list_all_usable_releases(osystems):
-    """Return dictionary of usable `releases` for each opertaing system."""
-    distro_series = {}
-    nodegroups = list(NodeGroup.objects.all())
-    for osystem in osystems:
-        releases = set()
-        for nodegroup in nodegroups:
-            releases = releases.union(
-                BootImage.objects.get_usable_releases(nodegroup, osystem.name))
-        distro_series[osystem.name] = sorted(releases)
-    return distro_series
-
-
-def get_release_requires_key(osystem, release):
-    """Return astrisk for any release that requires
-    a license key.
-
-    This is used by the JS, to display the licese_key field.
-    """
-    if osystem.requires_license_key(release):
-        return '*'
-    return ''
-
-
-def list_release_choices(releases):
-    """Return Django "choices" list for `releases`."""
-    choices = [('', 'Default OS Release')]
-    for key, value in releases.items():
-        osystem = OperatingSystemRegistry[key]
-        options = osystem.format_release_choices(value)
-        requires_key = get_release_requires_key(osystem, '')
-        choices += [(
-            '%s/%s' % (osystem.name, requires_key),
-            'Latest %s Release' % osystem.title
-            )]
-        choices += [(
-            '%s/%s%s' % (
-                osystem.name,
-                name,
-                get_release_requires_key(osystem, name)
-                ),
-            title
-            )
-            for name, title in options
-            ]
-    return choices
-
-
-def get_distro_series_inital(instance):
-    """Returns the distro_series initial value for the instance."""
-    osystem = instance.osystem
-    series = instance.distro_series
-    os_obj = OperatingSystemRegistry.get_item(osystem)
-    if os_obj is not None:
-        key_required = get_release_requires_key(os_obj, series)
-    else:
-        key_required = ''
-    if osystem is not None and osystem != '':
-        if series is None:
-            series = ''
-        return '%s/%s%s' % (osystem, series, key_required)
-    return None
-
-
 def clean_distro_series_field(form, field, os_field):
     """Cleans the distro_series field in the form. Validating that
     the selected operating system matches the distro_series.
@@ -1080,13 +995,18 @@
 
 class DeployForm(ConfigForm):
     """Settings page, Deploy section."""
-    default_osystem = get_config_field('default_osystem')
-    default_distro_series = get_config_field('default_distro_series')
+
+    def __init__(self, *args, **kwargs):
+        Form.__init__(self, *args, **kwargs)
+        self.fields['default_osystem'] = get_config_field('default_osystem')
+        self.fields['default_distro_series'] = get_config_field(
+            'default_distro_series')
+        self._load_initials()
 
     def _load_initials(self):
         super(DeployForm, self)._load_initials()
-        initial_os = self.initial['default_osystem']
-        initial_series = self.initial['default_distro_series']
+        initial_os = self.fields['default_osystem'].initial
+        initial_series = self.fields['default_distro_series'].initial
         self.initial['default_distro_series'] = '%s/%s' % (
             initial_os,
             initial_series

=== modified file 'src/maasserver/forms_settings.py'
--- src/maasserver/forms_settings.py	2014-06-10 14:45:14 +0000
+++ src/maasserver/forms_settings.py	2014-06-11 09:53:56 +0000
@@ -23,43 +23,58 @@
 from socket import gethostname
 
 from django import forms
-from maasserver.models.config import DEFAULT_OS
+from maasserver.models.config import (
+    Config,
+    DEFAULT_OS,
+    )
 from maasserver.utils.forms import compose_invalid_choice_text
-from provisioningserver.drivers.osystem import OperatingSystemRegistry
+from maasserver.utils.os import (
+    list_all_usable_osystems,
+    list_all_usable_releases,
+    list_osystem_choices,
+    list_release_choices,
+    )
 
 
 INVALID_URL_MESSAGE = "Enter a valid url (e.g. http://host.example.com)."
 
 
-def list_osystem_choices():
-    osystems = [osystem for _, osystem in OperatingSystemRegistry]
-    osystems = sorted(osystems, key=lambda osystem: osystem.title)
-    return [
-        (osystem.name, osystem.title)
-        for osystem in osystems
-        ]
-
-
-def list_release_choices():
-    osystems = [osystem for _, osystem in OperatingSystemRegistry]
-    choices = []
-    for osystem in osystems:
-        supported = sorted(osystem.get_supported_releases())
-        options = osystem.format_release_choices(supported)
-        options = [
-            ('%s/%s' % (osystem.name, name), title)
-            for name, title in options
-            ]
-        choices += options
-    return choices
-
-
 def list_commisioning_choices():
     releases = DEFAULT_OS.get_supported_commissioning_releases()
     options = DEFAULT_OS.format_release_choices(releases)
     return list(options)
 
 
+def make_default_osystem_field(*args, **kwargs):
+    """Build and return the default_osystem field."""
+    usable_oses = list_all_usable_osystems()
+    os_choices = list_osystem_choices(usable_oses)
+    field = forms.ChoiceField(
+        initial = Config.objects.get_config('default_osystem'),
+        choices = os_choices,
+        error_messages = {
+            'invalid_choice': compose_invalid_choice_text(
+                'osystem', os_choices)
+        },
+        **kwargs)
+    return field
+
+
+def make_default_distro_series_field(*args, **kwargs):
+    usable_oses = list_all_usable_osystems()
+    release_choices = list_release_choices(
+        list_all_usable_releases(usable_oses))
+    field = forms.ChoiceField(
+        initial = Config.objects.get_config('default_distro_series'),
+        choices = release_choices,
+        error_messages = {
+            'invalid_choice': compose_invalid_choice_text(
+                'release', release_choices)
+        },
+        **kwargs)
+    return field
+
+
 CONFIG_ITEMS = {
     'check_compatibility': {
         'default': False,
@@ -162,32 +177,21 @@
         }
     },
     'default_osystem': {
-        'default': DEFAULT_OS.name,
-        'form': forms.ChoiceField,
+        'form': make_default_osystem_field,
         'form_kwargs': {
             'label': "Default operating system used for deployment",
-            'choices': list_osystem_choices(),
             'required': False,
-            'error_messages': {
-                'invalid_choice': compose_invalid_choice_text(
-                    'osystem',
-                    list_osystem_choices())},
+            # This field's `choices` and `error_messages` are populated
+            # at run-time to avoid a race condition.
         }
     },
     'default_distro_series': {
-        'default': '%s/%s' % (
-            DEFAULT_OS.name,
-            DEFAULT_OS.get_default_release()
-            ),
-        'form': forms.ChoiceField,
+        'form': make_default_distro_series_field,
         'form_kwargs': {
             'label': "Default OS release used for deployment",
-            'choices': list_release_choices(),
             'required': False,
-            'error_messages': {
-                'invalid_choice': compose_invalid_choice_text(
-                    'distro_series',
-                    list_release_choices())},
+            # This field's `choices` and `error_messages` are populated
+            # at run-time to avoid a race condition.
         }
     },
     'commissioning_distro_series': {

=== modified file 'src/maasserver/models/bootsourceselection.py'
--- src/maasserver/models/bootsourceselection.py	2014-06-10 14:45:14 +0000
+++ src/maasserver/models/bootsourceselection.py	2014-06-11 09:53:56 +0000
@@ -26,14 +26,6 @@
 from maasserver import DefaultMeta
 from maasserver.models.cleansave import CleanSave
 from maasserver.models.timestampedmodel import TimestampedModel
-from provisioningserver.drivers.osystem.ubuntu import UbuntuOS
-
-
-def list_release_choices():
-    """Return Django "choices" list for Ubuntu releases."""
-    osystem = UbuntuOS()
-    releases = osystem.get_supported_releases()
-    return osystem.format_release_choices(releases)
 
 
 class BootSourceSelectionManager(Manager):
@@ -51,8 +43,7 @@
     boot_source = ForeignKey('maasserver.BootSource', blank=False)
 
     release = CharField(
-        max_length=20, choices=list_release_choices(), blank=True,
-        default='',
+        max_length=20, blank=True, default='',
         help_text="The Ubuntu release for which to import resources.")
 
     arches = djorm_pgarray.fields.ArrayField(dbtype="text")

=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py	2014-06-11 04:38:41 +0000
+++ src/maasserver/testing/factory.py	2014-06-11 09:53:56 +0000
@@ -64,7 +64,10 @@
 # XXX 2014-05-13 blake-rouse bug=1319143
 # Need to not import directly, use RPC to info from cluster.
 from provisioningserver.drivers.osystem import OperatingSystemRegistry
-from provisioningserver.drivers.osystem.ubuntu import UbuntuOS
+from provisioningserver.drivers.osystem.ubuntu import (
+    UbuntuOS,
+    )
+from provisioningserver.testing.os import FakeOS
 
 # We have a limited number of public keys:
 # src/maasserver/tests/data/test_rsa{0, 1, 2, 3, 4}.pub
@@ -170,6 +173,19 @@
         finally:
             NODE_TRANSITIONS[None] = valid_initial_states
 
+    def make_operating_system(self, name=None, release=None):
+        """Make an :class:`OperatingSystem`."""
+        if name is None:
+            name = self.make_name('os')
+        if release is None:
+            release = self.make_name('release')
+        self.make_boot_image(release=release, osystem=name)
+        new_operating_system = FakeOS(
+            name=name, purpose="install", releases=[release])
+        OperatingSystemRegistry.register_item(
+            new_operating_system.name, new_operating_system)
+        return new_operating_system
+
     def make_node(self, mac=False, hostname=None, status=None,
                   architecture="i386/generic", updated=None,
                   created=None, nodegroup=None, routers=None, zone=None,

=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py	2014-06-11 04:38:41 +0000
+++ src/maasserver/tests/test_forms.py	2014-06-11 09:53:56 +0000
@@ -44,6 +44,7 @@
     CommissioningForm,
     CommissioningScriptForm,
     ConfigForm,
+    DeployForm,
     DownloadProgressForm,
     EditUserForm,
     get_action_form,
@@ -78,6 +79,7 @@
     ValidatorMultipleChoiceField,
     ZoneForm,
     )
+from provisioningserver.drivers.osystem import ubuntu
 from maasserver.models import (
     Config,
     MACAddress,
@@ -240,6 +242,28 @@
         self.assertEqual(
             [osystem], list_all_usable_osystems())
 
+    def test_list_all_usable_osystems_omits_oses_without_boot_images(self):
+        usable_os_name = factory.make_name('os')
+        unusable_os_name = factory.make_name('os')
+        self.make_usable_boot_images(osystem=usable_os_name)
+        usable_os = make_usable_osystem(self, usable_os_name)
+        unusable_os = make_usable_osystem(self, unusable_os_name)
+
+        usable_os_list = list_all_usable_osystems()
+        self.assertIn(usable_os, usable_os_list)
+        self.assertNotIn(unusable_os, usable_os_list)
+
+    def test_list_all_usable_osystems_omits_oses_not_supported(self):
+        usable_os_name = factory.make_name('os')
+        unusable_os_name = factory.make_name('os')
+        self.make_usable_boot_images(osystem=usable_os_name)
+        self.make_usable_boot_images(osystem=unusable_os_name)
+        usable_os = make_usable_osystem(self, usable_os_name)
+
+        usable_os_list = list_all_usable_osystems()
+        self.assertIn(usable_os, usable_os_list)
+        self.assertNotIn(unusable_os_name, [os.name for os in usable_os_list])
+
     def test_list_all_usable_releases_combines_nodegroups(self):
         expected = {}
         osystems = []
@@ -2146,3 +2170,39 @@
         self.assertTrue(form.is_valid(), form._errors)
         boot_source_selection = form.save()
         self.assertAttributes(boot_source_selection, params)
+
+
+class TestDeployForm(MAASServerTestCase):
+    """Tests for `DeployForm`."""
+
+    def test_uses_live_data(self):
+        # The DeployForm uses the database rather than just relying on
+        # hard-coded stuff.
+        os_name = factory.make_name('os')
+        release_name = factory.make_name('release')
+        factory.make_operating_system(
+            name=os_name, release=release_name)
+        release_name = "%s/%s" % (os_name, release_name)
+        deploy_form = DeployForm()
+        os_names = [
+            name for name, title in deploy_form.fields['default_osystem'].choices]
+        release_names = [
+            name for name, title in deploy_form.fields['default_distro_series'].choices]
+        self.assertIn(os_name, os_names)
+        self.assertIn(release_name, release_names)
+
+    def test_accepts_new_values(self):
+        os_name = 'ubuntu'
+        release_name = factory.make_name('release')
+        # XXX 2014-06-10 gmb:
+        #     We have to poke the release into the DISTRO_SERIES_CHOICES
+        #     dict, because otherwise it won't show up in the availble
+        #     releases on the DeployForm.
+        ubuntu.DISTRO_SERIES_CHOICES[release_name] = release_name.title()
+        factory.make_operating_system(name=os_name, release=release_name)
+        params = {
+            'default_osystem': os_name,
+            'default_distro_series': "%s/%s" % (os_name, release_name),
+            }
+        form = DeployForm(data=params)
+        self.assertTrue(form.is_valid())

=== added file 'src/maasserver/utils/os.py'
--- src/maasserver/utils/os.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/utils/os.py	2014-06-11 09:53:56 +0000
@@ -0,0 +1,126 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+"""Utilities for working with operating systems."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = [
+    'get_distro_series_inital',
+    'get_release_requires_key',
+    'list_all_usable_osystems',
+    'list_all_usable_releases',
+    'list_osystem_choices',
+    'list_release_choices',
+    ]
+
+
+from maasserver.models import (
+    BootImage,
+    NodeGroup,
+    )
+from provisioningserver.drivers.osystem import OperatingSystemRegistry
+
+
+def list_all_usable_osystems():
+    """Return all operating systems that can be used for nodes.
+
+    These are the operating systems for which any nodegroup has the boot images
+    required to boot the node.
+    """
+    # The Node edit form offers all usable operating systems as options for the
+    # osystem field.  Not all of these may be available in the node's
+    # nodegroup, but to represent that accurately in the UI would depend on
+    # the currently selected nodegroup.  Narrowing the options down further
+    # would have to happen browser-side.
+    osystems = set()
+    for nodegroup in NodeGroup.objects.all():
+        osystems = osystems.union(
+            BootImage.objects.get_usable_osystems(nodegroup))
+    osystems = [
+        OperatingSystemRegistry[osystem] for osystem in osystems
+        if osystem in OperatingSystemRegistry
+        ]
+    return sorted(osystems, key=lambda osystem: osystem.title)
+
+
+def list_osystem_choices(osystems):
+    """Return Django "choices" list for `osystem`."""
+    choices = [('', 'Default OS')]
+    choices += [
+        (osystem.name, osystem.title)
+        for osystem in osystems
+        ]
+    return choices
+
+
+def list_all_usable_releases(osystems):
+    """Return dictionary of usable `releases` for each opertaing system."""
+    distro_series = {}
+    nodegroups = list(NodeGroup.objects.all())
+    for osystem in osystems:
+        releases = set()
+        for nodegroup in nodegroups:
+            releases = releases.union(
+                BootImage.objects.get_usable_releases(nodegroup, osystem.name))
+        distro_series[osystem.name] = sorted(releases)
+    return distro_series
+
+
+def get_release_requires_key(osystem, release):
+    """Return astrisk for any release that requires
+    a license key.
+
+    This is used by the JS, to display the licese_key field.
+    """
+    if osystem.requires_license_key(release):
+        return '*'
+    return ''
+
+
+def list_release_choices(releases):
+    """Return Django "choices" list for `releases`."""
+    choices = [('', 'Default OS Release')]
+    for key, value in releases.items():
+        osystem = OperatingSystemRegistry[key]
+        options = osystem.format_release_choices(value)
+        requires_key = get_release_requires_key(osystem, '')
+        choices += [(
+            '%s/%s' % (osystem.name, requires_key),
+            'Latest %s Release' % osystem.title
+            )]
+        choices += [(
+            '%s/%s%s' % (
+                osystem.name,
+                name,
+                get_release_requires_key(osystem, name)
+                ),
+            title
+            )
+            for name, title in options
+            ]
+    return choices
+
+
+def get_distro_series_inital(instance):
+    """Returns the distro_series initial value for the instance."""
+    osystem = instance.osystem
+    series = instance.distro_series
+    os_obj = OperatingSystemRegistry.get_item(osystem)
+    if os_obj is not None:
+        key_required = get_release_requires_key(os_obj, series)
+    else:
+        key_required = ''
+    if osystem is not None and osystem != '':
+        if series is None:
+            series = ''
+        return '%s/%s%s' % (osystem, series, key_required)
+    return None
+
+

=== modified file 'src/maasserver/views/tests/test_settings.py'
--- src/maasserver/views/tests/test_settings.py	2014-05-29 18:20:58 +0000
+++ src/maasserver/views/tests/test_settings.py	2014-06-11 09:53:56 +0000
@@ -154,9 +154,11 @@
 
     def test_settings_deploy_POST(self):
         self.client_log_in(as_admin=True)
-        new_osystem = factory.getRandomOS()
+        osystem_name = factory.make_name('os')
+        release_name = factory.make_name('release')
+        new_osystem = factory.make_operating_system(
+            name=osystem_name, release=release_name)
         new_default_osystem = new_osystem.name
-        new_default_distro_series = factory.getRandomRelease(new_osystem)
         response = self.client.post(
             reverse('settings'),
             get_prefixed_form_data(
@@ -165,7 +167,7 @@
                     'default_osystem': new_default_osystem,
                     'default_distro_series': '%s/%s' % (
                         new_default_osystem,
-                        new_default_distro_series
+                        release_name,
                         ),
                 }))
 
@@ -173,7 +175,7 @@
         self.assertEqual(
             (
                 new_default_osystem,
-                new_default_distro_series,
+                release_name,
             ),
             (
                 Config.objects.get_config('default_osystem'),

=== modified file 'src/provisioningserver/boot/tests/test_tftppath.py'
--- src/provisioningserver/boot/tests/test_tftppath.py	2014-06-10 14:45:14 +0000
+++ src/provisioningserver/boot/tests/test_tftppath.py	2014-06-11 09:53:56 +0000
@@ -33,7 +33,6 @@
     locate_tftp_path,
     )
 from provisioningserver.drivers.osystem import (
-    OperatingSystem,
     OperatingSystemRegistry,
     )
 from provisioningserver.import_images.boot_image_mapping import (
@@ -48,6 +47,7 @@
     make_boot_image_storage_params,
     )
 from provisioningserver.testing.config import set_tftp_root
+from provisioningserver.testing.os import FakeOS
 from testtools.matchers import (
     Not,
     StartsWith,
@@ -55,40 +55,6 @@
 from testtools.testcase import ExpectedException
 
 
-class FakeOS(OperatingSystem):
-
-    name = ""
-    title = ""
-
-    def __init__(self, name, purpose, releases=None):
-        self.name = name
-        self.title = name
-        self.purpose = purpose
-        if releases is None:
-            self.fake_list = [
-                factory.getRandomString()
-                for _ in range(3)
-                ]
-        else:
-            self.fake_list = releases
-
-    def get_boot_image_purposes(self, *args):
-        return self.purpose
-
-    def get_supported_releases(self):
-        return self.fake_list
-
-    def get_default_release(self):
-        return self.fake_list[0]
-
-    def format_release_choices(self, releases):
-        return [
-            (release, release)
-            for release in releases
-            if release in self.fake_list
-            ]
-
-
 def make_image(params, purpose, metadata=None):
     """Describe an image as a dict similar to what `list_boot_images` returns.
 

=== added file 'src/provisioningserver/testing/os.py'
--- src/provisioningserver/testing/os.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/testing/os.py	2014-06-11 09:53:56 +0000
@@ -0,0 +1,54 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+"""Utilities for testing operating systems-related code."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = [
+    'FakeOS',
+    ]
+
+
+from maastesting.factory import factory
+from provisioningserver.drivers.osystem import OperatingSystem
+
+
+class FakeOS(OperatingSystem):
+
+    name = ""
+    title = ""
+
+    def __init__(self, name, purpose, releases=None):
+        self.name = name
+        self.title = name
+        self.purpose = purpose
+        if releases is None:
+            self.fake_list = [
+                factory.getRandomString()
+                for _ in range(3)
+                ]
+        else:
+            self.fake_list = releases
+
+    def get_boot_image_purposes(self, *args):
+        return self.purpose
+
+    def get_supported_releases(self):
+        return self.fake_list
+
+    def get_default_release(self):
+        return self.fake_list[0]
+
+    def format_release_choices(self, releases):
+        return [
+            (release, release)
+            for release in releases
+            if release in self.fake_list
+            ]