← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~rvb/maas/preseed-ui into lp:maas

 

Raphaël Badin has proposed merging lp:~rvb/maas/preseed-ui into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/preseed-ui/+merge/111557

This branch adds the UI to display a node's preseed and the enlistment preseed.

Here are the screenshots of the new UI:

1. Node's commissioning preseed:
http://people.canonical.com/~rvb/node_com_preseed.png

2. Node's preseed (non-commissioning)
http://people.canonical.com/~rvb/node_preseed.png

3. Enlist preseed
http://people.canonical.com/~rvb/enlist_preseed.png

4. Node page (with the link to the Node's preseed)
http://people.canonical.com/~rvb/node_page.png

5. Node list (with the link to the enlistment preseed)
http://people.canonical.com/~rvb/node_list.png

I confess I'm not really completely happy about that last one.  The enlist preseed is not linked to any particular node so it makes sense to have the link to it displayed on the node list page… but I couldn't find any obvious place on that page where it would fit perfectly.  After a few tests, I finally settled on putting it under the "Add node" button but I'm happy to move it somewhere else if someone has a better idea.  Note that we've got a mockup for that page but the mockup includes all the things that we're planning to do in the future (mass actions etc.) so the mockup looks very different from what we currently have and I could not really use it for inspiration.

= Pre-imp =

This change was briefly discussed with Gavin but I had no precise mockups at the time so, like I said earlier, I'm happy make changes to the UI if a reviewer has a better idea on how things should look like.

= Notes =

You'll note something weird in the content ("!!python/unicode" strings) of the commissioning preseed (node_com_preseed.png).  That comes from the fact that we use yaml.dump in compose_commissioning_preseed (src/maasserver/provisioning.py).  This was already in place long before this branch but I believe this is all right because that is  parsed by cloud init which uses python's yaml.load to load that bit.  Still, it's probably more safe to use yaml.safe_dump (which produces only standard YAML tags) everywhere.  I'll fix that in a follow-up branch.

>>> import yaml
>>> yaml.dump({u'test' : u'test'})
"{!!python/unicode 'test': !!python/unicode 'test'}\n"
>>> yaml.safe_dump({u'test' : u'test'})
'{test: test}\n'

Drive-by fix: fix the comment in contrib/preseeds_v2/preseed_master.
-- 
https://code.launchpad.net/~rvb/maas/preseed-ui/+merge/111557
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/preseed-ui into lp:maas.
=== modified file 'contrib/preseeds_v2/preseed_master'
--- contrib/preseeds_v2/preseed_master	2012-06-19 16:01:07 +0000
+++ contrib/preseeds_v2/preseed_master	2012-06-22 07:53:20 +0000
@@ -1,7 +1,7 @@
 # MAAS - Ubuntu Server Installation
 # * Minimal install
 # * Cloud-init for bare-metal
-# * maas_preseed snippet is expanded to provide cloud-init preseed data
+# * Cloud-init preseed data
 
 # Locale
 d-i     debian-installer/locale string en_US.UTF-8

=== added file 'src/maasserver/templates/maasserver/enlist_preseed.html'
--- src/maasserver/templates/maasserver/enlist_preseed.html	1970-01-01 00:00:00 +0000
+++ src/maasserver/templates/maasserver/enlist_preseed.html	2012-06-22 07:53:20 +0000
@@ -0,0 +1,11 @@
+{% extends "maasserver/base.html" %}
+
+{% block nav-active-settings %}active{% endblock %}
+{% block title %}Enlistment preseed{% endblock %}
+{% block page-title %}Enlistment preseed{% endblock %}
+
+{% block content %}
+  <pre>
+{{ preseed }}
+  </pre>
+{% endblock %}

=== modified file 'src/maasserver/templates/maasserver/node_list.html'
--- src/maasserver/templates/maasserver/node_list.html	2012-04-16 09:25:37 +0000
+++ src/maasserver/templates/maasserver/node_list.html	2012-06-22 07:53:20 +0000
@@ -61,5 +61,8 @@
       </table>
     {% endif%}
     <a id="addnode" href="#" class="button right space-top">+ Add node</a>
+    <div class="clear"></div>
+    <a class="right space-top" href="{% url "enlist-preseed-view" %}">View enlistment preseed</a>
+    <div class="clear"></div>
   </div>
 {% endblock %}

=== added file 'src/maasserver/templates/maasserver/node_preseed.html'
--- src/maasserver/templates/maasserver/node_preseed.html	1970-01-01 00:00:00 +0000
+++ src/maasserver/templates/maasserver/node_preseed.html	2012-06-22 07:53:20 +0000
@@ -0,0 +1,23 @@
+{% extends "maasserver/base.html" %}
+
+{% block nav-active-settings %}active{% endblock %}
+{% block title %}Preseed for node: {{ node.hostname }}{% endblock %}
+{% block page-title %}Preseed for node: {{ node.hostname }}{% endblock %}
+{% block layout-modifiers %}sidebar{% endblock %}
+
+{% block sidebar %}
+  <h4>Node details</h4>
+  <a href="{% url 'node-view' node.system_id %}" class="button secondary">
+    View node
+  </a>
+{% endblock %}
+
+{% block content %}
+  {% if is_commissioning %}
+  The node {{ node.hostname }} is comissioning.  This is the comissioning
+  preseed.
+  {% endif %}
+  <pre>
+{{ preseed }}
+  </pre>
+{% endblock %}

=== modified file 'src/maasserver/templates/maasserver/node_view.html'
--- src/maasserver/templates/maasserver/node_view.html	2012-05-08 16:22:18 +0000
+++ src/maasserver/templates/maasserver/node_view.html	2012-06-22 07:53:20 +0000
@@ -6,12 +6,15 @@
 {% block layout-modifiers %}sidebar{% endblock %}
 
 {% block sidebar %}
-  {% if can_edit %}
-    <h4>Node details</h4>
-    <a href="{% url 'node-edit' node.system_id %}" class="button secondary">
-      Edit node
+  <h4>Node details</h4>
+    {% if can_edit %}
+      <a href="{% url 'node-edit' node.system_id %}" class="button secondary">
+        Edit node
+      </a>
+    {% endif %}
+    <a href="{% url 'node-preseed-view' node.system_id %}" class="button secondary">
+      View preseed
     </a>
-  {% endif %}
   {% if form.action_buttons %}
     <h4>Actions</h4>
     <form id="node_actions" method="post" action=".">

=== modified file 'src/maasserver/tests/test_views_nodes.py'
--- src/maasserver/tests/test_views_nodes.py	2012-06-15 07:08:42 +0000
+++ src/maasserver/tests/test_views_nodes.py	2012-06-22 07:53:20 +0000
@@ -31,6 +31,10 @@
     Node,
     )
 from maasserver.node_action import StartNode
+from maasserver.preseed import (
+    get_enlist_preseed,
+    get_preseed,
+    )
 from maasserver.testing import (
     extract_redirect,
     get_content_links,
@@ -65,6 +69,11 @@
         node_link = reverse('node-view', args=[node.system_id])
         self.assertIn(node_link, get_content_links(response))
 
+    def test_node_list_contains_link_to_enlist_preseed_view(self):
+        response = self.client.get(reverse('node-list'))
+        enlist_preseed_link = reverse('enlist-preseed-view')
+        self.assertIn(enlist_preseed_link, get_content_links(response))
+
     def test_node_list_displays_sorted_list_of_nodes(self):
         # Nodes are sorted on the node list page, newest first.
         nodes = [factory.make_node() for i in range(3)]
@@ -84,7 +93,7 @@
         self.assertEqual(
             node_links,
             [link for link in get_content_links(response)
-                if link.startswith('/nodes')])
+                if link.startswith('/nodes/node')])
 
     def test_view_node_displays_node_info(self):
         # The node page features the basic information about the node.
@@ -106,6 +115,13 @@
         content_text = doc.cssselect('#content')[0].text_content()
         self.assertNotIn('Owner', content_text)
 
+    def test_view_node_displays_link_to_view_pressed(self):
+        node = factory.make_node(owner=self.logged_in_user)
+        node_link = reverse('node-view', args=[node.system_id])
+        response = self.client.get(node_link)
+        node_preseed_link = reverse('node-preseed-view', args=[node.system_id])
+        self.assertIn(node_preseed_link, get_content_links(response))
+
     def test_view_node_displays_link_to_edit_if_user_owns_node(self):
         node = factory.make_node(owner=self.logged_in_user)
         node_link = reverse('node-view', args=[node.system_id])
@@ -323,6 +339,39 @@
             '\n'.join(msg.message for msg in response.context['messages']))
 
 
+class NodePreseedViewTest(LoggedInTestCase):
+
+    def test_preseedview_node_displays_pressed_data(self):
+        node = factory.make_node(owner=self.logged_in_user)
+        node_preseed_link = reverse('node-preseed-view', args=[node.system_id])
+        response = self.client.get(node_preseed_link)
+        self.assertIn(get_preseed(node), response.content)
+
+    def test_preseedview_node_displays_message_if_commissioning(self):
+        node = factory.make_node(
+            owner=self.logged_in_user, status=NODE_STATUS.COMMISSIONING,
+            set_hostname=True)
+        node_preseed_link = reverse('node-preseed-view', args=[node.system_id])
+        response = self.client.get(node_preseed_link)
+        self.assertThat(
+            response.content,
+            ContainsAll(
+                [get_preseed(node),
+                 "The node %s is comissioning." % node.hostname]))
+
+    def test_preseedview_node_displays_link_to_view_node(self):
+        node = factory.make_node(owner=self.logged_in_user)
+        node_preseed_link = reverse('node-preseed-view', args=[node.system_id])
+        response = self.client.get(node_preseed_link)
+        node_link = reverse('node-view', args=[node.system_id])
+        self.assertIn(node_link, get_content_links(response))
+
+    def test_enlist_preseed_displays_enlist_preseed(self):
+        enlist_preseed_link = reverse('enlist-preseed-view')
+        response = self.client.get(enlist_preseed_link)
+        self.assertIn(get_enlist_preseed(), response.content)
+
+
 class NodeDeleteMacTest(LoggedInTestCase):
 
     def test_node_delete_not_found_if_node_does_not_exist(self):

=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py	2012-04-26 10:35:45 +0000
+++ src/maasserver/urls.py	2012-06-22 07:53:20 +0000
@@ -33,11 +33,13 @@
     )
 from maasserver.views.combo import combo_view
 from maasserver.views.nodes import (
+    enlist_preseed_view,
     MacAdd,
     MacDelete,
     NodeDelete,
     NodeEdit,
     NodeListView,
+    NodePreseedView,
     NodeView,
     )
 from maasserver.views.prefs import (
@@ -99,10 +101,15 @@
         NodeListView.as_view(template_name="maasserver/index.html"),
         name='index'),
     url(r'^nodes/$', NodeListView.as_view(model=Node), name='node-list'),
+    url(r'^nodes/enlist-preseed/$', enlist_preseed_view,
+        name='enlist-preseed-view'),
     url(
         r'^nodes/(?P<system_id>[\w\-]+)/view/$', NodeView.as_view(),
         name='node-view'),
     url(
+        r'^nodes/(?P<system_id>[\w\-]+)/preseedview/$',
+        NodePreseedView.as_view(), name='node-preseed-view'),
+     url(
         r'^nodes/(?P<system_id>[\w\-]+)/edit/$', NodeEdit.as_view(),
         name='node-edit'),
     url(

=== modified file 'src/maasserver/views/nodes.py'
--- src/maasserver/views/nodes.py	2012-05-28 11:58:22 +0000
+++ src/maasserver/views/nodes.py	2012-06-22 07:53:20 +0000
@@ -11,9 +11,11 @@
 
 __metaclass__ = type
 __all__ = [
+    'enlist_preseed_view',
     'MacAdd',
     'MacDelete',
     'NodeListView',
+    'NodePreseedView',
     'NodeView',
     'NodeEdit',
     ]
@@ -24,10 +26,15 @@
 from django.contrib import messages
 from django.core.exceptions import PermissionDenied
 from django.core.urlresolvers import reverse
-from django.shortcuts import get_object_or_404
+from django.shortcuts import (
+    get_object_or_404,
+    render_to_response,
+    )
+from django.template import RequestContext
 from django.utils.safestring import mark_safe
 from django.views.generic import (
     CreateView,
+    DetailView,
     ListView,
     UpdateView,
     )
@@ -49,6 +56,10 @@
     MACAddress,
     Node,
     )
+from maasserver.preseed import (
+    get_enlist_preseed,
+    get_preseed,
+    )
 from maasserver.views import HelpfulDeleteView
 
 
@@ -83,6 +94,43 @@
         return context
 
 
+def enlist_preseed_view(request):
+    """View method to display the enlistment preseed."""
+    return render_to_response(
+        'maasserver/enlist_preseed.html',
+        {'preseed': mark_safe(get_enlist_preseed())},
+        context_instance=RequestContext(request))
+
+
+class NodeViewMixin:
+    """Mixin class used to fetch a node by system_id if the logged-in user
+    has the required permission.
+    """
+
+    context_object_name = 'node'
+
+    def get_object(self):
+        system_id = self.kwargs.get('system_id', None)
+        node = Node.objects.get_node_or_404(
+            system_id=system_id, user=self.request.user,
+            perm=NODE_PERMISSION.VIEW)
+        return node
+
+
+class NodePreseedView(NodeViewMixin, DetailView):
+    """View class to display a node's preseed."""
+
+    template_name = 'maasserver/node_preseed.html'
+
+    def get_context_data(self, **kwargs):
+        context = super(NodePreseedView, self).get_context_data(**kwargs)
+        node = self.get_object()
+        context['preseed'] = mark_safe(get_preseed(node))
+        context['is_commissioning'] = (
+            node.status == NODE_STATUS.COMMISSIONING)
+        return context
+
+
 # Info message displayed on the node page for COMMISSIONING
 # or READY nodes.
 NODE_BOOT_INFO = mark_safe("""
@@ -93,19 +141,13 @@
 """)
 
 
-class NodeView(UpdateView):
+class NodeView(NodeViewMixin, UpdateView):
+    """View class to display a node's information and buttons for the actions
+    which can be performed on this node.
+    """
 
     template_name = 'maasserver/node_view.html'
 
-    context_object_name = 'node'
-
-    def get_object(self):
-        system_id = self.kwargs.get('system_id', None)
-        node = Node.objects.get_node_or_404(
-            system_id=system_id, user=self.request.user,
-            perm=NODE_PERMISSION.VIEW)
-        return node
-
     def get_form_class(self):
         return get_action_form(self.request.user, self.request)
 


Follow ups