Merge lp:~blake-rouse/maas/add-osystem-to-node-form-api into lp:~maas-committers/maas/trunk

Proposed by Blake Rouse
Status: Merged
Approved by: Blake Rouse
Approved revision: no longer in the source branch.
Merged at revision: 2383
Proposed branch: lp:~blake-rouse/maas/add-osystem-to-node-form-api
Merge into: lp:~maas-committers/maas/trunk
Prerequisite: lp:~blake-rouse/maas/add-osystem-to-node-migration
Diff against target: 1355 lines (+572/-114)
15 files modified
src/maasserver/api.py (+33/-9)
src/maasserver/forms.py (+173/-13)
src/maasserver/forms_settings.py (+55/-15)
src/maasserver/models/config.py (+9/-3)
src/maasserver/models/node.py (+20/-11)
src/maasserver/models/tests/test_node.py (+13/-11)
src/maasserver/preseed.py (+1/-1)
src/maasserver/testing/osystems.py (+12/-4)
src/maasserver/tests/test_api_node.py (+24/-11)
src/maasserver/tests/test_api_pxeconfig.py (+29/-4)
src/maasserver/tests/test_forms.py (+150/-4)
src/maasserver/tests/test_preseed.py (+11/-15)
src/maasserver/views/settings.py (+9/-0)
src/maasserver/views/tests/test_settings.py (+32/-12)
src/metadataserver/tests/test_api.py (+1/-1)
To merge this branch: bzr merge lp:~blake-rouse/maas/add-osystem-to-node-form-api
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Approve
Jason Hobbs (community) Approve
Review via email: mp+219521@code.launchpad.net

Commit message

Add operating system selection to NodeForm, and os selection from distro series in start api call.

Description of the change

This replaces the large merge of add-osystem-to-node, and separates it into 3 different mergers. This is the second one.

The start node API uses the same distro_series options so that the api call does not change. The osystem is selected based on the operating system that is found to contain that distro_series/release. The valid choices that get displayed in the WebUI are based on the BootImage's that are available, removing the ability for a user to select an operating system and release combination that would not boot.

Note: osystem was used throughout the code instead of os, as the python os module would conflict throughout the code base.

To post a comment you must log in.
Revision history for this message
Jason Hobbs (jason-hobbs) wrote :
Download full text (12.3 KiB)

I am new to a lot of this part of MAAS and as a result, this review is probably more of the trees than of the forest. Over all it looks good to me, but I do have some questions and comments.

[1] Needs make format/make lint.

[2]

@@ -403,7 +407,7 @@
             available to the nodes through the metadata service.
         :type user_data: base64-encoded unicode
         :param distro_series: If present, this parameter specifies the
- Ubuntu Release the node will use.
+ os relase the node will use.

Typo 'relase'. Maybe s/os/OS too.

[3]

-def get_boot_purpose(node):
+def get_boot_purpose(node, osystem, arch, subarch, series, label):
     """Return a suitable "purpose" for this boot, e.g. "install"."""
     # XXX: allenap bug=1031406 2012-07-31: The boot purpose is still in
     # flux. It may be that there will just be an "ephemeral" environment and
@@ -2364,7 +2368,15 @@
             if node.should_use_traditional_installer():
                 return "install"
             else:
- return "xinstall"
+ # Check that the booting operating system, actually supports
+ # fast-path installation. If it does not then, we need to
+ # return normal install.
+ osystem_obj = OperatingSystemRegistry[osystem]
+ purposes = osystem_obj.get_boot_image_purposes(
+ arch, subarch, series, label)
+ if BOOT_IMAGE_PURPOSE.XINSTALL in purposes:
+ return "xinstall"
+ return "install"

Why isn't this error state handled when we configure the node? It is
confusing to have a node configured for xinstall, but to have it really
use install behind the scenes.

[4]
+ # Get the purpose, checking that if node is using xinstall, the operating
+ # system actaully supports that mode. We pass None for the label, here
+ # because it is unknown which label will be used yet.
+ purpose = get_boot_purpose(node, osystem, arch, subarch, series, None)

Same comment as [3].

Also, don't need the comma between 'label' and 'here'.

[5]
     label = get_optional_param(request.GET, 'label', latest_label)

+ # Now that we have the correct label, lets check the boot purpose again
+ # to make sure that the boot image with that label, is the correct purpose.
+ purpose = get_boot_purpose(node, osystem, arch, subarch, series, label)

Same comment as [3] again - didn't need to grab purpose more than once
with the previous implementation.

Also, s/lets/let's and drop comma between 'label' and 'is'.

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2014-05-14 07:12:10 +0000
+++ src/maasserver/forms.py 2014-05-14 14:40:00 +0000
@@ -71,9 +71,6 @@
     )
 from maasserver.config_forms import SKIP_CHECK_NAME
 from maasserver.enum import (
- COMMISSIONING_DISTRO_SERIES_CHOICES,
- DISTRO_SERIES,
- DISTRO_SERIES_CHOICES,
     NODE_STATUS,
     NODEGROUPINTERFACE_MANAGEMENT,
     NODEGROUPINTERFACE_MANAGEMENT_CHOICES,
@@ -86,7 +83,7 @@
 from maasserver.forms_settings import (
     CONFIG_ITEMS_KEYS,
     get_config_field,
- INVALID_DISTRO_SERIES_MESSAGE,
+ list_commisioni...

review: Needs Information
Revision history for this message
Blake Rouse (blake-rouse) wrote :
Download full text (3.1 KiB)

[1,2] Fixed

[3] Why fail a node from working? What is the downside of using install instead of xinstall, you still end up with a system that is similar after finish installation, its just speed. So if the operating system installs using fast-path great, if not then use the old way. Also since fast-path booting is only set by a tag, then it makes it even harder to catch. As you wouldn't see the error until you go to turn on the node, at which case it has already been acquired and it will fail, causing JuJu to fail.

[4,5] Comments fixed.
[5] You need to grab the purpose first only because the boot label is unknown on the first call. So first it gets the purpose without the label, then once the correct label is select. Just to check the label supports that purpose we call again. Its just a good check to make sure that the final selected boot image can handle this method.

[6] Early out used!
[6] It needs to handle two form inputs as the OS selection is used with NodeForm, and DeployForm. Didn't want to duplicate the code, when it would be the same so I split it out into a method that can handle multiple forms.

[7,8] Fixed

[9] Merge conflict resolved

[10] Its always a good check to make sure that it is not None, so I check. It really would only be a blank string, but doesn't hurt to just make sure. Being explicit.

[11] Split into two tests.

[12] This is testing that the set method is actually still being called. So if the code was to change where it wasn't this would break the API and the test would fail.

[13] This actually uses a different method on Node, release. The release call resets the osystem and distro_series. Also a good check to make sure that the release call is called.

[14] Belief that it makes it easier to see that the test is testing it multiple times.

[15] Differ in one tests that the items are there, and the other tests that the items are sorted. This is a copy of how the architecture code is tested.

[16]
Added test for maasserver.api.get_boot_purpose() on return "install" when os doesn't support "xinstall".

NodeForm.set_up_osystem_and_distro_series_fields() and maasserver.forms.clean_distro_series_field() is being tested in maasserver.tests.test_forms.TestNodeForm by the following:
  - test_contains_limited_set_of_fields
  - test_accepts_osystem
  - test_rejects_invalid_osystem
  - test_starts_with_default_osystem
  - test_accepts_osystem_distro_series
  - test_rejects_invalid_osystem_distro_series
  - test_starts_with_default_distro_series
  - test_rejects_mismatch_osystem_distro_series

maasserver.models.node.validate_osystem() and maasserver.models.node.validate_distro_series() are getting tested with the TestNodeForm and the start API call.

These are getting covered by the usage of the forms, views, and api throughout the testing code base, as you stated.
  - maasserver.forms.list_release_choices()
  - maasserver.forms.list_osystem_choices()
  - NodeForm.clean_distro_series()
  - maasserver.form_settings.list_osystem_choices()
  - maasserver.form_settings.list_release_choices()
  - maasserver.form_settings.list_commissioning_choices()
  - maasserver.models.node.get_all_releases()
  - maasserver.models.node.get_o...

Read more...

Revision history for this message
Jason Hobbs (jason-hobbs) wrote :

Thanks! A couple more comments inline below.

On Thu, May 15, 2014 at 4:22 PM, Blake Rouse <email address hidden>wrote:
>
> [3] Why fail a node from working?

I don't mean that we should fail the node instead of reverting to install,
but that we should never be able to get a Node into the state where it's
configured to xinstall with an OS that can't xinstall. Ideally we would
raise a validation error whenever someone configured a node to xinstall
with an OS/release that couldn't xinstall. Maybe it's harder than it should
be because we use a tag to indicate xinstall use.

> What is the downside of using install instead of xinstall, you still end
> up with a system that is similar after finish installation, its just speed.

What if you have a modified preseed or pxeconfig for xinstall, with stuff
that's critical in it? Or a feature that is enabled for xinstall but not
install (which will likely happen with debian-installer being deprecated).
It seems fragile to assume that they are now and will forever be equivalent
other than speed.

>
> [7,8] Fixed
>

You may want to double check the new validate_osystem() - I think you need
to invert your logic for using the early out way. If it's passing tests as
is I would also double check those :).

>
> [9] Merge conflict resolved
>
> [10] Its always a good check to make sure that it is not None, so I check.
> It really would only be a blank string, but doesn't hurt to just make sure.
> Being explicit.
>
> [11] Split into two tests.
>
> [12] This is testing that the set method is actually still being called.
> So if the code was to change where it wasn't this would break the API and
> the test would fail.
>
> [13] This actually uses a different method on Node, release. The release
> call resets the osystem and distro_series. Also a good check to make sure
> that the release call is called.
>
> [14] Belief that it makes it easier to see that the test is testing it
> multiple times.
>
> [15] Differ in one tests that the items are there, and the other tests
> that the items are sorted. This is a copy of how the architecture code is
> tested.
>

Yes, the difference just makes them more fun to refactor though :).

Jason

Revision history for this message
Jason Hobbs (jason-hobbs) :
review: Approve
Revision history for this message
Blake Rouse (blake-rouse) wrote :

Nice catch, validate_osystem was incorrect and was not being tested for a bad value. Added a test for bad value for validate_osystem and validate_distro_series.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

I'm going to review this in detail, but time has run out for me today; I'll have to get back to it tomorrow. Smaller branches have less of this problem!

For the record, I agree with Jason that it's better to fail early than tohide potentially problematic settings. But of course that's assuming that we can reasonably catch the problem at the right stage. Once we're trying to boot a node, it's probably too late.

As for the get_boot_purpose call where the label is None, I'd suggest passing it as a keyword for clarity: label=None.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Damn those big branches! It's good, but far too big. I'm attaching inline review notes for the first 500 lines or so, up to (but not including) src/maasserver/models/node.py. More tomorrow...

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

I just reviewed src/maasserver/models/node.py as well (again using inline comments) and I hit one sticking point: I don't think validation of a node's operating system and release against the database belongs on the model. The model can validate for basic sanity of the input string, but I feel the checks for supported systems/releases belongs at the form level.

If these were proper foreign keys then yes, clearly it's a model integrity issue. But then we'd also have model-level protection against osystems being deleted while nodes still refer to them. We would have the full stack of referential-integrity mechanisms to do the work for us, efficiently and reliably. But where possible I would avoid model validation that needs to query the database.

As it is, don't these validators break tests? I would expect factory.make_node to set a random operating system and release, and unless you surprised the caller by also creating boot images (probably breaking other tests), the validators would wreak havoc on the test suite.

Revision history for this message
Blake Rouse (blake-rouse) wrote :

osystem has validation in the model, as this field can also be set with the start API call. distro_series always had validation using the choices=, but now its just using a function to perform the validation. It gives the same result.

factory.make_node does not break tests, as the osystem field just like the distro_series field was can be blank. If the field is blank then the default is used, so no tests break.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

One of our architecture decisions is that the API should validate through the forms, so validating in the form should also cover the API.

We normally like our factories to produce randomised fields, not reliable blanks. (If a field can be blank, it can be useful to flip a coin first — heads, leave blank; tails, make up random value.)

Revision history for this message
Blake Rouse (blake-rouse) wrote :

I was just implementing it the exact way it was implemented before, I just
added a new field osystem. If you would like me to implement it
differently, then I can. Let me know what you would like me to do. I can do
this in a later branch, as this one long enough. :)

On Thu, May 22, 2014 at 10:15 AM, Jeroen T. Vermeulen <email address hidden>wrote:

> One of our architecture decisions is that the API should validate through
> the forms, so validating in the form should also cover the API.
>
> We normally like our factories to produce randomised fields, not reliable
> blanks. (If a field can be blank, it can be useful to flip a coin first —
> heads, leave blank; tails, make up random value.)
> --
>
> https://code.launchpad.net/~blake-rouse/maas/add-osystem-to-node-form-api/+merge/219521
> You are the owner of lp:~blake-rouse/maas/add-osystem-to-node-form-api.
>

Revision history for this message
Raphaël Badin (rvb) wrote :

My general take on how validation should be performed is that we should use model-level validation only to avoid inconsistencies in the DB. The rest of the validation belongs to forms. They give us a much more flexible way to validate the input from the UI and the API. More flexible because we can still manipulate objects directly in tests and migrations. This is just a matter of properly separating the M and the C (in MVC) because they are fundamentally separate steps; mingling them together reduces the flexibility of the whole model.

> osystem has validation in the model, as this field can also be set with the start API call.
> distro_series always had validation using the choices=, but now its just using a function to
> perform the validation. It gives the same result.

The result is not quite the same, no. Doing this kind of application-level integrity enforcement at the model level reduces our ability to deal gracefully with inconsistencies. Consider for instance the consequence of having an OS removed: one would be incapable of modifying the nodes referencing this OS unless he also changes the OS (because the validation would fail at the model level); if the validation happens at the form level, at least we could write a migration to deal with these nodes.

> factory.make_node does not break tests, as the osystem field just like the distro_series
> field was can be blank. If the field is blank then the default is used, so no tests break.

But then all your tests operate with a very special case (osystem=blank). It's much better to have the tests operate on data that is as similar to production data as possible. This is also why we chose to use randomness when creating test objects.

Revision history for this message
Blake Rouse (blake-rouse) wrote :

> My general take on how validation should be performed is that we should use

> The result is not quite the same, no. Doing this kind of application-level
> integrity enforcement at the model level reduces our ability to deal
> gracefully with inconsistencies. Consider for instance the consequence of
> having an OS removed: one would be incapable of modifying the nodes
> referencing this OS unless he also changes the OS (because the validation
> would fail at the model level); if the validation happens at the form level,
> at least we could write a migration to deal with these nodes.

This makes since, it would fail to save the node if an os was removed. I will refactor the code to remove the validation from the model.

> But then all your tests operate with a very special case (osystem=blank).
> It's much better to have the tests operate on data that is as similar to
> production data as possible. This is also why we chose to use randomness when
> creating test objects.

That is not a special case, it is the default case for all created nodes. Even nodes created by the user, use that value. Its the same implementation that was there for distro_series. If there was no issue the way it was before, why is there now?

Revision history for this message
Raphaël Badin (rvb) wrote :

> Its the same implementation that was there for distro_series.
> If there was no issue the way it was before, why is there now?

The `distro_series` stuff was implemented when the possible choices where not the dynamic object it is now. One could argue that even at that time it was a mistake but the point is that we shouldn't persist doing things the way `distro_series` was implemented for the many reasons stated above. This means that, ideally, we should also perform the validation for `distro_series` at the form level.

Revision history for this message
Blake Rouse (blake-rouse) wrote :

> The `distro_series` stuff was implemented when the possible choices where not
> the dynamic object it is now. One could argue that even at that time it was a
> mistake but the point is that we shouldn't persist doing things the way
> `distro_series` was implemented for the many reasons stated above. This means
> that, ideally, we should also perform the validation for `distro_series` at
> the form level.

Sorry I was talking specifically about factory.make_node. I will refactor distro_series and osystem to use form validation.

Revision history for this message
Blake Rouse (blake-rouse) wrote :

Removed the validation of osystem and distro_series from the NodeModel. The start API now uses the NodeForm to set the distro_series and osystem.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

I'm supposed to be reviewing the review here, I think, but things got so big that I just went back to the code! Anyway, thanks for doing this and for bearing with us when we made you jump through the additional hoops...

Notes about superficialities follow.

.

Doesn't that last line in list_commisioning_choices just return "options" as it already was? If you're trying to force eager evaluation, you could just return list(options).

.

In test_get_osystem_returns_default_osystem, you create a Node and assume that it won't have its osystem set. I think it would make sense to set a random one by default, in which case it would make more sense to set it to '' explicitly. (Yes, Django's handling of blank strings can be infuriating.) Same for test_get_distro_series_returns_default_series just below that.

.

In make_osystem_with_releases, and make_usable_osystem below that, I guess the “purpose” parameter should really be called “purposes” (plural).

.

Nice tests for list_all_usable_osystems, list_all_usable_releases, and the new form validation! Very focused, clearly named, distinct setup / execution / verification phases.

.

It'd be nice to rename _all_ mention of “series” to “releases”... maybe someday.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2014-05-27 15:05:37 +0000
+++ src/maasserver/api.py 2014-05-29 17:03:44 +0000
@@ -253,6 +253,10 @@
253from piston.emitters import JSONEmitter253from piston.emitters import JSONEmitter
254from piston.handler import typemapper254from piston.handler import typemapper
255from piston.utils import rc255from piston.utils import rc
256from provisioningserver.driver import (
257 BOOT_IMAGE_PURPOSE,
258 OperatingSystemRegistry,
259 )
256from provisioningserver.kernel_opts import KernelParameters260from provisioningserver.kernel_opts import KernelParameters
257from provisioningserver.power_schema import UNKNOWN_POWER_TYPE261from provisioningserver.power_schema import UNKNOWN_POWER_TYPE
258import simplejson as json262import simplejson as json
@@ -411,7 +415,7 @@
411 available to the nodes through the metadata service.415 available to the nodes through the metadata service.
412 :type user_data: base64-encoded unicode416 :type user_data: base64-encoded unicode
413 :param distro_series: If present, this parameter specifies the417 :param distro_series: If present, this parameter specifies the
414 Ubuntu Release the node will use.418 OS release the node will use.
415 :type distro_series: unicode419 :type distro_series: unicode
416420
417 Ideally we'd have MIME multipart and content-transfer-encoding etc.421 Ideally we'd have MIME multipart and content-transfer-encoding etc.
@@ -427,7 +431,13 @@
427 node = Node.objects.get_node_or_404(431 node = Node.objects.get_node_or_404(
428 system_id=system_id, user=request.user,432 system_id=system_id, user=request.user,
429 perm=NODE_PERMISSION.EDIT)433 perm=NODE_PERMISSION.EDIT)
430 node.set_distro_series(series=series)434 Form = get_node_edit_form(request.user)
435 form = Form(instance=node)
436 form.set_distro_series(series=series)
437 if form.is_valid():
438 form.save()
439 else:
440 raise ValidationError(form.errors)
431 nodes = Node.objects.start_nodes(441 nodes = Node.objects.start_nodes(
432 [system_id], request.user, user_data=user_data)442 [system_id], request.user, user_data=user_data)
433 if len(nodes) == 0:443 if len(nodes) == 0:
@@ -2369,7 +2379,7 @@
2369 context_instance=RequestContext(request))2379 context_instance=RequestContext(request))
23702380
23712381
2372def get_boot_purpose(node):2382def get_boot_purpose(node, osystem, arch, subarch, series, label):
2373 """Return a suitable "purpose" for this boot, e.g. "install"."""2383 """Return a suitable "purpose" for this boot, e.g. "install"."""
2374 # XXX: allenap bug=1031406 2012-07-31: The boot purpose is still in2384 # XXX: allenap bug=1031406 2012-07-31: The boot purpose is still in
2375 # flux. It may be that there will just be an "ephemeral" environment and2385 # flux. It may be that there will just be an "ephemeral" environment and
@@ -2389,7 +2399,14 @@
2389 if node.should_use_traditional_installer():2399 if node.should_use_traditional_installer():
2390 return "install"2400 return "install"
2391 else:2401 else:
2392 return "xinstall"2402 # Return normal install if the booting operating system
2403 # doesn't support xinstall.
2404 osystem_obj = OperatingSystemRegistry[osystem]
2405 purposes = osystem_obj.get_boot_image_purposes(
2406 arch, subarch, series, label)
2407 if BOOT_IMAGE_PURPOSE.XINSTALL in purposes:
2408 return "xinstall"
2409 return "install"
2393 else:2410 else:
2394 return "local" # TODO: Investigate.2411 return "local" # TODO: Investigate.
2395 else:2412 else:
@@ -2457,12 +2474,12 @@
2457 node = get_node_from_mac_string(request.GET.get('mac', None))2474 node = get_node_from_mac_string(request.GET.get('mac', None))
24582475
2459 if node is None or node.status == NODE_STATUS.COMMISSIONING:2476 if node is None or node.status == NODE_STATUS.COMMISSIONING:
2477 osystem = Config.objects.get_config('commissioning_osystem')
2460 series = Config.objects.get_config('commissioning_distro_series')2478 series = Config.objects.get_config('commissioning_distro_series')
2461 else:2479 else:
2480 osystem = node.get_osystem()
2462 series = node.get_distro_series()2481 series = node.get_distro_series()
24632482
2464 purpose = get_boot_purpose(node)
2465
2466 if node:2483 if node:
2467 arch, subarch = node.architecture.split('/')2484 arch, subarch = node.architecture.split('/')
2468 preseed_url = compose_preseed_url(node)2485 preseed_url = compose_preseed_url(node)
@@ -2488,7 +2505,7 @@
2488 # current series. If nothing is found, fall back to i386 like2505 # current series. If nothing is found, fall back to i386 like
2489 # we used to. LP #11813342506 # we used to. LP #1181334
2490 image = BootImage.objects.get_default_arch_image_in_nodegroup(2507 image = BootImage.objects.get_default_arch_image_in_nodegroup(
2491 nodegroup, 'ubuntu', series, purpose=purpose)2508 nodegroup, osystem, series, purpose='commissioning')
2492 if image is None:2509 if image is None:
2493 arch = 'i386'2510 arch = 'i386'
2494 else:2511 else:
@@ -2496,12 +2513,16 @@
24962513
2497 subarch = get_optional_param(request.GET, 'subarch', 'generic')2514 subarch = get_optional_param(request.GET, 'subarch', 'generic')
24982515
2516 # Get the purpose, without a selected label
2517 purpose = get_boot_purpose(
2518 node, osystem, arch, subarch, series, label=None)
2519
2499 # We use as our default label the label of the most recent image for2520 # We use as our default label the label of the most recent image for
2500 # the criteria we've assembled above. If there is no latest image2521 # the criteria we've assembled above. If there is no latest image
2501 # (which should never happen in reality but may happen in tests), we2522 # (which should never happen in reality but may happen in tests), we
2502 # fall back to using 'no-such-image' as our default.2523 # fall back to using 'no-such-image' as our default.
2503 latest_image = BootImage.objects.get_latest_image(2524 latest_image = BootImage.objects.get_latest_image(
2504 nodegroup, 'ubuntu', arch, subarch, series, purpose)2525 nodegroup, osystem, arch, subarch, series, purpose)
2505 if latest_image is None:2526 if latest_image is None:
2506 # XXX 2014-03-18 gmb bug=1294131:2527 # XXX 2014-03-18 gmb bug=1294131:
2507 # We really ought to raise an exception here so that client2528 # We really ought to raise an exception here so that client
@@ -2519,6 +2540,9 @@
2519 subarch = latest_image.subarchitecture2540 subarch = latest_image.subarchitecture
2520 label = get_optional_param(request.GET, 'label', latest_label)2541 label = get_optional_param(request.GET, 'label', latest_label)
25212542
2543 # Get the supported purpose with the boot label
2544 purpose = get_boot_purpose(node, osystem, arch, subarch, series, label)
2545
2522 if node is not None:2546 if node is not None:
2523 # We don't care if the kernel opts is from the global setting or a tag,2547 # We don't care if the kernel opts is from the global setting or a tag,
2524 # just get the options2548 # just get the options
@@ -2548,7 +2572,7 @@
2548 cluster_address = get_mandatory_param(request.GET, "local")2572 cluster_address = get_mandatory_param(request.GET, "local")
25492573
2550 params = KernelParameters(2574 params = KernelParameters(
2551 osystem='ubuntu', arch=arch, subarch=subarch, release=series,2575 osystem=osystem, arch=arch, subarch=subarch, release=series,
2552 label=label, purpose=purpose, hostname=hostname, domain=domain,2576 label=label, purpose=purpose, hostname=hostname, domain=domain,
2553 preseed_url=preseed_url, log_host=server_address,2577 preseed_url=preseed_url, log_host=server_address,
2554 fs_host=cluster_address, extra_opts=extra_kernel_opts)2578 fs_host=cluster_address, extra_opts=extra_kernel_opts)
25552579
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2014-05-26 14:14:57 +0000
+++ src/maasserver/forms.py 2014-05-29 17:03:44 +0000
@@ -73,9 +73,6 @@
73 )73 )
74from maasserver.config_forms import SKIP_CHECK_NAME74from maasserver.config_forms import SKIP_CHECK_NAME
75from maasserver.enum import (75from maasserver.enum import (
76 COMMISSIONING_DISTRO_SERIES_CHOICES,
77 DISTRO_SERIES,
78 DISTRO_SERIES_CHOICES,
79 NODE_STATUS,76 NODE_STATUS,
80 NODEGROUPINTERFACE_MANAGEMENT,77 NODEGROUPINTERFACE_MANAGEMENT,
81 NODEGROUPINTERFACE_MANAGEMENT_CHOICES,78 NODEGROUPINTERFACE_MANAGEMENT_CHOICES,
@@ -88,8 +85,8 @@
88from maasserver.forms_settings import (85from maasserver.forms_settings import (
89 CONFIG_ITEMS_KEYS,86 CONFIG_ITEMS_KEYS,
90 get_config_field,87 get_config_field,
91 INVALID_DISTRO_SERIES_MESSAGE,
92 INVALID_SETTING_MSG_TEMPLATE,88 INVALID_SETTING_MSG_TEMPLATE,
89 list_commisioning_choices,
93 )90 )
94from maasserver.models import (91from maasserver.models import (
95 BootImage,92 BootImage,
@@ -117,6 +114,7 @@
117from maasserver.utils.network import make_network114from maasserver.utils.network import make_network
118from metadataserver.fields import Bin115from metadataserver.fields import Bin
119from metadataserver.models import CommissioningScript116from metadataserver.models import CommissioningScript
117from provisioningserver.driver import OperatingSystemRegistry
120118
121# A reusable null-option for choice fields.119# A reusable null-option for choice fields.
122BLANK_CHOICE = ('', '-------')120BLANK_CHOICE = ('', '-------')
@@ -217,6 +215,106 @@
217 return all_architectures[0]215 return all_architectures[0]
218216
219217
218def list_all_usable_osystems():
219 """Return all operating systems that can be used for nodes.
220
221 These are the operating systems for which any nodegroup has the boot images
222 required to boot the node.
223 """
224 # The Node edit form offers all usable operating systems as options for the
225 # osystem field. Not all of these may be available in the node's
226 # nodegroup, but to represent that accurately in the UI would depend on
227 # the currently selected nodegroup. Narrowing the options down further
228 # would have to happen browser-side.
229 osystems = set()
230 for nodegroup in NodeGroup.objects.all():
231 osystems = osystems.union(
232 BootImage.objects.get_usable_osystems(nodegroup))
233 osystems = [OperatingSystemRegistry[osystem] for osystem in osystems]
234 return sorted(osystems, key=lambda osystem: osystem.title)
235
236
237def list_osystem_choices(osystems):
238 """Return Django "choices" list for `osystem`."""
239 choices = [('', 'Default OS')]
240 choices += [
241 (osystem.name, osystem.title)
242 for osystem in osystems
243 ]
244 return choices
245
246
247def list_all_usable_releases(osystems):
248 """Return dictionary of usable `releases` for each opertaing system."""
249 distro_series = {}
250 nodegroups = list(NodeGroup.objects.all())
251 for osystem in osystems:
252 releases = set()
253 for nodegroup in nodegroups:
254 releases = releases.union(
255 BootImage.objects.get_usable_releases(nodegroup, osystem.name))
256 distro_series[osystem.name] = sorted(releases)
257 return distro_series
258
259
260def list_release_choices(releases):
261 """Return Django "choices" list for `releases`."""
262 choices = [('', 'Default OS Release')]
263 for key, value in releases.items():
264 osystem = OperatingSystemRegistry[key]
265 options = osystem.format_release_choices(value)
266 choices += [(
267 '%s/' % osystem.name,
268 'Latest %s Release' % osystem.title
269 )]
270 choices += [
271 ('%s/%s' % (osystem.name, name), title)
272 for name, title in options
273 ]
274 return choices
275
276
277def get_distro_series_inital(instance):
278 """Returns the distro_series initial value for the instance."""
279 osystem = instance.osystem
280 series = instance.distro_series
281 if osystem is not None and osystem != '':
282 if series is None:
283 series = ''
284 return '%s/%s' % (osystem, series)
285 return None
286
287
288def clean_distro_series_field(form, field, os_field):
289 """Cleans the distro_series field in the form. Validating that
290 the selected operating system matches the distro_series.
291
292 :param form: `Form` class
293 :param field: distro_series field name
294 :param os_field: osystem field name
295 :returns: clean distro_series field value
296 """
297 new_distro_series = form.cleaned_data.get(field)
298 if new_distro_series is None or '/' not in new_distro_series:
299 return new_distro_series
300 os, release = new_distro_series.split('/', 1)
301 if os_field in form.cleaned_data:
302 new_os = form.cleaned_data[os_field]
303 if os != new_os:
304 raise ValidationError(
305 "%s in %s does not match with "
306 "operating system %s" % (release, field, os))
307 return release
308
309
310def get_osystem_from_release(release):
311 """Returns the operating system that supports that release."""
312 for _, osystem in OperatingSystemRegistry:
313 if release in osystem.get_supported_releases():
314 return osystem
315 return None
316
317
220class NodeForm(ModelForm):318class NodeForm(ModelForm):
221319
222 def __init__(self, request=None, *args, **kwargs):320 def __init__(self, request=None, *args, **kwargs):
@@ -229,6 +327,7 @@
229 self.fields['nodegroup'] = NodeGroupFormField(327 self.fields['nodegroup'] = NodeGroupFormField(
230 required=False, empty_label="Default (master)")328 required=False, empty_label="Default (master)")
231 self.set_up_architecture_field()329 self.set_up_architecture_field()
330 self.set_up_osystem_and_distro_series_fields(kwargs.get('instance'))
232331
233 def set_up_architecture_field(self):332 def set_up_architecture_field(self):
234 """Create the `architecture` field.333 """Create the `architecture` field.
@@ -248,6 +347,32 @@
248 choices=choices, required=True, initial=default_arch,347 choices=choices, required=True, initial=default_arch,
249 error_messages={'invalid_choice': invalid_arch_message})348 error_messages={'invalid_choice': invalid_arch_message})
250349
350 def set_up_osystem_and_distro_series_fields(self, instance):
351 """Create the `osystem` and `distro_series` fields.
352
353 This needs to be done on the fly so that we can pass a dynamic list of
354 usable operating systems and distro_series.
355 """
356 osystems = list_all_usable_osystems()
357 releases = list_all_usable_releases(osystems)
358 os_choices = list_osystem_choices(osystems)
359 distro_choices = list_release_choices(releases)
360 invalid_osystem_message = compose_invalid_choice_text(
361 'osystem', os_choices)
362 invalid_distro_series_message = compose_invalid_choice_text(
363 'distro_series', distro_choices)
364 self.fields['osystem'] = forms.ChoiceField(
365 label="OS", choices=os_choices, required=False, initial='',
366 error_messages={'invalid_choice': invalid_osystem_message})
367 self.fields['distro_series'] = forms.ChoiceField(
368 label="Release", choices=distro_choices,
369 required=False, initial='',
370 error_messages={'invalid_choice': invalid_distro_series_message})
371 if instance is not None:
372 initial_value = get_distro_series_inital(instance)
373 if instance is not None:
374 self.initial['distro_series'] = initial_value
375
251 def clean_hostname(self):376 def clean_hostname(self):
252 # Don't allow the hostname to be changed if the node is377 # Don't allow the hostname to be changed if the node is
253 # currently allocated. Juju knows the node by its old name, so378 # currently allocated. Juju knows the node by its old name, so
@@ -261,6 +386,9 @@
261386
262 return new_hostname387 return new_hostname
263388
389 def clean_distro_series(self):
390 return clean_distro_series_field(self, 'distro_series', 'osystem')
391
264 def is_valid(self):392 def is_valid(self):
265 is_valid = super(NodeForm, self).is_valid()393 is_valid = super(NodeForm, self).is_valid()
266 if len(list_all_usable_architectures()) == 0:394 if len(list_all_usable_architectures()) == 0:
@@ -269,11 +397,24 @@
269 is_valid = False397 is_valid = False
270 return is_valid398 return is_valid
271399
272 distro_series = forms.ChoiceField(400 def set_distro_series(self, series=''):
273 choices=DISTRO_SERIES_CHOICES, required=False,401 """Sets the osystem and distro_series, from the provided
274 initial=DISTRO_SERIES.default,402 distro_series.
275 label="Release",403 """
276 error_messages={'invalid_choice': INVALID_DISTRO_SERIES_MESSAGE})404 # This implementation is used so that current API, is not broken. This
405 # makes the distro_series a flat namespace. The distro_series is used
406 # to search through the supporting operating systems, to find the
407 # correct operating system that supports this distro_series.
408 self.is_bound = True
409 self.data['osystem'] = ''
410 self.data['distro_series'] = ''
411 if series is not None and series != '':
412 osystem = get_osystem_from_release(series)
413 if osystem is not None:
414 self.data['osystem'] = osystem.name
415 self.data['distro_series'] = '%s/%s' % (osystem.name, series)
416 else:
417 self.data['distro_series'] = series
277418
278 hostname = forms.CharField(419 hostname = forms.CharField(
279 label="Host name", required=False, help_text=(420 label="Host name", required=False, help_text=(
@@ -292,6 +433,7 @@
292 fields = (433 fields = (
293 'hostname',434 'hostname',
294 'architecture',435 'architecture',
436 'osystem',
295 'distro_series',437 'distro_series',
296 )438 )
297439
@@ -872,16 +1014,34 @@
872 """Settings page, Commissioning section."""1014 """Settings page, Commissioning section."""
873 check_compatibility = get_config_field('check_compatibility')1015 check_compatibility = get_config_field('check_compatibility')
874 commissioning_distro_series = forms.ChoiceField(1016 commissioning_distro_series = forms.ChoiceField(
875 choices=COMMISSIONING_DISTRO_SERIES_CHOICES, required=False,1017 choices=list_commisioning_choices(), required=False,
876 label="Default distro series used for commissioning",1018 label="Default Ubuntu release used for commissioning",
877 error_messages={'invalid_choice': compose_invalid_choice_text(1019 error_messages={'invalid_choice': compose_invalid_choice_text(
878 'commissioning_distro_series',1020 'commissioning_distro_series',
879 COMMISSIONING_DISTRO_SERIES_CHOICES)})1021 list_commisioning_choices())})
1022
1023
1024class DeployForm(ConfigForm):
1025 """Settings page, Deploy section."""
1026 default_osystem = get_config_field('default_osystem')
1027 default_distro_series = get_config_field('default_distro_series')
1028
1029 def _load_initials(self):
1030 super(DeployForm, self)._load_initials()
1031 initial_os = self.initial['default_osystem']
1032 initial_series = self.initial['default_distro_series']
1033 self.initial['default_distro_series'] = '%s/%s' % (
1034 initial_os,
1035 initial_series
1036 )
1037
1038 def clean_default_distro_series(self):
1039 return clean_distro_series_field(
1040 self, 'default_distro_series', 'default_osystem')
8801041
8811042
882class UbuntuForm(ConfigForm):1043class UbuntuForm(ConfigForm):
883 """Settings page, Ubuntu section."""1044 """Settings page, Ubuntu section."""
884 default_distro_series = get_config_field('default_distro_series')
885 main_archive = get_config_field('main_archive')1045 main_archive = get_config_field('main_archive')
886 ports_archive = get_config_field('ports_archive')1046 ports_archive = get_config_field('ports_archive')
8871047
8881048
=== modified file 'src/maasserver/forms_settings.py'
--- src/maasserver/forms_settings.py 2014-04-10 20:21:24 +0000
+++ src/maasserver/forms_settings.py 2014-05-29 17:03:44 +0000
@@ -23,19 +23,41 @@
23from socket import gethostname23from socket import gethostname
2424
25from django import forms25from django import forms
26from maasserver.enum import (26from maasserver.models.config import DEFAULT_OS
27 COMMISSIONING_DISTRO_SERIES_CHOICES,
28 DISTRO_SERIES,
29 DISTRO_SERIES_CHOICES,
30 )
31from maasserver.utils.forms import compose_invalid_choice_text27from maasserver.utils.forms import compose_invalid_choice_text
28from provisioningserver.driver import OperatingSystemRegistry
3229
3330
34INVALID_URL_MESSAGE = "Enter a valid url (e.g. http://host.example.com)."31INVALID_URL_MESSAGE = "Enter a valid url (e.g. http://host.example.com)."
3532
3633
37INVALID_DISTRO_SERIES_MESSAGE = compose_invalid_choice_text(34def list_osystem_choices():
38 'distro_series', DISTRO_SERIES_CHOICES)35 osystems = [osystem for _, osystem in OperatingSystemRegistry]
36 osystems = sorted(osystems, key=lambda osystem: osystem.title)
37 return [
38 (osystem.name, osystem.title)
39 for osystem in osystems
40 ]
41
42
43def list_release_choices():
44 osystems = [osystem for _, osystem in OperatingSystemRegistry]
45 choices = []
46 for osystem in osystems:
47 supported = sorted(osystem.get_supported_releases())
48 options = osystem.format_release_choices(supported)
49 options = [
50 ('%s/%s' % (osystem.name, name), title)
51 for name, title in options
52 ]
53 choices += options
54 return choices
55
56
57def list_commisioning_choices():
58 releases = DEFAULT_OS.get_supported_commissioning_releases()
59 options = DEFAULT_OS.format_release_choices(releases)
60 return list(options)
3961
4062
41CONFIG_ITEMS = {63CONFIG_ITEMS = {
@@ -139,28 +161,46 @@
139 "e.g. for ntp.ubuntu.com: '91.189.94.4'")161 "e.g. for ntp.ubuntu.com: '91.189.94.4'")
140 }162 }
141 },163 },
164 'default_osystem': {
165 'default': DEFAULT_OS.name,
166 'form': forms.ChoiceField,
167 'form_kwargs': {
168 'label': "Default operating system used for deployment",
169 'choices': list_osystem_choices(),
170 'required': False,
171 'error_messages': {
172 'invalid_choice': compose_invalid_choice_text(
173 'osystem',
174 list_osystem_choices())},
175 }
176 },
142 'default_distro_series': {177 'default_distro_series': {
143 'default': DISTRO_SERIES.trusty,178 'default': '%s/%s' % (
179 DEFAULT_OS.name,
180 DEFAULT_OS.get_default_release()
181 ),
144 'form': forms.ChoiceField,182 'form': forms.ChoiceField,
145 'form_kwargs': {183 'form_kwargs': {
146 'label': "Default distro series used for deployment",184 'label': "Default OS release used for deployment",
147 'choices': DISTRO_SERIES_CHOICES,185 'choices': list_release_choices(),
148 'required': False,186 'required': False,
149 'error_messages': {187 'error_messages': {
150 'invalid_choice': INVALID_DISTRO_SERIES_MESSAGE},188 'invalid_choice': compose_invalid_choice_text(
189 'distro_series',
190 list_release_choices())},
151 }191 }
152 },192 },
153 'commissioning_distro_series': {193 'commissioning_distro_series': {
154 'default': DISTRO_SERIES.trusty,194 'default': DEFAULT_OS.get_default_commissioning_release(),
155 'form': forms.ChoiceField,195 'form': forms.ChoiceField,
156 'form_kwargs': {196 'form_kwargs': {
157 'label': "Default distro series used for commissioning",197 'label': "Default Ubuntu release used for commissioning",
158 'choices': COMMISSIONING_DISTRO_SERIES_CHOICES,198 'choices': list_commisioning_choices(),
159 'required': False,199 'required': False,
160 'error_messages': {200 'error_messages': {
161 'invalid_choice': compose_invalid_choice_text(201 'invalid_choice': compose_invalid_choice_text(
162 'commissioning_distro_series',202 'commissioning_distro_series',
163 COMMISSIONING_DISTRO_SERIES_CHOICES)},203 list_commisioning_choices())},
164 }204 }
165 },205 },
166 'enable_third_party_drivers': {206 'enable_third_party_drivers': {
167207
=== modified file 'src/maasserver/models/config.py'
--- src/maasserver/models/config.py 2014-04-14 21:42:00 +0000
+++ src/maasserver/models/config.py 2014-05-29 17:03:44 +0000
@@ -28,8 +28,11 @@
28 )28 )
29from django.db.models.signals import post_save29from django.db.models.signals import post_save
30from maasserver import DefaultMeta30from maasserver import DefaultMeta
31from maasserver.enum import DISTRO_SERIES
32from maasserver.fields import JSONObjectField31from maasserver.fields import JSONObjectField
32from provisioningserver.driver.os_ubuntu import UbuntuOS
33
34
35DEFAULT_OS = UbuntuOS()
3336
3437
35def get_default_config():38def get_default_config():
@@ -40,11 +43,14 @@
40 # Ubuntu section configuration.43 # Ubuntu section configuration.
41 'main_archive': 'http://archive.ubuntu.com/ubuntu',44 'main_archive': 'http://archive.ubuntu.com/ubuntu',
42 'ports_archive': 'http://ports.ubuntu.com/ubuntu-ports',45 'ports_archive': 'http://ports.ubuntu.com/ubuntu-ports',
43 'commissioning_distro_series': DISTRO_SERIES.trusty,46 'commissioning_osystem': DEFAULT_OS.name,
47 'commissioning_distro_series':
48 DEFAULT_OS.get_default_commissioning_release(),
44 # Network section configuration.49 # Network section configuration.
45 'maas_name': gethostname(),50 'maas_name': gethostname(),
46 'enlistment_domain': b'local',51 'enlistment_domain': b'local',
47 'default_distro_series': DISTRO_SERIES.trusty,52 'default_osystem': DEFAULT_OS.name,
53 'default_distro_series': DEFAULT_OS.get_default_release(),
48 'http_proxy': None,54 'http_proxy': None,
49 'upstream_dns': None,55 'upstream_dns': None,
50 'ntp_server': '91.189.94.4', # ntp.ubuntu.com56 'ntp_server': '91.189.94.4', # ntp.ubuntu.com
5157
=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py 2014-05-27 07:11:28 +0000
+++ src/maasserver/models/node.py 2014-05-29 17:03:44 +0000
@@ -49,8 +49,6 @@
49 logger,49 logger,
50 )50 )
51from maasserver.enum import (51from maasserver.enum import (
52 DISTRO_SERIES,
53 DISTRO_SERIES_CHOICES,
54 NODE_PERMISSION,52 NODE_PERMISSION,
55 NODE_STATUS,53 NODE_STATUS,
56 NODE_STATUS_CHOICES,54 NODE_STATUS_CHOICES,
@@ -72,6 +70,7 @@
72 strip_domain,70 strip_domain,
73 )71 )
74from piston.models import Token72from piston.models import Token
73from provisioningserver.driver import OperatingSystemRegistry
75from provisioningserver.tasks import (74from provisioningserver.tasks import (
76 power_off,75 power_off,
77 power_on,76 power_on,
@@ -486,7 +485,7 @@
486 max_length=20, blank=True, default='')485 max_length=20, blank=True, default='')
487486
488 distro_series = CharField(487 distro_series = CharField(
489 max_length=20, choices=DISTRO_SERIES_CHOICES, blank=True, default='')488 max_length=20, blank=True, default='')
490489
491 architecture = CharField(max_length=31, blank=False)490 architecture = CharField(max_length=31, blank=False)
492491
@@ -793,21 +792,30 @@
793 """The name of the queue for tasks specific to this node."""792 """The name of the queue for tasks specific to this node."""
794 return self.nodegroup.work_queue793 return self.nodegroup.work_queue
795794
795 def get_osystem(self):
796 """Return the operating system to install that node."""
797 use_default_osystem = (self.osystem is None or self.osystem == '')
798 if use_default_osystem:
799 return Config.objects.get_config('default_osystem')
800 else:
801 return self.osystem
802
796 def get_distro_series(self):803 def get_distro_series(self):
797 """Return the distro series to install that node."""804 """Return the distro series to install that node."""
805 use_default_osystem = (
806 self.osystem is None or
807 self.osystem == '')
798 use_default_distro_series = (808 use_default_distro_series = (
799 not self.distro_series or809 self.distro_series is None or
800 self.distro_series == DISTRO_SERIES.default)810 self.distro_series == '')
801 if use_default_distro_series:811 if use_default_osystem and use_default_distro_series:
802 return Config.objects.get_config('default_distro_series')812 return Config.objects.get_config('default_distro_series')
813 elif use_default_distro_series:
814 osystem = OperatingSystemRegistry[self.osystem]
815 return osystem.get_default_release()
803 else:816 else:
804 return self.distro_series817 return self.distro_series
805818
806 def set_distro_series(self, series=''):
807 """Set the distro series to install that node."""
808 self.distro_series = series
809 self.save()
810
811 def get_effective_power_parameters(self):819 def get_effective_power_parameters(self):
812 """Return effective power parameters, including any defaults."""820 """Return effective power parameters, including any defaults."""
813 if self.power_parameters:821 if self.power_parameters:
@@ -860,6 +868,7 @@
860 self.token = None868 self.token = None
861 self.agent_name = ''869 self.agent_name = ''
862 self.set_netboot()870 self.set_netboot()
871 self.osystem = ''
863 self.distro_series = ''872 self.distro_series = ''
864 self.save()873 self.save()
865874
866875
=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py 2014-05-27 12:35:35 +0000
+++ src/maasserver/models/tests/test_node.py 2014-05-29 17:03:44 +0000
@@ -20,7 +20,6 @@
20from django.core.exceptions import ValidationError20from django.core.exceptions import ValidationError
21from maasserver.clusterrpc.power_parameters import get_power_types21from maasserver.clusterrpc.power_parameters import get_power_types
22from maasserver.enum import (22from maasserver.enum import (
23 DISTRO_SERIES,
24 NODE_PERMISSION,23 NODE_PERMISSION,
25 NODE_STATUS,24 NODE_STATUS,
26 NODE_STATUS_CHOICES,25 NODE_STATUS_CHOICES,
@@ -270,15 +269,14 @@
270 offset += timedelta(1)269 offset += timedelta(1)
271 self.assertEqual(macs[0], node.get_primary_mac().mac_address)270 self.assertEqual(macs[0], node.get_primary_mac().mac_address)
272271
272 def test_get_osystem_returns_default_osystem(self):
273 node = factory.make_node(osystem='')
274 osystem = Config.objects.get_config('default_osystem')
275 self.assertEqual(osystem, node.get_osystem())
276
273 def test_get_distro_series_returns_default_series(self):277 def test_get_distro_series_returns_default_series(self):
274 node = factory.make_node()278 node = factory.make_node(distro_series='')
275 series = Config.objects.get_config('commissioning_distro_series')279 series = Config.objects.get_config('default_distro_series')
276 self.assertEqual(series, node.get_distro_series())
277
278 def test_set_get_distro_series_returns_series(self):
279 node = factory.make_node()
280 series = DISTRO_SERIES.quantal
281 node.set_distro_series(series)
282 self.assertEqual(series, node.get_distro_series())280 self.assertEqual(series, node.get_distro_series())
283281
284 def test_delete_node_deletes_related_mac(self):282 def test_delete_node_deletes_related_mac(self):
@@ -597,11 +595,15 @@
597 node.release()595 node.release()
598 self.assertTrue(node.netboot)596 self.assertTrue(node.netboot)
599597
600 def test_release_clears_distro_series(self):598 def test_release_clears_osystem_and_distro_series(self):
601 node = factory.make_node(599 node = factory.make_node(
602 status=NODE_STATUS.ALLOCATED, owner=factory.make_user())600 status=NODE_STATUS.ALLOCATED, owner=factory.make_user())
603 node.set_distro_series(series=DISTRO_SERIES.quantal)601 osystem = factory.getRandomOS()
602 release = factory.getRandomRelease(osystem)
603 node.osystem = osystem.name
604 node.distro_series = release
604 node.release()605 node.release()
606 self.assertEqual("", node.osystem)
605 self.assertEqual("", node.distro_series)607 self.assertEqual("", node.distro_series)
606608
607 def test_release_powers_off_node(self):609 def test_release_powers_off_node(self):
608610
=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py 2014-04-23 16:09:39 +0000
+++ src/maasserver/preseed.py 2014-05-29 17:03:44 +0000
@@ -93,7 +93,7 @@
9393
94def get_curtin_installer_url(node):94def get_curtin_installer_url(node):
95 """Return the URL where curtin on the node can download its installer."""95 """Return the URL where curtin on the node can download its installer."""
96 osystem = 'ubuntu'96 osystem = node.get_osystem()
97 series = node.get_distro_series()97 series = node.get_distro_series()
98 cluster_host = pick_cluster_controller_address(node)98 cluster_host = pick_cluster_controller_address(node)
99 # XXX rvb(?): The path shouldn't be hardcoded like this, but rather synced99 # XXX rvb(?): The path shouldn't be hardcoded like this, but rather synced
100100
=== modified file 'src/maasserver/testing/osystems.py'
--- src/maasserver/testing/osystems.py 2014-05-14 15:12:48 +0000
+++ src/maasserver/testing/osystems.py 2014-05-29 17:03:44 +0000
@@ -25,23 +25,27 @@
25from provisioningserver.driver import BOOT_IMAGE_PURPOSE25from provisioningserver.driver import BOOT_IMAGE_PURPOSE
2626
2727
28def make_osystem_with_releases(testcase, osystem_name=None, releases=None):28def make_osystem_with_releases(testcase, osystem_name=None, releases=None,
29 purposes=None):
29 """Generate an arbitrary operating system.30 """Generate an arbitrary operating system.
3031
31 :param osystem_name: The operating system name. Useful in cases where32 :param osystem_name: The operating system name. Useful in cases where
32 we need to test that not supplying an os works correctly.33 we need to test that not supplying an os works correctly.
33 :param releases: The list of releases name. Useful in cases where34 :param releases: The list of releases name. Useful in cases where
34 we need to test that not supplying a release works correctly.35 we need to test that not supplying a release works correctly.
36 :param purposes: The purpose's of the boot images.
35 """37 """
36 if osystem_name is None:38 if osystem_name is None:
37 osystem_name = factory.make_name('os')39 osystem_name = factory.make_name('os')
38 if releases is None:40 if releases is None:
39 releases = [factory.make_name('release') for _ in range(3)]41 releases = [factory.make_name('release') for _ in range(3)]
42 if purposes is None:
43 purposes = [BOOT_IMAGE_PURPOSE.INSTALL, BOOT_IMAGE_PURPOSE.XINSTALL]
4044
41 osystem = make_osystem(45 osystem = make_osystem(
42 testcase,46 testcase,
43 osystem_name,47 osystem_name,
44 [BOOT_IMAGE_PURPOSE.INSTALL, BOOT_IMAGE_PURPOSE.XINSTALL])48 purposes)
45 if releases is not None and releases != []:49 if releases is not None and releases != []:
46 osystem.fake_list = releases50 osystem.fake_list = releases
47 return osystem51 return osystem
@@ -72,7 +76,8 @@
72 forms, 'list_all_usable_releases').return_value = distro_series76 forms, 'list_all_usable_releases').return_value = distro_series
7377
7478
75def make_usable_osystem(testcase, osystem_name=None, releases=None):79def make_usable_osystem(testcase, osystem_name=None, releases=None,
80 purposes=None):
76 """Return arbitrary operating system, and make it "usable."81 """Return arbitrary operating system, and make it "usable."
7782
78 A usable operating system is one for which boot images are available.83 A usable operating system is one for which boot images are available.
@@ -83,8 +88,11 @@
83 we need to test that not supplying an os works correctly.88 we need to test that not supplying an os works correctly.
84 :param releases: The list of releases name. Useful in cases where89 :param releases: The list of releases name. Useful in cases where
85 we need to test that not supplying a release works correctly.90 we need to test that not supplying a release works correctly.
91 :param purposse: The list of purposes. Useful in cases where
92 we need to test that not supplying a purpose works correctly.
86 """93 """
87 osystem = make_osystem_with_releases(94 osystem = make_osystem_with_releases(
88 testcase, osystem_name=osystem_name, releases=releases)95 testcase, osystem_name=osystem_name, releases=releases,
96 purposes=purposes)
89 patch_usable_osystems(testcase, [osystem])97 patch_usable_osystems(testcase, [osystem])
90 return osystem98 return osystem
9199
=== modified file 'src/maasserver/tests/test_api_node.py'
--- src/maasserver/tests/test_api_node.py 2014-04-25 18:03:38 +0000
+++ src/maasserver/tests/test_api_node.py 2014-05-29 17:03:44 +0000
@@ -23,7 +23,6 @@
23import bson23import bson
24from django.core.urlresolvers import reverse24from django.core.urlresolvers import reverse
25from maasserver.enum import (25from maasserver.enum import (
26 DISTRO_SERIES,
27 NODE_STATUS,26 NODE_STATUS,
28 NODE_STATUS_CHOICES_DICT,27 NODE_STATUS_CHOICES_DICT,
29 )28 )
@@ -40,6 +39,7 @@
40from maasserver.testing.architecture import make_usable_architecture39from maasserver.testing.architecture import make_usable_architecture
41from maasserver.testing.factory import factory40from maasserver.testing.factory import factory
42from maasserver.testing.oauthclient import OAuthAuthenticatedClient41from maasserver.testing.oauthclient import OAuthAuthenticatedClient
42from maasserver.testing.osystems import make_usable_osystem
43from maasserver.testing.testcase import MAASServerTestCase43from maasserver.testing.testcase import MAASServerTestCase
44from maasserver.utils import map_enum44from maasserver.utils import map_enum
45from metadataserver.models import (45from metadataserver.models import (
@@ -221,24 +221,31 @@
221 self.assertEqual(221 self.assertEqual(
222 node.system_id, json.loads(response.content)['system_id'])222 node.system_id, json.loads(response.content)['system_id'])
223223
224 def test_POST_start_sets_distro_series(self):224 def test_POST_start_sets_osystem_and_distro_series(self):
225 node = factory.make_node(225 node = factory.make_node(
226 owner=self.logged_in_user, mac=True,226 owner=self.logged_in_user, mac=True,
227 power_type='ether_wake')227 power_type='ether_wake',
228 distro_series = factory.getRandomEnum(DISTRO_SERIES)228 architecture=make_usable_architecture(self))
229 osystem = make_usable_osystem(self)
230 distro_series = factory.getRandomRelease(osystem)
229 response = self.client.post(231 response = self.client.post(
230 self.get_node_uri(node),232 self.get_node_uri(node), {
231 {'op': 'start', 'distro_series': distro_series})233 'op': 'start',
234 'distro_series': distro_series
235 })
232 self.assertEqual(236 self.assertEqual(
233 (httplib.OK, node.system_id),237 (httplib.OK, node.system_id),
234 (response.status_code, json.loads(response.content)['system_id']))238 (response.status_code, json.loads(response.content)['system_id']))
235 self.assertEqual(239 self.assertEqual(
240 osystem.name, reload_object(node).osystem)
241 self.assertEqual(
236 distro_series, reload_object(node).distro_series)242 distro_series, reload_object(node).distro_series)
237243
238 def test_POST_start_validates_distro_series(self):244 def test_POST_start_validates_distro_series(self):
239 node = factory.make_node(245 node = factory.make_node(
240 owner=self.logged_in_user, mac=True,246 owner=self.logged_in_user, mac=True,
241 power_type='ether_wake')247 power_type='ether_wake',
248 architecture=make_usable_architecture(self))
242 invalid_distro_series = factory.getRandomString()249 invalid_distro_series = factory.getRandomString()
243 response = self.client.post(250 response = self.client.post(
244 self.get_node_uri(node),251 self.get_node_uri(node),
@@ -247,7 +254,8 @@
247 (254 (
248 httplib.BAD_REQUEST,255 httplib.BAD_REQUEST,
249 {'distro_series': [256 {'distro_series': [
250 "Value u'%s' is not a valid choice." %257 "'%s' is not a valid distro_series. "
258 "It should be one of: ''." %
251 invalid_distro_series]}259 invalid_distro_series]}
252 ),260 ),
253 (response.status_code, json.loads(response.content)))261 (response.status_code, json.loads(response.content)))
@@ -300,18 +308,23 @@
300 self.client.post(self.get_node_uri(node), {'op': 'release'})308 self.client.post(self.get_node_uri(node), {'op': 'release'})
301 self.assertTrue(reload_object(node).netboot)309 self.assertTrue(reload_object(node).netboot)
302310
303 def test_POST_release_resets_distro_series(self):311 def test_POST_release_resets_osystem_and_distro_series(self):
312 osystem = factory.getRandomOS()
313 release = factory.getRandomRelease(osystem)
304 node = factory.make_node(314 node = factory.make_node(
305 status=NODE_STATUS.ALLOCATED, owner=self.logged_in_user,315 status=NODE_STATUS.ALLOCATED, owner=self.logged_in_user,
306 distro_series=factory.getRandomEnum(DISTRO_SERIES))316 osystem=osystem.name, distro_series=release)
307 self.client.post(self.get_node_uri(node), {'op': 'release'})317 self.client.post(self.get_node_uri(node), {'op': 'release'})
318 self.assertEqual('', reload_object(node).osystem)
308 self.assertEqual('', reload_object(node).distro_series)319 self.assertEqual('', reload_object(node).distro_series)
309320
310 def test_POST_release_resets_agent_name(self):321 def test_POST_release_resets_agent_name(self):
311 agent_name = factory.make_name('agent-name')322 agent_name = factory.make_name('agent-name')
323 osystem = factory.getRandomOS()
324 release = factory.getRandomRelease(osystem)
312 node = factory.make_node(325 node = factory.make_node(
313 status=NODE_STATUS.ALLOCATED, owner=self.logged_in_user,326 status=NODE_STATUS.ALLOCATED, owner=self.logged_in_user,
314 distro_series=factory.getRandomEnum(DISTRO_SERIES),327 osystem=osystem.name, distro_series=release,
315 agent_name=agent_name)328 agent_name=agent_name)
316 self.client.post(self.get_node_uri(node), {'op': 'release'})329 self.client.post(self.get_node_uri(node), {'op': 'release'})
317 self.assertEqual('', reload_object(node).agent_name)330 self.assertEqual('', reload_object(node).agent_name)
318331
=== modified file 'src/maasserver/tests/test_api_pxeconfig.py'
--- src/maasserver/tests/test_api_pxeconfig.py 2014-05-20 02:58:09 +0000
+++ src/maasserver/tests/test_api_pxeconfig.py 2014-05-29 17:03:44 +0000
@@ -35,11 +35,13 @@
35 )35 )
36from maasserver.testing.architecture import make_usable_architecture36from maasserver.testing.architecture import make_usable_architecture
37from maasserver.testing.factory import factory37from maasserver.testing.factory import factory
38from maasserver.testing.osystems import make_usable_osystem
38from maasserver.testing.testcase import MAASServerTestCase39from maasserver.testing.testcase import MAASServerTestCase
39from maastesting.fakemethod import FakeMethod40from maastesting.fakemethod import FakeMethod
40from mock import Mock41from mock import Mock
41from netaddr import IPNetwork42from netaddr import IPNetwork
42from provisioningserver import kernel_opts43from provisioningserver import kernel_opts
44from provisioningserver.driver import BOOT_IMAGE_PURPOSE
43from provisioningserver.kernel_opts import KernelParameters45from provisioningserver.kernel_opts import KernelParameters
44from testtools.matchers import (46from testtools.matchers import (
45 Contains,47 Contains,
@@ -133,7 +135,7 @@
133 self.assertEqual(value, response_dict['extra_opts'])135 self.assertEqual(value, response_dict['extra_opts'])
134136
135 def test_pxeconfig_uses_present_boot_image(self):137 def test_pxeconfig_uses_present_boot_image(self):
136 osystem = 'ubuntu'138 osystem = Config.objects.get_config('commissioning_osystem')
137 release = Config.objects.get_config('commissioning_distro_series')139 release = Config.objects.get_config('commissioning_distro_series')
138 nodegroup = factory.make_node_group()140 nodegroup = factory.make_node_group()
139 factory.make_boot_image(141 factory.make_boot_image(
@@ -284,7 +286,8 @@
284 def test_get_boot_purpose_unknown_node(self):286 def test_get_boot_purpose_unknown_node(self):
285 # A node that's not yet known to MAAS is assumed to be enlisting,287 # A node that's not yet known to MAAS is assumed to be enlisting,
286 # which uses a "commissioning" image.288 # which uses a "commissioning" image.
287 self.assertEqual("commissioning", api.get_boot_purpose(None))289 self.assertEqual("commissioning", api.get_boot_purpose(
290 None, None, None, None, None, None))
288291
289 def test_get_boot_purpose_known_node(self):292 def test_get_boot_purpose_known_node(self):
290 # The following table shows the expected boot "purpose" for each set293 # The following table shows the expected boot "purpose" for each set
@@ -307,11 +310,33 @@
307 node.use_fastpath_installer()310 node.use_fastpath_installer()
308 for name, value in parameters.items():311 for name, value in parameters.items():
309 setattr(node, name, value)312 setattr(node, name, value)
310 self.assertEqual(purpose, api.get_boot_purpose(node))313 osystem = node.get_osystem()
314 series = node.get_distro_series()
315 arch, subarch = node.architecture.split('/')
316 self.assertEqual(
317 purpose,
318 api.get_boot_purpose(
319 node, osystem, arch, subarch, series, None))
320
321 def test_get_boot_purpose_osystem_no_xinstall_support(self):
322 osystem = make_usable_osystem(
323 self, purposes=[BOOT_IMAGE_PURPOSE.INSTALL])
324 release = factory.getRandomRelease(osystem)
325 node = factory.make_node(
326 status=NODE_STATUS.ALLOCATED, netboot=True,
327 osystem=osystem.name, distro_series=release)
328 node.use_fastpath_installer()
329 node_os = node.get_osystem()
330 node_series = node.get_distro_series()
331 arch, subarch = node.architecture.split('/')
332 self.assertEqual(
333 'install',
334 api.get_boot_purpose(
335 node, node_os, arch, subarch, node_series, None))
311336
312 def test_pxeconfig_uses_boot_purpose(self):337 def test_pxeconfig_uses_boot_purpose(self):
313 fake_boot_purpose = factory.make_name("purpose")338 fake_boot_purpose = factory.make_name("purpose")
314 self.patch(api, "get_boot_purpose", lambda node: fake_boot_purpose)339 self.patch(api, "get_boot_purpose").return_value = fake_boot_purpose
315 response = self.client.get(reverse('pxeconfig'),340 response = self.client.get(reverse('pxeconfig'),
316 self.get_default_params())341 self.get_default_params())
317 self.assertEqual(342 self.assertEqual(
318343
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py 2014-05-20 14:16:58 +0000
+++ src/maasserver/tests/test_forms.py 2014-05-29 17:03:44 +0000
@@ -53,6 +53,8 @@
53 InstanceListField,53 InstanceListField,
54 INTERFACES_VALIDATION_ERROR_MESSAGE,54 INTERFACES_VALIDATION_ERROR_MESSAGE,
55 list_all_usable_architectures,55 list_all_usable_architectures,
56 list_all_usable_osystems,
57 list_all_usable_releases,
56 MACAddressForm,58 MACAddressForm,
57 MAX_MESSAGES,59 MAX_MESSAGES,
58 merge_error_messages,60 merge_error_messages,
@@ -98,6 +100,11 @@
98 patch_usable_architectures,100 patch_usable_architectures,
99 )101 )
100from maasserver.testing.factory import factory102from maasserver.testing.factory import factory
103from maasserver.testing.osystems import (
104 make_osystem_with_releases,
105 make_usable_osystem,
106 patch_usable_osystems,
107 )
101from maasserver.testing.testcase import MAASServerTestCase108from maasserver.testing.testcase import MAASServerTestCase
102from maasserver.utils import map_enum109from maasserver.utils import map_enum
103from maasserver.utils.forms import compose_invalid_choice_text110from maasserver.utils.forms import compose_invalid_choice_text
@@ -120,8 +127,8 @@
120127
121class TestHelpers(MAASServerTestCase):128class TestHelpers(MAASServerTestCase):
122129
123 def make_usable_boot_images(self, nodegroup=None, arch=None,130 def make_usable_boot_images(self, nodegroup=None, osystem=None,
124 subarchitecture=None):131 arch=None, subarchitecture=None, release=None):
125 """Create a set of boot images, so the architecture becomes "usable".132 """Create a set of boot images, so the architecture becomes "usable".
126133
127 This will make the images' architecture show up in the list of usable134 This will make the images' architecture show up in the list of usable
@@ -131,14 +138,19 @@
131 """138 """
132 if nodegroup is None:139 if nodegroup is None:
133 nodegroup = factory.make_node_group()140 nodegroup = factory.make_node_group()
141 if osystem is None:
142 osystem = factory.make_name('os')
134 if arch is None:143 if arch is None:
135 arch = factory.make_name('arch')144 arch = factory.make_name('arch')
136 if subarchitecture is None:145 if subarchitecture is None:
137 subarchitecture = factory.make_name('subarch')146 subarchitecture = factory.make_name('subarch')
147 if release is None:
148 release = factory.make_name('release')
138 for purpose in ['install', 'commissioning']:149 for purpose in ['install', 'commissioning']:
139 factory.make_boot_image(150 factory.make_boot_image(
140 nodegroup=nodegroup, architecture=arch,151 nodegroup=nodegroup, osystem=osystem, architecture=arch,
141 subarchitecture=subarchitecture, purpose=purpose)152 subarchitecture=subarchitecture, release=release,
153 purpose=purpose)
142154
143 def test_initialize_node_group_leaves_nodegroup_reference_intact(self):155 def test_initialize_node_group_leaves_nodegroup_reference_intact(self):
144 preselected_nodegroup = factory.make_node_group()156 preselected_nodegroup = factory.make_node_group()
@@ -201,6 +213,67 @@
201 arches = [factory.make_name('arch') for _ in range(5)]213 arches = [factory.make_name('arch') for _ in range(5)]
202 self.assertEqual(arches[0], pick_default_architecture(arches))214 self.assertEqual(arches[0], pick_default_architecture(arches))
203215
216 def test_list_all_usable_osystems_combines_nodegroups(self):
217 osystem_names = [factory.make_name('os') for _ in range(3)]
218 expected = []
219 for name in osystem_names:
220 self.make_usable_boot_images(osystem=name)
221 expected.append(make_usable_osystem(self, name))
222 self.assertItemsEqual(expected, list_all_usable_osystems())
223
224 def test_list_all_usable_osystems_sorts_output(self):
225 osystem_names = [factory.make_name('os') for _ in range(3)]
226 expected = []
227 for name in osystem_names:
228 self.make_usable_boot_images(osystem=name)
229 expected.append(make_usable_osystem(self, name))
230 expected = sorted(expected, key=lambda osystem: osystem.title)
231 self.assertEqual(expected, list_all_usable_osystems())
232
233 def test_list_all_usable_osystems_returns_no_duplicates(self):
234 os_name = factory.make_name('os')
235 self.make_usable_boot_images(osystem=os_name)
236 self.make_usable_boot_images(osystem=os_name)
237 osystem = make_usable_osystem(self, os_name)
238 self.assertEqual(
239 [osystem], list_all_usable_osystems())
240
241 def test_list_all_usable_releases_combines_nodegroups(self):
242 expected = {}
243 osystems = []
244 os_names = [factory.make_name('os') for _ in range(3)]
245 for name in os_names:
246 releases = [factory.make_name('release') for _ in range(3)]
247 for release in releases:
248 self.make_usable_boot_images(osystem=name, release=release)
249 osystems.append(
250 make_usable_osystem(self, name, releases=releases))
251 expected[name] = releases
252 self.assertItemsEqual(expected, list_all_usable_releases(osystems))
253
254 def test_list_all_usable_releases_sorts_output(self):
255 expected = {}
256 osystems = []
257 os_names = [factory.make_name('os') for _ in range(3)]
258 for name in os_names:
259 releases = [factory.make_name('release') for _ in range(3)]
260 for release in releases:
261 self.make_usable_boot_images(osystem=name, release=release)
262 osystems.append(
263 make_usable_osystem(self, name, releases=releases))
264 expected[name] = sorted(releases)
265 self.assertEqual(expected, list_all_usable_releases(osystems))
266
267 def test_list_all_usable_releases_returns_no_duplicates(self):
268 os_name = factory.make_name('os')
269 release = factory.make_name('release')
270 self.make_usable_boot_images(osystem=os_name, release=release)
271 self.make_usable_boot_images(osystem=os_name, release=release)
272 osystem = make_usable_osystem(self, os_name, releases=[release])
273 expected = {}
274 expected[os_name] = [release]
275 self.assertEqual(expected, list_all_usable_releases([osystem]))
276
204 def test_remove_None_values_removes_None_values_in_dict(self):277 def test_remove_None_values_removes_None_values_in_dict(self):
205 random_input = factory.getRandomString()278 random_input = factory.getRandomString()
206 self.assertEqual(279 self.assertEqual(
@@ -430,6 +503,7 @@
430 [503 [
431 'hostname',504 'hostname',
432 'architecture',505 'architecture',
506 'osystem',
433 'distro_series',507 'distro_series',
434 'nodegroup',508 'nodegroup',
435 ], list(form.fields))509 ], list(form.fields))
@@ -489,6 +563,77 @@
489 [NO_ARCHITECTURES_AVAILABLE],563 [NO_ARCHITECTURES_AVAILABLE],
490 form.errors['architecture'])564 form.errors['architecture'])
491565
566 def test_accepts_osystem(self):
567 osystem = make_usable_osystem(self)
568 form = NodeForm(data={
569 'hostname': factory.make_name('host'),
570 'architecture': make_usable_architecture(self),
571 'osystem': osystem.name,
572 })
573 self.assertTrue(form.is_valid(), form._errors)
574
575 def test_rejects_invalid_osystem(self):
576 patch_usable_osystems(self)
577 form = NodeForm(data={
578 'hostname': factory.make_name('host'),
579 'architecture': make_usable_architecture(self),
580 'osystem': factory.make_name('os'),
581 })
582 self.assertFalse(form.is_valid())
583 self.assertItemsEqual(['osystem'], form._errors.keys())
584
585 def test_starts_with_default_osystem(self):
586 osystems = [make_osystem_with_releases(self) for _ in range(5)]
587 patch_usable_osystems(self, osystems)
588 form = NodeForm()
589 self.assertEqual(
590 '',
591 form.fields['osystem'].initial)
592
593 def test_accepts_osystem_distro_series(self):
594 osystem = make_usable_osystem(self)
595 release = osystem.get_default_release()
596 form = NodeForm(data={
597 'hostname': factory.make_name('host'),
598 'architecture': make_usable_architecture(self),
599 'osystem': osystem.name,
600 'distro_series': '%s/%s' % (osystem.name, release),
601 })
602 self.assertTrue(form.is_valid(), form._errors)
603
604 def test_rejects_invalid_osystem_distro_series(self):
605 osystem = make_usable_osystem(self)
606 release = factory.make_name('release')
607 form = NodeForm(data={
608 'hostname': factory.make_name('host'),
609 'architecture': make_usable_architecture(self),
610 'osystem': osystem.name,
611 'distro_series': '%s/%s' % (osystem.name, release),
612 })
613 self.assertFalse(form.is_valid())
614 self.assertItemsEqual(['distro_series'], form._errors.keys())
615
616 def test_starts_with_default_distro_series(self):
617 osystems = [make_osystem_with_releases(self) for _ in range(5)]
618 patch_usable_osystems(self, osystems)
619 form = NodeForm()
620 self.assertEqual(
621 '',
622 form.fields['distro_series'].initial)
623
624 def test_rejects_mismatch_osystem_distro_series(self):
625 osystem = make_usable_osystem(self)
626 release = osystem.get_default_release()
627 invalid = factory.make_name('invalid_os')
628 form = NodeForm(data={
629 'hostname': factory.make_name('host'),
630 'architecture': make_usable_architecture(self),
631 'osystem': osystem.name,
632 'distro_series': '%s/%s' % (invalid, release),
633 })
634 self.assertFalse(form.is_valid())
635 self.assertItemsEqual(['distro_series'], form._errors.keys())
636
492637
493class TestAdminNodeForm(MAASServerTestCase):638class TestAdminNodeForm(MAASServerTestCase):
494639
@@ -500,6 +645,7 @@
500 [645 [
501 'hostname',646 'hostname',
502 'architecture',647 'architecture',
648 'osystem',
503 'distro_series',649 'distro_series',
504 'power_type',650 'power_type',
505 'power_parameters',651 'power_parameters',
506652
=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py 2014-05-13 18:43:40 +0000
+++ src/maasserver/tests/test_preseed.py 2014-05-29 17:03:44 +0000
@@ -22,7 +22,6 @@
22from django.conf import settings22from django.conf import settings
23from django.core.urlresolvers import reverse23from django.core.urlresolvers import reverse
24from maasserver.enum import (24from maasserver.enum import (
25 DISTRO_SERIES,
26 NODE_STATUS,25 NODE_STATUS,
27 NODEGROUPINTERFACE_MANAGEMENT,26 NODEGROUPINTERFACE_MANAGEMENT,
28 PRESEED_TYPE,27 PRESEED_TYPE,
@@ -590,7 +589,7 @@
590 node = factory.make_node()589 node = factory.make_node()
591 arch, subarch = node.architecture.split('/')590 arch, subarch = node.architecture.split('/')
592 factory.make_boot_image(591 factory.make_boot_image(
593 osystem='ubuntu',592 osystem=node.get_osystem(),
594 architecture=arch, subarchitecture=subarch,593 architecture=arch, subarchitecture=subarch,
595 release=node.get_distro_series(), purpose='xinstall',594 release=node.get_distro_series(), purpose='xinstall',
596 nodegroup=node.nodegroup)595 nodegroup=node.nodegroup)
@@ -678,18 +677,15 @@
678 self.assertIn('cloud-init', context['curtin_preseed'])677 self.assertIn('cloud-init', context['curtin_preseed'])
679678
680 def test_get_curtin_installer_url_returns_url(self):679 def test_get_curtin_installer_url_returns_url(self):
681 osystem = 'ubuntu'680 osystem = factory.getRandomOS()
682 # Exclude DISTRO_SERIES.default. It's a special value that defers681 series = factory.getRandomRelease(osystem)
683 # to a run-time setting which we don't provide in this test.
684 series = factory.getRandomEnum(
685 DISTRO_SERIES, but_not=DISTRO_SERIES.default)
686 architecture = make_usable_architecture(self)682 architecture = make_usable_architecture(self)
687 node = factory.make_node(683 node = factory.make_node(
688 architecture=architecture,684 osystem=osystem.name, architecture=architecture,
689 distro_series=series)685 distro_series=series)
690 arch, subarch = architecture.split('/')686 arch, subarch = architecture.split('/')
691 boot_image = factory.make_boot_image(687 boot_image = factory.make_boot_image(
692 osystem=osystem, architecture=arch,688 osystem=osystem.name, architecture=arch,
693 subarchitecture=subarch, release=series,689 subarchitecture=subarch, release=series,
694 purpose='xinstall', nodegroup=node.nodegroup)690 purpose='xinstall', nodegroup=node.nodegroup)
695691
@@ -698,20 +694,20 @@
698 [interface] = node.nodegroup.get_managed_interfaces()694 [interface] = node.nodegroup.get_managed_interfaces()
699 self.assertEqual(695 self.assertEqual(
700 'http://%s/MAAS/static/images/%s/%s/%s/%s/%s/root-tgz' % (696 'http://%s/MAAS/static/images/%s/%s/%s/%s/%s/root-tgz' % (
701 interface.ip, osystem, arch, subarch,697 interface.ip, osystem.name, arch, subarch,
702 series, boot_image.label),698 series, boot_image.label),
703 installer_url)699 installer_url)
704700
705 def test_get_curtin_installer_url_fails_if_no_boot_image(self):701 def test_get_curtin_installer_url_fails_if_no_boot_image(self):
706 osystem = 'ubuntu'702 osystem = factory.getRandomOS()
707 series = factory.getRandomEnum(703 series = factory.getRandomRelease(osystem)
708 DISTRO_SERIES, but_not=DISTRO_SERIES.default)
709 architecture = make_usable_architecture(self)704 architecture = make_usable_architecture(self)
710 node = factory.make_node(705 node = factory.make_node(
706 osystem=osystem.name,
711 architecture=architecture, distro_series=series)707 architecture=architecture, distro_series=series)
712 # Generate a boot image with a different arch/subarch.708 # Generate a boot image with a different arch/subarch.
713 factory.make_boot_image(709 factory.make_boot_image(
714 osystem=osystem,710 osystem=osystem.name,
715 architecture=factory.make_name('arch'),711 architecture=factory.make_name('arch'),
716 subarchitecture=factory.make_name('subarch'), release=series,712 subarchitecture=factory.make_name('subarch'), release=series,
717 purpose='xinstall', nodegroup=node.nodegroup)713 purpose='xinstall', nodegroup=node.nodegroup)
@@ -722,7 +718,7 @@
722 msg = (718 msg = (
723 "No image could be found for the given selection: "719 "No image could be found for the given selection: "
724 "os=%s, arch=%s, subarch=%s, series=%s, purpose=xinstall." % (720 "os=%s, arch=%s, subarch=%s, series=%s, purpose=xinstall." % (
725 osystem,721 osystem.name,
726 arch,722 arch,
727 subarch,723 subarch,
728 node.get_distro_series(),724 node.get_distro_series(),
729725
=== modified file 'src/maasserver/views/settings.py'
--- src/maasserver/views/settings.py 2014-04-10 13:43:33 +0000
+++ src/maasserver/views/settings.py 2014-05-29 17:03:44 +0000
@@ -41,6 +41,7 @@
41from maasserver.exceptions import CannotDeleteUserException41from maasserver.exceptions import CannotDeleteUserException
42from maasserver.forms import (42from maasserver.forms import (
43 CommissioningForm,43 CommissioningForm,
44 DeployForm,
44 EditUserForm,45 EditUserForm,
45 GlobalKernelOptsForm,46 GlobalKernelOptsForm,
46 MAASAndNetworkForm,47 MAASAndNetworkForm,
@@ -177,6 +178,13 @@
177 if response is not None:178 if response is not None:
178 return response179 return response
179180
181 # Process the Deploy form.
182 deploy_form, response = process_form(
183 request, DeployForm, reverse('settings'), 'deploy',
184 "Configuration updated.")
185 if response is not None:
186 return response
187
180 # Process the Ubuntu form.188 # Process the Ubuntu form.
181 ubuntu_form, response = process_form(189 ubuntu_form, response = process_form(
182 request, UbuntuForm, reverse('settings'), 'ubuntu',190 request, UbuntuForm, reverse('settings'), 'ubuntu',
@@ -202,6 +210,7 @@
202 'maas_and_network_form': maas_and_network_form,210 'maas_and_network_form': maas_and_network_form,
203 'third_party_drivers_form': third_party_drivers_form,211 'third_party_drivers_form': third_party_drivers_form,
204 'commissioning_form': commissioning_form,212 'commissioning_form': commissioning_form,
213 'deploy_form': deploy_form,
205 'ubuntu_form': ubuntu_form,214 'ubuntu_form': ubuntu_form,
206 'kernelopts_form': kernelopts_form,215 'kernelopts_form': kernelopts_form,
207 },216 },
208217
=== modified file 'src/maasserver/views/tests/test_settings.py'
--- src/maasserver/views/tests/test_settings.py 2014-04-10 13:43:33 +0000
+++ src/maasserver/views/tests/test_settings.py 2014-05-29 17:03:44 +0000
@@ -20,14 +20,11 @@
20from django.contrib.auth.models import User20from django.contrib.auth.models import User
21from django.core.urlresolvers import reverse21from django.core.urlresolvers import reverse
22from lxml.html import fromstring22from lxml.html import fromstring
23from maasserver.enum import (
24 COMMISSIONING_DISTRO_SERIES_CHOICES,
25 DISTRO_SERIES,
26 )
27from maasserver.models import (23from maasserver.models import (
28 Config,24 Config,
29 UserProfile,25 UserProfile,
30 )26 )
27from maasserver.models.config import DEFAULT_OS
31from maasserver.testing import (28from maasserver.testing import (
32 extract_redirect,29 extract_redirect,
33 get_prefixed_form_data,30 get_prefixed_form_data,
@@ -112,8 +109,7 @@
112 def test_settings_commissioning_POST(self):109 def test_settings_commissioning_POST(self):
113 self.client_log_in(as_admin=True)110 self.client_log_in(as_admin=True)
114 new_check_compatibility = factory.getRandomBoolean()111 new_check_compatibility = factory.getRandomBoolean()
115 new_commissioning_distro_series = factory.getRandomChoice(112 new_commissioning = factory.getRandomCommissioningRelease(DEFAULT_OS)
116 COMMISSIONING_DISTRO_SERIES_CHOICES)
117 response = self.client.post(113 response = self.client.post(
118 reverse('settings'),114 reverse('settings'),
119 get_prefixed_form_data(115 get_prefixed_form_data(
@@ -121,14 +117,14 @@
121 data={117 data={
122 'check_compatibility': new_check_compatibility,118 'check_compatibility': new_check_compatibility,
123 'commissioning_distro_series': (119 'commissioning_distro_series': (
124 new_commissioning_distro_series),120 new_commissioning),
125 }))121 }))
126122
127 self.assertEqual(httplib.FOUND, response.status_code)123 self.assertEqual(httplib.FOUND, response.status_code)
128 self.assertEqual(124 self.assertEqual(
129 (125 (
130 new_check_compatibility,126 new_check_compatibility,
131 new_commissioning_distro_series,127 new_commissioning,
132 ),128 ),
133 (129 (
134 Config.objects.get_config('check_compatibility'),130 Config.objects.get_config('check_compatibility'),
@@ -156,11 +152,38 @@
156 Config.objects.get_config('enable_third_party_drivers'),152 Config.objects.get_config('enable_third_party_drivers'),
157 ))153 ))
158154
155 def test_settings_deploy_POST(self):
156 self.client_log_in(as_admin=True)
157 new_osystem = factory.getRandomOS()
158 new_default_osystem = new_osystem.name
159 new_default_distro_series = factory.getRandomRelease(new_osystem)
160 response = self.client.post(
161 reverse('settings'),
162 get_prefixed_form_data(
163 prefix='deploy',
164 data={
165 'default_osystem': new_default_osystem,
166 'default_distro_series': '%s/%s' % (
167 new_default_osystem,
168 new_default_distro_series
169 ),
170 }))
171
172 self.assertEqual(httplib.FOUND, response.status_code, response.content)
173 self.assertEqual(
174 (
175 new_default_osystem,
176 new_default_distro_series,
177 ),
178 (
179 Config.objects.get_config('default_osystem'),
180 Config.objects.get_config('default_distro_series'),
181 ))
182
159 def test_settings_ubuntu_POST(self):183 def test_settings_ubuntu_POST(self):
160 self.client_log_in(as_admin=True)184 self.client_log_in(as_admin=True)
161 new_main_archive = 'http://test.example.com/archive'185 new_main_archive = 'http://test.example.com/archive'
162 new_ports_archive = 'http://test2.example.com/archive'186 new_ports_archive = 'http://test2.example.com/archive'
163 new_default_distro_series = factory.getRandomEnum(DISTRO_SERIES)
164 response = self.client.post(187 response = self.client.post(
165 reverse('settings'),188 reverse('settings'),
166 get_prefixed_form_data(189 get_prefixed_form_data(
@@ -168,7 +191,6 @@
168 data={191 data={
169 'main_archive': new_main_archive,192 'main_archive': new_main_archive,
170 'ports_archive': new_ports_archive,193 'ports_archive': new_ports_archive,
171 'default_distro_series': new_default_distro_series,
172 }))194 }))
173195
174 self.assertEqual(httplib.FOUND, response.status_code, response.content)196 self.assertEqual(httplib.FOUND, response.status_code, response.content)
@@ -176,12 +198,10 @@
176 (198 (
177 new_main_archive,199 new_main_archive,
178 new_ports_archive,200 new_ports_archive,
179 new_default_distro_series,
180 ),201 ),
181 (202 (
182 Config.objects.get_config('main_archive'),203 Config.objects.get_config('main_archive'),
183 Config.objects.get_config('ports_archive'),204 Config.objects.get_config('ports_archive'),
184 Config.objects.get_config('default_distro_series'),
185 ))205 ))
186206
187 def test_settings_kernelopts_POST(self):207 def test_settings_kernelopts_POST(self):
188208
=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py 2014-04-23 16:09:39 +0000
+++ src/metadataserver/tests/test_api.py 2014-05-29 17:03:44 +0000
@@ -394,7 +394,7 @@
394 node = factory.make_node()394 node = factory.make_node()
395 arch, subarch = node.architecture.split('/')395 arch, subarch = node.architecture.split('/')
396 factory.make_boot_image(396 factory.make_boot_image(
397 osystem='ubuntu',397 osystem=node.get_osystem(),
398 architecture=arch, subarchitecture=subarch,398 architecture=arch, subarchitecture=subarch,
399 release=node.get_distro_series(), purpose='xinstall',399 release=node.get_distro_series(), purpose='xinstall',
400 nodegroup=node.nodegroup)400 nodegroup=node.nodegroup)