launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15513
[Merge] lp:~rvb/maas/api-update-cluster into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/api-update-cluster into lp:maas.
Commit message:
Add method to update a cluster using the API.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1172193 in MAAS: "The clusters' information cannot be updated via the API/CLI"
https://bugs.launchpad.net/maas/+bug/1172193
For more details, see:
https://code.launchpad.net/~rvb/maas/api-update-cluster/+merge/160641
This has been discussed with Julian. We need this (in trunk and 1.2) because we need to improve the migration story from the version of MAAS that used cobbler and this will allow us to simplify the migration script.
The testing is pretty minimal because the form itself is already tested in src/maasserver/tests/test_forms.py:TestNodeGroupEdit.
--
https://code.launchpad.net/~rvb/maas/api-update-cluster/+merge/160641
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/api-update-cluster into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2013-03-12 16:00:25 +0000
+++ src/maasserver/api.py 2013-04-24 13:35:30 +0000
@@ -67,6 +67,7 @@
"FilesHandler",
"get_oauth_token",
"MaasHandler",
+ "NodeGroupHandler",
"NodeGroupsHandler",
"NodeGroupInterfaceHandler",
"NodeGroupInterfacesHandler",
@@ -161,6 +162,7 @@
from maasserver.forms import (
get_node_create_form,
get_node_edit_form,
+ NodeGroupEdit,
NodeGroupInterfaceForm,
NodeGroupWithInterfacesForm,
TagForm,
@@ -1186,7 +1188,7 @@
Each NodeGroup has its own uuid.
"""
- create = update = delete = None
+ create = delete = None
fields = DISPLAYED_NODEGROUP_FIELDS
def read(self, request, uuid):
@@ -1201,6 +1203,27 @@
uuid = nodegroup.uuid
return ('nodegroup_handler', [uuid])
+ def update(self, request, uuid):
+ """Update a specific cluster.
+
+ :param name: The new DNS name for this cluster.
+ :type name: basestring
+ :param cluster_name: The new name for this cluster.
+ :type cluster_name: basestring
+ :param status: The new status for this cluster (see
+ vocabulary `NODEGROUP_STATUS`).
+ :type status: int
+ """
+ if not request.user.is_superuser:
+ raise PermissionDenied("That method is reserved to admin users.")
+ nodegroup = get_object_or_404(NodeGroup, uuid=uuid)
+ data = get_overrided_query_dict(model_to_dict(nodegroup), request.data)
+ form = NodeGroupEdit(instance=nodegroup, data=data)
+ if form.is_valid():
+ return form.save()
+ else:
+ raise ValidationError(form.errors)
+
@operation(idempotent=False)
def update_leases(self, request, uuid):
"""Submit latest state of DHCP leases within the cluster.
=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py 2013-03-07 14:41:14 +0000
+++ src/maasserver/testing/factory.py 2013-04-24 13:35:30 +0000
@@ -214,6 +214,25 @@
ng.save()
return ng
+ def make_unrenamable_nodegroup_with_node(self):
+ """Create a `NodeGroup` that can't be renamed, and `Node`.
+
+ Node groups can't be renamed while they are in an accepted state, have
+ DHCP and DNS management enabled, and have a node that is in allocated
+ state.
+
+ :return: tuple: (`NodeGroup`, `Node`).
+ """
+ name = self.make_name('original-name')
+ nodegroup = self.make_node_group(
+ name=name, status=NODEGROUP_STATUS.ACCEPTED)
+ interface = nodegroup.get_managed_interface()
+ interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS
+ interface.save()
+ node = self.make_node(
+ nodegroup=nodegroup, status=NODE_STATUS.ALLOCATED)
+ return nodegroup, node
+
def make_node_group_interface(self, nodegroup, ip=None,
router_ip=None, network=None,
subnet_mask=None, broadcast_ip=None,
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2013-03-12 15:25:43 +0000
+++ src/maasserver/tests/test_api.py 2013-04-24 13:35:30 +0000
@@ -72,6 +72,7 @@
NODE_STATUS,
NODE_STATUS_CHOICES_DICT,
NODEGROUP_STATUS,
+ NODEGROUP_STATUS_CHOICES,
NODEGROUPINTERFACE_MANAGEMENT,
)
from maasserver.exceptions import MAASAPIBadRequest
@@ -4348,6 +4349,51 @@
self.get_uri('nodegroups/%s/' % factory.make_name('nodegroup')))
self.assertEqual(httplib.NOT_FOUND, response.status_code)
+ def test_PUT_reserved_to_admin_users(self):
+ nodegroup = factory.make_node_group()
+ response = self.client.put(
+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
+ {'name': factory.make_name("new-name")})
+
+ self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
+ def test_PUT_updates_nodegroup(self):
+ # The api allows the updating of a NodeGroup.
+ nodegroup = factory.make_node_group()
+ self.become_admin()
+ new_name = factory.make_name("new-name")
+ new_cluster_name = factory.make_name("new-cluster-name")
+ new_status = factory.getRandomChoice(
+ NODEGROUP_STATUS_CHOICES, but_not=[nodegroup.status])
+ response = self.client.put(
+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
+ {
+ 'name': new_name,
+ 'cluster_name': new_cluster_name,
+ 'status': new_status,
+ })
+
+ self.assertEqual(httplib.OK, response.status_code, response.content)
+ nodegroup = reload_object(nodegroup)
+ self.assertEqual(
+ (new_name, new_cluster_name, new_status),
+ (nodegroup.name, nodegroup.cluster_name, nodegroup.status))
+
+ def test_PUT_updates_nodegroup_validates_data(self):
+ nodegroup, _ = factory.make_unrenamable_nodegroup_with_node()
+ self.become_admin()
+ new_name = factory.make_name("new-name")
+ response = self.client.put(
+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
+ {'name': new_name})
+
+ parsed_result = json.loads(response.content)
+
+ self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+ self.assertIn(
+ "Can't rename DNS zone",
+ parsed_result['name'][0])
+
def test_update_leases_processes_empty_leases_dict(self):
nodegroup = factory.make_node_group()
factory.make_dhcp_lease(nodegroup=nodegroup)
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py 2012-12-20 10:08:36 +0000
+++ src/maasserver/tests/test_forms.py 2013-04-24 13:35:30 +0000
@@ -842,25 +842,6 @@
])
-def make_unrenamable_nodegroup_with_node():
- """Create a `NodeGroup` that can't be renamed, and `Node`.
-
- Node groups can't be renamed while they are in an accepted state, have
- DHCP and DNS management enabled, and have a node that is in allocated
- state.
-
- :return: tuple: (`NodeGroup`, `Node`).
- """
- name = factory.make_name('original-name')
- nodegroup = factory.make_node_group(
- name=name, status=NODEGROUP_STATUS.ACCEPTED)
- interface = nodegroup.get_managed_interface()
- interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS
- interface.save()
- node = factory.make_node(nodegroup=nodegroup, status=NODE_STATUS.ALLOCATED)
- return nodegroup, node
-
-
class TestNodeGroupEdit(TestCase):
def make_form_data(self, nodegroup):
@@ -882,14 +863,14 @@
self.assertEqual(new_name, reload_object(nodegroup).name)
def test_refuses_name_change_if_dns_managed_and_nodes_in_use(self):
- nodegroup, node = make_unrenamable_nodegroup_with_node()
+ nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
data = self.make_form_data(nodegroup)
data['name'] = factory.make_name('new-name')
form = NodeGroupEdit(instance=nodegroup, data=data)
self.assertFalse(form.is_valid())
def test_accepts_unchanged_name(self):
- nodegroup, node = make_unrenamable_nodegroup_with_node()
+ nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
original_name = nodegroup.name
form = NodeGroupEdit(
instance=nodegroup, data=self.make_form_data(nodegroup))
@@ -898,7 +879,7 @@
self.assertEqual(original_name, reload_object(nodegroup).name)
def test_accepts_omitted_name(self):
- nodegroup, node = make_unrenamable_nodegroup_with_node()
+ nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
original_name = nodegroup.name
data = self.make_form_data(nodegroup)
del data['name']
@@ -908,7 +889,7 @@
self.assertEqual(original_name, reload_object(nodegroup).name)
def test_accepts_name_change_if_nodegroup_not_accepted(self):
- nodegroup, node = make_unrenamable_nodegroup_with_node()
+ nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
nodegroup.status = NODEGROUP_STATUS.PENDING
data = self.make_form_data(nodegroup)
data['name'] = factory.make_name('new-name')
@@ -916,7 +897,7 @@
self.assertTrue(form.is_valid())
def test_accepts_name_change_if_dns_managed_but_no_nodes_in_use(self):
- nodegroup, node = make_unrenamable_nodegroup_with_node()
+ nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
node.status = NODE_STATUS.READY
node.save()
data = self.make_form_data(nodegroup)
@@ -927,7 +908,7 @@
self.assertEqual(data['name'], reload_object(nodegroup).name)
def test_accepts_name_change_if_nodes_in_use_but_dns_not_managed(self):
- nodegroup, node = make_unrenamable_nodegroup_with_node()
+ nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
interface = nodegroup.get_managed_interface()
interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP
interface.save()
@@ -939,7 +920,7 @@
self.assertEqual(data['name'], reload_object(nodegroup).name)
def test_accepts_name_change_if_nodegroup_has_no_interface(self):
- nodegroup, node = make_unrenamable_nodegroup_with_node()
+ nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
NodeGroupInterface.objects.filter(nodegroup=nodegroup).delete()
data = self.make_form_data(nodegroup)
data['name'] = factory.make_name('new-name')
