Merge lp:~jason-hobbs/maas/allow-hostname-change-while-allocated into lp:~maas-committers/maas/trunk

[Jason Hobbs]

Allow changing a hostname while a node is acquired.

This rolls back most of r1314. A similar check for "node started" instead of "node acquired" is expected to be added in the future, once MAAS grows the ability to track a node's start/stop state. In the mean time, I think it makes sense to be less restrictive and allow more use cases.

[Jason Hobbs (jason-hobbs)]

We will revisit this towards the end of the sprint next week - there is a plan to implement acquired vs. acquired+started tracking in MAAS.

[Julian Edwards (julian-edwards)]

I think this is fine, but please file a bug with the intended behaviour and XXX with a reference to it in the code.

review: Approve

[Jason Hobbs (jason-hobbs)]

Approving minor comment change.

review: Approve

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2014-08-14 05:57:19 +0000
+++ src/maasserver/forms.py	2014-08-14 13:49:41 +0000
@@ -335,18 +335,16 @@
                 self.initial['distro_series'] = initial_value
 
     def clean_hostname(self):
-        # Don't allow the hostname to be changed if the node is
-        # currently allocated.  Juju knows the node by its old name, so
-        # changing the name would confuse things.
-        hostname = self.instance.hostname
-        status = self.instance.status
-        new_hostname = self.cleaned_data.get('hostname', hostname)
-        if new_hostname != hostname and status == NODE_STATUS.ALLOCATED:
-            raise ValidationError(
-                "Can't change hostname to %s: node is in use." % new_hostname)
-
-        # XXX 2014-07-30 jason-hobbs, bug=1350459: This check shouldn't be
-        # necessary, but many tests don't provide a nodegroup to NodeForm.
+        # XXX 2014-08-14 jason-hobbs, bug=1356880; MAAS shouldn't allow
+        # a hostname to be changed on a "deployed" node, but it doesn't
+        # yet have the ability to distinguish between an "acquired" node
+        # and a "deployed" node.
+
+        new_hostname = self.cleaned_data.get('hostname')
+
+        # XXX 2014-07-30 jason-hobbs, bug=1350459: This check for no
+        # nodegroup shouldn't be necessary, but many tests don't provide
+        # a nodegroup to NodeForm.
         if self.instance.nodegroup is None:
             return new_hostname
 

=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py	2014-08-12 13:52:04 +0000
+++ src/maasserver/tests/test_forms.py	2014-08-14 13:49:41 +0000
@@ -710,37 +710,6 @@
             (node.hostname, node.power_type, node.zone),
             (hostname, power_type, zone))
 
-    def test_AdminNodeForm_refuses_to_update_hostname_on_allocated_node(self):
-        old_name = factory.make_name('old-hostname')
-        new_name = factory.make_name('new-hostname')
-        node = factory.make_node(
-            hostname=old_name, status=NODE_STATUS.ALLOCATED)
-        form = AdminNodeForm(
-            data={
-                'hostname': new_name,
-                'architecture': node.architecture,
-                },
-            instance=node)
-        self.assertFalse(form.is_valid())
-        self.assertEqual(
-            ["Can't change hostname to %s: node is in use." % new_name],
-            form._errors['hostname'])
-
-    def test_AdminNodeForm_accepts_unchanged_hostname_on_allocated_node(self):
-        old_name = factory.make_name('old-hostname')
-        node = factory.make_node(
-            hostname=old_name, status=NODE_STATUS.ALLOCATED)
-        patch_usable_architectures(self, [node.architecture])
-        form = AdminNodeForm(
-            data={
-                'hostname': old_name,
-                'architecture': node.architecture,
-            },
-            instance=node)
-        self.assertTrue(form.is_valid(), form._errors)
-        form.save()
-        self.assertEqual(old_name, reload_object(node).hostname)
-
     def test_AdminNodeForm_populates_power_type_choices(self):
         form = AdminNodeForm()
         self.assertEqual(