Merge lp:~blake-rouse/maas/add-osystem-to-node-form-api into lp:~maas-committers/maas/trunk
- add-osystem-to-node-form-api
- Merge into trunk
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 | ||||
Related bugs: |
|
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-
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_
Note: osystem was used throughout the code instead of os, as the python os module would conflict throughout the code base.
Blake Rouse (blake-rouse) wrote : | # |
[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.
NodeForm.
- test_contains_
- test_accepts_
- test_rejects_
- test_starts_
- test_accepts_
- test_rejects_
- test_starts_
- test_rejects_
maasserver.
These are getting covered by the usage of the forms, views, and api throughout the testing code base, as you stated.
- maasserver.
- maasserver.
- NodeForm.
- maasserver.
- maasserver.
- maasserver.
- maasserver.
- maasserver.
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
Jason Hobbs (jason-hobbs) : | # |
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_
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.
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/
Jeroen T. Vermeulen (jtv) wrote : | # |
I just reviewed src/maasserver/
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-
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.
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.
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.)
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:/
> You are the owner of lp:~blake-rouse/maas/add-osystem-to-node-form-api.
>
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.
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?
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.
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.
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.
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_commisioni
.
In test_get_
.
In make_osystem_
.
Nice tests for list_all_
.
It'd be nice to rename _all_ mention of “series” to “releases”... maybe someday.
Preview Diff
1 | === modified file 'src/maasserver/api.py' | |||
2 | --- src/maasserver/api.py 2014-05-27 15:05:37 +0000 | |||
3 | +++ src/maasserver/api.py 2014-05-29 17:03:44 +0000 | |||
4 | @@ -253,6 +253,10 @@ | |||
5 | 253 | from piston.emitters import JSONEmitter | 253 | from piston.emitters import JSONEmitter |
6 | 254 | from piston.handler import typemapper | 254 | from piston.handler import typemapper |
7 | 255 | from piston.utils import rc | 255 | from piston.utils import rc |
8 | 256 | from provisioningserver.driver import ( | ||
9 | 257 | BOOT_IMAGE_PURPOSE, | ||
10 | 258 | OperatingSystemRegistry, | ||
11 | 259 | ) | ||
12 | 256 | from provisioningserver.kernel_opts import KernelParameters | 260 | from provisioningserver.kernel_opts import KernelParameters |
13 | 257 | from provisioningserver.power_schema import UNKNOWN_POWER_TYPE | 261 | from provisioningserver.power_schema import UNKNOWN_POWER_TYPE |
14 | 258 | import simplejson as json | 262 | import simplejson as json |
15 | @@ -411,7 +415,7 @@ | |||
16 | 411 | available to the nodes through the metadata service. | 415 | available to the nodes through the metadata service. |
17 | 412 | :type user_data: base64-encoded unicode | 416 | :type user_data: base64-encoded unicode |
18 | 413 | :param distro_series: If present, this parameter specifies the | 417 | :param distro_series: If present, this parameter specifies the |
20 | 414 | Ubuntu Release the node will use. | 418 | OS release the node will use. |
21 | 415 | :type distro_series: unicode | 419 | :type distro_series: unicode |
22 | 416 | 420 | ||
23 | 417 | Ideally we'd have MIME multipart and content-transfer-encoding etc. | 421 | Ideally we'd have MIME multipart and content-transfer-encoding etc. |
24 | @@ -427,7 +431,13 @@ | |||
25 | 427 | node = Node.objects.get_node_or_404( | 431 | node = Node.objects.get_node_or_404( |
26 | 428 | system_id=system_id, user=request.user, | 432 | system_id=system_id, user=request.user, |
27 | 429 | perm=NODE_PERMISSION.EDIT) | 433 | perm=NODE_PERMISSION.EDIT) |
29 | 430 | node.set_distro_series(series=series) | 434 | Form = get_node_edit_form(request.user) |
30 | 435 | form = Form(instance=node) | ||
31 | 436 | form.set_distro_series(series=series) | ||
32 | 437 | if form.is_valid(): | ||
33 | 438 | form.save() | ||
34 | 439 | else: | ||
35 | 440 | raise ValidationError(form.errors) | ||
36 | 431 | nodes = Node.objects.start_nodes( | 441 | nodes = Node.objects.start_nodes( |
37 | 432 | [system_id], request.user, user_data=user_data) | 442 | [system_id], request.user, user_data=user_data) |
38 | 433 | if len(nodes) == 0: | 443 | if len(nodes) == 0: |
39 | @@ -2369,7 +2379,7 @@ | |||
40 | 2369 | context_instance=RequestContext(request)) | 2379 | context_instance=RequestContext(request)) |
41 | 2370 | 2380 | ||
42 | 2371 | 2381 | ||
44 | 2372 | def get_boot_purpose(node): | 2382 | def get_boot_purpose(node, osystem, arch, subarch, series, label): |
45 | 2373 | """Return a suitable "purpose" for this boot, e.g. "install".""" | 2383 | """Return a suitable "purpose" for this boot, e.g. "install".""" |
46 | 2374 | # XXX: allenap bug=1031406 2012-07-31: The boot purpose is still in | 2384 | # XXX: allenap bug=1031406 2012-07-31: The boot purpose is still in |
47 | 2375 | # flux. It may be that there will just be an "ephemeral" environment and | 2385 | # flux. It may be that there will just be an "ephemeral" environment and |
48 | @@ -2389,7 +2399,14 @@ | |||
49 | 2389 | if node.should_use_traditional_installer(): | 2399 | if node.should_use_traditional_installer(): |
50 | 2390 | return "install" | 2400 | return "install" |
51 | 2391 | else: | 2401 | else: |
53 | 2392 | return "xinstall" | 2402 | # Return normal install if the booting operating system |
54 | 2403 | # doesn't support xinstall. | ||
55 | 2404 | osystem_obj = OperatingSystemRegistry[osystem] | ||
56 | 2405 | purposes = osystem_obj.get_boot_image_purposes( | ||
57 | 2406 | arch, subarch, series, label) | ||
58 | 2407 | if BOOT_IMAGE_PURPOSE.XINSTALL in purposes: | ||
59 | 2408 | return "xinstall" | ||
60 | 2409 | return "install" | ||
61 | 2393 | else: | 2410 | else: |
62 | 2394 | return "local" # TODO: Investigate. | 2411 | return "local" # TODO: Investigate. |
63 | 2395 | else: | 2412 | else: |
64 | @@ -2457,12 +2474,12 @@ | |||
65 | 2457 | node = get_node_from_mac_string(request.GET.get('mac', None)) | 2474 | node = get_node_from_mac_string(request.GET.get('mac', None)) |
66 | 2458 | 2475 | ||
67 | 2459 | if node is None or node.status == NODE_STATUS.COMMISSIONING: | 2476 | if node is None or node.status == NODE_STATUS.COMMISSIONING: |
68 | 2477 | osystem = Config.objects.get_config('commissioning_osystem') | ||
69 | 2460 | series = Config.objects.get_config('commissioning_distro_series') | 2478 | series = Config.objects.get_config('commissioning_distro_series') |
70 | 2461 | else: | 2479 | else: |
71 | 2480 | osystem = node.get_osystem() | ||
72 | 2462 | series = node.get_distro_series() | 2481 | series = node.get_distro_series() |
73 | 2463 | 2482 | ||
74 | 2464 | purpose = get_boot_purpose(node) | ||
75 | 2465 | |||
76 | 2466 | if node: | 2483 | if node: |
77 | 2467 | arch, subarch = node.architecture.split('/') | 2484 | arch, subarch = node.architecture.split('/') |
78 | 2468 | preseed_url = compose_preseed_url(node) | 2485 | preseed_url = compose_preseed_url(node) |
79 | @@ -2488,7 +2505,7 @@ | |||
80 | 2488 | # current series. If nothing is found, fall back to i386 like | 2505 | # current series. If nothing is found, fall back to i386 like |
81 | 2489 | # we used to. LP #1181334 | 2506 | # we used to. LP #1181334 |
82 | 2490 | image = BootImage.objects.get_default_arch_image_in_nodegroup( | 2507 | image = BootImage.objects.get_default_arch_image_in_nodegroup( |
84 | 2491 | nodegroup, 'ubuntu', series, purpose=purpose) | 2508 | nodegroup, osystem, series, purpose='commissioning') |
85 | 2492 | if image is None: | 2509 | if image is None: |
86 | 2493 | arch = 'i386' | 2510 | arch = 'i386' |
87 | 2494 | else: | 2511 | else: |
88 | @@ -2496,12 +2513,16 @@ | |||
89 | 2496 | 2513 | ||
90 | 2497 | subarch = get_optional_param(request.GET, 'subarch', 'generic') | 2514 | subarch = get_optional_param(request.GET, 'subarch', 'generic') |
91 | 2498 | 2515 | ||
92 | 2516 | # Get the purpose, without a selected label | ||
93 | 2517 | purpose = get_boot_purpose( | ||
94 | 2518 | node, osystem, arch, subarch, series, label=None) | ||
95 | 2519 | |||
96 | 2499 | # We use as our default label the label of the most recent image for | 2520 | # We use as our default label the label of the most recent image for |
97 | 2500 | # the criteria we've assembled above. If there is no latest image | 2521 | # the criteria we've assembled above. If there is no latest image |
98 | 2501 | # (which should never happen in reality but may happen in tests), we | 2522 | # (which should never happen in reality but may happen in tests), we |
99 | 2502 | # fall back to using 'no-such-image' as our default. | 2523 | # fall back to using 'no-such-image' as our default. |
100 | 2503 | latest_image = BootImage.objects.get_latest_image( | 2524 | latest_image = BootImage.objects.get_latest_image( |
102 | 2504 | nodegroup, 'ubuntu', arch, subarch, series, purpose) | 2525 | nodegroup, osystem, arch, subarch, series, purpose) |
103 | 2505 | if latest_image is None: | 2526 | if latest_image is None: |
104 | 2506 | # XXX 2014-03-18 gmb bug=1294131: | 2527 | # XXX 2014-03-18 gmb bug=1294131: |
105 | 2507 | # We really ought to raise an exception here so that client | 2528 | # We really ought to raise an exception here so that client |
106 | @@ -2519,6 +2540,9 @@ | |||
107 | 2519 | subarch = latest_image.subarchitecture | 2540 | subarch = latest_image.subarchitecture |
108 | 2520 | label = get_optional_param(request.GET, 'label', latest_label) | 2541 | label = get_optional_param(request.GET, 'label', latest_label) |
109 | 2521 | 2542 | ||
110 | 2543 | # Get the supported purpose with the boot label | ||
111 | 2544 | purpose = get_boot_purpose(node, osystem, arch, subarch, series, label) | ||
112 | 2545 | |||
113 | 2522 | if node is not None: | 2546 | if node is not None: |
114 | 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, |
115 | 2524 | # just get the options | 2548 | # just get the options |
116 | @@ -2548,7 +2572,7 @@ | |||
117 | 2548 | cluster_address = get_mandatory_param(request.GET, "local") | 2572 | cluster_address = get_mandatory_param(request.GET, "local") |
118 | 2549 | 2573 | ||
119 | 2550 | params = KernelParameters( | 2574 | params = KernelParameters( |
121 | 2551 | osystem='ubuntu', arch=arch, subarch=subarch, release=series, | 2575 | osystem=osystem, arch=arch, subarch=subarch, release=series, |
122 | 2552 | label=label, purpose=purpose, hostname=hostname, domain=domain, | 2576 | label=label, purpose=purpose, hostname=hostname, domain=domain, |
123 | 2553 | preseed_url=preseed_url, log_host=server_address, | 2577 | preseed_url=preseed_url, log_host=server_address, |
124 | 2554 | fs_host=cluster_address, extra_opts=extra_kernel_opts) | 2578 | fs_host=cluster_address, extra_opts=extra_kernel_opts) |
125 | 2555 | 2579 | ||
126 | === modified file 'src/maasserver/forms.py' | |||
127 | --- src/maasserver/forms.py 2014-05-26 14:14:57 +0000 | |||
128 | +++ src/maasserver/forms.py 2014-05-29 17:03:44 +0000 | |||
129 | @@ -73,9 +73,6 @@ | |||
130 | 73 | ) | 73 | ) |
131 | 74 | from maasserver.config_forms import SKIP_CHECK_NAME | 74 | from maasserver.config_forms import SKIP_CHECK_NAME |
132 | 75 | from maasserver.enum import ( | 75 | from maasserver.enum import ( |
133 | 76 | COMMISSIONING_DISTRO_SERIES_CHOICES, | ||
134 | 77 | DISTRO_SERIES, | ||
135 | 78 | DISTRO_SERIES_CHOICES, | ||
136 | 79 | NODE_STATUS, | 76 | NODE_STATUS, |
137 | 80 | NODEGROUPINTERFACE_MANAGEMENT, | 77 | NODEGROUPINTERFACE_MANAGEMENT, |
138 | 81 | NODEGROUPINTERFACE_MANAGEMENT_CHOICES, | 78 | NODEGROUPINTERFACE_MANAGEMENT_CHOICES, |
139 | @@ -88,8 +85,8 @@ | |||
140 | 88 | from maasserver.forms_settings import ( | 85 | from maasserver.forms_settings import ( |
141 | 89 | CONFIG_ITEMS_KEYS, | 86 | CONFIG_ITEMS_KEYS, |
142 | 90 | get_config_field, | 87 | get_config_field, |
143 | 91 | INVALID_DISTRO_SERIES_MESSAGE, | ||
144 | 92 | INVALID_SETTING_MSG_TEMPLATE, | 88 | INVALID_SETTING_MSG_TEMPLATE, |
145 | 89 | list_commisioning_choices, | ||
146 | 93 | ) | 90 | ) |
147 | 94 | from maasserver.models import ( | 91 | from maasserver.models import ( |
148 | 95 | BootImage, | 92 | BootImage, |
149 | @@ -117,6 +114,7 @@ | |||
150 | 117 | from maasserver.utils.network import make_network | 114 | from maasserver.utils.network import make_network |
151 | 118 | from metadataserver.fields import Bin | 115 | from metadataserver.fields import Bin |
152 | 119 | from metadataserver.models import CommissioningScript | 116 | from metadataserver.models import CommissioningScript |
153 | 117 | from provisioningserver.driver import OperatingSystemRegistry | ||
154 | 120 | 118 | ||
155 | 121 | # A reusable null-option for choice fields. | 119 | # A reusable null-option for choice fields. |
156 | 122 | BLANK_CHOICE = ('', '-------') | 120 | BLANK_CHOICE = ('', '-------') |
157 | @@ -217,6 +215,106 @@ | |||
158 | 217 | return all_architectures[0] | 215 | return all_architectures[0] |
159 | 218 | 216 | ||
160 | 219 | 217 | ||
161 | 218 | def list_all_usable_osystems(): | ||
162 | 219 | """Return all operating systems that can be used for nodes. | ||
163 | 220 | |||
164 | 221 | These are the operating systems for which any nodegroup has the boot images | ||
165 | 222 | required to boot the node. | ||
166 | 223 | """ | ||
167 | 224 | # The Node edit form offers all usable operating systems as options for the | ||
168 | 225 | # osystem field. Not all of these may be available in the node's | ||
169 | 226 | # nodegroup, but to represent that accurately in the UI would depend on | ||
170 | 227 | # the currently selected nodegroup. Narrowing the options down further | ||
171 | 228 | # would have to happen browser-side. | ||
172 | 229 | osystems = set() | ||
173 | 230 | for nodegroup in NodeGroup.objects.all(): | ||
174 | 231 | osystems = osystems.union( | ||
175 | 232 | BootImage.objects.get_usable_osystems(nodegroup)) | ||
176 | 233 | osystems = [OperatingSystemRegistry[osystem] for osystem in osystems] | ||
177 | 234 | return sorted(osystems, key=lambda osystem: osystem.title) | ||
178 | 235 | |||
179 | 236 | |||
180 | 237 | def list_osystem_choices(osystems): | ||
181 | 238 | """Return Django "choices" list for `osystem`.""" | ||
182 | 239 | choices = [('', 'Default OS')] | ||
183 | 240 | choices += [ | ||
184 | 241 | (osystem.name, osystem.title) | ||
185 | 242 | for osystem in osystems | ||
186 | 243 | ] | ||
187 | 244 | return choices | ||
188 | 245 | |||
189 | 246 | |||
190 | 247 | def list_all_usable_releases(osystems): | ||
191 | 248 | """Return dictionary of usable `releases` for each opertaing system.""" | ||
192 | 249 | distro_series = {} | ||
193 | 250 | nodegroups = list(NodeGroup.objects.all()) | ||
194 | 251 | for osystem in osystems: | ||
195 | 252 | releases = set() | ||
196 | 253 | for nodegroup in nodegroups: | ||
197 | 254 | releases = releases.union( | ||
198 | 255 | BootImage.objects.get_usable_releases(nodegroup, osystem.name)) | ||
199 | 256 | distro_series[osystem.name] = sorted(releases) | ||
200 | 257 | return distro_series | ||
201 | 258 | |||
202 | 259 | |||
203 | 260 | def list_release_choices(releases): | ||
204 | 261 | """Return Django "choices" list for `releases`.""" | ||
205 | 262 | choices = [('', 'Default OS Release')] | ||
206 | 263 | for key, value in releases.items(): | ||
207 | 264 | osystem = OperatingSystemRegistry[key] | ||
208 | 265 | options = osystem.format_release_choices(value) | ||
209 | 266 | choices += [( | ||
210 | 267 | '%s/' % osystem.name, | ||
211 | 268 | 'Latest %s Release' % osystem.title | ||
212 | 269 | )] | ||
213 | 270 | choices += [ | ||
214 | 271 | ('%s/%s' % (osystem.name, name), title) | ||
215 | 272 | for name, title in options | ||
216 | 273 | ] | ||
217 | 274 | return choices | ||
218 | 275 | |||
219 | 276 | |||
220 | 277 | def get_distro_series_inital(instance): | ||
221 | 278 | """Returns the distro_series initial value for the instance.""" | ||
222 | 279 | osystem = instance.osystem | ||
223 | 280 | series = instance.distro_series | ||
224 | 281 | if osystem is not None and osystem != '': | ||
225 | 282 | if series is None: | ||
226 | 283 | series = '' | ||
227 | 284 | return '%s/%s' % (osystem, series) | ||
228 | 285 | return None | ||
229 | 286 | |||
230 | 287 | |||
231 | 288 | def clean_distro_series_field(form, field, os_field): | ||
232 | 289 | """Cleans the distro_series field in the form. Validating that | ||
233 | 290 | the selected operating system matches the distro_series. | ||
234 | 291 | |||
235 | 292 | :param form: `Form` class | ||
236 | 293 | :param field: distro_series field name | ||
237 | 294 | :param os_field: osystem field name | ||
238 | 295 | :returns: clean distro_series field value | ||
239 | 296 | """ | ||
240 | 297 | new_distro_series = form.cleaned_data.get(field) | ||
241 | 298 | if new_distro_series is None or '/' not in new_distro_series: | ||
242 | 299 | return new_distro_series | ||
243 | 300 | os, release = new_distro_series.split('/', 1) | ||
244 | 301 | if os_field in form.cleaned_data: | ||
245 | 302 | new_os = form.cleaned_data[os_field] | ||
246 | 303 | if os != new_os: | ||
247 | 304 | raise ValidationError( | ||
248 | 305 | "%s in %s does not match with " | ||
249 | 306 | "operating system %s" % (release, field, os)) | ||
250 | 307 | return release | ||
251 | 308 | |||
252 | 309 | |||
253 | 310 | def get_osystem_from_release(release): | ||
254 | 311 | """Returns the operating system that supports that release.""" | ||
255 | 312 | for _, osystem in OperatingSystemRegistry: | ||
256 | 313 | if release in osystem.get_supported_releases(): | ||
257 | 314 | return osystem | ||
258 | 315 | return None | ||
259 | 316 | |||
260 | 317 | |||
261 | 220 | class NodeForm(ModelForm): | 318 | class NodeForm(ModelForm): |
262 | 221 | 319 | ||
263 | 222 | def __init__(self, request=None, *args, **kwargs): | 320 | def __init__(self, request=None, *args, **kwargs): |
264 | @@ -229,6 +327,7 @@ | |||
265 | 229 | self.fields['nodegroup'] = NodeGroupFormField( | 327 | self.fields['nodegroup'] = NodeGroupFormField( |
266 | 230 | required=False, empty_label="Default (master)") | 328 | required=False, empty_label="Default (master)") |
267 | 231 | self.set_up_architecture_field() | 329 | self.set_up_architecture_field() |
268 | 330 | self.set_up_osystem_and_distro_series_fields(kwargs.get('instance')) | ||
269 | 232 | 331 | ||
270 | 233 | def set_up_architecture_field(self): | 332 | def set_up_architecture_field(self): |
271 | 234 | """Create the `architecture` field. | 333 | """Create the `architecture` field. |
272 | @@ -248,6 +347,32 @@ | |||
273 | 248 | choices=choices, required=True, initial=default_arch, | 347 | choices=choices, required=True, initial=default_arch, |
274 | 249 | error_messages={'invalid_choice': invalid_arch_message}) | 348 | error_messages={'invalid_choice': invalid_arch_message}) |
275 | 250 | 349 | ||
276 | 350 | def set_up_osystem_and_distro_series_fields(self, instance): | ||
277 | 351 | """Create the `osystem` and `distro_series` fields. | ||
278 | 352 | |||
279 | 353 | This needs to be done on the fly so that we can pass a dynamic list of | ||
280 | 354 | usable operating systems and distro_series. | ||
281 | 355 | """ | ||
282 | 356 | osystems = list_all_usable_osystems() | ||
283 | 357 | releases = list_all_usable_releases(osystems) | ||
284 | 358 | os_choices = list_osystem_choices(osystems) | ||
285 | 359 | distro_choices = list_release_choices(releases) | ||
286 | 360 | invalid_osystem_message = compose_invalid_choice_text( | ||
287 | 361 | 'osystem', os_choices) | ||
288 | 362 | invalid_distro_series_message = compose_invalid_choice_text( | ||
289 | 363 | 'distro_series', distro_choices) | ||
290 | 364 | self.fields['osystem'] = forms.ChoiceField( | ||
291 | 365 | label="OS", choices=os_choices, required=False, initial='', | ||
292 | 366 | error_messages={'invalid_choice': invalid_osystem_message}) | ||
293 | 367 | self.fields['distro_series'] = forms.ChoiceField( | ||
294 | 368 | label="Release", choices=distro_choices, | ||
295 | 369 | required=False, initial='', | ||
296 | 370 | error_messages={'invalid_choice': invalid_distro_series_message}) | ||
297 | 371 | if instance is not None: | ||
298 | 372 | initial_value = get_distro_series_inital(instance) | ||
299 | 373 | if instance is not None: | ||
300 | 374 | self.initial['distro_series'] = initial_value | ||
301 | 375 | |||
302 | 251 | def clean_hostname(self): | 376 | def clean_hostname(self): |
303 | 252 | # Don't allow the hostname to be changed if the node is | 377 | # Don't allow the hostname to be changed if the node is |
304 | 253 | # currently allocated. Juju knows the node by its old name, so | 378 | # currently allocated. Juju knows the node by its old name, so |
305 | @@ -261,6 +386,9 @@ | |||
306 | 261 | 386 | ||
307 | 262 | return new_hostname | 387 | return new_hostname |
308 | 263 | 388 | ||
309 | 389 | def clean_distro_series(self): | ||
310 | 390 | return clean_distro_series_field(self, 'distro_series', 'osystem') | ||
311 | 391 | |||
312 | 264 | def is_valid(self): | 392 | def is_valid(self): |
313 | 265 | is_valid = super(NodeForm, self).is_valid() | 393 | is_valid = super(NodeForm, self).is_valid() |
314 | 266 | if len(list_all_usable_architectures()) == 0: | 394 | if len(list_all_usable_architectures()) == 0: |
315 | @@ -269,11 +397,24 @@ | |||
316 | 269 | is_valid = False | 397 | is_valid = False |
317 | 270 | return is_valid | 398 | return is_valid |
318 | 271 | 399 | ||
324 | 272 | distro_series = forms.ChoiceField( | 400 | def set_distro_series(self, series=''): |
325 | 273 | choices=DISTRO_SERIES_CHOICES, required=False, | 401 | """Sets the osystem and distro_series, from the provided |
326 | 274 | initial=DISTRO_SERIES.default, | 402 | distro_series. |
327 | 275 | label="Release", | 403 | """ |
328 | 276 | error_messages={'invalid_choice': INVALID_DISTRO_SERIES_MESSAGE}) | 404 | # This implementation is used so that current API, is not broken. This |
329 | 405 | # makes the distro_series a flat namespace. The distro_series is used | ||
330 | 406 | # to search through the supporting operating systems, to find the | ||
331 | 407 | # correct operating system that supports this distro_series. | ||
332 | 408 | self.is_bound = True | ||
333 | 409 | self.data['osystem'] = '' | ||
334 | 410 | self.data['distro_series'] = '' | ||
335 | 411 | if series is not None and series != '': | ||
336 | 412 | osystem = get_osystem_from_release(series) | ||
337 | 413 | if osystem is not None: | ||
338 | 414 | self.data['osystem'] = osystem.name | ||
339 | 415 | self.data['distro_series'] = '%s/%s' % (osystem.name, series) | ||
340 | 416 | else: | ||
341 | 417 | self.data['distro_series'] = series | ||
342 | 277 | 418 | ||
343 | 278 | hostname = forms.CharField( | 419 | hostname = forms.CharField( |
344 | 279 | label="Host name", required=False, help_text=( | 420 | label="Host name", required=False, help_text=( |
345 | @@ -292,6 +433,7 @@ | |||
346 | 292 | fields = ( | 433 | fields = ( |
347 | 293 | 'hostname', | 434 | 'hostname', |
348 | 294 | 'architecture', | 435 | 'architecture', |
349 | 436 | 'osystem', | ||
350 | 295 | 'distro_series', | 437 | 'distro_series', |
351 | 296 | ) | 438 | ) |
352 | 297 | 439 | ||
353 | @@ -872,16 +1014,34 @@ | |||
354 | 872 | """Settings page, Commissioning section.""" | 1014 | """Settings page, Commissioning section.""" |
355 | 873 | check_compatibility = get_config_field('check_compatibility') | 1015 | check_compatibility = get_config_field('check_compatibility') |
356 | 874 | commissioning_distro_series = forms.ChoiceField( | 1016 | commissioning_distro_series = forms.ChoiceField( |
359 | 875 | choices=COMMISSIONING_DISTRO_SERIES_CHOICES, required=False, | 1017 | choices=list_commisioning_choices(), required=False, |
360 | 876 | label="Default distro series used for commissioning", | 1018 | label="Default Ubuntu release used for commissioning", |
361 | 877 | error_messages={'invalid_choice': compose_invalid_choice_text( | 1019 | error_messages={'invalid_choice': compose_invalid_choice_text( |
362 | 878 | 'commissioning_distro_series', | 1020 | 'commissioning_distro_series', |
364 | 879 | COMMISSIONING_DISTRO_SERIES_CHOICES)}) | 1021 | list_commisioning_choices())}) |
365 | 1022 | |||
366 | 1023 | |||
367 | 1024 | class DeployForm(ConfigForm): | ||
368 | 1025 | """Settings page, Deploy section.""" | ||
369 | 1026 | default_osystem = get_config_field('default_osystem') | ||
370 | 1027 | default_distro_series = get_config_field('default_distro_series') | ||
371 | 1028 | |||
372 | 1029 | def _load_initials(self): | ||
373 | 1030 | super(DeployForm, self)._load_initials() | ||
374 | 1031 | initial_os = self.initial['default_osystem'] | ||
375 | 1032 | initial_series = self.initial['default_distro_series'] | ||
376 | 1033 | self.initial['default_distro_series'] = '%s/%s' % ( | ||
377 | 1034 | initial_os, | ||
378 | 1035 | initial_series | ||
379 | 1036 | ) | ||
380 | 1037 | |||
381 | 1038 | def clean_default_distro_series(self): | ||
382 | 1039 | return clean_distro_series_field( | ||
383 | 1040 | self, 'default_distro_series', 'default_osystem') | ||
384 | 880 | 1041 | ||
385 | 881 | 1042 | ||
386 | 882 | class UbuntuForm(ConfigForm): | 1043 | class UbuntuForm(ConfigForm): |
387 | 883 | """Settings page, Ubuntu section.""" | 1044 | """Settings page, Ubuntu section.""" |
388 | 884 | default_distro_series = get_config_field('default_distro_series') | ||
389 | 885 | main_archive = get_config_field('main_archive') | 1045 | main_archive = get_config_field('main_archive') |
390 | 886 | ports_archive = get_config_field('ports_archive') | 1046 | ports_archive = get_config_field('ports_archive') |
391 | 887 | 1047 | ||
392 | 888 | 1048 | ||
393 | === modified file 'src/maasserver/forms_settings.py' | |||
394 | --- src/maasserver/forms_settings.py 2014-04-10 20:21:24 +0000 | |||
395 | +++ src/maasserver/forms_settings.py 2014-05-29 17:03:44 +0000 | |||
396 | @@ -23,19 +23,41 @@ | |||
397 | 23 | from socket import gethostname | 23 | from socket import gethostname |
398 | 24 | 24 | ||
399 | 25 | from django import forms | 25 | from django import forms |
405 | 26 | from maasserver.enum import ( | 26 | from maasserver.models.config import DEFAULT_OS |
401 | 27 | COMMISSIONING_DISTRO_SERIES_CHOICES, | ||
402 | 28 | DISTRO_SERIES, | ||
403 | 29 | DISTRO_SERIES_CHOICES, | ||
404 | 30 | ) | ||
406 | 31 | from maasserver.utils.forms import compose_invalid_choice_text | 27 | from maasserver.utils.forms import compose_invalid_choice_text |
407 | 28 | from provisioningserver.driver import OperatingSystemRegistry | ||
408 | 32 | 29 | ||
409 | 33 | 30 | ||
410 | 34 | INVALID_URL_MESSAGE = "Enter a valid url (e.g. http://host.example.com)." | 31 | INVALID_URL_MESSAGE = "Enter a valid url (e.g. http://host.example.com)." |
411 | 35 | 32 | ||
412 | 36 | 33 | ||
415 | 37 | INVALID_DISTRO_SERIES_MESSAGE = compose_invalid_choice_text( | 34 | def list_osystem_choices(): |
416 | 38 | 'distro_series', DISTRO_SERIES_CHOICES) | 35 | osystems = [osystem for _, osystem in OperatingSystemRegistry] |
417 | 36 | osystems = sorted(osystems, key=lambda osystem: osystem.title) | ||
418 | 37 | return [ | ||
419 | 38 | (osystem.name, osystem.title) | ||
420 | 39 | for osystem in osystems | ||
421 | 40 | ] | ||
422 | 41 | |||
423 | 42 | |||
424 | 43 | def list_release_choices(): | ||
425 | 44 | osystems = [osystem for _, osystem in OperatingSystemRegistry] | ||
426 | 45 | choices = [] | ||
427 | 46 | for osystem in osystems: | ||
428 | 47 | supported = sorted(osystem.get_supported_releases()) | ||
429 | 48 | options = osystem.format_release_choices(supported) | ||
430 | 49 | options = [ | ||
431 | 50 | ('%s/%s' % (osystem.name, name), title) | ||
432 | 51 | for name, title in options | ||
433 | 52 | ] | ||
434 | 53 | choices += options | ||
435 | 54 | return choices | ||
436 | 55 | |||
437 | 56 | |||
438 | 57 | def list_commisioning_choices(): | ||
439 | 58 | releases = DEFAULT_OS.get_supported_commissioning_releases() | ||
440 | 59 | options = DEFAULT_OS.format_release_choices(releases) | ||
441 | 60 | return list(options) | ||
442 | 39 | 61 | ||
443 | 40 | 62 | ||
444 | 41 | CONFIG_ITEMS = { | 63 | CONFIG_ITEMS = { |
445 | @@ -139,28 +161,46 @@ | |||
446 | 139 | "e.g. for ntp.ubuntu.com: '91.189.94.4'") | 161 | "e.g. for ntp.ubuntu.com: '91.189.94.4'") |
447 | 140 | } | 162 | } |
448 | 141 | }, | 163 | }, |
449 | 164 | 'default_osystem': { | ||
450 | 165 | 'default': DEFAULT_OS.name, | ||
451 | 166 | 'form': forms.ChoiceField, | ||
452 | 167 | 'form_kwargs': { | ||
453 | 168 | 'label': "Default operating system used for deployment", | ||
454 | 169 | 'choices': list_osystem_choices(), | ||
455 | 170 | 'required': False, | ||
456 | 171 | 'error_messages': { | ||
457 | 172 | 'invalid_choice': compose_invalid_choice_text( | ||
458 | 173 | 'osystem', | ||
459 | 174 | list_osystem_choices())}, | ||
460 | 175 | } | ||
461 | 176 | }, | ||
462 | 142 | 'default_distro_series': { | 177 | 'default_distro_series': { |
464 | 143 | 'default': DISTRO_SERIES.trusty, | 178 | 'default': '%s/%s' % ( |
465 | 179 | DEFAULT_OS.name, | ||
466 | 180 | DEFAULT_OS.get_default_release() | ||
467 | 181 | ), | ||
468 | 144 | 'form': forms.ChoiceField, | 182 | 'form': forms.ChoiceField, |
469 | 145 | 'form_kwargs': { | 183 | 'form_kwargs': { |
472 | 146 | 'label': "Default distro series used for deployment", | 184 | 'label': "Default OS release used for deployment", |
473 | 147 | 'choices': DISTRO_SERIES_CHOICES, | 185 | 'choices': list_release_choices(), |
474 | 148 | 'required': False, | 186 | 'required': False, |
475 | 149 | 'error_messages': { | 187 | 'error_messages': { |
477 | 150 | 'invalid_choice': INVALID_DISTRO_SERIES_MESSAGE}, | 188 | 'invalid_choice': compose_invalid_choice_text( |
478 | 189 | 'distro_series', | ||
479 | 190 | list_release_choices())}, | ||
480 | 151 | } | 191 | } |
481 | 152 | }, | 192 | }, |
482 | 153 | 'commissioning_distro_series': { | 193 | 'commissioning_distro_series': { |
484 | 154 | 'default': DISTRO_SERIES.trusty, | 194 | 'default': DEFAULT_OS.get_default_commissioning_release(), |
485 | 155 | 'form': forms.ChoiceField, | 195 | 'form': forms.ChoiceField, |
486 | 156 | 'form_kwargs': { | 196 | 'form_kwargs': { |
489 | 157 | 'label': "Default distro series used for commissioning", | 197 | 'label': "Default Ubuntu release used for commissioning", |
490 | 158 | 'choices': COMMISSIONING_DISTRO_SERIES_CHOICES, | 198 | 'choices': list_commisioning_choices(), |
491 | 159 | 'required': False, | 199 | 'required': False, |
492 | 160 | 'error_messages': { | 200 | 'error_messages': { |
493 | 161 | 'invalid_choice': compose_invalid_choice_text( | 201 | 'invalid_choice': compose_invalid_choice_text( |
494 | 162 | 'commissioning_distro_series', | 202 | 'commissioning_distro_series', |
496 | 163 | COMMISSIONING_DISTRO_SERIES_CHOICES)}, | 203 | list_commisioning_choices())}, |
497 | 164 | } | 204 | } |
498 | 165 | }, | 205 | }, |
499 | 166 | 'enable_third_party_drivers': { | 206 | 'enable_third_party_drivers': { |
500 | 167 | 207 | ||
501 | === modified file 'src/maasserver/models/config.py' | |||
502 | --- src/maasserver/models/config.py 2014-04-14 21:42:00 +0000 | |||
503 | +++ src/maasserver/models/config.py 2014-05-29 17:03:44 +0000 | |||
504 | @@ -28,8 +28,11 @@ | |||
505 | 28 | ) | 28 | ) |
506 | 29 | from django.db.models.signals import post_save | 29 | from django.db.models.signals import post_save |
507 | 30 | from maasserver import DefaultMeta | 30 | from maasserver import DefaultMeta |
508 | 31 | from maasserver.enum import DISTRO_SERIES | ||
509 | 32 | from maasserver.fields import JSONObjectField | 31 | from maasserver.fields import JSONObjectField |
510 | 32 | from provisioningserver.driver.os_ubuntu import UbuntuOS | ||
511 | 33 | |||
512 | 34 | |||
513 | 35 | DEFAULT_OS = UbuntuOS() | ||
514 | 33 | 36 | ||
515 | 34 | 37 | ||
516 | 35 | def get_default_config(): | 38 | def get_default_config(): |
517 | @@ -40,11 +43,14 @@ | |||
518 | 40 | # Ubuntu section configuration. | 43 | # Ubuntu section configuration. |
519 | 41 | 'main_archive': 'http://archive.ubuntu.com/ubuntu', | 44 | 'main_archive': 'http://archive.ubuntu.com/ubuntu', |
520 | 42 | 'ports_archive': 'http://ports.ubuntu.com/ubuntu-ports', | 45 | 'ports_archive': 'http://ports.ubuntu.com/ubuntu-ports', |
522 | 43 | 'commissioning_distro_series': DISTRO_SERIES.trusty, | 46 | 'commissioning_osystem': DEFAULT_OS.name, |
523 | 47 | 'commissioning_distro_series': | ||
524 | 48 | DEFAULT_OS.get_default_commissioning_release(), | ||
525 | 44 | # Network section configuration. | 49 | # Network section configuration. |
526 | 45 | 'maas_name': gethostname(), | 50 | 'maas_name': gethostname(), |
527 | 46 | 'enlistment_domain': b'local', | 51 | 'enlistment_domain': b'local', |
529 | 47 | 'default_distro_series': DISTRO_SERIES.trusty, | 52 | 'default_osystem': DEFAULT_OS.name, |
530 | 53 | 'default_distro_series': DEFAULT_OS.get_default_release(), | ||
531 | 48 | 'http_proxy': None, | 54 | 'http_proxy': None, |
532 | 49 | 'upstream_dns': None, | 55 | 'upstream_dns': None, |
533 | 50 | 'ntp_server': '91.189.94.4', # ntp.ubuntu.com | 56 | 'ntp_server': '91.189.94.4', # ntp.ubuntu.com |
534 | 51 | 57 | ||
535 | === modified file 'src/maasserver/models/node.py' | |||
536 | --- src/maasserver/models/node.py 2014-05-27 07:11:28 +0000 | |||
537 | +++ src/maasserver/models/node.py 2014-05-29 17:03:44 +0000 | |||
538 | @@ -49,8 +49,6 @@ | |||
539 | 49 | logger, | 49 | logger, |
540 | 50 | ) | 50 | ) |
541 | 51 | from maasserver.enum import ( | 51 | from maasserver.enum import ( |
542 | 52 | DISTRO_SERIES, | ||
543 | 53 | DISTRO_SERIES_CHOICES, | ||
544 | 54 | NODE_PERMISSION, | 52 | NODE_PERMISSION, |
545 | 55 | NODE_STATUS, | 53 | NODE_STATUS, |
546 | 56 | NODE_STATUS_CHOICES, | 54 | NODE_STATUS_CHOICES, |
547 | @@ -72,6 +70,7 @@ | |||
548 | 72 | strip_domain, | 70 | strip_domain, |
549 | 73 | ) | 71 | ) |
550 | 74 | from piston.models import Token | 72 | from piston.models import Token |
551 | 73 | from provisioningserver.driver import OperatingSystemRegistry | ||
552 | 75 | from provisioningserver.tasks import ( | 74 | from provisioningserver.tasks import ( |
553 | 76 | power_off, | 75 | power_off, |
554 | 77 | power_on, | 76 | power_on, |
555 | @@ -486,7 +485,7 @@ | |||
556 | 486 | max_length=20, blank=True, default='') | 485 | max_length=20, blank=True, default='') |
557 | 487 | 486 | ||
558 | 488 | distro_series = CharField( | 487 | distro_series = CharField( |
560 | 489 | max_length=20, choices=DISTRO_SERIES_CHOICES, blank=True, default='') | 488 | max_length=20, blank=True, default='') |
561 | 490 | 489 | ||
562 | 491 | architecture = CharField(max_length=31, blank=False) | 490 | architecture = CharField(max_length=31, blank=False) |
563 | 492 | 491 | ||
564 | @@ -793,21 +792,30 @@ | |||
565 | 793 | """The name of the queue for tasks specific to this node.""" | 792 | """The name of the queue for tasks specific to this node.""" |
566 | 794 | return self.nodegroup.work_queue | 793 | return self.nodegroup.work_queue |
567 | 795 | 794 | ||
568 | 795 | def get_osystem(self): | ||
569 | 796 | """Return the operating system to install that node.""" | ||
570 | 797 | use_default_osystem = (self.osystem is None or self.osystem == '') | ||
571 | 798 | if use_default_osystem: | ||
572 | 799 | return Config.objects.get_config('default_osystem') | ||
573 | 800 | else: | ||
574 | 801 | return self.osystem | ||
575 | 802 | |||
576 | 796 | def get_distro_series(self): | 803 | def get_distro_series(self): |
577 | 797 | """Return the distro series to install that node.""" | 804 | """Return the distro series to install that node.""" |
578 | 805 | use_default_osystem = ( | ||
579 | 806 | self.osystem is None or | ||
580 | 807 | self.osystem == '') | ||
581 | 798 | use_default_distro_series = ( | 808 | use_default_distro_series = ( |
585 | 799 | not self.distro_series or | 809 | self.distro_series is None or |
586 | 800 | self.distro_series == DISTRO_SERIES.default) | 810 | self.distro_series == '') |
587 | 801 | if use_default_distro_series: | 811 | if use_default_osystem and use_default_distro_series: |
588 | 802 | return Config.objects.get_config('default_distro_series') | 812 | return Config.objects.get_config('default_distro_series') |
589 | 813 | elif use_default_distro_series: | ||
590 | 814 | osystem = OperatingSystemRegistry[self.osystem] | ||
591 | 815 | return osystem.get_default_release() | ||
592 | 803 | else: | 816 | else: |
593 | 804 | return self.distro_series | 817 | return self.distro_series |
594 | 805 | 818 | ||
595 | 806 | def set_distro_series(self, series=''): | ||
596 | 807 | """Set the distro series to install that node.""" | ||
597 | 808 | self.distro_series = series | ||
598 | 809 | self.save() | ||
599 | 810 | |||
600 | 811 | def get_effective_power_parameters(self): | 819 | def get_effective_power_parameters(self): |
601 | 812 | """Return effective power parameters, including any defaults.""" | 820 | """Return effective power parameters, including any defaults.""" |
602 | 813 | if self.power_parameters: | 821 | if self.power_parameters: |
603 | @@ -860,6 +868,7 @@ | |||
604 | 860 | self.token = None | 868 | self.token = None |
605 | 861 | self.agent_name = '' | 869 | self.agent_name = '' |
606 | 862 | self.set_netboot() | 870 | self.set_netboot() |
607 | 871 | self.osystem = '' | ||
608 | 863 | self.distro_series = '' | 872 | self.distro_series = '' |
609 | 864 | self.save() | 873 | self.save() |
610 | 865 | 874 | ||
611 | 866 | 875 | ||
612 | === modified file 'src/maasserver/models/tests/test_node.py' | |||
613 | --- src/maasserver/models/tests/test_node.py 2014-05-27 12:35:35 +0000 | |||
614 | +++ src/maasserver/models/tests/test_node.py 2014-05-29 17:03:44 +0000 | |||
615 | @@ -20,7 +20,6 @@ | |||
616 | 20 | from django.core.exceptions import ValidationError | 20 | from django.core.exceptions import ValidationError |
617 | 21 | from maasserver.clusterrpc.power_parameters import get_power_types | 21 | from maasserver.clusterrpc.power_parameters import get_power_types |
618 | 22 | from maasserver.enum import ( | 22 | from maasserver.enum import ( |
619 | 23 | DISTRO_SERIES, | ||
620 | 24 | NODE_PERMISSION, | 23 | NODE_PERMISSION, |
621 | 25 | NODE_STATUS, | 24 | NODE_STATUS, |
622 | 26 | NODE_STATUS_CHOICES, | 25 | NODE_STATUS_CHOICES, |
623 | @@ -270,15 +269,14 @@ | |||
624 | 270 | offset += timedelta(1) | 269 | offset += timedelta(1) |
625 | 271 | self.assertEqual(macs[0], node.get_primary_mac().mac_address) | 270 | self.assertEqual(macs[0], node.get_primary_mac().mac_address) |
626 | 272 | 271 | ||
627 | 272 | def test_get_osystem_returns_default_osystem(self): | ||
628 | 273 | node = factory.make_node(osystem='') | ||
629 | 274 | osystem = Config.objects.get_config('default_osystem') | ||
630 | 275 | self.assertEqual(osystem, node.get_osystem()) | ||
631 | 276 | |||
632 | 273 | def test_get_distro_series_returns_default_series(self): | 277 | def test_get_distro_series_returns_default_series(self): |
641 | 274 | node = factory.make_node() | 278 | node = factory.make_node(distro_series='') |
642 | 275 | series = Config.objects.get_config('commissioning_distro_series') | 279 | series = Config.objects.get_config('default_distro_series') |
635 | 276 | self.assertEqual(series, node.get_distro_series()) | ||
636 | 277 | |||
637 | 278 | def test_set_get_distro_series_returns_series(self): | ||
638 | 279 | node = factory.make_node() | ||
639 | 280 | series = DISTRO_SERIES.quantal | ||
640 | 281 | node.set_distro_series(series) | ||
643 | 282 | self.assertEqual(series, node.get_distro_series()) | 280 | self.assertEqual(series, node.get_distro_series()) |
644 | 283 | 281 | ||
645 | 284 | def test_delete_node_deletes_related_mac(self): | 282 | def test_delete_node_deletes_related_mac(self): |
646 | @@ -597,11 +595,15 @@ | |||
647 | 597 | node.release() | 595 | node.release() |
648 | 598 | self.assertTrue(node.netboot) | 596 | self.assertTrue(node.netboot) |
649 | 599 | 597 | ||
651 | 600 | def test_release_clears_distro_series(self): | 598 | def test_release_clears_osystem_and_distro_series(self): |
652 | 601 | node = factory.make_node( | 599 | node = factory.make_node( |
653 | 602 | status=NODE_STATUS.ALLOCATED, owner=factory.make_user()) | 600 | status=NODE_STATUS.ALLOCATED, owner=factory.make_user()) |
655 | 603 | node.set_distro_series(series=DISTRO_SERIES.quantal) | 601 | osystem = factory.getRandomOS() |
656 | 602 | release = factory.getRandomRelease(osystem) | ||
657 | 603 | node.osystem = osystem.name | ||
658 | 604 | node.distro_series = release | ||
659 | 604 | node.release() | 605 | node.release() |
660 | 606 | self.assertEqual("", node.osystem) | ||
661 | 605 | self.assertEqual("", node.distro_series) | 607 | self.assertEqual("", node.distro_series) |
662 | 606 | 608 | ||
663 | 607 | def test_release_powers_off_node(self): | 609 | def test_release_powers_off_node(self): |
664 | 608 | 610 | ||
665 | === modified file 'src/maasserver/preseed.py' | |||
666 | --- src/maasserver/preseed.py 2014-04-23 16:09:39 +0000 | |||
667 | +++ src/maasserver/preseed.py 2014-05-29 17:03:44 +0000 | |||
668 | @@ -93,7 +93,7 @@ | |||
669 | 93 | 93 | ||
670 | 94 | def get_curtin_installer_url(node): | 94 | def get_curtin_installer_url(node): |
671 | 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.""" |
673 | 96 | osystem = 'ubuntu' | 96 | osystem = node.get_osystem() |
674 | 97 | series = node.get_distro_series() | 97 | series = node.get_distro_series() |
675 | 98 | cluster_host = pick_cluster_controller_address(node) | 98 | cluster_host = pick_cluster_controller_address(node) |
676 | 99 | # XXX rvb(?): The path shouldn't be hardcoded like this, but rather synced | 99 | # XXX rvb(?): The path shouldn't be hardcoded like this, but rather synced |
677 | 100 | 100 | ||
678 | === modified file 'src/maasserver/testing/osystems.py' | |||
679 | --- src/maasserver/testing/osystems.py 2014-05-14 15:12:48 +0000 | |||
680 | +++ src/maasserver/testing/osystems.py 2014-05-29 17:03:44 +0000 | |||
681 | @@ -25,23 +25,27 @@ | |||
682 | 25 | from provisioningserver.driver import BOOT_IMAGE_PURPOSE | 25 | from provisioningserver.driver import BOOT_IMAGE_PURPOSE |
683 | 26 | 26 | ||
684 | 27 | 27 | ||
686 | 28 | def make_osystem_with_releases(testcase, osystem_name=None, releases=None): | 28 | def make_osystem_with_releases(testcase, osystem_name=None, releases=None, |
687 | 29 | purposes=None): | ||
688 | 29 | """Generate an arbitrary operating system. | 30 | """Generate an arbitrary operating system. |
689 | 30 | 31 | ||
690 | 31 | :param osystem_name: The operating system name. Useful in cases where | 32 | :param osystem_name: The operating system name. Useful in cases where |
691 | 32 | we need to test that not supplying an os works correctly. | 33 | we need to test that not supplying an os works correctly. |
692 | 33 | :param releases: The list of releases name. Useful in cases where | 34 | :param releases: The list of releases name. Useful in cases where |
693 | 34 | we need to test that not supplying a release works correctly. | 35 | we need to test that not supplying a release works correctly. |
694 | 36 | :param purposes: The purpose's of the boot images. | ||
695 | 35 | """ | 37 | """ |
696 | 36 | if osystem_name is None: | 38 | if osystem_name is None: |
697 | 37 | osystem_name = factory.make_name('os') | 39 | osystem_name = factory.make_name('os') |
698 | 38 | if releases is None: | 40 | if releases is None: |
699 | 39 | releases = [factory.make_name('release') for _ in range(3)] | 41 | releases = [factory.make_name('release') for _ in range(3)] |
700 | 42 | if purposes is None: | ||
701 | 43 | purposes = [BOOT_IMAGE_PURPOSE.INSTALL, BOOT_IMAGE_PURPOSE.XINSTALL] | ||
702 | 40 | 44 | ||
703 | 41 | osystem = make_osystem( | 45 | osystem = make_osystem( |
704 | 42 | testcase, | 46 | testcase, |
705 | 43 | osystem_name, | 47 | osystem_name, |
707 | 44 | [BOOT_IMAGE_PURPOSE.INSTALL, BOOT_IMAGE_PURPOSE.XINSTALL]) | 48 | purposes) |
708 | 45 | if releases is not None and releases != []: | 49 | if releases is not None and releases != []: |
709 | 46 | osystem.fake_list = releases | 50 | osystem.fake_list = releases |
710 | 47 | return osystem | 51 | return osystem |
711 | @@ -72,7 +76,8 @@ | |||
712 | 72 | forms, 'list_all_usable_releases').return_value = distro_series | 76 | forms, 'list_all_usable_releases').return_value = distro_series |
713 | 73 | 77 | ||
714 | 74 | 78 | ||
716 | 75 | def make_usable_osystem(testcase, osystem_name=None, releases=None): | 79 | def make_usable_osystem(testcase, osystem_name=None, releases=None, |
717 | 80 | purposes=None): | ||
718 | 76 | """Return arbitrary operating system, and make it "usable." | 81 | """Return arbitrary operating system, and make it "usable." |
719 | 77 | 82 | ||
720 | 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. |
721 | @@ -83,8 +88,11 @@ | |||
722 | 83 | we need to test that not supplying an os works correctly. | 88 | we need to test that not supplying an os works correctly. |
723 | 84 | :param releases: The list of releases name. Useful in cases where | 89 | :param releases: The list of releases name. Useful in cases where |
724 | 85 | we need to test that not supplying a release works correctly. | 90 | we need to test that not supplying a release works correctly. |
725 | 91 | :param purposse: The list of purposes. Useful in cases where | ||
726 | 92 | we need to test that not supplying a purpose works correctly. | ||
727 | 86 | """ | 93 | """ |
728 | 87 | osystem = make_osystem_with_releases( | 94 | osystem = make_osystem_with_releases( |
730 | 88 | testcase, osystem_name=osystem_name, releases=releases) | 95 | testcase, osystem_name=osystem_name, releases=releases, |
731 | 96 | purposes=purposes) | ||
732 | 89 | patch_usable_osystems(testcase, [osystem]) | 97 | patch_usable_osystems(testcase, [osystem]) |
733 | 90 | return osystem | 98 | return osystem |
734 | 91 | 99 | ||
735 | === modified file 'src/maasserver/tests/test_api_node.py' | |||
736 | --- src/maasserver/tests/test_api_node.py 2014-04-25 18:03:38 +0000 | |||
737 | +++ src/maasserver/tests/test_api_node.py 2014-05-29 17:03:44 +0000 | |||
738 | @@ -23,7 +23,6 @@ | |||
739 | 23 | import bson | 23 | import bson |
740 | 24 | from django.core.urlresolvers import reverse | 24 | from django.core.urlresolvers import reverse |
741 | 25 | from maasserver.enum import ( | 25 | from maasserver.enum import ( |
742 | 26 | DISTRO_SERIES, | ||
743 | 27 | NODE_STATUS, | 26 | NODE_STATUS, |
744 | 28 | NODE_STATUS_CHOICES_DICT, | 27 | NODE_STATUS_CHOICES_DICT, |
745 | 29 | ) | 28 | ) |
746 | @@ -40,6 +39,7 @@ | |||
747 | 40 | from maasserver.testing.architecture import make_usable_architecture | 39 | from maasserver.testing.architecture import make_usable_architecture |
748 | 41 | from maasserver.testing.factory import factory | 40 | from maasserver.testing.factory import factory |
749 | 42 | from maasserver.testing.oauthclient import OAuthAuthenticatedClient | 41 | from maasserver.testing.oauthclient import OAuthAuthenticatedClient |
750 | 42 | from maasserver.testing.osystems import make_usable_osystem | ||
751 | 43 | from maasserver.testing.testcase import MAASServerTestCase | 43 | from maasserver.testing.testcase import MAASServerTestCase |
752 | 44 | from maasserver.utils import map_enum | 44 | from maasserver.utils import map_enum |
753 | 45 | from metadataserver.models import ( | 45 | from metadataserver.models import ( |
754 | @@ -221,24 +221,31 @@ | |||
755 | 221 | self.assertEqual( | 221 | self.assertEqual( |
756 | 222 | node.system_id, json.loads(response.content)['system_id']) | 222 | node.system_id, json.loads(response.content)['system_id']) |
757 | 223 | 223 | ||
759 | 224 | def test_POST_start_sets_distro_series(self): | 224 | def test_POST_start_sets_osystem_and_distro_series(self): |
760 | 225 | node = factory.make_node( | 225 | node = factory.make_node( |
761 | 226 | owner=self.logged_in_user, mac=True, | 226 | owner=self.logged_in_user, mac=True, |
764 | 227 | power_type='ether_wake') | 227 | power_type='ether_wake', |
765 | 228 | distro_series = factory.getRandomEnum(DISTRO_SERIES) | 228 | architecture=make_usable_architecture(self)) |
766 | 229 | osystem = make_usable_osystem(self) | ||
767 | 230 | distro_series = factory.getRandomRelease(osystem) | ||
768 | 229 | response = self.client.post( | 231 | response = self.client.post( |
771 | 230 | self.get_node_uri(node), | 232 | self.get_node_uri(node), { |
772 | 231 | {'op': 'start', 'distro_series': distro_series}) | 233 | 'op': 'start', |
773 | 234 | 'distro_series': distro_series | ||
774 | 235 | }) | ||
775 | 232 | self.assertEqual( | 236 | self.assertEqual( |
776 | 233 | (httplib.OK, node.system_id), | 237 | (httplib.OK, node.system_id), |
777 | 234 | (response.status_code, json.loads(response.content)['system_id'])) | 238 | (response.status_code, json.loads(response.content)['system_id'])) |
778 | 235 | self.assertEqual( | 239 | self.assertEqual( |
779 | 240 | osystem.name, reload_object(node).osystem) | ||
780 | 241 | self.assertEqual( | ||
781 | 236 | distro_series, reload_object(node).distro_series) | 242 | distro_series, reload_object(node).distro_series) |
782 | 237 | 243 | ||
783 | 238 | def test_POST_start_validates_distro_series(self): | 244 | def test_POST_start_validates_distro_series(self): |
784 | 239 | node = factory.make_node( | 245 | node = factory.make_node( |
785 | 240 | owner=self.logged_in_user, mac=True, | 246 | owner=self.logged_in_user, mac=True, |
787 | 241 | power_type='ether_wake') | 247 | power_type='ether_wake', |
788 | 248 | architecture=make_usable_architecture(self)) | ||
789 | 242 | invalid_distro_series = factory.getRandomString() | 249 | invalid_distro_series = factory.getRandomString() |
790 | 243 | response = self.client.post( | 250 | response = self.client.post( |
791 | 244 | self.get_node_uri(node), | 251 | self.get_node_uri(node), |
792 | @@ -247,7 +254,8 @@ | |||
793 | 247 | ( | 254 | ( |
794 | 248 | httplib.BAD_REQUEST, | 255 | httplib.BAD_REQUEST, |
795 | 249 | {'distro_series': [ | 256 | {'distro_series': [ |
797 | 250 | "Value u'%s' is not a valid choice." % | 257 | "'%s' is not a valid distro_series. " |
798 | 258 | "It should be one of: ''." % | ||
799 | 251 | invalid_distro_series]} | 259 | invalid_distro_series]} |
800 | 252 | ), | 260 | ), |
801 | 253 | (response.status_code, json.loads(response.content))) | 261 | (response.status_code, json.loads(response.content))) |
802 | @@ -300,18 +308,23 @@ | |||
803 | 300 | self.client.post(self.get_node_uri(node), {'op': 'release'}) | 308 | self.client.post(self.get_node_uri(node), {'op': 'release'}) |
804 | 301 | self.assertTrue(reload_object(node).netboot) | 309 | self.assertTrue(reload_object(node).netboot) |
805 | 302 | 310 | ||
807 | 303 | def test_POST_release_resets_distro_series(self): | 311 | def test_POST_release_resets_osystem_and_distro_series(self): |
808 | 312 | osystem = factory.getRandomOS() | ||
809 | 313 | release = factory.getRandomRelease(osystem) | ||
810 | 304 | node = factory.make_node( | 314 | node = factory.make_node( |
811 | 305 | status=NODE_STATUS.ALLOCATED, owner=self.logged_in_user, | 315 | status=NODE_STATUS.ALLOCATED, owner=self.logged_in_user, |
813 | 306 | distro_series=factory.getRandomEnum(DISTRO_SERIES)) | 316 | osystem=osystem.name, distro_series=release) |
814 | 307 | self.client.post(self.get_node_uri(node), {'op': 'release'}) | 317 | self.client.post(self.get_node_uri(node), {'op': 'release'}) |
815 | 318 | self.assertEqual('', reload_object(node).osystem) | ||
816 | 308 | self.assertEqual('', reload_object(node).distro_series) | 319 | self.assertEqual('', reload_object(node).distro_series) |
817 | 309 | 320 | ||
818 | 310 | def test_POST_release_resets_agent_name(self): | 321 | def test_POST_release_resets_agent_name(self): |
819 | 311 | agent_name = factory.make_name('agent-name') | 322 | agent_name = factory.make_name('agent-name') |
820 | 323 | osystem = factory.getRandomOS() | ||
821 | 324 | release = factory.getRandomRelease(osystem) | ||
822 | 312 | node = factory.make_node( | 325 | node = factory.make_node( |
823 | 313 | status=NODE_STATUS.ALLOCATED, owner=self.logged_in_user, | 326 | status=NODE_STATUS.ALLOCATED, owner=self.logged_in_user, |
825 | 314 | distro_series=factory.getRandomEnum(DISTRO_SERIES), | 327 | osystem=osystem.name, distro_series=release, |
826 | 315 | agent_name=agent_name) | 328 | agent_name=agent_name) |
827 | 316 | self.client.post(self.get_node_uri(node), {'op': 'release'}) | 329 | self.client.post(self.get_node_uri(node), {'op': 'release'}) |
828 | 317 | self.assertEqual('', reload_object(node).agent_name) | 330 | self.assertEqual('', reload_object(node).agent_name) |
829 | 318 | 331 | ||
830 | === modified file 'src/maasserver/tests/test_api_pxeconfig.py' | |||
831 | --- src/maasserver/tests/test_api_pxeconfig.py 2014-05-20 02:58:09 +0000 | |||
832 | +++ src/maasserver/tests/test_api_pxeconfig.py 2014-05-29 17:03:44 +0000 | |||
833 | @@ -35,11 +35,13 @@ | |||
834 | 35 | ) | 35 | ) |
835 | 36 | from maasserver.testing.architecture import make_usable_architecture | 36 | from maasserver.testing.architecture import make_usable_architecture |
836 | 37 | from maasserver.testing.factory import factory | 37 | from maasserver.testing.factory import factory |
837 | 38 | from maasserver.testing.osystems import make_usable_osystem | ||
838 | 38 | from maasserver.testing.testcase import MAASServerTestCase | 39 | from maasserver.testing.testcase import MAASServerTestCase |
839 | 39 | from maastesting.fakemethod import FakeMethod | 40 | from maastesting.fakemethod import FakeMethod |
840 | 40 | from mock import Mock | 41 | from mock import Mock |
841 | 41 | from netaddr import IPNetwork | 42 | from netaddr import IPNetwork |
842 | 42 | from provisioningserver import kernel_opts | 43 | from provisioningserver import kernel_opts |
843 | 44 | from provisioningserver.driver import BOOT_IMAGE_PURPOSE | ||
844 | 43 | from provisioningserver.kernel_opts import KernelParameters | 45 | from provisioningserver.kernel_opts import KernelParameters |
845 | 44 | from testtools.matchers import ( | 46 | from testtools.matchers import ( |
846 | 45 | Contains, | 47 | Contains, |
847 | @@ -133,7 +135,7 @@ | |||
848 | 133 | self.assertEqual(value, response_dict['extra_opts']) | 135 | self.assertEqual(value, response_dict['extra_opts']) |
849 | 134 | 136 | ||
850 | 135 | def test_pxeconfig_uses_present_boot_image(self): | 137 | def test_pxeconfig_uses_present_boot_image(self): |
852 | 136 | osystem = 'ubuntu' | 138 | osystem = Config.objects.get_config('commissioning_osystem') |
853 | 137 | release = Config.objects.get_config('commissioning_distro_series') | 139 | release = Config.objects.get_config('commissioning_distro_series') |
854 | 138 | nodegroup = factory.make_node_group() | 140 | nodegroup = factory.make_node_group() |
855 | 139 | factory.make_boot_image( | 141 | factory.make_boot_image( |
856 | @@ -284,7 +286,8 @@ | |||
857 | 284 | def test_get_boot_purpose_unknown_node(self): | 286 | def test_get_boot_purpose_unknown_node(self): |
858 | 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, |
859 | 286 | # which uses a "commissioning" image. | 288 | # which uses a "commissioning" image. |
861 | 287 | self.assertEqual("commissioning", api.get_boot_purpose(None)) | 289 | self.assertEqual("commissioning", api.get_boot_purpose( |
862 | 290 | None, None, None, None, None, None)) | ||
863 | 288 | 291 | ||
864 | 289 | def test_get_boot_purpose_known_node(self): | 292 | def test_get_boot_purpose_known_node(self): |
865 | 290 | # The following table shows the expected boot "purpose" for each set | 293 | # The following table shows the expected boot "purpose" for each set |
866 | @@ -307,11 +310,33 @@ | |||
867 | 307 | node.use_fastpath_installer() | 310 | node.use_fastpath_installer() |
868 | 308 | for name, value in parameters.items(): | 311 | for name, value in parameters.items(): |
869 | 309 | setattr(node, name, value) | 312 | setattr(node, name, value) |
871 | 310 | self.assertEqual(purpose, api.get_boot_purpose(node)) | 313 | osystem = node.get_osystem() |
872 | 314 | series = node.get_distro_series() | ||
873 | 315 | arch, subarch = node.architecture.split('/') | ||
874 | 316 | self.assertEqual( | ||
875 | 317 | purpose, | ||
876 | 318 | api.get_boot_purpose( | ||
877 | 319 | node, osystem, arch, subarch, series, None)) | ||
878 | 320 | |||
879 | 321 | def test_get_boot_purpose_osystem_no_xinstall_support(self): | ||
880 | 322 | osystem = make_usable_osystem( | ||
881 | 323 | self, purposes=[BOOT_IMAGE_PURPOSE.INSTALL]) | ||
882 | 324 | release = factory.getRandomRelease(osystem) | ||
883 | 325 | node = factory.make_node( | ||
884 | 326 | status=NODE_STATUS.ALLOCATED, netboot=True, | ||
885 | 327 | osystem=osystem.name, distro_series=release) | ||
886 | 328 | node.use_fastpath_installer() | ||
887 | 329 | node_os = node.get_osystem() | ||
888 | 330 | node_series = node.get_distro_series() | ||
889 | 331 | arch, subarch = node.architecture.split('/') | ||
890 | 332 | self.assertEqual( | ||
891 | 333 | 'install', | ||
892 | 334 | api.get_boot_purpose( | ||
893 | 335 | node, node_os, arch, subarch, node_series, None)) | ||
894 | 311 | 336 | ||
895 | 312 | def test_pxeconfig_uses_boot_purpose(self): | 337 | def test_pxeconfig_uses_boot_purpose(self): |
896 | 313 | fake_boot_purpose = factory.make_name("purpose") | 338 | fake_boot_purpose = factory.make_name("purpose") |
898 | 314 | self.patch(api, "get_boot_purpose", lambda node: fake_boot_purpose) | 339 | self.patch(api, "get_boot_purpose").return_value = fake_boot_purpose |
899 | 315 | response = self.client.get(reverse('pxeconfig'), | 340 | response = self.client.get(reverse('pxeconfig'), |
900 | 316 | self.get_default_params()) | 341 | self.get_default_params()) |
901 | 317 | self.assertEqual( | 342 | self.assertEqual( |
902 | 318 | 343 | ||
903 | === modified file 'src/maasserver/tests/test_forms.py' | |||
904 | --- src/maasserver/tests/test_forms.py 2014-05-20 14:16:58 +0000 | |||
905 | +++ src/maasserver/tests/test_forms.py 2014-05-29 17:03:44 +0000 | |||
906 | @@ -53,6 +53,8 @@ | |||
907 | 53 | InstanceListField, | 53 | InstanceListField, |
908 | 54 | INTERFACES_VALIDATION_ERROR_MESSAGE, | 54 | INTERFACES_VALIDATION_ERROR_MESSAGE, |
909 | 55 | list_all_usable_architectures, | 55 | list_all_usable_architectures, |
910 | 56 | list_all_usable_osystems, | ||
911 | 57 | list_all_usable_releases, | ||
912 | 56 | MACAddressForm, | 58 | MACAddressForm, |
913 | 57 | MAX_MESSAGES, | 59 | MAX_MESSAGES, |
914 | 58 | merge_error_messages, | 60 | merge_error_messages, |
915 | @@ -98,6 +100,11 @@ | |||
916 | 98 | patch_usable_architectures, | 100 | patch_usable_architectures, |
917 | 99 | ) | 101 | ) |
918 | 100 | from maasserver.testing.factory import factory | 102 | from maasserver.testing.factory import factory |
919 | 103 | from maasserver.testing.osystems import ( | ||
920 | 104 | make_osystem_with_releases, | ||
921 | 105 | make_usable_osystem, | ||
922 | 106 | patch_usable_osystems, | ||
923 | 107 | ) | ||
924 | 101 | from maasserver.testing.testcase import MAASServerTestCase | 108 | from maasserver.testing.testcase import MAASServerTestCase |
925 | 102 | from maasserver.utils import map_enum | 109 | from maasserver.utils import map_enum |
926 | 103 | from maasserver.utils.forms import compose_invalid_choice_text | 110 | from maasserver.utils.forms import compose_invalid_choice_text |
927 | @@ -120,8 +127,8 @@ | |||
928 | 120 | 127 | ||
929 | 121 | class TestHelpers(MAASServerTestCase): | 128 | class TestHelpers(MAASServerTestCase): |
930 | 122 | 129 | ||
933 | 123 | def make_usable_boot_images(self, nodegroup=None, arch=None, | 130 | def make_usable_boot_images(self, nodegroup=None, osystem=None, |
934 | 124 | subarchitecture=None): | 131 | arch=None, subarchitecture=None, release=None): |
935 | 125 | """Create a set of boot images, so the architecture becomes "usable". | 132 | """Create a set of boot images, so the architecture becomes "usable". |
936 | 126 | 133 | ||
937 | 127 | This will make the images' architecture show up in the list of usable | 134 | This will make the images' architecture show up in the list of usable |
938 | @@ -131,14 +138,19 @@ | |||
939 | 131 | """ | 138 | """ |
940 | 132 | if nodegroup is None: | 139 | if nodegroup is None: |
941 | 133 | nodegroup = factory.make_node_group() | 140 | nodegroup = factory.make_node_group() |
942 | 141 | if osystem is None: | ||
943 | 142 | osystem = factory.make_name('os') | ||
944 | 134 | if arch is None: | 143 | if arch is None: |
945 | 135 | arch = factory.make_name('arch') | 144 | arch = factory.make_name('arch') |
946 | 136 | if subarchitecture is None: | 145 | if subarchitecture is None: |
947 | 137 | subarchitecture = factory.make_name('subarch') | 146 | subarchitecture = factory.make_name('subarch') |
948 | 147 | if release is None: | ||
949 | 148 | release = factory.make_name('release') | ||
950 | 138 | for purpose in ['install', 'commissioning']: | 149 | for purpose in ['install', 'commissioning']: |
951 | 139 | factory.make_boot_image( | 150 | factory.make_boot_image( |
954 | 140 | nodegroup=nodegroup, architecture=arch, | 151 | nodegroup=nodegroup, osystem=osystem, architecture=arch, |
955 | 141 | subarchitecture=subarchitecture, purpose=purpose) | 152 | subarchitecture=subarchitecture, release=release, |
956 | 153 | purpose=purpose) | ||
957 | 142 | 154 | ||
958 | 143 | def test_initialize_node_group_leaves_nodegroup_reference_intact(self): | 155 | def test_initialize_node_group_leaves_nodegroup_reference_intact(self): |
959 | 144 | preselected_nodegroup = factory.make_node_group() | 156 | preselected_nodegroup = factory.make_node_group() |
960 | @@ -201,6 +213,67 @@ | |||
961 | 201 | arches = [factory.make_name('arch') for _ in range(5)] | 213 | arches = [factory.make_name('arch') for _ in range(5)] |
962 | 202 | self.assertEqual(arches[0], pick_default_architecture(arches)) | 214 | self.assertEqual(arches[0], pick_default_architecture(arches)) |
963 | 203 | 215 | ||
964 | 216 | def test_list_all_usable_osystems_combines_nodegroups(self): | ||
965 | 217 | osystem_names = [factory.make_name('os') for _ in range(3)] | ||
966 | 218 | expected = [] | ||
967 | 219 | for name in osystem_names: | ||
968 | 220 | self.make_usable_boot_images(osystem=name) | ||
969 | 221 | expected.append(make_usable_osystem(self, name)) | ||
970 | 222 | self.assertItemsEqual(expected, list_all_usable_osystems()) | ||
971 | 223 | |||
972 | 224 | def test_list_all_usable_osystems_sorts_output(self): | ||
973 | 225 | osystem_names = [factory.make_name('os') for _ in range(3)] | ||
974 | 226 | expected = [] | ||
975 | 227 | for name in osystem_names: | ||
976 | 228 | self.make_usable_boot_images(osystem=name) | ||
977 | 229 | expected.append(make_usable_osystem(self, name)) | ||
978 | 230 | expected = sorted(expected, key=lambda osystem: osystem.title) | ||
979 | 231 | self.assertEqual(expected, list_all_usable_osystems()) | ||
980 | 232 | |||
981 | 233 | def test_list_all_usable_osystems_returns_no_duplicates(self): | ||
982 | 234 | os_name = factory.make_name('os') | ||
983 | 235 | self.make_usable_boot_images(osystem=os_name) | ||
984 | 236 | self.make_usable_boot_images(osystem=os_name) | ||
985 | 237 | osystem = make_usable_osystem(self, os_name) | ||
986 | 238 | self.assertEqual( | ||
987 | 239 | [osystem], list_all_usable_osystems()) | ||
988 | 240 | |||
989 | 241 | def test_list_all_usable_releases_combines_nodegroups(self): | ||
990 | 242 | expected = {} | ||
991 | 243 | osystems = [] | ||
992 | 244 | os_names = [factory.make_name('os') for _ in range(3)] | ||
993 | 245 | for name in os_names: | ||
994 | 246 | releases = [factory.make_name('release') for _ in range(3)] | ||
995 | 247 | for release in releases: | ||
996 | 248 | self.make_usable_boot_images(osystem=name, release=release) | ||
997 | 249 | osystems.append( | ||
998 | 250 | make_usable_osystem(self, name, releases=releases)) | ||
999 | 251 | expected[name] = releases | ||
1000 | 252 | self.assertItemsEqual(expected, list_all_usable_releases(osystems)) | ||
1001 | 253 | |||
1002 | 254 | def test_list_all_usable_releases_sorts_output(self): | ||
1003 | 255 | expected = {} | ||
1004 | 256 | osystems = [] | ||
1005 | 257 | os_names = [factory.make_name('os') for _ in range(3)] | ||
1006 | 258 | for name in os_names: | ||
1007 | 259 | releases = [factory.make_name('release') for _ in range(3)] | ||
1008 | 260 | for release in releases: | ||
1009 | 261 | self.make_usable_boot_images(osystem=name, release=release) | ||
1010 | 262 | osystems.append( | ||
1011 | 263 | make_usable_osystem(self, name, releases=releases)) | ||
1012 | 264 | expected[name] = sorted(releases) | ||
1013 | 265 | self.assertEqual(expected, list_all_usable_releases(osystems)) | ||
1014 | 266 | |||
1015 | 267 | def test_list_all_usable_releases_returns_no_duplicates(self): | ||
1016 | 268 | os_name = factory.make_name('os') | ||
1017 | 269 | release = factory.make_name('release') | ||
1018 | 270 | self.make_usable_boot_images(osystem=os_name, release=release) | ||
1019 | 271 | self.make_usable_boot_images(osystem=os_name, release=release) | ||
1020 | 272 | osystem = make_usable_osystem(self, os_name, releases=[release]) | ||
1021 | 273 | expected = {} | ||
1022 | 274 | expected[os_name] = [release] | ||
1023 | 275 | self.assertEqual(expected, list_all_usable_releases([osystem])) | ||
1024 | 276 | |||
1025 | 204 | def test_remove_None_values_removes_None_values_in_dict(self): | 277 | def test_remove_None_values_removes_None_values_in_dict(self): |
1026 | 205 | random_input = factory.getRandomString() | 278 | random_input = factory.getRandomString() |
1027 | 206 | self.assertEqual( | 279 | self.assertEqual( |
1028 | @@ -430,6 +503,7 @@ | |||
1029 | 430 | [ | 503 | [ |
1030 | 431 | 'hostname', | 504 | 'hostname', |
1031 | 432 | 'architecture', | 505 | 'architecture', |
1032 | 506 | 'osystem', | ||
1033 | 433 | 'distro_series', | 507 | 'distro_series', |
1034 | 434 | 'nodegroup', | 508 | 'nodegroup', |
1035 | 435 | ], list(form.fields)) | 509 | ], list(form.fields)) |
1036 | @@ -489,6 +563,77 @@ | |||
1037 | 489 | [NO_ARCHITECTURES_AVAILABLE], | 563 | [NO_ARCHITECTURES_AVAILABLE], |
1038 | 490 | form.errors['architecture']) | 564 | form.errors['architecture']) |
1039 | 491 | 565 | ||
1040 | 566 | def test_accepts_osystem(self): | ||
1041 | 567 | osystem = make_usable_osystem(self) | ||
1042 | 568 | form = NodeForm(data={ | ||
1043 | 569 | 'hostname': factory.make_name('host'), | ||
1044 | 570 | 'architecture': make_usable_architecture(self), | ||
1045 | 571 | 'osystem': osystem.name, | ||
1046 | 572 | }) | ||
1047 | 573 | self.assertTrue(form.is_valid(), form._errors) | ||
1048 | 574 | |||
1049 | 575 | def test_rejects_invalid_osystem(self): | ||
1050 | 576 | patch_usable_osystems(self) | ||
1051 | 577 | form = NodeForm(data={ | ||
1052 | 578 | 'hostname': factory.make_name('host'), | ||
1053 | 579 | 'architecture': make_usable_architecture(self), | ||
1054 | 580 | 'osystem': factory.make_name('os'), | ||
1055 | 581 | }) | ||
1056 | 582 | self.assertFalse(form.is_valid()) | ||
1057 | 583 | self.assertItemsEqual(['osystem'], form._errors.keys()) | ||
1058 | 584 | |||
1059 | 585 | def test_starts_with_default_osystem(self): | ||
1060 | 586 | osystems = [make_osystem_with_releases(self) for _ in range(5)] | ||
1061 | 587 | patch_usable_osystems(self, osystems) | ||
1062 | 588 | form = NodeForm() | ||
1063 | 589 | self.assertEqual( | ||
1064 | 590 | '', | ||
1065 | 591 | form.fields['osystem'].initial) | ||
1066 | 592 | |||
1067 | 593 | def test_accepts_osystem_distro_series(self): | ||
1068 | 594 | osystem = make_usable_osystem(self) | ||
1069 | 595 | release = osystem.get_default_release() | ||
1070 | 596 | form = NodeForm(data={ | ||
1071 | 597 | 'hostname': factory.make_name('host'), | ||
1072 | 598 | 'architecture': make_usable_architecture(self), | ||
1073 | 599 | 'osystem': osystem.name, | ||
1074 | 600 | 'distro_series': '%s/%s' % (osystem.name, release), | ||
1075 | 601 | }) | ||
1076 | 602 | self.assertTrue(form.is_valid(), form._errors) | ||
1077 | 603 | |||
1078 | 604 | def test_rejects_invalid_osystem_distro_series(self): | ||
1079 | 605 | osystem = make_usable_osystem(self) | ||
1080 | 606 | release = factory.make_name('release') | ||
1081 | 607 | form = NodeForm(data={ | ||
1082 | 608 | 'hostname': factory.make_name('host'), | ||
1083 | 609 | 'architecture': make_usable_architecture(self), | ||
1084 | 610 | 'osystem': osystem.name, | ||
1085 | 611 | 'distro_series': '%s/%s' % (osystem.name, release), | ||
1086 | 612 | }) | ||
1087 | 613 | self.assertFalse(form.is_valid()) | ||
1088 | 614 | self.assertItemsEqual(['distro_series'], form._errors.keys()) | ||
1089 | 615 | |||
1090 | 616 | def test_starts_with_default_distro_series(self): | ||
1091 | 617 | osystems = [make_osystem_with_releases(self) for _ in range(5)] | ||
1092 | 618 | patch_usable_osystems(self, osystems) | ||
1093 | 619 | form = NodeForm() | ||
1094 | 620 | self.assertEqual( | ||
1095 | 621 | '', | ||
1096 | 622 | form.fields['distro_series'].initial) | ||
1097 | 623 | |||
1098 | 624 | def test_rejects_mismatch_osystem_distro_series(self): | ||
1099 | 625 | osystem = make_usable_osystem(self) | ||
1100 | 626 | release = osystem.get_default_release() | ||
1101 | 627 | invalid = factory.make_name('invalid_os') | ||
1102 | 628 | form = NodeForm(data={ | ||
1103 | 629 | 'hostname': factory.make_name('host'), | ||
1104 | 630 | 'architecture': make_usable_architecture(self), | ||
1105 | 631 | 'osystem': osystem.name, | ||
1106 | 632 | 'distro_series': '%s/%s' % (invalid, release), | ||
1107 | 633 | }) | ||
1108 | 634 | self.assertFalse(form.is_valid()) | ||
1109 | 635 | self.assertItemsEqual(['distro_series'], form._errors.keys()) | ||
1110 | 636 | |||
1111 | 492 | 637 | ||
1112 | 493 | class TestAdminNodeForm(MAASServerTestCase): | 638 | class TestAdminNodeForm(MAASServerTestCase): |
1113 | 494 | 639 | ||
1114 | @@ -500,6 +645,7 @@ | |||
1115 | 500 | [ | 645 | [ |
1116 | 501 | 'hostname', | 646 | 'hostname', |
1117 | 502 | 'architecture', | 647 | 'architecture', |
1118 | 648 | 'osystem', | ||
1119 | 503 | 'distro_series', | 649 | 'distro_series', |
1120 | 504 | 'power_type', | 650 | 'power_type', |
1121 | 505 | 'power_parameters', | 651 | 'power_parameters', |
1122 | 506 | 652 | ||
1123 | === modified file 'src/maasserver/tests/test_preseed.py' | |||
1124 | --- src/maasserver/tests/test_preseed.py 2014-05-13 18:43:40 +0000 | |||
1125 | +++ src/maasserver/tests/test_preseed.py 2014-05-29 17:03:44 +0000 | |||
1126 | @@ -22,7 +22,6 @@ | |||
1127 | 22 | from django.conf import settings | 22 | from django.conf import settings |
1128 | 23 | from django.core.urlresolvers import reverse | 23 | from django.core.urlresolvers import reverse |
1129 | 24 | from maasserver.enum import ( | 24 | from maasserver.enum import ( |
1130 | 25 | DISTRO_SERIES, | ||
1131 | 26 | NODE_STATUS, | 25 | NODE_STATUS, |
1132 | 27 | NODEGROUPINTERFACE_MANAGEMENT, | 26 | NODEGROUPINTERFACE_MANAGEMENT, |
1133 | 28 | PRESEED_TYPE, | 27 | PRESEED_TYPE, |
1134 | @@ -590,7 +589,7 @@ | |||
1135 | 590 | node = factory.make_node() | 589 | node = factory.make_node() |
1136 | 591 | arch, subarch = node.architecture.split('/') | 590 | arch, subarch = node.architecture.split('/') |
1137 | 592 | factory.make_boot_image( | 591 | factory.make_boot_image( |
1139 | 593 | osystem='ubuntu', | 592 | osystem=node.get_osystem(), |
1140 | 594 | architecture=arch, subarchitecture=subarch, | 593 | architecture=arch, subarchitecture=subarch, |
1141 | 595 | release=node.get_distro_series(), purpose='xinstall', | 594 | release=node.get_distro_series(), purpose='xinstall', |
1142 | 596 | nodegroup=node.nodegroup) | 595 | nodegroup=node.nodegroup) |
1143 | @@ -678,18 +677,15 @@ | |||
1144 | 678 | self.assertIn('cloud-init', context['curtin_preseed']) | 677 | self.assertIn('cloud-init', context['curtin_preseed']) |
1145 | 679 | 678 | ||
1146 | 680 | def test_get_curtin_installer_url_returns_url(self): | 679 | def test_get_curtin_installer_url_returns_url(self): |
1152 | 681 | osystem = 'ubuntu' | 680 | osystem = factory.getRandomOS() |
1153 | 682 | # Exclude DISTRO_SERIES.default. It's a special value that defers | 681 | series = factory.getRandomRelease(osystem) |
1149 | 683 | # to a run-time setting which we don't provide in this test. | ||
1150 | 684 | series = factory.getRandomEnum( | ||
1151 | 685 | DISTRO_SERIES, but_not=DISTRO_SERIES.default) | ||
1154 | 686 | architecture = make_usable_architecture(self) | 682 | architecture = make_usable_architecture(self) |
1155 | 687 | node = factory.make_node( | 683 | node = factory.make_node( |
1157 | 688 | architecture=architecture, | 684 | osystem=osystem.name, architecture=architecture, |
1158 | 689 | distro_series=series) | 685 | distro_series=series) |
1159 | 690 | arch, subarch = architecture.split('/') | 686 | arch, subarch = architecture.split('/') |
1160 | 691 | boot_image = factory.make_boot_image( | 687 | boot_image = factory.make_boot_image( |
1162 | 692 | osystem=osystem, architecture=arch, | 688 | osystem=osystem.name, architecture=arch, |
1163 | 693 | subarchitecture=subarch, release=series, | 689 | subarchitecture=subarch, release=series, |
1164 | 694 | purpose='xinstall', nodegroup=node.nodegroup) | 690 | purpose='xinstall', nodegroup=node.nodegroup) |
1165 | 695 | 691 | ||
1166 | @@ -698,20 +694,20 @@ | |||
1167 | 698 | [interface] = node.nodegroup.get_managed_interfaces() | 694 | [interface] = node.nodegroup.get_managed_interfaces() |
1168 | 699 | self.assertEqual( | 695 | self.assertEqual( |
1169 | 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' % ( |
1171 | 701 | interface.ip, osystem, arch, subarch, | 697 | interface.ip, osystem.name, arch, subarch, |
1172 | 702 | series, boot_image.label), | 698 | series, boot_image.label), |
1173 | 703 | installer_url) | 699 | installer_url) |
1174 | 704 | 700 | ||
1175 | 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): |
1179 | 706 | osystem = 'ubuntu' | 702 | osystem = factory.getRandomOS() |
1180 | 707 | series = factory.getRandomEnum( | 703 | series = factory.getRandomRelease(osystem) |
1178 | 708 | DISTRO_SERIES, but_not=DISTRO_SERIES.default) | ||
1181 | 709 | architecture = make_usable_architecture(self) | 704 | architecture = make_usable_architecture(self) |
1182 | 710 | node = factory.make_node( | 705 | node = factory.make_node( |
1183 | 706 | osystem=osystem.name, | ||
1184 | 711 | architecture=architecture, distro_series=series) | 707 | architecture=architecture, distro_series=series) |
1185 | 712 | # Generate a boot image with a different arch/subarch. | 708 | # Generate a boot image with a different arch/subarch. |
1186 | 713 | factory.make_boot_image( | 709 | factory.make_boot_image( |
1188 | 714 | osystem=osystem, | 710 | osystem=osystem.name, |
1189 | 715 | architecture=factory.make_name('arch'), | 711 | architecture=factory.make_name('arch'), |
1190 | 716 | subarchitecture=factory.make_name('subarch'), release=series, | 712 | subarchitecture=factory.make_name('subarch'), release=series, |
1191 | 717 | purpose='xinstall', nodegroup=node.nodegroup) | 713 | purpose='xinstall', nodegroup=node.nodegroup) |
1192 | @@ -722,7 +718,7 @@ | |||
1193 | 722 | msg = ( | 718 | msg = ( |
1194 | 723 | "No image could be found for the given selection: " | 719 | "No image could be found for the given selection: " |
1195 | 724 | "os=%s, arch=%s, subarch=%s, series=%s, purpose=xinstall." % ( | 720 | "os=%s, arch=%s, subarch=%s, series=%s, purpose=xinstall." % ( |
1197 | 725 | osystem, | 721 | osystem.name, |
1198 | 726 | arch, | 722 | arch, |
1199 | 727 | subarch, | 723 | subarch, |
1200 | 728 | node.get_distro_series(), | 724 | node.get_distro_series(), |
1201 | 729 | 725 | ||
1202 | === modified file 'src/maasserver/views/settings.py' | |||
1203 | --- src/maasserver/views/settings.py 2014-04-10 13:43:33 +0000 | |||
1204 | +++ src/maasserver/views/settings.py 2014-05-29 17:03:44 +0000 | |||
1205 | @@ -41,6 +41,7 @@ | |||
1206 | 41 | from maasserver.exceptions import CannotDeleteUserException | 41 | from maasserver.exceptions import CannotDeleteUserException |
1207 | 42 | from maasserver.forms import ( | 42 | from maasserver.forms import ( |
1208 | 43 | CommissioningForm, | 43 | CommissioningForm, |
1209 | 44 | DeployForm, | ||
1210 | 44 | EditUserForm, | 45 | EditUserForm, |
1211 | 45 | GlobalKernelOptsForm, | 46 | GlobalKernelOptsForm, |
1212 | 46 | MAASAndNetworkForm, | 47 | MAASAndNetworkForm, |
1213 | @@ -177,6 +178,13 @@ | |||
1214 | 177 | if response is not None: | 178 | if response is not None: |
1215 | 178 | return response | 179 | return response |
1216 | 179 | 180 | ||
1217 | 181 | # Process the Deploy form. | ||
1218 | 182 | deploy_form, response = process_form( | ||
1219 | 183 | request, DeployForm, reverse('settings'), 'deploy', | ||
1220 | 184 | "Configuration updated.") | ||
1221 | 185 | if response is not None: | ||
1222 | 186 | return response | ||
1223 | 187 | |||
1224 | 180 | # Process the Ubuntu form. | 188 | # Process the Ubuntu form. |
1225 | 181 | ubuntu_form, response = process_form( | 189 | ubuntu_form, response = process_form( |
1226 | 182 | request, UbuntuForm, reverse('settings'), 'ubuntu', | 190 | request, UbuntuForm, reverse('settings'), 'ubuntu', |
1227 | @@ -202,6 +210,7 @@ | |||
1228 | 202 | 'maas_and_network_form': maas_and_network_form, | 210 | 'maas_and_network_form': maas_and_network_form, |
1229 | 203 | 'third_party_drivers_form': third_party_drivers_form, | 211 | 'third_party_drivers_form': third_party_drivers_form, |
1230 | 204 | 'commissioning_form': commissioning_form, | 212 | 'commissioning_form': commissioning_form, |
1231 | 213 | 'deploy_form': deploy_form, | ||
1232 | 205 | 'ubuntu_form': ubuntu_form, | 214 | 'ubuntu_form': ubuntu_form, |
1233 | 206 | 'kernelopts_form': kernelopts_form, | 215 | 'kernelopts_form': kernelopts_form, |
1234 | 207 | }, | 216 | }, |
1235 | 208 | 217 | ||
1236 | === modified file 'src/maasserver/views/tests/test_settings.py' | |||
1237 | --- src/maasserver/views/tests/test_settings.py 2014-04-10 13:43:33 +0000 | |||
1238 | +++ src/maasserver/views/tests/test_settings.py 2014-05-29 17:03:44 +0000 | |||
1239 | @@ -20,14 +20,11 @@ | |||
1240 | 20 | from django.contrib.auth.models import User | 20 | from django.contrib.auth.models import User |
1241 | 21 | from django.core.urlresolvers import reverse | 21 | from django.core.urlresolvers import reverse |
1242 | 22 | from lxml.html import fromstring | 22 | from lxml.html import fromstring |
1243 | 23 | from maasserver.enum import ( | ||
1244 | 24 | COMMISSIONING_DISTRO_SERIES_CHOICES, | ||
1245 | 25 | DISTRO_SERIES, | ||
1246 | 26 | ) | ||
1247 | 27 | from maasserver.models import ( | 23 | from maasserver.models import ( |
1248 | 28 | Config, | 24 | Config, |
1249 | 29 | UserProfile, | 25 | UserProfile, |
1250 | 30 | ) | 26 | ) |
1251 | 27 | from maasserver.models.config import DEFAULT_OS | ||
1252 | 31 | from maasserver.testing import ( | 28 | from maasserver.testing import ( |
1253 | 32 | extract_redirect, | 29 | extract_redirect, |
1254 | 33 | get_prefixed_form_data, | 30 | get_prefixed_form_data, |
1255 | @@ -112,8 +109,7 @@ | |||
1256 | 112 | def test_settings_commissioning_POST(self): | 109 | def test_settings_commissioning_POST(self): |
1257 | 113 | self.client_log_in(as_admin=True) | 110 | self.client_log_in(as_admin=True) |
1258 | 114 | new_check_compatibility = factory.getRandomBoolean() | 111 | new_check_compatibility = factory.getRandomBoolean() |
1261 | 115 | new_commissioning_distro_series = factory.getRandomChoice( | 112 | new_commissioning = factory.getRandomCommissioningRelease(DEFAULT_OS) |
1260 | 116 | COMMISSIONING_DISTRO_SERIES_CHOICES) | ||
1262 | 117 | response = self.client.post( | 113 | response = self.client.post( |
1263 | 118 | reverse('settings'), | 114 | reverse('settings'), |
1264 | 119 | get_prefixed_form_data( | 115 | get_prefixed_form_data( |
1265 | @@ -121,14 +117,14 @@ | |||
1266 | 121 | data={ | 117 | data={ |
1267 | 122 | 'check_compatibility': new_check_compatibility, | 118 | 'check_compatibility': new_check_compatibility, |
1268 | 123 | 'commissioning_distro_series': ( | 119 | 'commissioning_distro_series': ( |
1270 | 124 | new_commissioning_distro_series), | 120 | new_commissioning), |
1271 | 125 | })) | 121 | })) |
1272 | 126 | 122 | ||
1273 | 127 | self.assertEqual(httplib.FOUND, response.status_code) | 123 | self.assertEqual(httplib.FOUND, response.status_code) |
1274 | 128 | self.assertEqual( | 124 | self.assertEqual( |
1275 | 129 | ( | 125 | ( |
1276 | 130 | new_check_compatibility, | 126 | new_check_compatibility, |
1278 | 131 | new_commissioning_distro_series, | 127 | new_commissioning, |
1279 | 132 | ), | 128 | ), |
1280 | 133 | ( | 129 | ( |
1281 | 134 | Config.objects.get_config('check_compatibility'), | 130 | Config.objects.get_config('check_compatibility'), |
1282 | @@ -156,11 +152,38 @@ | |||
1283 | 156 | Config.objects.get_config('enable_third_party_drivers'), | 152 | Config.objects.get_config('enable_third_party_drivers'), |
1284 | 157 | )) | 153 | )) |
1285 | 158 | 154 | ||
1286 | 155 | def test_settings_deploy_POST(self): | ||
1287 | 156 | self.client_log_in(as_admin=True) | ||
1288 | 157 | new_osystem = factory.getRandomOS() | ||
1289 | 158 | new_default_osystem = new_osystem.name | ||
1290 | 159 | new_default_distro_series = factory.getRandomRelease(new_osystem) | ||
1291 | 160 | response = self.client.post( | ||
1292 | 161 | reverse('settings'), | ||
1293 | 162 | get_prefixed_form_data( | ||
1294 | 163 | prefix='deploy', | ||
1295 | 164 | data={ | ||
1296 | 165 | 'default_osystem': new_default_osystem, | ||
1297 | 166 | 'default_distro_series': '%s/%s' % ( | ||
1298 | 167 | new_default_osystem, | ||
1299 | 168 | new_default_distro_series | ||
1300 | 169 | ), | ||
1301 | 170 | })) | ||
1302 | 171 | |||
1303 | 172 | self.assertEqual(httplib.FOUND, response.status_code, response.content) | ||
1304 | 173 | self.assertEqual( | ||
1305 | 174 | ( | ||
1306 | 175 | new_default_osystem, | ||
1307 | 176 | new_default_distro_series, | ||
1308 | 177 | ), | ||
1309 | 178 | ( | ||
1310 | 179 | Config.objects.get_config('default_osystem'), | ||
1311 | 180 | Config.objects.get_config('default_distro_series'), | ||
1312 | 181 | )) | ||
1313 | 182 | |||
1314 | 159 | def test_settings_ubuntu_POST(self): | 183 | def test_settings_ubuntu_POST(self): |
1315 | 160 | self.client_log_in(as_admin=True) | 184 | self.client_log_in(as_admin=True) |
1316 | 161 | new_main_archive = 'http://test.example.com/archive' | 185 | new_main_archive = 'http://test.example.com/archive' |
1317 | 162 | new_ports_archive = 'http://test2.example.com/archive' | 186 | new_ports_archive = 'http://test2.example.com/archive' |
1318 | 163 | new_default_distro_series = factory.getRandomEnum(DISTRO_SERIES) | ||
1319 | 164 | response = self.client.post( | 187 | response = self.client.post( |
1320 | 165 | reverse('settings'), | 188 | reverse('settings'), |
1321 | 166 | get_prefixed_form_data( | 189 | get_prefixed_form_data( |
1322 | @@ -168,7 +191,6 @@ | |||
1323 | 168 | data={ | 191 | data={ |
1324 | 169 | 'main_archive': new_main_archive, | 192 | 'main_archive': new_main_archive, |
1325 | 170 | 'ports_archive': new_ports_archive, | 193 | 'ports_archive': new_ports_archive, |
1326 | 171 | 'default_distro_series': new_default_distro_series, | ||
1327 | 172 | })) | 194 | })) |
1328 | 173 | 195 | ||
1329 | 174 | self.assertEqual(httplib.FOUND, response.status_code, response.content) | 196 | self.assertEqual(httplib.FOUND, response.status_code, response.content) |
1330 | @@ -176,12 +198,10 @@ | |||
1331 | 176 | ( | 198 | ( |
1332 | 177 | new_main_archive, | 199 | new_main_archive, |
1333 | 178 | new_ports_archive, | 200 | new_ports_archive, |
1334 | 179 | new_default_distro_series, | ||
1335 | 180 | ), | 201 | ), |
1336 | 181 | ( | 202 | ( |
1337 | 182 | Config.objects.get_config('main_archive'), | 203 | Config.objects.get_config('main_archive'), |
1338 | 183 | Config.objects.get_config('ports_archive'), | 204 | Config.objects.get_config('ports_archive'), |
1339 | 184 | Config.objects.get_config('default_distro_series'), | ||
1340 | 185 | )) | 205 | )) |
1341 | 186 | 206 | ||
1342 | 187 | def test_settings_kernelopts_POST(self): | 207 | def test_settings_kernelopts_POST(self): |
1343 | 188 | 208 | ||
1344 | === modified file 'src/metadataserver/tests/test_api.py' | |||
1345 | --- src/metadataserver/tests/test_api.py 2014-04-23 16:09:39 +0000 | |||
1346 | +++ src/metadataserver/tests/test_api.py 2014-05-29 17:03:44 +0000 | |||
1347 | @@ -394,7 +394,7 @@ | |||
1348 | 394 | node = factory.make_node() | 394 | node = factory.make_node() |
1349 | 395 | arch, subarch = node.architecture.split('/') | 395 | arch, subarch = node.architecture.split('/') |
1350 | 396 | factory.make_boot_image( | 396 | factory.make_boot_image( |
1352 | 397 | osystem='ubuntu', | 397 | osystem=node.get_osystem(), |
1353 | 398 | architecture=arch, subarchitecture=subarch, | 398 | architecture=arch, subarchitecture=subarch, |
1354 | 399 | release=node.get_distro_series(), purpose='xinstall', | 399 | release=node.get_distro_series(), purpose='xinstall', |
1355 | 400 | nodegroup=node.nodegroup) | 400 | nodegroup=node.nodegroup) |
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): purpose( node, osystem, arch, subarch, series, label): use_traditional _installer( ):
return "install" Registry[ osystem] obj.get_ boot_image_ purposes( PURPOSE. XINSTALL in purposes:
+def get_boot_
"""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_
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 = OperatingSystem
+ purposes = osystem_
+ arch, subarch, series, label)
+ if BOOT_IMAGE_
+ 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] purpose( node, osystem, arch, subarch, series, None)
+ # 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_
Same comment as [3].
Also, don't need the comma between 'label' and 'here'.
[5] param(request. GET, 'label', latest_label)
label = get_optional_
+ # Now that we have the correct label, lets check the boot purpose again purpose( node, osystem, arch, subarch, series, label)
+ # to make sure that the boot image with that label, is the correct purpose.
+ purpose = get_boot_
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' forms.py 2014-05-14 07:12:10 +0000 forms.py 2014-05-14 14:40:00 +0000 config_ forms import SKIP_CHECK_NAME DISTRO_ SERIES_ CHOICES, SERIES_ CHOICES, NTERFACE_ MANAGEMENT, NTERFACE_ MANAGEMENT_ CHOICES, forms_settings import ( ITEMS_KEYS, config_ field, DISTRO_ SERIES_ MESSAGE,
--- src/maasserver/
+++ src/maasserver/
@@ -71,9 +71,6 @@
)
from maasserver.
from maasserver.enum import (
- COMMISSIONING_
- DISTRO_SERIES,
- DISTRO_
NODE_STATUS,
NODEGROUPI
NODEGROUPI
@@ -86,7 +83,7 @@
from maasserver.
CONFIG_
get_
- INVALID_
+ list_commisioni...