launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/start-node into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/start-node/+merge/92993

Add a “start” action on Node in the MaaS REST API, and underlying forwarding to model, provisioning API, and Cobbler API.  This is completely analogous to the “stop” action; everything you see here from design to tests to implementation & documentation mirrors an existing equivalent for stopping a node.  The start node had to establish some additional infrastructure but this branch re-uses it unchanged.

As with stopping a node, there is no mechanism yet for awaiting and checking for success.  If we need that, we will have to come up with something that will make things a bit more complicated.  But it may also be acceptable to have the user notice that nothing happened, and investigate.
-- 
https://code.launchpad.net/~jtv/maas/start-node/+merge/92993
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/start-node into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-02-14 12:50:15 +0000
+++ src/maasserver/api.py	2012-02-14 14:34:19 +0000
@@ -243,6 +243,15 @@
                 "You are not allowed to shut down this node.")
         return nodes[0]
 
+    @api_exported('start', 'POST')
+    def start(self, request, system_id):
+        """Power up a node."""
+        nodes = Node.objects.start_nodes([system_id], request.user)
+        if len(nodes) == 0:
+            raise PermissionDenied(
+                "You are not allowed to start up this node.")
+        return nodes[0]
+
 
 @api_operations
 class NodesHandler(BaseHandler):

=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py	2012-02-14 12:54:01 +0000
+++ src/maasserver/models.py	2012-02-14 14:34:19 +0000
@@ -250,6 +250,25 @@
         self.provisioning_proxy.stop_nodes([node.system_id for node in nodes])
         return nodes
 
+    def start_nodes(self, ids, by_user):
+        """Request on given user's behalf that the given nodes be started up.
+
+        Power-on is only requested for nodes that the user has ownership
+        privileges for; any other nodes in the request are ignored.
+
+        :param ids: Sequence of `system_id` values for nodes to start.
+        :type ids: QuerySet
+        :param by_user: Requesting user.
+        :type by_user: User_
+        :return: Those Nodes for which power-on was actually requested.
+        :rtype: list
+        """
+        self._set_provisioning_proxy()
+        nodes = self.get_editable_nodes(by_user, ids=ids)
+        self.provisioning_proxy.start_nodes(
+            [node.system_id for node in nodes])
+        return nodes
+
 
 class Node(CommonInfo):
     """A `Node` represents a physical machine used by the MaaS Server.

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-02-14 13:55:46 +0000
+++ src/maasserver/tests/test_api.py	2012-02-14 14:34:19 +0000
@@ -178,6 +178,24 @@
         response = self.client.post(self.get_uri(node), {'op': 'stop'})
         self.assertEqual(httplib.OK, response.status_code)
 
+    def test_POST_start_checks_permission(self):
+        node = factory.make_node()
+        response = self.client.post(self.get_uri(node), {'op': 'start'})
+        self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
+    def test_POST_start_returns_node(self):
+        node = factory.make_node(owner=self.logged_in_user)
+        response = self.client.post(self.get_uri(node), {'op': 'start'})
+        self.assertEqual(httplib.OK, response.status_code)
+        self.assertEqual(
+            node.system_id, json.loads(response.content)['system_id'])
+
+    def test_POST_start_may_be_repeated(self):
+        node = factory.make_node(owner=self.logged_in_user)
+        self.client.post(self.get_uri(node), {'op': 'start'})
+        response = self.client.post(self.get_uri(node), {'op': 'start'})
+        self.assertEqual(httplib.OK, response.status_code)
+
     def test_PUT_updates_node(self):
         # The api allows to update a Node.
         node = factory.make_node(hostname='diane')

=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py	2012-02-14 13:31:04 +0000
+++ src/maasserver/tests/test_models.py	2012-02-14 14:34:19 +0000
@@ -224,6 +224,24 @@
             [stoppable_node],
             Node.objects.stop_nodes(ids, stoppable_node.owner))
 
+    def test_start_nodes_starts_nodes(self):
+        user = factory.make_user()
+        node = self.make_node(user)
+        output = Node.objects.start_nodes([node.system_id], user)
+
+        self.assertItemsEqual([node], output)
+        self.assertEqual(
+            'start',
+            Node.objects.provisioning_proxy.power_status[node.system_id])
+
+    def test_start_nodes_ignores_uneditable_nodes(self):
+        nodes = [self.make_node(factory.make_user()) for counter in range(3)]
+        ids = [node.system_id for node in nodes]
+        startable_node = nodes[0]
+        self.assertItemsEqual(
+            [startable_node],
+            Node.objects.start_nodes(ids, startable_node.owner))
+
 
 class MACAddressTest(TestCase):
 

=== modified file 'src/provisioningserver/tests/test_api.py'
--- src/provisioningserver/tests/test_api.py	2012-02-14 13:51:36 +0000
+++ src/provisioningserver/tests/test_api.py	2012-02-14 14:34:19 +0000
@@ -432,6 +432,16 @@
         # The test is that we get here without error.
         pass
 
+    @inlineCallbacks
+    def test_start_nodes(self):
+        papi = self.get_provisioning_api()
+        distro = yield papi.add_distro("distro", "initrd", "kernel")
+        profile = yield papi.add_profile("profile", distro)
+        yield papi.add_node("alice", profile)
+        yield papi.start_nodes(["alice"])
+        # The test is that we get here without error.
+        pass
+
 
 class TestFakeProvisioningAPI(TestProvisioningAPI):
     """Test :class:`FakeAsynchronousProvisioningAPI`."""