Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-987629 into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #987629 in MAAS: "No way to move out of FAILED_TESTS state"
https://bugs.launchpad.net/maas/+bug/987629
For more details, see:
https://code.launchpad.net/~jtv/maas/bug-987629/+merge/103416
There was nobody around to pre-imp this with, so I just went ahead and did the
absolute minimum that bug 987629 required: if a node is in Failed Tests state,
allow an admin to retry the commissioning process (e.g. after swapping out bad
hardware or remembering to plug the machine into the network).
In order of diff:
In the sampledata, node “moon” was in state Failed Tests but had an owner.
This is unrealistic. A node in this state does not have an owner. (And thus,
it can be deleted which makes a lot of sense!)
In NODE_ACTIONS, I added the retry button (as well as a missing comma at the
end of the last entry in the preceding dict). All it does is send the node
straight back into commissioning. There's no need to send it back into
Declared state first, which would only have added unnecessary code paths.
Why is there no need? As you see in NODE_TRANSITIONS, all I had to do was
allow the node transition from Failed Tests back to Commissioning. In fact we
didn't have any transitions from that state defined, so I defined what seemed
sensible — a superset of what we actually use.
I did *not* add a “Retire” button for failed nodes, although I did allow the
transition. The reason is that we don't seem to have that option anywhere else
in the UI, and we might as well add that, wherever appropriate, in one fell
swoop. Including it in this branch would water down its purpose, and doing it
only for failed nodes would give a false impression that the job was done.
Oh, and finally of course there's a test. There's no negative test to prove
that non-admins can't do this; that's purely data-driven, so essentially a
configuration matter. There's little point in a test duplicating that
information.
I toyed with making the Retry button require only View permission on the node,
since after all at this stage there's no longer any objection to wiping its
disks. The only scenario I could think of where it mattered was if an admin
owned a node that failed commissioning, and then lost admin privileges but
remained a valid user of the MAAS; they could then retain responsibility for
dealing with the broken node. —And then I realized that a Failed node does not
have an owner. Never mind; Admin is nice and simple.
Jeroen
--
https://code.launchpad.net/~jtv/maas/bug-987629/+merge/103416
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/maas/bug-987629 into lp:maas.
=== modified file 'src/maasserver/fixtures/dev_fixture.yaml'
--- src/maasserver/fixtures/dev_fixture.yaml 2012-03-12 07:25:31 +0000
+++ src/maasserver/fixtures/dev_fixture.yaml 2012-04-25 07:21:18 +0000
@@ -7,7 +7,7 @@
pk: 15
- fields: {
created: 2012-01-24, hostname: moon, architecture: i386,
- owner: 106, status: 2,
+ owner: null, status: 2,
system_id: node-29d7ad70-4671-11e1-93b8-00225f89f211,
updated: !!timestamp '2012-01-24 10:52:44.507777'}
model: maasserver.node
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2012-04-25 02:23:38 +0000
+++ src/maasserver/forms.py 2012-04-25 07:21:18 +0000
@@ -287,7 +287,15 @@
'display': "Accept & commission",
'permission': NODE_PERMISSION.ADMIN,
'execute': start_commissioning_node,
- 'message': "Node commissioning started."
+ 'message': "Node commissioning started.",
+ },
+ ],
+ NODE_STATUS.FAILED_TESTS: [
+ {
+ 'display': "Retry commissioning",
+ 'permission': NODE_PERMISSION.ADMIN,
+ 'execute': start_commissioning_node,
+ 'message': "Started a new attempt to commission this node.",
},
],
NODE_STATUS.READY: [
=== modified file 'src/maasserver/models/__init__.py'
--- src/maasserver/models/__init__.py 2012-04-24 16:06:16 +0000
+++ src/maasserver/models/__init__.py 2012-04-25 07:21:18 +0000
@@ -159,6 +159,11 @@
NODE_STATUS.RETIRED,
NODE_STATUS.MISSING,
],
+ NODE_STATUS.FAILED_TESTS: [
+ NODE_STATUS.COMMISSIONING,
+ NODE_STATUS.MISSING,
+ NODE_STATUS.RETIRED,
+ ],
NODE_STATUS.READY: [
NODE_STATUS.ALLOCATED,
NODE_STATUS.RESERVED,
=== modified file 'src/maasserver/tests/test_views_nodes.py'
--- src/maasserver/tests/test_views_nodes.py 2012-04-25 02:23:38 +0000
+++ src/maasserver/tests/test_views_nodes.py 2012-04-25 07:21:18 +0000
@@ -265,6 +265,17 @@
self.assertEqual(
NODE_STATUS.COMMISSIONING, reload_object(node).status)
+ def test_view_node_POST_admin_can_retry_failed_commissioning(self):
+ self.become_admin()
+ node = factory.make_node(status=NODE_STATUS.FAILED_TESTS)
+ node_link = reverse('node-view', args=[node.system_id])
+ response = self.client.post(
+ node_link,
+ data={NodeActionForm.input_name: "Retry commissioning"})
+ self.assertEqual(httplib.FOUND, response.status_code)
+ self.assertEqual(
+ NODE_STATUS.COMMISSIONING, reload_object(node).status)
+
def perform_action_and_get_node_page(self, node, action_name):
"""POST to perform a node action, then load the resulting page."""
node_link = reverse('node-view', args=[node.system_id])
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
