Merge lp:~ltrager/maas/lp1593991_2.0 into lp:maas/2.0

[Lee Trager]

Backport from 2.1.1 - Validate power parameters in the node form.

Backport from 2.1.1, reviewed in https://code.launchpad.net/~ltrager/maas/lp1593991/+merge/308893

[Mike Pontillo (mpontillo)]

You can self-approve and immediately land backports, unless there was a merge conflict and you'd like to get another pair of eyes. I'm assuming that is the case here.

review: Approve

=== modified file 'src/maasserver/api/machines.py'
--- src/maasserver/api/machines.py	2016-08-18 12:30:42 +0000
+++ src/maasserver/api/machines.py	2016-10-24 18:44:21 +0000
@@ -30,7 +30,6 @@
     OwnerDataMixin,
     PowerMixin,
     PowersMixin,
-    store_node_power_parameters,
 )
 from maasserver.api.support import (
     admin_method,
@@ -737,8 +736,6 @@
     form = Form(data=altered_query_data, request=request)
     if form.is_valid():
         machine = form.save()
-        # Hack in the power parameters here.
-        store_node_power_parameters(machine, request)
         maaslog.info("%s: Enlisted new machine", machine.hostname)
         return machine
     else:

=== modified file 'src/maasserver/api/tests/test_enlistment.py'
--- src/maasserver/api/tests/test_enlistment.py	2016-06-14 18:31:07 +0000
+++ src/maasserver/api/tests/test_enlistment.py	2016-10-24 18:44:21 +0000
@@ -87,6 +87,7 @@
         architecture = make_usable_architecture(self)
         power_type = 'ipmi'
         power_parameters = {
+            "power_address": factory.make_ip_address(),
             "power_user": factory.make_name("power-user"),
             "power_pass": factory.make_name("power-pass"),
             }
@@ -95,14 +96,16 @@
             {
                 'hostname': hostname,
                 'architecture': architecture,
-                'power_type': 'manual',
+                'power_type': power_type,
                 'mac_addresses': factory.make_mac_address(),
                 'power_parameters': json.dumps(power_parameters),
-                'power_type': power_type,
             })
+        # Add the default values.
+        power_parameters['power_driver'] = 'LAN_2_0'
+        power_parameters['mac_address'] = ''
         self.assertEqual(http.client.OK, response.status_code)
         [machine] = Machine.objects.filter(hostname=hostname)
-        self.assertEqual(power_parameters, machine.power_parameters)
+        self.assertItemsEqual(power_parameters, machine.power_parameters)
         self.assertEqual(power_type, machine.power_type)
 
     def test_POST_create_creates_machine_with_arch_only(self):
@@ -454,23 +457,29 @@
         self.assertEqual([], machines_returned)
 
     def test_POST_simple_user_can_set_power_type_and_parameters(self):
-        new_power_address = factory.make_string()
+        new_power_address = factory.make_url()
+        new_power_id = factory.make_name('power_id')
         response = self.client.post(
             reverse('machines_handler'), {
                 'architecture': make_usable_architecture(self),
-                'power_type': 'manual',
+                'power_type': 'virsh',
                 'power_parameters': json.dumps(
-                    {"power_address": new_power_address}),
+                    {
+                        "power_address": new_power_address,
+                        "power_id": new_power_id,
+                    }),
                 'mac_addresses': ['AA:BB:CC:DD:EE:FF'],
                 })
-
         machine = Machine.objects.get(
             system_id=json_load_bytes(response.content)['system_id'])
-        self.assertEqual(
-            (http.client.OK, {"power_address": new_power_address},
-             'manual'),
-            (response.status_code, machine.power_parameters,
-             machine.power_type))
+        self.assertEqual(http.client.OK, response.status_code)
+        self.assertEqual('virsh', machine.power_type)
+        self.assertItemsEqual(
+            {
+                'power_pass': '',
+                'power_id': new_power_id,
+                'power_address': new_power_address,
+            }, machine.power_parameters)
 
     def test_POST_returns_limited_fields(self):
         response = self.client.post(

=== modified file 'src/maasserver/api/tests/test_machine.py'
--- src/maasserver/api/tests/test_machine.py	2016-05-24 22:05:45 +0000
+++ src/maasserver/api/tests/test_machine.py	2016-10-24 18:44:21 +0000
@@ -808,7 +808,7 @@
         # The api allows the updating of a Machine.
         machine = factory.make_Node(
             hostname='diane', owner=self.user,
-            architecture=make_usable_architecture(self))
+            architecture=make_usable_architecture(self), power_type='manual')
         response = self.client.put(
             self.get_machine_uri(machine), {'hostname': 'francis'})
         parsed_result = json_load_bytes(response.content)
@@ -825,7 +825,8 @@
         hostname = factory.make_name('hostname')
         arch = make_usable_architecture(self)
         machine = factory.make_Node(
-            hostname=hostname, owner=self.user, architecture=arch)
+            hostname=hostname, owner=self.user, architecture=arch,
+            power_type='manual')
         response = self.client.put(
             self.get_machine_uri(machine),
             {'architecture': arch})
@@ -848,7 +849,8 @@
         self.become_admin()
         machine = factory.make_Node(
             owner=self.user,
-            architecture=make_usable_architecture(self))
+            architecture=make_usable_architecture(self),
+            power_type='manual')
         field = factory.make_string()
         response = self.client.put(
             self.get_machine_uri(machine),
@@ -866,7 +868,11 @@
             power_type=original_power_type,
             architecture=make_usable_architecture(self))
         response = self.client.put(
-            self.get_machine_uri(machine), {'power_type': new_power_type})
+            self.get_machine_uri(machine),
+            {
+                'power_type': new_power_type,
+                'power_parameters_skip_check': 'true',
+            })
 
         self.assertEqual(http.client.OK, response.status_code)
         self.assertEqual(
@@ -890,7 +896,8 @@
         # provides the URI for this Machine.
         machine = factory.make_Node(
             hostname='diane', owner=self.user,
-            architecture=make_usable_architecture(self))
+            architecture=make_usable_architecture(self),
+            power_type='manual')
         response = self.client.put(
             self.get_machine_uri(machine), {'hostname': 'francis'})
         parsed_result = json_load_bytes(response.content)
@@ -906,7 +913,8 @@
         self.become_admin()
         machine = factory.make_Node(
             hostname='diane', owner=self.user,
-            architecture=make_usable_architecture(self))
+            architecture=make_usable_architecture(self),
+            power_type='manual')
         response = self.client.put(
             self.get_machine_uri(machine), {'hostname': '.'})
         parsed_result = json_load_bytes(response.content)
@@ -959,8 +967,8 @@
         self.become_admin()
         machine = factory.make_Node(
             owner=self.user,
-            power_type=factory.pick_power_type(),
-            architecture=make_usable_architecture(self))
+            architecture=make_usable_architecture(self),
+            power_type='manual')
         response = self.client.put(
             self.get_machine_uri(machine),
             {'cpu_count': 1, 'memory': 1024})
@@ -1067,7 +1075,8 @@
         self.become_admin()
         machine = factory.make_Node(
             owner=self.user,
-            architecture=make_usable_architecture(self))
+            architecture=make_usable_architecture(self),
+            power_parameters={})
         new_param = factory.make_string()
         new_value = factory.make_string()
         response = self.client.put(
@@ -1083,7 +1092,11 @@
 
     def test_PUT_updates_power_parameters_empty_string(self):
         self.become_admin()
-        power_parameters = {factory.make_string(): factory.make_string()}
+        power_parameters = {
+            'power_address': factory.make_url(),
+            'power_id': factory.make_name('power_id'),
+            'power_pass': factory.make_name('power_pass'),
+        }
         machine = factory.make_Node(
             owner=self.user,
             power_type='virsh',
@@ -1091,22 +1104,17 @@
             architecture=make_usable_architecture(self))
         response = self.client.put(
             self.get_machine_uri(machine),
-            {'power_parameters_power_id': ''})
+            {'power_parameters_power_pass': ''})
 
         self.assertEqual(http.client.OK, response.status_code)
         self.assertEqual(
-            {
-                'power_id': '',
-                'power_pass': '',
-                'power_address': '',
-            },
-            reload_object(machine).power_parameters)
+            '', reload_object(machine).power_parameters['power_pass'])
 
     def test_PUT_sets_zone(self):
         self.become_admin()
         new_zone = factory.make_Zone()
         machine = factory.make_Node(
-            architecture=make_usable_architecture(self))
+            architecture=make_usable_architecture(self), power_type='manual')
 
         response = self.client.put(
             self.get_machine_uri(machine), {'zone': new_zone.name})
@@ -1119,7 +1127,7 @@
         self.become_admin()
         new_name = factory.make_name()
         machine = factory.make_Node(
-            architecture=make_usable_architecture(self))
+            architecture=make_usable_architecture(self), power_type='manual')
         old_zone = machine.zone
 
         response = self.client.put(
@@ -1148,7 +1156,8 @@
         self.become_admin()
         zone = factory.make_Zone()
         machine = factory.make_Node(
-            zone=zone, architecture=make_usable_architecture(self))
+            zone=zone, architecture=make_usable_architecture(self),
+            power_type='manual')
 
         response = self.client.put(self.get_machine_uri(machine), {})
 
@@ -1186,6 +1195,7 @@
         machine = factory.make_Node(
             owner=self.user,
             architecture=make_usable_architecture(self),
+            power_type='manual',
             disable_ipv4=original_setting)
         new_setting = not original_setting
 
@@ -1202,6 +1212,7 @@
         machine = factory.make_Node(
             owner=self.user,
             architecture=make_usable_architecture(self),
+            power_type='manual',
             disable_ipv4=original_setting)
         self.assertEqual(original_setting, machine.disable_ipv4)
 
@@ -1216,7 +1227,8 @@
         self.become_admin()
         machine = factory.make_Node(
             owner=self.user,
-            architecture=make_usable_architecture(self))
+            architecture=make_usable_architecture(self),
+            power_type='manual')
         response = self.client.put(
             reverse('machine_handler', args=[machine.system_id]),
             {'swap_size': 5 * 1000 ** 3})  # Making sure we overflow 32 bits
@@ -1229,7 +1241,8 @@
         self.become_admin()
         machine = factory.make_Node(
             owner=self.user,
-            architecture=make_usable_architecture(self))
+            architecture=make_usable_architecture(self),
+            power_type='manual')
 
         response = self.client.put(
             reverse('machine_handler', args=[machine.system_id]),

=== modified file 'src/maasserver/api/tests/test_node.py'
--- src/maasserver/api/tests/test_node.py	2016-06-17 07:16:39 +0000
+++ src/maasserver/api/tests/test_node.py	2016-10-24 18:44:21 +0000
@@ -302,7 +302,7 @@
         self.assertEqual(
             http.client.OK, response.status_code, response.content)
         parsed_params = json_load_bytes(response.content)
-        self.assertEqual({}, parsed_params)
+        self.assertEqual(node.power_parameters, parsed_params)
 
     def test_power_parameters_requires_admin(self):
         node = factory.make_Node()

=== modified file 'src/maasserver/api/tests/test_rackcontroller.py'
--- src/maasserver/api/tests/test_rackcontroller.py	2016-06-08 20:20:40 +0000
+++ src/maasserver/api/tests/test_rackcontroller.py	2016-10-24 18:44:21 +0000
@@ -36,7 +36,8 @@
 
     def test_PUT_updates_rack_controller(self):
         self.become_admin()
-        rack = factory.make_RackController(owner=self.user)
+        rack = factory.make_RackController(
+            owner=self.user, power_type='manual')
         zone = factory.make_zone()
         response = self.client.put(
             self.get_rack_uri(rack), {'zone': zone.name})

=== modified file 'src/maasserver/api/tests/test_utils.py'
--- src/maasserver/api/tests/test_utils.py	2016-05-24 13:51:36 +0000
+++ src/maasserver/api/tests/test_utils.py	2016-10-24 18:44:21 +0000
@@ -7,6 +7,7 @@
 
 from collections import namedtuple
 
+from django.forms import CharField
 from django.http import QueryDict
 from maasserver.api.utils import (
     extract_bool,
@@ -15,6 +16,7 @@
     get_oauth_token,
     get_overridden_query_dict,
 )
+from maasserver.config_forms import DictCharField
 from maasserver.exceptions import Unauthorized
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
@@ -94,6 +96,24 @@
         results = get_overridden_query_dict(defaults, data, [key1])
         self.assertEqual([data_value2], results.getlist(key2))
 
+    def test_expands_dict_fields(self):
+        field_name = factory.make_name('field_name')
+        sub_fields = {
+            factory.make_name('sub_field'): CharField() for _ in range(3)
+        }
+        fields = {
+            field_name: DictCharField(sub_fields)
+        }
+        defaults = {
+            "%s_%s" % (field_name, field): factory.make_name('subfield')
+            for field in sub_fields.keys()
+        }
+        data = {field_name: DictCharField(fields)}
+        results = get_overridden_query_dict(defaults, data, fields)
+        expected = {key: [value] for key, value in defaults.items()}
+        expected.update(fields)
+        self.assertItemsEqual(expected, results)
+
 
 def make_fake_request(auth_header):
     """Create a very simple fake request, with just an auth header."""

=== modified file 'src/maasserver/api/utils.py'
--- src/maasserver/api/utils.py	2016-03-28 13:54:47 +0000
+++ src/maasserver/api/utils.py	2016-10-24 18:44:21 +0000
@@ -16,6 +16,7 @@
 
 from django.http import QueryDict
 from formencode.validators import Invalid
+from maasserver.config_forms import DictCharField
 from maasserver.exceptions import (
     MAASAPIValidationError,
     Unauthorized,
@@ -149,11 +150,23 @@
     """
     # Create a writable query dict.
     new_data = QueryDict('').copy()
+    # If the fields are a dict of django Fields see if one is a DictCharField.
+    # DictCharField must have their values prefixed with the DictField name in
+    # the returned data or defaults don't get carried.
+    if isinstance(fields, dict):
+        acceptable_fields = []
+        for field_name, field in fields.items():
+            acceptable_fields.append(field_name)
+            if isinstance(field, DictCharField):
+                for sub_field in field.names:
+                    acceptable_fields.append("%s_%s" % (field_name, sub_field))
+    else:
+        acceptable_fields = fields
     # Missing fields will be taken from the node's current values.  This
     # is to circumvent Django's ModelForm (form created from a model)
     # default behaviour that requires all the fields to be defined.
     for k, v in defaults.items():
-        if k in fields:
+        if k in acceptable_fields:
             new_data.setlist(k, listify(v))
     # We can't use update here because data is a QueryDict and 'update'
     # does not replaces the old values with the new as one would expect.

=== modified file 'src/maasserver/clusterrpc/power_parameters.py'
--- src/maasserver/clusterrpc/power_parameters.py	2016-03-28 13:54:47 +0000
+++ src/maasserver/clusterrpc/power_parameters.py	2016-10-24 18:44:21 +0000
@@ -65,12 +65,16 @@
             json_field['name'], json_field['choices'])
         extra_parameters = {
             'choices': json_field['choices'],
-            'initial': json_field['default'],
             'error_messages': {
                 'invalid_choice': invalid_choice_message},
             }
     else:
         extra_parameters = {}
+
+    default = json_field.get('default')
+    if default is not None:
+        extra_parameters['initial'] = default
+
     form_field = field_class(
         label=json_field['label'], required=json_field['required'],
         **extra_parameters)
@@ -111,13 +115,20 @@
          'missing_packages': missing_packages})
 
 
-def get_power_type_parameters_from_json(json_power_type_parameters):
+def get_power_type_parameters_from_json(
+        json_power_type_parameters, initial_power_params=None,
+        skip_check=False):
     """Return power type parameters.
 
     :param json_power_type_parameters: Power type parameters expressed
         as a JSON string or as set of JSONSchema-verifiable objects.
         Will be validated using jsonschema.validate().
     :type json_power_type_parameters: JSON string or iterable.
+    :param initial_power_params: Power paramaters that were already set, any
+        field which matches will have its initial value set.
+    :type initial_power_params: dict
+    :param skip_check: Whether the field should be checked or not.
+    :type skip_check: bool
     :return: A dict of power parameters for all power types, indexed by
         power type name.
     """
@@ -127,19 +138,27 @@
         '': DictCharField(
             [], required=False, skip_check=True),
     }
+    if initial_power_params is None:
+        initial_power_params = []
     for power_type in json_power_type_parameters:
         fields = []
+        has_required_field = False
         for json_field in power_type['fields']:
+            field_name = json_field['name']
+            if field_name in initial_power_params:
+                json_field['default'] = initial_power_params[field_name]
+            has_required_field = has_required_field or json_field['required']
             fields.append((
                 json_field['name'], make_form_field(json_field)))
-        params = DictCharField(fields, required=False, skip_check=True)
+        params = DictCharField(
+            fields, required=has_required_field, skip_check=skip_check)
         power_parameters[power_type['name']] = params
     return power_parameters
 
 
-def get_power_type_parameters():
+def get_power_type_parameters(initial_power_params=None, skip_check=False):
     return get_power_type_parameters_from_json(
-        get_all_power_types_from_clusters())
+        get_all_power_types_from_clusters(), initial_power_params, skip_check)
 
 
 def get_power_type_choices():

=== modified file 'src/maasserver/clusterrpc/tests/test_power_parameters.py'
--- src/maasserver/clusterrpc/tests/test_power_parameters.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/clusterrpc/tests/test_power_parameters.py	2016-10-24 18:44:21 +0000
@@ -20,6 +20,7 @@
 )
 from maasserver.config_forms import DictCharField
 from maasserver.fields import MACAddressFormField
+from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils.forms import compose_invalid_choice_text
 from maastesting.matchers import MockCalledOnceWith
@@ -70,6 +71,42 @@
         for name, field in power_type_parameters.items():
             self.assertIsInstance(field, DictCharField)
 
+    def test__overrides_defaults(self):
+        name = factory.make_name('name')
+        field_name = factory.make_name('field_name')
+        new_default = factory.make_name('new default')
+        json_parameters = [{
+            'name': name,
+            'description': factory.make_name('description'),
+            'fields': [{
+                'name': field_name,
+                'label': factory.make_name('field label'),
+                'field_type': factory.make_name('field type'),
+                'default': factory.make_name('field default'),
+                'required': False,
+            }],
+        }]
+        power_type_parameters = get_power_type_parameters_from_json(
+            json_parameters, {field_name: new_default})
+        self.assertEqual(
+            new_default, power_type_parameters[name].fields[0].initial)
+
+    def test__manual_does_not_require_power_params(self):
+        json_parameters = [{
+            'name': 'manual',
+            'description': factory.make_name('description'),
+            'fields': [{
+                'name': factory.make_name('field name'),
+                'label': factory.make_name('field label'),
+                'field_type': factory.make_name('field type'),
+                'default': factory.make_name('field default'),
+                'required': False,
+            }],
+        }]
+        power_type_parameters = get_power_type_parameters_from_json(
+            json_parameters)
+        self.assertFalse(power_type_parameters['manual'].required)
+
 
 class TestMakeFormField(MAASServerTestCase):
     """Test that make_form_field() converts JSON fields to Django."""
@@ -137,6 +174,17 @@
             (json_field['label'], json_field['required']),
             (django_field.label, django_field.required))
 
+    def test__sets_initial_to_default(self):
+        json_field = {
+            'name': 'some_field',
+            'label': 'Some Field',
+            'field_type': 'string',
+            'required': False,
+            'default': 'some default',
+        }
+        django_field = make_form_field(json_field)
+        self.assertEquals(json_field['default'], django_field.initial)
+
 
 class TestMakeJSONField(MAASServerTestCase):
     """Test that make_json_field() creates JSON-verifiable fields."""

=== modified file 'src/maasserver/config_forms.py'
--- src/maasserver/config_forms.py	2015-12-01 18:12:59 +0000
+++ src/maasserver/config_forms.py	2016-10-24 18:44:21 +0000
@@ -186,6 +186,9 @@
                 field_value = value[index]
             except IndexError:
                 field_value = None
+            # Set the field_value to the default value if not set.
+            if field_value is None and field.initial not in (None, ''):
+                field_value = field.initial
             # Check the field's 'required' field instead of the global
             # 'required' field to allow subfields to be required or not.
             if field.required and field_value in validators.EMPTY_VALUES:

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2016-08-09 17:19:45 +0000
+++ src/maasserver/forms.py	2016-10-24 18:44:21 +0000
@@ -36,6 +36,7 @@
 
 from collections import Counter
 from functools import partial
+import json
 import pipes
 import re
 
@@ -244,6 +245,27 @@
         return power_type
 
     @staticmethod
+    def _get_power_parameters(form, data, machine):
+        if data is None:
+            data = {}
+
+        power_parameters = data.get(
+            'power_parameters', form.initial.get('power_parameters', {}))
+
+        if isinstance(power_parameters, str):
+            try:
+                power_parameters = json.loads(power_parameters)
+            except json.JSONDecodeError:
+                raise ValidationError("Failed to parse JSON power_parameters")
+
+        # Integrate the machines existing power_parameters if unset by form.
+        if machine:
+            for key, value in machine.power_parameters.items():
+                if power_parameters.get(key) is None:
+                    power_parameters[key] = value
+        return power_parameters
+
+    @staticmethod
     def set_up_power_type(form, data, machine=None):
         """Set up the 'power_type' and 'power_parameters' fields.
 
@@ -254,10 +276,18 @@
         choices = [BLANK_CHOICE] + get_power_type_choices()
         form.fields['power_type'] = forms.ChoiceField(
             required=False, choices=choices, initial=power_type)
-        form.fields['power_parameters'] = get_power_type_parameters()[
-            power_type]
-        if form.instance is not None and form.instance.power_type != '':
-            form.initial['power_type'] = form.instance.power_type
+        power_parameters = WithPowerMixin._get_power_parameters(
+            form, data, machine)
+        skip_check = (
+            form.data.get('power_parameters_%s' % SKIP_CHECK_NAME) == 'true')
+        form.fields['power_parameters'] = get_power_type_parameters(
+            power_parameters, skip_check=skip_check)[power_type]
+        if form.instance is not None:
+            if form.instance.power_type != '':
+                form.initial['power_type'] = form.instance.power_type
+            if form.instance.power_parameters != '':
+                for key, value in power_parameters.items():
+                    form.initial['power_parameters_%s' % key] = value
 
     @staticmethod
     def check_power_type(form, cleaned_data):
@@ -280,8 +310,9 @@
 
             # If power_type is not set and power_parameters_skip_check is not
             # on, reset power_parameters (set it to the empty string).
-            if cleaned_data.get('power_type', '') == '':
-                cleaned_data['power_parameters'] = ''
+            power_type = cleaned_data.get('power_type', '')
+            if power_type == '':
+                cleaned_data['power_parameters'] = {}
         return cleaned_data
 
     @staticmethod

=== modified file 'src/maasserver/rpc/nodes.py'
--- src/maasserver/rpc/nodes.py	2016-07-25 23:25:53 +0000
+++ src/maasserver/rpc/nodes.py	2016-10-24 18:44:21 +0000
@@ -222,10 +222,6 @@
     form = AdminMachineWithMACAddressesForm(data_query_dict)
     if form.is_valid():
         node = form.save()
-        # We have to explicitly save the power parameters; the form
-        # won't do it for us.
-        node.power_parameters = json.loads(power_parameters)
-        node.save()
         return node
     else:
         raise ValidationError(form.errors)

=== modified file 'src/maasserver/rpc/tests/test_nodes.py'
--- src/maasserver/rpc/tests/test_nodes.py	2016-05-11 00:25:14 +0000
+++ src/maasserver/rpc/tests/test_nodes.py	2016-10-24 18:44:21 +0000
@@ -7,7 +7,6 @@
 
 from datetime import timedelta
 import json
-from json import dumps
 from operator import attrgetter
 import random
 from random import randint
@@ -79,17 +78,13 @@
         mac_addresses = [
             factory.make_mac_address() for _ in range(3)]
         architecture = make_usable_architecture(self)
-        power_type = random.choice(self.power_types)['name']
-        power_parameters = dumps({})
 
-        node = create_node(
-            architecture, power_type, power_parameters,
-            mac_addresses)
+        node = create_node(architecture, 'manual', {}, mac_addresses)
 
         self.assertEqual(
             (
                 architecture,
-                power_type,
+                'manual',
                 {},
             ),
             (
@@ -113,17 +108,14 @@
             factory.make_mac_address() for _ in range(3)]
         architecture = make_usable_architecture(self)
         hostname = factory.make_hostname()
-        power_type = random.choice(self.power_types)['name']
-        power_parameters = dumps({})
 
         node = create_node(
-            architecture, power_type, power_parameters,
-            mac_addresses, hostname=hostname)
+            architecture, 'manual', {}, mac_addresses, hostname=hostname)
 
         self.assertEqual(
             (
                 architecture,
-                power_type,
+                'manual',
                 {},
                 hostname
             ),
@@ -149,7 +141,7 @@
             "Microsoft Windows",
             ])
         power_type = random.choice(self.power_types)['name']
-        power_parameters = dumps({})
+        power_parameters = {}
 
         with ExpectedException(ValidationError):
             create_node(
@@ -164,17 +156,15 @@
         architecture = make_usable_architecture(self)
         hostname = factory.make_hostname()
         domain = factory.make_Domain()
-        power_type = random.choice(self.power_types)['name']
-        power_parameters = dumps({})
 
         node = create_node(
-            architecture, power_type, power_parameters,
-            mac_addresses, domain=domain.name, hostname=hostname)
+            architecture, 'manual', {}, mac_addresses, domain=domain.name,
+            hostname=hostname)
 
         self.assertEqual(
             (
                 architecture,
-                power_type,
+                'manual',
                 {},
                 domain.id,
                 hostname,
@@ -198,7 +188,7 @@
             factory.make_mac_address() for _ in range(3)]
         architecture = make_usable_architecture(self)
         power_type = random.choice(self.power_types)['name']
-        power_parameters = dumps({})
+        power_parameters = {}
 
         with ExpectedException(ValidationError):
             create_node(
@@ -211,7 +201,7 @@
         self.assertRaises(
             ValidationError, create_node,
             architecture="spam/eggs", power_type="scrambled",
-            power_parameters=dumps({}),
+            power_parameters={},
             mac_addresses=[factory.make_mac_address()])
 
     def test__raises_error_if_node_already_exists(self):
@@ -220,8 +210,8 @@
         mac_addresses = [
             factory.make_mac_address() for _ in range(3)]
         architecture = make_usable_architecture(self)
-        power_type = random.choice(self.power_types)['name']
-        power_parameters = dumps({})
+        power_type = 'manual'
+        power_parameters = {}
 
         create_node(
             architecture, power_type, power_parameters,
@@ -236,20 +226,20 @@
         mac_addresses = [
             factory.make_mac_address() for _ in range(3)]
         architecture = make_usable_architecture(self)
-        power_type = random.choice(self.power_types)['name']
         power_parameters = {
-            factory.make_name('key'): factory.make_name('value')
-            for _ in range(3)
+            'power_address': factory.make_url(),
+            'power_pass': factory.make_name('power_pass'),
+            'power_id': factory.make_name('power_id'),
         }
 
         node = create_node(
-            architecture, power_type, dumps(power_parameters),
+            architecture, 'virsh', power_parameters,
             mac_addresses)
 
         # Reload the object from the DB so that we're sure its power
         # parameters are being persisted.
         node = reload_object(node)
-        self.assertEqual(power_parameters, node.power_parameters)
+        self.assertItemsEqual(power_parameters, node.power_parameters)
 
     def test__forces_generic_subarchitecture_if_missing(self):
         self.prepare_rack_rpc()
@@ -257,13 +247,9 @@
         mac_addresses = [
             factory.make_mac_address() for _ in range(3)]
         architecture = make_usable_architecture(self, subarch_name='generic')
-        power_type = random.choice(self.power_types)['name']
-        power_parameters = dumps({})
 
         arch, subarch = architecture.split('/')
-        node = create_node(
-            arch, power_type, power_parameters,
-            mac_addresses)
+        node = create_node(arch, 'manual', {}, mac_addresses)
 
         self.assertEqual(architecture, node.architecture)
 

=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py	2016-07-21 16:43:37 +0000
+++ src/maasserver/testing/factory.py	2016-10-24 18:44:21 +0000
@@ -367,8 +367,6 @@
             zone = self.make_Zone()
         if power_type is None:
             power_type = 'virsh'
-        if power_parameters is None:
-            power_parameters = {}
         if power_state is None:
             power_state = self.pick_enum(POWER_STATE)
         if power_state_updated is undefined:
@@ -430,7 +428,8 @@
                 ip_address = existing_static_ips[0]
             bmc_ip_address = self.pick_ip_in_Subnet(ip_address.subnet)
             node.power_parameters = {
-                "power_address": "qemu+ssh://user@%s/system" % bmc_ip_address
+                "power_address": "qemu+ssh://user@%s/system" % bmc_ip_address,
+                "power_id": factory.make_name("power_id"),
             }
             node.save()
 

=== modified file 'src/maasserver/tests/test_config_forms.py'
--- src/maasserver/tests/test_config_forms.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/tests/test_config_forms.py	2016-10-24 18:44:21 +0000
@@ -165,6 +165,15 @@
             {'char_field': char_value, 'multi_field': None},
             form.cleaned_data)
 
+    def test_DictCharField_sets_default_value_for_subfields(self):
+        default_value = factory.make_name('default_value')
+        multi_field = DictCharField(
+            [('field_a', forms.CharField(
+                label='Field a', initial=default_value))],
+            required=False)
+        self.assertEquals(
+            default_value, multi_field.clean_sub_fields('')['field_a'])
+
 
 class TestUtilities(MAASServerTestCase):
 

=== modified file 'src/maasserver/tests/test_forms_controller.py'
--- src/maasserver/tests/test_forms_controller.py	2016-04-13 02:20:16 +0000
+++ src/maasserver/tests/test_forms_controller.py	2016-10-24 18:44:21 +0000
@@ -40,6 +40,7 @@
         form = ControllerForm(
             data={
                 'power_type': power_type,
+                'power_parameters_skip_check': 'true',
             },
             instance=rack)
         rack = form.save()
@@ -51,12 +52,12 @@
         form = ControllerForm(
             data={
                 'power_parameters_field': power_parameters_field,
-                'power_parameters_skip_check': True,
+                'power_parameters_skip_check': 'true',
             },
             instance=rack)
         rack = form.save()
         self.assertEqual(
-            {'field': power_parameters_field}, rack.power_parameters)
+            power_parameters_field, rack.power_parameters['field'])
 
     def test__sets_zone(self):
         rack = factory.make_RackController()
@@ -64,6 +65,7 @@
         form = ControllerForm(
             data={
                 'zone': zone.name,
+                'power_parameters_skip_check': 'true',
             },
             instance=rack)
         rack = form.save()

=== modified file 'src/maasserver/tests/test_forms_machine.py'
--- src/maasserver/tests/test_forms_machine.py	2016-04-12 22:25:44 +0000
+++ src/maasserver/tests/test_forms_machine.py	2016-10-24 18:44:21 +0000
@@ -373,7 +373,7 @@
                 'architecture': arch,
                 'power_type': power_type,
                 'power_parameters_field': power_parameters_field,
-                'power_parameters_skip_check': True,
+                'power_parameters_skip_check': 'true',
                 },
             instance=node)
         form.save()
@@ -393,6 +393,7 @@
             data={
                 'hostname': hostname,
                 'architecture': arch,
+                'power_parameters_skip_check': 'true',
                 },
             instance=node)
         node = form.save()
@@ -407,6 +408,7 @@
             data={
                 'hostname': hostname,
                 'architecture': arch,
+                'power_parameters_skip_check': 'true',
                 },
             instance=node)
         node = form.save()
@@ -422,6 +424,7 @@
                 'hostname': hostname,
                 'architecture': arch,
                 'power_type': power_type,
+                'power_parameters_skip_check': 'true',
                 },
             instance=node)
         node = form.save()

=== modified file 'src/maasserver/websockets/handlers/machine.py'
--- src/maasserver/websockets/handlers/machine.py	2016-04-22 21:02:20 +0000
+++ src/maasserver/websockets/handlers/machine.py	2016-10-24 18:44:21 +0000
@@ -214,6 +214,7 @@
         new_params["hostname"] = params.get("hostname")
         new_params["architecture"] = params.get("architecture")
         new_params["power_type"] = params.get("power_type")
+        new_params["power_parameters"] = params.get("power_parameters")
         if "zone" in params:
             new_params["zone"] = params["zone"]["name"]
         if "domain" in params:
@@ -236,13 +237,8 @@
         if not self.user.is_superuser:
             raise HandlerPermissionError()
 
-        # Create the object, then save the power parameters because the
-        # form will not save this information.
         data = super(NodeHandler, self).create(params)
         node_obj = Node.objects.get(system_id=data['system_id'])
-        node_obj.power_type = params.get("power_type", '')
-        node_obj.power_parameters = params.get("power_parameters", {})
-        node_obj.save()
 
         # Start the commissioning process right away, which has the
         # desired side effect of initializing the node's power state.
@@ -256,16 +252,13 @@
         if not self.user.is_superuser:
             raise HandlerPermissionError()
 
-        # Update the node with the form. The form will not update the
-        # power_type or power_parameters, so we perform that here.
         data = super(NodeHandler, self).update(params)
         node_obj = Node.objects.get(system_id=data['system_id'])
-        node_obj.power_type = params.get("power_type", '')
-        node_obj.power_parameters = params.get("power_parameters", {})
 
         # Update the tags for the node and disks.
         self.update_tags(node_obj, params['tags'])
         node_obj.save()
+
         return self.full_dehydrate(node_obj)
 
     def mount_special(self, params):

=== modified file 'src/maasserver/websockets/handlers/tests/test_machine.py'
--- src/maasserver/websockets/handlers/tests/test_machine.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/websockets/handlers/tests/test_machine.py	2016-10-24 18:44:21 +0000
@@ -1094,7 +1094,7 @@
     def test_update_raises_validation_error_for_invalid_architecture(self):
         user = factory.make_admin()
         handler = MachineHandler(user, {})
-        node = factory.make_Node(interface=True)
+        node = factory.make_Node(interface=True, power_type='manual')
         node_data = self.dehydrate_node(node, handler)
         arch = factory.make_name("arch")
         node_data["architecture"] = arch
@@ -1144,7 +1144,8 @@
         user = factory.make_admin()
         handler = MachineHandler(user, {})
         architecture = make_usable_architecture(self)
-        node = factory.make_Node(interface=True, architecture=architecture)
+        node = factory.make_Node(
+            interface=True, architecture=architecture, power_type='manual')
         tags = [
             factory.make_Tag(definition='').name
             for _ in range(3)
@@ -1158,7 +1159,8 @@
         user = factory.make_admin()
         handler = MachineHandler(user, {})
         architecture = make_usable_architecture(self)
-        node = factory.make_Node(interface=True, architecture=architecture)
+        node = factory.make_Node(
+            interface=True, architecture=architecture, power_type='manual')
         tags = []
         for _ in range(3):
             tag = factory.make_Tag(definition='')
@@ -1175,7 +1177,8 @@
         user = factory.make_admin()
         handler = MachineHandler(user, {})
         architecture = make_usable_architecture(self)
-        node = factory.make_Node(interface=True, architecture=architecture)
+        node = factory.make_Node(
+            interface=True, architecture=architecture, power_type='manual')
         tag_name = factory.make_name("tag")
         node_data = self.dehydrate_node(node, handler)
         node_data["tags"].append(tag_name)

=== modified file 'src/maasserver/websockets/tests/test_base.py'
--- src/maasserver/websockets/tests/test_base.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/websockets/tests/test_base.py	2016-10-24 18:44:21 +0000
@@ -536,7 +536,7 @@
 
     def test_update_with_form_updates_node(self):
         arch = make_usable_architecture(self)
-        node = factory.make_Node(architecture=arch)
+        node = factory.make_Node(architecture=arch, power_type='manual')
         hostname = factory.make_name("hostname")
         handler = self.make_nodes_handler(
             fields=['hostname'], form=AdminMachineForm)
@@ -552,7 +552,7 @@
 
     def test_update_with_form_uses_form_from_get_form_class(self):
         arch = make_usable_architecture(self)
-        node = factory.make_Node(architecture=arch)
+        node = factory.make_Node(architecture=arch, power_type='manual')
         hostname = factory.make_name("hostname")
         handler = self.make_nodes_handler(fields=['hostname'])
         self.patch(

=== modified file 'src/provisioningserver/power/schema.py'
--- src/provisioningserver/power/schema.py	2016-05-20 19:58:33 +0000
+++ src/provisioningserver/power/schema.py	2016-10-24 18:44:21 +0000
@@ -224,9 +224,10 @@
         'name': 'virsh',
         'description': 'Virsh (virtual systems)',
         'fields': [
-            make_json_field('power_address', "Power address"),
+            make_json_field('power_address', "Power address", required=True),
             make_json_field(
-                'power_id', "Power ID", scope=POWER_PARAMETER_SCOPE.NODE),
+                'power_id', "Power ID", scope=POWER_PARAMETER_SCOPE.NODE,
+                required=True),
             make_json_field(
                 'power_pass', "Power password (optional)",
                 required=False, field_type='password'),
@@ -244,10 +245,11 @@
             make_json_field(
                 'power_uuid', "VM UUID (if known)", required=False,
                 scope=POWER_PARAMETER_SCOPE.NODE),
-            make_json_field('power_address', "VMware hostname"),
-            make_json_field('power_user', "VMware username"),
+            make_json_field('power_address', "VMware hostname", required=True),
+            make_json_field('power_user', "VMware username", required=True),
             make_json_field(
-                'power_pass', "VMware password", field_type='password'),
+                'power_pass', "VMware password", field_type='password',
+                required=True),
             make_json_field(
                 'power_port', "VMware API port (optional)", required=False),
             make_json_field(
@@ -259,9 +261,10 @@
         'name': 'fence_cdu',
         'description': 'Sentry Switch CDU',
         'fields': [
-            make_json_field('power_address', "Power address"),
+            make_json_field('power_address', "Power address", required=True),
             make_json_field(
-                'power_id', "Power ID", scope=POWER_PARAMETER_SCOPE.NODE),
+                'power_id', "Power ID", scope=POWER_PARAMETER_SCOPE.NODE,
+                required=True),
             make_json_field('power_user', "Power user"),
             make_json_field(
                 'power_pass', "Power password", field_type='password'),
@@ -274,8 +277,9 @@
         'fields': [
             make_json_field(
                 'power_driver', "Power driver", field_type='choice',
-                choices=IPMI_DRIVER_CHOICES, default=IPMI_DRIVER.LAN_2_0),
-            make_json_field('power_address', "IP address"),
+                choices=IPMI_DRIVER_CHOICES, default=IPMI_DRIVER.LAN_2_0,
+                required=True),
+            make_json_field('power_address', "IP address", required=True),
             make_json_field('power_user', "Power user"),
             make_json_field(
                 'power_pass', "Power password", field_type='password'),
@@ -288,13 +292,13 @@
         'name': 'moonshot',
         'description': 'HP Moonshot - iLO4 (IPMI)',
         'fields': [
-            make_json_field('power_address', "Power address"),
+            make_json_field('power_address', "Power address", required=True),
             make_json_field('power_user', "Power user"),
             make_json_field(
                 'power_pass', "Power password", field_type='password'),
             make_json_field(
                 'power_hwaddress', "Power hardware address",
-                scope=POWER_PARAMETER_SCOPE.NODE),
+                scope=POWER_PARAMETER_SCOPE.NODE, required=True),
         ],
         'ip_extractor': make_ip_extractor('power_address'),
     },
@@ -303,14 +307,16 @@
         'description': 'SeaMicro 15000',
         'fields': [
             make_json_field(
-                'system_id', "System ID", scope=POWER_PARAMETER_SCOPE.NODE),
-            make_json_field('power_address', "Power address"),
+                'system_id', "System ID", scope=POWER_PARAMETER_SCOPE.NODE,
+                required=True),
+            make_json_field('power_address', "Power address", required=True),
             make_json_field('power_user', "Power user"),
             make_json_field(
                 'power_pass', "Power password", field_type='password'),
             make_json_field(
                 'power_control', "Power control type", field_type='choice',
-                choices=SM15K_POWER_CONTROL_CHOICES, default='ipmi'),
+                choices=SM15K_POWER_CONTROL_CHOICES, default='ipmi',
+                required=True),
         ],
         'ip_extractor': make_ip_extractor('power_address'),
     },
@@ -320,7 +326,7 @@
         'fields': [
             make_json_field(
                 'power_pass', "Power password", field_type='password'),
-            make_json_field('power_address', "Power address")
+            make_json_field('power_address', "Power address", required=True)
         ],
         'ip_extractor': make_ip_extractor('power_address'),
     },
@@ -329,8 +335,9 @@
         'description': 'Digital Loggers, Inc. PDU',
         'fields': [
             make_json_field(
-                'outlet_id', "Outlet ID", scope=POWER_PARAMETER_SCOPE.NODE),
-            make_json_field('power_address', "Power address"),
+                'outlet_id', "Outlet ID", scope=POWER_PARAMETER_SCOPE.NODE,
+                required=True),
+            make_json_field('power_address', "Power address", required=True),
             make_json_field('power_user', "Power user"),
             make_json_field(
                 'power_pass', "Power password", field_type='password'),
@@ -342,8 +349,9 @@
         'description': "Cisco UCS Manager",
         'fields': [
             make_json_field(
-                'uuid', "Server UUID", scope=POWER_PARAMETER_SCOPE.NODE),
-            make_json_field('power_address', "URL for XML API"),
+                'uuid', "Server UUID", scope=POWER_PARAMETER_SCOPE.NODE,
+                required=True),
+            make_json_field('power_address', "URL for XML API", required=True),
             make_json_field('power_user', "API user"),
             make_json_field(
                 'power_pass', "API password", field_type='password'),
@@ -355,7 +363,8 @@
         'name': 'mscm',
         'description': "HP Moonshot - iLO Chassis Manager",
         'fields': [
-            make_json_field('power_address', "IP for MSCM CLI API"),
+            make_json_field(
+                'power_address', "IP for MSCM CLI API", required=True),
             make_json_field('power_user', "MSCM CLI API user"),
             make_json_field(
                 'power_pass', "MSCM CLI API password", field_type='password'),
@@ -363,7 +372,7 @@
                 'node_id',
                 "Node ID - Must adhere to cXnY format "
                 "(X=cartridge number, Y=node number).",
-                scope=POWER_PARAMETER_SCOPE.NODE),
+                scope=POWER_PARAMETER_SCOPE.NODE, required=True),
         ],
         'ip_extractor': make_ip_extractor('power_address'),
     },
@@ -371,14 +380,14 @@
         'name': 'msftocs',
         'description': "Microsoft OCS - Chassis Manager",
         'fields': [
-            make_json_field('power_address', "Power address"),
+            make_json_field('power_address', "Power address", required=True),
             make_json_field('power_port', "Power port"),
             make_json_field('power_user', "Power user"),
             make_json_field(
                 'power_pass', "Power password", field_type='password'),
             make_json_field(
                 'blade_id', "Blade ID (Typically 1-24)",
-                scope=POWER_PARAMETER_SCOPE.NODE),
+                scope=POWER_PARAMETER_SCOPE.NODE, required=True),
         ],
         'ip_extractor': make_ip_extractor('power_address'),
     },
@@ -386,10 +395,10 @@
         'name': 'apc',
         'description': "American Power Conversion (APC) PDU",
         'fields': [
-            make_json_field('power_address', "IP for APC PDU"),
+            make_json_field('power_address', "IP for APC PDU", required=True),
             make_json_field(
                 'node_outlet', "APC PDU node outlet number (1-16)",
-                scope=POWER_PARAMETER_SCOPE.NODE),
+                scope=POWER_PARAMETER_SCOPE.NODE, required=True),
             make_json_field(
                 'power_on_delay', "Power ON outlet delay (seconds)",
                 default='5'),
@@ -400,16 +409,16 @@
         'name': 'hmc',
         'description': "IBM Hardware Management Console (HMC)",
         'fields': [
-            make_json_field('power_address', "IP for HMC"),
+            make_json_field('power_address', "IP for HMC", required=True),
             make_json_field('power_user', "HMC username"),
             make_json_field(
                 'power_pass', "HMC password", field_type='password'),
             make_json_field(
                 'server_name', "HMC Managed System server name",
-                scope=POWER_PARAMETER_SCOPE.NODE),
+                scope=POWER_PARAMETER_SCOPE.NODE, required=True),
             make_json_field(
                 'lpar', "HMC logical partition",
-                scope=POWER_PARAMETER_SCOPE.NODE),
+                scope=POWER_PARAMETER_SCOPE.NODE, required=True),
         ],
         'ip_extractor': make_ip_extractor('power_address'),
     },
@@ -417,11 +426,13 @@
         'name': 'nova',
         'description': 'OpenStack Nova',
         'fields': [
-            make_json_field('nova_id', "Host UUID"),
-            make_json_field('os_tenantname', "Tenant name"),
-            make_json_field('os_username', "Username"),
-            make_json_field('os_password', "Password", field_type='password'),
-            make_json_field('os_authurl', "Auth URL"),
+            make_json_field('nova_id', "Host UUID", required=True),
+            make_json_field('os_tenantname', "Tenant name", required=True),
+            make_json_field('os_username', "Username", required=True),
+            make_json_field(
+                'os_password', "Password", field_type='password',
+                required=True),
+            make_json_field('os_authurl', "Auth URL", required=True),
         ],
     },
 ]