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

[Raphaël Badin]

Add buttons to enable/disable curtin.

Since I extended the meaning of is_permitted(), I changed the error messages a bit. A better fix would have been to extend the logic of the action classes to include a is_enabled() or something but this would require much more work; besides, I think the logic we have in place is complicated enough already.

Please carefully review the wordings on the buttons. I'm happy with any suggestion but keep in mind that the "display" and "display_bulk" fields have to be small to fit in our design!

[Julian Edwards (julian-edwards)]

review: Approve

=== modified file 'src/maasserver/node_action.py'
--- src/maasserver/node_action.py	2013-10-07 09:12:40 +0000
+++ src/maasserver/node_action.py	2013-10-09 09:08:56 +0000
@@ -44,6 +44,7 @@
     Node,
     SSHKey,
     )
+from maasserver.utils import map_enum
 
 # All node statuses.
 ALL_STATUSES = set(NODE_STATUS_CHOICES_DICT.keys())
@@ -180,6 +181,42 @@
         return "Node commissioning started."
 
 
+class UseCurtin(NodeAction):
+    """Set this node to use curtin for installation."""
+    name = "usecurtin"
+    display = "Use the fast installer"
+    display_bulk = "Mark the selected nodes as using the fast installer"
+    actionable_statuses = map_enum(NODE_STATUS).values()
+    permission = NODE_PERMISSION.EDIT
+
+    def is_permitted(self):
+        permitted = super(UseCurtin, self).is_permitted()
+        return permitted and self.node.should_use_traditional_installer()
+
+    def execute(self, allow_redirect=True):
+        """See `NodeAction.execute`."""
+        self.node.use_fastpath_installer()
+        return "Node marked as using curtin for install."
+
+
+class UseDI(NodeAction):
+    """Set this node to use d-i for installation."""
+    name = "usedi"
+    display = "Use the default installer"
+    display_bulk = "Mark the selected nodes as using the default installer"
+    actionable_statuses = map_enum(NODE_STATUS).values()
+    permission = NODE_PERMISSION.EDIT
+
+    def is_permitted(self):
+        permitted = super(UseDI, self).is_permitted()
+        return permitted and self.node.should_use_fastpath_installer()
+
+    def execute(self, allow_redirect=True):
+        """See `NodeAction.execute`."""
+        self.node.use_traditional_installer()
+        return "Node marked as using the default installer."
+
+
 class StartNode(NodeAction):
     """Acquire and start a node."""
     name = "start"
@@ -236,6 +273,8 @@
     Commission,
     StartNode,
     StopNode,
+    UseCurtin,
+    UseDI,
     )
 
 

=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py	2013-10-08 07:58:09 +0000
+++ src/maasserver/tests/test_forms.py	2013-10-09 09:08:56 +0000
@@ -69,6 +69,7 @@
     Delete,
     StartNode,
     StopNode,
+    UseCurtin,
     )
 from maasserver.testing import reload_object
 from maasserver.testing.factory import factory
@@ -474,10 +475,11 @@
     def test_get_action_form_for_admin(self):
         admin = factory.make_admin()
         node = factory.make_node(status=NODE_STATUS.DECLARED)
+        node.use_traditional_installer()
         form = get_action_form(admin)(node)
 
         self.assertItemsEqual(
-            [Commission.name, Delete.name],
+            [Commission.name, Delete.name, UseCurtin.name],
             form.actions)
 
     def test_get_action_form_for_user(self):

=== modified file 'src/maasserver/tests/test_node_action.py'
--- src/maasserver/tests/test_node_action.py	2013-10-07 09:12:40 +0000
+++ src/maasserver/tests/test_node_action.py	2013-10-09 09:08:56 +0000
@@ -30,6 +30,8 @@
     NodeAction,
     StartNode,
     StopNode,
+    UseCurtin,
+    UseDI,
     )
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
@@ -261,3 +263,51 @@
         self.assertEqual(
             'provisioningserver.tasks.power_off',
             self.celery.tasks[0]['task'].name)
+
+
+class TestUseCurtinNodeAction(MAASServerTestCase):
+
+    def test_sets_tag(self):
+        user = factory.make_user()
+        node = factory.make_node(owner=user)
+        node.use_traditional_installer()
+        action = UseCurtin(node, user)
+        self.assertTrue(action.is_permitted())
+        action.execute()
+        self.assertTrue(node.should_use_fastpath_installer())
+
+    def test_requires_edit_permission(self):
+        user = factory.make_user()
+        node = factory.make_node()
+        node.use_traditional_installer()
+        self.assertFalse(UseCurtin(node, user).is_permitted())
+
+    def test_not_permitted_if_already_uses_curtin(self):
+        node = factory.make_node()
+        node.use_fastpath_installer()
+        user = factory.make_admin()
+        self.assertFalse(UseCurtin(node, user).is_permitted())
+
+
+class TestUseDINodeAction(MAASServerTestCase):
+
+    def test_sets_tag(self):
+        user = factory.make_user()
+        node = factory.make_node(owner=user)
+        node.use_fastpath_installer()
+        action = UseDI(node, user)
+        self.assertTrue(action.is_permitted())
+        action.execute()
+        self.assertTrue(node.should_use_traditional_installer())
+
+    def test_requires_edit_permission(self):
+        user = factory.make_user()
+        node = factory.make_node()
+        node.use_fastpath_installer()
+        self.assertFalse(UseDI(node, user).is_permitted())
+
+    def test_not_permitted_if_already_uses_di(self):
+        node = factory.make_node()
+        node.use_traditional_installer()
+        user = factory.make_admin()
+        self.assertFalse(UseDI(node, user).is_permitted())

=== modified file 'src/maasserver/tests/test_views_nodes.py'
--- src/maasserver/tests/test_views_nodes.py	2013-10-07 09:12:40 +0000
+++ src/maasserver/tests/test_views_nodes.py	2013-10-09 09:08:56 +0000
@@ -915,7 +915,7 @@
                 (Delete, 0, 1, 1),
                 (
                     'could not be performed on 1 node because its state',
-                    "could not be performed on 1 node because you don't",
+                    "could not be performed on 1 node because that action",
                 )
             ),
              (
@@ -930,14 +930,14 @@
                      % Delete.display_bulk,
                     'It could not be performed on 4 nodes because their '
                     'state does not allow that action.',
-                    "It could not be performed on 2 nodes because you "
-                    "don't have the required permissions.",
+                    "It could not be performed on 2 nodes because that "
+                    "action is not permitted on these nodes.",
                 ),
             ),
             (
                 (Delete, 0, 0, 3),
                 ('The action "%s" could not be performed on 3 nodes '
-                 "because you don't have the required permissions."
+                 "because that action is not permitted on these nodes."
                  % Delete.display_bulk,),
             ),
         ]

=== modified file 'src/maasserver/views/nodes.py'
--- src/maasserver/views/nodes.py	2013-10-07 09:12:40 +0000
+++ src/maasserver/views/nodes.py	2013-10-09 09:08:56 +0000
@@ -131,10 +131,10 @@
          'state does not allow that action.'),
     ]
     not_permitted_templates = [
-        ('%s could not be performed on %d node because you '
-         "don't have the required permissions."),
-        ('%s could not be performed on %d nodes because you '
-         "don't have the required permissions."),
+        ('%s could not be performed on %d node because that '
+         "action is not permitted on that node."),
+        ('%s could not be performed on %d nodes because that '
+         "action is not permitted on these nodes."),
     ]
     number_message = [
         (done, done_templates),