Merge lp:~rvb/maas/stop-button into lp:~maas-committers/maas/trunk

Proposed by Raphaël Badin
Status: Merged
Approved by: Raphaël Badin
Approved revision: no longer in the source branch.
Merged at revision: 1489
Proposed branch: lp:~rvb/maas/stop-button
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 113 lines (+51/-0)
2 files modified
src/maasserver/node_action.py (+15/-0)
src/maasserver/tests/test_node_action.py (+36/-0)
To merge this branch: bzr merge lp:~rvb/maas/stop-button
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Approve
Review via email: mp+165054@code.launchpad.net

Commit message

Add stop node button.

Description of the change

Add a "stop node" button in the UI.

Drive-by fix: refactored the tests so that each button has it's own test class. It makes it more simple to spot how many tests we have for each type of button and copy the tests over when one creates a new button (and the tests that goes with it).

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Good stuff. Only one very mild suggestion: it would be more specific to tell the user that the node has been asked to "shut down," rather than to "stop."

That makes it more obvious that this is a "stop" at the system level, not at the service level. I'm guessing that it might be slightly puzzling if the request "stop this node" translates to "ask the node to stop" plus something else.

Jeroen

review: Approve
Revision history for this message
Raphaël Badin (rvb) wrote :

> Good stuff. Only one very mild suggestion: it would be more specific to tell
> the user that the node has been asked to "shut down," rather than to "stop."
>
> That makes it more obvious that this is a "stop" at the system level, not at
> the service level. I'm guessing that it might be slightly puzzling if the
> request "stop this node" translates to "ask the node to stop" plus something
> else.

good point, done.

Thanks for the review!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/node_action.py'
2--- src/maasserver/node_action.py 2012-05-08 16:22:48 +0000
3+++ src/maasserver/node_action.py 2013-05-22 15:31:25 +0000
4@@ -196,11 +196,26 @@
5 """)
6
7
8+class StopNode(NodeAction):
9+ """Stop and release a node."""
10+ display = "Stop node"
11+ actionable_statuses = (NODE_STATUS.ALLOCATED, )
12+ permission = NODE_PERMISSION.EDIT
13+
14+ def execute(self):
15+ self.node.release()
16+ return dedent("""\
17+ This node is no longer allocated to you.
18+ It has been asked to shut down.
19+ """)
20+
21+
22 ACTION_CLASSES = (
23 Delete,
24 AcceptAndCommission,
25 RetryCommissioning,
26 StartNode,
27+ StopNode,
28 )
29
30
31
32=== modified file 'src/maasserver/tests/test_node_action.py'
33--- src/maasserver/tests/test_node_action.py 2012-06-07 11:44:14 +0000
34+++ src/maasserver/tests/test_node_action.py 2013-05-22 15:31:25 +0000
35@@ -28,10 +28,12 @@
36 NodeAction,
37 RetryCommissioning,
38 StartNode,
39+ StopNode,
40 )
41 from maasserver.testing.factory import factory
42 from maasserver.testing.testcase import TestCase
43 from provisioningserver.enum import POWER_TYPE
44+from provisioningserver.power.poweraction import PowerAction
45
46
47 ALL_STATUSES = NODE_STATUS_CHOICES_DICT.keys()
48@@ -162,6 +164,9 @@
49 action.fake_inhibition = factory.getRandomString()
50 self.assertIsNone(action.inhibition)
51
52+
53+class TestDeleteNodeAction(TestCase):
54+
55 def test_Delete_inhibit_when_node_is_allocated(self):
56 node = factory.make_node(status=NODE_STATUS.ALLOCATED)
57 action = Delete(node, factory.make_admin())
58@@ -186,6 +191,9 @@
59 reverse('node-delete', args=[node.system_id]),
60 urlparse(unicode(e)).path)
61
62+
63+class TestAcceptAndCommissionNodeAction(TestCase):
64+
65 def test_AcceptAndCommission_starts_commissioning(self):
66 node = factory.make_node(
67 mac=True, status=NODE_STATUS.DECLARED,
68@@ -197,6 +205,9 @@
69 'provisioningserver.tasks.power_on',
70 self.celery.tasks[0]['task'].name)
71
72+
73+class TestRetryCommissioningNodeAction(TestCase):
74+
75 def test_RetryCommissioning_starts_commissioning(self):
76 node = factory.make_node(
77 mac=True, status=NODE_STATUS.FAILED_TESTS,
78@@ -208,6 +219,9 @@
79 'provisioningserver.tasks.power_on',
80 self.celery.tasks[0]['task'].name)
81
82+
83+class TestStartNodeNodeAction(TestCase):
84+
85 def test_StartNode_inhibit_allows_user_with_SSH_key(self):
86 user_with_key = factory.make_user()
87 factory.make_sshkey(user_with_key)
88@@ -232,3 +246,25 @@
89 self.assertEqual(
90 'provisioningserver.tasks.power_on',
91 self.celery.tasks[0]['task'].name)
92+
93+
94+class TestStopNodeNodeAction(TestCase):
95+
96+ def test_StopNode_stops_and_releases_node(self):
97+ self.patch(PowerAction, 'run_shell', lambda *args, **kwargs: ('', ''))
98+ user = factory.make_user()
99+ params = dict(
100+ power_address=factory.getRandomString(),
101+ power_user=factory.getRandomString(),
102+ power_pass=factory.getRandomString())
103+ node = factory.make_node(
104+ mac=True, status=NODE_STATUS.ALLOCATED,
105+ power_type=POWER_TYPE.IPMI,
106+ owner=user, power_parameters=params)
107+ StopNode(node, user).execute()
108+
109+ self.assertEqual(NODE_STATUS.READY, node.status)
110+ self.assertIsNone(node.owner)
111+ self.assertEqual(
112+ 'provisioningserver.tasks.power_off',
113+ self.celery.tasks[0]['task'].name)