Merge lp:~jtv/maas/1.2-bug-1070775 into lp:maas/1.2

Proposed by Jeroen T. Vermeulen on 2012-10-30
Status: Merged
Approved by: Jeroen T. Vermeulen on 2012-10-30
Approved revision: 1281
Merged at revision: 1277
Proposed branch: lp:~jtv/maas/1.2-bug-1070775
Merge into: lp:maas/1.2
Diff against target: 156 lines (+124/-0)
2 files modified
src/maasserver/forms.py (+26/-0)
src/maasserver/tests/test_forms.py (+98/-0)
To merge this branch: bzr merge lp:~jtv/maas/1.2-bug-1070775
Reviewer Review Type Date Requested Status
Gavin Panella (community) 2012-10-30 Approve on 2012-10-30
Review via email: mp+132054@code.launchpad.net

Commit message

Disallow nodegroup (DNS zone) name changes while allocated nodes are still using that zone name.

Description of the change

This fixes bug 1070775, after discussion with Raphaël: changing a NodeGroup.name will confuse Juju when it has nodes in use in that nodegroup — at least if we're managing DNS for that nodegroup. If we're not managing DNS, we can only assume that the admin knows what they're doing.

I'll be forward-porting this to trunk as well.

Jeroen

To post a comment you must log in.
Gavin Panella (allenap) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/forms.py'
2--- src/maasserver/forms.py 2012-10-29 11:04:39 +0000
3+++ src/maasserver/forms.py 2012-10-30 10:55:27 +0000
4@@ -62,6 +62,7 @@
5 NODE_AFTER_COMMISSIONING_ACTION,
6 NODE_AFTER_COMMISSIONING_ACTION_CHOICES,
7 NODE_STATUS,
8+ NODEGROUP_STATUS,
9 NODEGROUPINTERFACE_MANAGEMENT,
10 NODEGROUPINTERFACE_MANAGEMENT_CHOICES,
11 )
12@@ -801,6 +802,31 @@
13 'name',
14 )
15
16+ def clean_name(self):
17+ old_name = self.instance.name
18+ new_name = self.cleaned_data['name']
19+ if new_name == old_name or not new_name:
20+ # No change to the name. Return old name.
21+ return old_name
22+
23+ if self.instance.status != NODEGROUP_STATUS.ACCEPTED:
24+ # This nodegroup is not in use. Change it at will.
25+ return new_name
26+
27+ interface = self.instance.get_managed_interface()
28+ if interface.management != NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS:
29+ # MAAS is not managing DNS on this network, so the user can
30+ # rename the zone at will.
31+ return new_name
32+
33+ nodes_in_use = Node.objects.filter(
34+ nodegroup=self.instance, status=NODE_STATUS.ALLOCATED)
35+ if nodes_in_use.exists():
36+ raise ValidationError(
37+ "Can't rename DNS zone to %s; nodes are in use." % new_name)
38+
39+ return new_name
40+
41
42 class TagForm(ModelForm):
43
44
45=== modified file 'src/maasserver/tests/test_forms.py'
46--- src/maasserver/tests/test_forms.py 2012-10-29 16:53:58 +0000
47+++ src/maasserver/tests/test_forms.py 2012-10-30 10:55:27 +0000
48@@ -44,6 +44,7 @@
49 NewUserCreationForm,
50 NodeActionForm,
51 NodeForm,
52+ NodeGroupEdit,
53 NodeGroupInterfaceForm,
54 NodeGroupWithInterfacesForm,
55 NodeWithMACAddressesForm,
56@@ -879,3 +880,100 @@
57 nodegroup.management for nodegroup in
58 nodegroup.nodegroupinterface_set.all()
59 ])
60+
61+
62+def make_unrenamable_nodegroup_with_node():
63+ """Create a `NodeGroup` that can't be renamed, and `Node`.
64+
65+ Node groups can't be renamed while they are in an accepted state, have
66+ DHCP and DNS management enabled, and have a node that is in allocated
67+ state.
68+
69+ :return: tuple: (`NodeGroup`, `Node`).
70+ """
71+ name = factory.make_name('original-name')
72+ nodegroup = factory.make_node_group(
73+ name=name, status=NODEGROUP_STATUS.ACCEPTED)
74+ interface = nodegroup.get_managed_interface()
75+ interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS
76+ interface.save()
77+ node = factory.make_node(nodegroup=nodegroup, status=NODE_STATUS.ALLOCATED)
78+ return nodegroup, node
79+
80+
81+class TestNodeGroupEdit(TestCase):
82+
83+ def make_form_data(self, nodegroup):
84+ """Create `NodeGroupEdit` form data based on `nodegroup`."""
85+ return {
86+ 'name': nodegroup.name,
87+ 'cluster_name': nodegroup.cluster_name,
88+ 'status': nodegroup.status,
89+ }
90+
91+ def test_changes_name(self):
92+ nodegroup = factory.make_node_group(name=factory.make_name('old-name'))
93+ new_name = factory.make_name('new-name')
94+ data = self.make_form_data(nodegroup)
95+ data['name'] = new_name
96+ form = NodeGroupEdit(instance=nodegroup, data=data)
97+ self.assertTrue(form.is_valid())
98+ form.save()
99+ self.assertEqual(new_name, reload_object(nodegroup).name)
100+
101+ def test_refuses_name_change_if_dns_managed_and_nodes_in_use(self):
102+ nodegroup, node = make_unrenamable_nodegroup_with_node()
103+ data = self.make_form_data(nodegroup)
104+ data['name'] = factory.make_name('new-name')
105+ form = NodeGroupEdit(instance=nodegroup, data=data)
106+ self.assertFalse(form.is_valid())
107+
108+ def test_accepts_unchanged_name(self):
109+ nodegroup, node = make_unrenamable_nodegroup_with_node()
110+ original_name = nodegroup.name
111+ form = NodeGroupEdit(
112+ instance=nodegroup, data=self.make_form_data(nodegroup))
113+ self.assertTrue(form.is_valid())
114+ form.save()
115+ self.assertEqual(original_name, reload_object(nodegroup).name)
116+
117+ def test_accepts_omitted_name(self):
118+ nodegroup, node = make_unrenamable_nodegroup_with_node()
119+ original_name = nodegroup.name
120+ data = self.make_form_data(nodegroup)
121+ del data['name']
122+ form = NodeGroupEdit(instance=nodegroup, data=data)
123+ self.assertTrue(form.is_valid())
124+ form.save()
125+ self.assertEqual(original_name, reload_object(nodegroup).name)
126+
127+ def test_accepts_name_change_if_nodegroup_not_accepted(self):
128+ nodegroup, node = make_unrenamable_nodegroup_with_node()
129+ nodegroup.status = NODEGROUP_STATUS.PENDING
130+ data = self.make_form_data(nodegroup)
131+ data['name'] = factory.make_name('new-name')
132+ form = NodeGroupEdit(instance=nodegroup, data=data)
133+ self.assertTrue(form.is_valid())
134+
135+ def test_accepts_name_change_if_dns_managed_but_no_nodes_in_use(self):
136+ nodegroup, node = make_unrenamable_nodegroup_with_node()
137+ node.status = NODE_STATUS.READY
138+ node.save()
139+ data = self.make_form_data(nodegroup)
140+ data['name'] = factory.make_name('new-name')
141+ form = NodeGroupEdit(instance=nodegroup, data=data)
142+ self.assertTrue(form.is_valid())
143+ form.save()
144+ self.assertEqual(data['name'], reload_object(nodegroup).name)
145+
146+ def test_accepts_name_change_if_nodes_in_use_but_dns_not_managed(self):
147+ nodegroup, node = make_unrenamable_nodegroup_with_node()
148+ interface = nodegroup.get_managed_interface()
149+ interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP
150+ interface.save()
151+ data = self.make_form_data(nodegroup)
152+ data['name'] = factory.make_name('new-name')
153+ form = NodeGroupEdit(instance=nodegroup, data=data)
154+ self.assertTrue(form.is_valid())
155+ form.save()
156+ self.assertEqual(data['name'], reload_object(nodegroup).name)

Subscribers

People subscribed via source and target branches

to status/vote changes: