Merge lp:~jtv/maas/take-node-disable-ipv4-from-nodegroup into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Make Node.disable_ipv4 default to NodeGroup.default_disable_ipv4 (on the node's own nodegroup). This allows users to set cluster-wide default policies for whether IPv4 should be disabled on deployed nodes.

Doing this on the form seemed more appropriate to me than doing it on the model: this default value is basically a user-interface concept. Making such connections between model objects during model cleaning seems altogether too intricate.

The UI for disabling IPv4 is still hidden behind the REVEAL_IPv6 feature flag, and this new default setting does not have any UI at all yet. We can change that in a later branch.

While I was there I also updated some existing Node API tests with factory helpers we didn't have at the time, and made the basic test for creating a node through the API verify that the node actually gets created.

Jeroen

[Graham Binns (gmb)]

review: Approve

=== modified file 'src/maasserver/api/tests/test_nodes.py'
--- src/maasserver/api/tests/test_nodes.py	2014-09-22 05:33:12 +0000
+++ src/maasserver/api/tests/test_nodes.py	2014-09-22 07:17:20 +0000
@@ -30,7 +30,10 @@
     )
 from maasserver.exceptions import ClusterUnavailable
 from maasserver.fields import MAC
-from maasserver.models import Node
+from maasserver.models import (
+    Node,
+    NodeGroup,
+    )
 from maasserver.models.node import RELEASABLE_STATUSES
 from maasserver.models.user import (
     create_auth_token,
@@ -159,17 +162,29 @@
 
     def test_POST_new_creates_node(self):
         # The API allows a non-admin logged-in user to create a Node.
+        hostname = factory.make_name('host')
+        architecture = make_usable_architecture(self)
+        macs = {
+            factory.make_mac_address()
+            for _ in range(random.randint(1, 2))
+            }
         response = self.client.post(
             reverse('nodes_handler'),
             {
                 'op': 'new',
                 'autodetect_nodegroup': '1',
-                'hostname': factory.make_string(),
-                'architecture': make_usable_architecture(self),
-                'mac_addresses': ['aa:bb:cc:dd:ee:ff', '22:bb:cc:dd:ee:ff'],
+                'hostname': hostname,
+                'architecture': architecture,
+                'mac_addresses': macs,
             })
-
         self.assertEqual(httplib.OK, response.status_code)
+        system_id = json.loads(response.content)['system_id']
+        node = Node.objects.get(system_id=system_id)
+        self.expectThat(node.hostname, Equals(hostname))
+        self.expectThat(node.architecture, Equals(architecture))
+        self.expectThat(
+            {mac.mac_address for mac in node.macaddress_set.all()},
+            Equals(macs))
 
     def test_POST_new_when_logged_in_creates_node_in_declared_state(self):
         # When a user enlists a node, it goes into the New state.
@@ -179,9 +194,9 @@
             {
                 'op': 'new',
                 'autodetect_nodegroup': '1',
-                'hostname': factory.make_string(),
+                'hostname': factory.make_name('host'),
                 'architecture': make_usable_architecture(self),
-                'mac_addresses': ['aa:bb:cc:dd:ee:ff'],
+                'mac_addresses': [factory.make_mac_address()],
             })
         self.assertEqual(httplib.OK, response.status_code)
         system_id = json.loads(response.content)['system_id']
@@ -189,6 +204,41 @@
             NODE_STATUS.NEW,
             Node.objects.get(system_id=system_id).status)
 
+    def test_POST_new_takes_default_for_disable_ipv4_from_given_cluster(self):
+        default_disable_ipv4 = factory.pick_bool()
+        cluster = factory.make_NodeGroup(
+            default_disable_ipv4=default_disable_ipv4)
+        response = self.client.post(
+            reverse('nodes_handler'),
+            {
+                'op': 'new',
+                'nodegroup': cluster.id,
+                'architecture': make_usable_architecture(self),
+                'mac_addresses': [factory.make_mac_address()],
+            })
+        self.assertEqual(httplib.OK, response.status_code)
+        system_id = json.loads(response.content)['system_id']
+        node = Node.objects.get(system_id=system_id)
+        self.assertEqual(default_disable_ipv4, node.disable_ipv4)
+
+    def test_POST_new_takes_default_disable_ipv4_from_guessed_cluster(self):
+        default_disable_ipv4 = factory.pick_bool()
+        master_cluster = NodeGroup.objects.ensure_master()
+        master_cluster.default_disable_ipv4 = default_disable_ipv4
+        master_cluster.save()
+        response = self.client.post(
+            reverse('nodes_handler'),
+            {
+                'op': 'new',
+                'autodetect_nodegroup': '1',
+                'architecture': make_usable_architecture(self),
+                'mac_addresses': [factory.make_mac_address()],
+            })
+        self.assertEqual(httplib.OK, response.status_code)
+        system_id = json.loads(response.content)['system_id']
+        node = Node.objects.get(system_id=system_id)
+        self.assertEqual(default_disable_ipv4, node.disable_ipv4)
+
     def test_POST_new_when_no_RPC_to_cluster_defaults_empty_power(self):
         # Test for bug 1305061, if there is no cluster RPC connection
         # then make sure that power_type is defaulted to the empty

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2014-09-17 13:18:15 +0000
+++ src/maasserver/forms.py	2014-09-22 07:17:20 +0000
@@ -283,8 +283,11 @@
         # Even though it doesn't need it and doesn't use it, this form accepts
         # a parameter named 'request' because it is used interchangingly
         # with NodeAdminForm which actually uses this parameter.
-        if kwargs.get('instance') is None:
-            # Creating a new node.  Offer choice of nodegroup.
+
+        # Are we creating a new node object?
+        self.new_node = (kwargs.get('instance') is None)
+        if self.new_node:
+            # Offer choice of nodegroup.
             self.fields['nodegroup'] = NodeGroupFormField(
                 required=False, empty_label="Default (master)")
         self.set_up_architecture_field()
@@ -372,6 +375,15 @@
     def clean_distro_series(self):
         return clean_distro_series_field(self, 'distro_series', 'osystem')
 
+    def clean(self):
+        cleaned_data = super(NodeForm, self).clean()
+        if self.new_node and self.data.get('disable_ipv4') is None:
+            # Creating a new node, without a value for disable_ipv4 given.
+            # Take the default value from the node's cluster.
+            nodegroup = cleaned_data['nodegroup']
+            cleaned_data['disable_ipv4'] = nodegroup.default_disable_ipv4
+        return cleaned_data
+
     def is_valid(self):
         is_valid = super(NodeForm, self).is_valid()
         if len(list_all_usable_architectures()) == 0:

=== modified file 'src/maasserver/tests/test_forms_node.py'
--- src/maasserver/tests/test_forms_node.py	2014-09-15 14:28:28 +0000
+++ src/maasserver/tests/test_forms_node.py	2014-09-22 07:17:20 +0000
@@ -262,6 +262,43 @@
         form.instance.nodegroup = nodegroup
         self.assertFalse(form.is_valid())
 
+    def test_obeys_disable_ipv4_if_given(self):
+        setting = factory.pick_bool()
+        cluster = factory.make_NodeGroup(default_disable_ipv4=(not setting))
+        form = NodeForm(
+            data={
+                'nodegroup': cluster,
+                'architecture': make_usable_architecture(self),
+                'disable_ipv4': setting,
+                })
+        form.instance.nodegroup = cluster
+        node = form.save()
+        self.assertEqual(setting, node.disable_ipv4)
+
+    def test_takes_True_disable_ipv4_from_cluster_by_default(self):
+        setting = True
+        cluster = factory.make_NodeGroup(default_disable_ipv4=setting)
+        form = NodeForm(
+            data={
+                'nodegroup': cluster,
+                'architecture': make_usable_architecture(self),
+                })
+        form.instance.nodegroup = cluster
+        node = form.save()
+        self.assertEqual(setting, node.disable_ipv4)
+
+    def test_takes_False_disable_ipv4_from_cluster_by_default(self):
+        setting = False
+        cluster = factory.make_NodeGroup(default_disable_ipv4=setting)
+        form = NodeForm(
+            data={
+                'nodegroup': cluster,
+                'architecture': make_usable_architecture(self),
+                })
+        form.instance.nodegroup = cluster
+        node = form.save()
+        self.assertEqual(setting, node.disable_ipv4)
+
 
 class TestAdminNodeForm(MAASServerTestCase):