Merge lp:~blake-rouse/maas/node-list-columns into lp:~maas-committers/maas/trunk

[Blake Rouse]

New design of the nodes listing page.

New design of the nodes listing page.

[Gavin Panella (allenap)]

Looks good, but some attention is needed before landing.

review: Approve

[Blake Rouse (blake-rouse)]

Added comment, fixing others.

[MAAS Lander (maas-lander)]

The attempt to merge lp:~blake-rouse/maas/node-list-columns into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Hit http://security.ubuntu.com trusty-security Release
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release
Hit http://security.ubuntu.com trusty-security/main Sources
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Ign http://security.ubuntu.com trusty-security/main Translation-en_US
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Ign http://security.ubuntu.com trusty-security/universe Translation-en_US
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en_US
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make postgresql python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-netaddr python-netifaces python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-twisted py...

=== modified file 'src/maasserver/models/macaddress.py'
--- src/maasserver/models/macaddress.py	2014-07-02 10:41:48 +0000
+++ src/maasserver/models/macaddress.py	2014-07-31 14:27:53 +0000
@@ -69,6 +69,7 @@
     class Meta(DefaultMeta):
         verbose_name = "MAC address"
         verbose_name_plural = "MAC addresses"
+        ordering = ('created', )
 
     def __unicode__(self):
         address = self.mac_address

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2014-07-31 13:43:31 +0000
+++ src/maasserver/models/node.py	2014-07-31 14:27:53 +0000
@@ -837,20 +837,20 @@
         self.clean_status()
 
     def display_status(self):
-        """Return status text as displayed to the user.
-
-        The UI representation is taken from NODE_STATUS_CHOICES_DICT and may
-        interpolate the variable "owner" to reflect the username of the node's
-        current owner, if any.
-        """
-        status_text = NODE_STATUS_CHOICES_DICT[self.status]
-        if self.status == NODE_STATUS.ALLOCATED:
-            # The User is represented as its username in interpolation.
-            # Don't just say self.owner.username here, or there will be
-            # trouble with unowned nodes!
-            return "%s to %s" % (status_text, self.owner)
-        else:
-            return status_text
+        """Return status text as displayed to the user."""
+        return NODE_STATUS_CHOICES_DICT[self.status]
+
+    def display_memory(self):
+        """Return memory in GB."""
+        if self.memory < 1024:
+            return '%.1f' % (self.memory / 1024.0)
+        return '%d' % (self.memory / 1024)
+
+    def display_storage(self):
+        """Return storage in GB."""
+        if self.storage < 1024:
+            return '%.1f' % (self.storage / 1024.0)
+        return '%d' % (self.storage / 1024)
 
     def add_mac_address(self, mac_address):
         """Add a new MAC address to this `Node`.

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2014-07-31 11:39:10 +0000
+++ src/maasserver/models/tests/test_node.py	2014-07-31 14:27:53 +0000
@@ -243,12 +243,21 @@
             NODE_STATUS_CHOICES_DICT[node.status],
             node.display_status())
 
-    def test_display_status_for_allocated_node_shows_owner(self):
-        node = factory.make_node(
-            owner=factory.make_user(), status=NODE_STATUS.ALLOCATED)
-        self.assertEqual(
-            "Allocated to %s" % node.owner.username,
-            node.display_status())
+    def test_display_memory_returns_decimal_less_than_1024(self):
+        node = factory.make_node(memory=512)
+        self.assertEqual('0.5', node.display_memory())
+
+    def test_display_memory_returns_value_divided_by_1024(self):
+        node = factory.make_node(memory=2048)
+        self.assertEqual('2', node.display_memory())
+
+    def test_display_storage_returns_decimal_less_than_1024(self):
+        node = factory.make_node(storage=512)
+        self.assertEqual('0.5', node.display_storage())
+
+    def test_display_storage_returns_value_divided_by_1024(self):
+        node = factory.make_node(storage=2048)
+        self.assertEqual('2', node.display_storage())
 
     def test_add_node_with_token(self):
         user = factory.make_user()

=== modified file 'src/maasserver/templates/maasserver/nodes_listing.html'
--- src/maasserver/templates/maasserver/nodes_listing.html	2014-01-07 09:49:25 +0000
+++ src/maasserver/templates/maasserver/nodes_listing.html	2014-07-31 14:27:53 +0000
@@ -10,23 +10,48 @@
              class="{{ sort_classes.hostname }}">
         <acronym title="Fully Qualified Domain Name">FQDN</acronym>
       </a></th>
-      <th>
-        <acronym
-           title="Media Access Control addresses">MAC</acronym>
-      </th>
+      <th></th>
       <th>
         <a href="{{ sort_links.status }}"
 	       class="{{ sort_classes.status }}">Status</a>
       </th>
       <th>
+        <a href="{{ sort_links.owner }}"
+         class="{{ sort_classes.owner }}">Owner</a>
+      </th>
+      <th>
+        <a href="{{ sort_links.cpu_count }}"
+         class="{{ sort_classes.cpu_count }}">Cores</a>
+      </th>
+      <th>
+        <a href="{{ sort_links.memory }}"
+         class="{{ sort_classes.memory }}">RAM (GB)</a>
+      </th>
+      <th>
+        <a href="{{ sort_links.storage }}"
+         class="{{ sort_classes.storage }}">Disk (GB)</a>
+      </th>
+      <th>
+        <a href="{{ sort_links.primary_mac }}"
+         class="{{ sort_classes.primary_mac }}">
+           <acronym
+            title="Media Access Control addresses">MAC</acronym>
+        </a>
+      </th>
+      <th>
         <a href="{{ sort_links.zone }}"
            class="{{ sort_classes.zone }}">Zone</a>
       </th>
       {% else %}
         <th><acronym title="Fully Qualified Domain Name">FQDN</acronym></th>
+        <th></th>
+        <th>Status</th>
+        <th>Owner</th>
+        <th>Cores</th>
+        <th>RAM (GB)</th>
+        <th>Disk (GB)</th>
         <th><acronym
           title="Media Access Control addresses">MAC</acronym></th>
-        <th>Status</th>
         <th>Zone</th>
       {% endif %}
       </tr>
@@ -43,14 +68,23 @@
           <a href="{% url 'node-view' node.system_id %}">
           {{ node.fqdn }}
           </a>
-          {% if node.power_type == '' %} <img src="{{ STATIC_URL }}img/warning.png" title="No power type defined"/> {% endif %}
         </td>
+        <td>{% if node.power_type == '' %} <img src="{{ STATIC_URL }}img/warning.png" title="No power type defined"/> {% endif %}</td>
+        <td>{{ node.display_status }}</td>
+        <th>{{ node.owner|default_if_none:"" }}</th>
+        <th>{{ node.cpu_count }}</th>
+        <th>{{ node.display_memory }}</th>
+        <th>{{ node.display_storage }}</th>
         <td>
-          {% for macaddress in node.macaddress_set.all reversed %}
-            {{ macaddress }}{% if not forloop.last %},{% endif %}
-          {% endfor %}
+          <span title="{{node.primary_mac_vendor}}">{{node.primary_mac}}</span>
+          {% if node.extra_macs %}
+          <span title="
+            {% for mac in node.extra_macs %}
+              {{mac}}
+            {% endfor %}
+          ">(+{{ node.extra_macs|length }})</span>
+          {% endif %}
         </td>
-        <td>{{ node.display_status }}</td>
         <td class="zone-column">
           <a href="{% url 'zone-view' node.zone.name %}">{{ node.zone }}</a>
         </td>

=== modified file 'src/maasserver/views/nodes.py'
--- src/maasserver/views/nodes.py	2014-06-09 19:58:06 +0000
+++ src/maasserver/views/nodes.py	2014-07-31 14:27:53 +0000
@@ -25,6 +25,11 @@
     ]
 
 from cgi import escape
+from operator import attrgetter
+from netaddr import (
+    EUI,
+    NotRegisteredError,
+    )
 from textwrap import dedent
 from urllib import urlencode
 
@@ -192,11 +197,44 @@
     return mark_safe("[\n%s\n]" % ',\n'.join(names))
 
 
+def get_vendor_for_mac(mac):
+    """Return vendor for MAC."""
+    data = EUI(mac)
+    try:
+        return data.oui.registration().org
+    except NotRegisteredError:
+        return 'Unknown Vendor'
+
+
+def configure_macs(nodes):
+    """Configures the each node in the query to have an "macs" attribute,
+    that contains a list of macs, sorted by created.
+
+    The list is structed to contain the MAC and its vendor.
+    """
+    for node in nodes:
+        macs = node.macaddress_set.all()
+        macs = sorted(macs, key=lambda mac: mac.created)
+        macs = ['%s' % mac.mac_address for mac in macs]
+        if len(macs) == 0:
+            node.primary_mac = None
+            node.primary_mac_vendor = None
+            node.extra_macs = []
+        else:
+            node.primary_mac = macs[0]
+            node.primary_mac_vendor = get_vendor_for_mac(node.primary_mac)
+            node.extra_macs = macs[1:]
+    return nodes
+
+
 class NodeListView(PaginatedListView, FormMixin, ProcessFormView):
 
     context_object_name = "node_list"
     form_class = BulkNodeActionForm
-    sort_fields = ('hostname', 'status', 'zone')
+    sort_fields = (
+        'hostname', 'status', 'owner', 'cpu_count',
+        'memory', 'storage', 'zone')
+    late_sort_fields = ('primary_mac', )
 
     def populate_modifiers(self, request):
         self.query = request.GET.get("query")
@@ -284,7 +322,6 @@
             if self.sort_dir == 'desc':
                 custom_order = '-%s' % custom_order
             order_by = (custom_order, )
-
         return order_by + ('-created', )
 
     def _constrain_nodes(self, nodes_query):
@@ -315,7 +352,8 @@
         nodes = nodes.order_by(*self._compose_sort_order())
         if self.query:
             nodes = self._constrain_nodes(nodes)
-        return prefetch_nodes_listing(nodes)
+        nodes = prefetch_nodes_listing(nodes)
+        return configure_macs(nodes)
 
     def _prepare_sort_links(self):
         """Returns 2 dicts, with sort fields as keys and
@@ -323,13 +361,14 @@
         """
 
         # Build relative URLs for the links, just with the params
-        links = {field: '?' for field in self.sort_fields}
-        classes = {field: 'sort-none' for field in self.sort_fields}
+        fields = self.sort_fields + self.late_sort_fields
+        links = {field: '?' for field in fields}
+        classes = {field: 'sort-none' for field in fields}
 
         params = self.request.GET.copy()
         reverse_dir = 'asc' if self.sort_dir == 'desc' else 'desc'
 
-        for field in self.sort_fields:
+        for field in fields:
             params['sort'] = field
             if field == self.sort_by:
                 params['dir'] = reverse_dir
@@ -341,8 +380,22 @@
 
         return links, classes
 
+    def late_sort(self, context):
+        """Sorts the node_list with sorting arguments that require
+        late sorting.
+        """
+        node_list = context['node_list']
+        reverse = (self.sort_dir == 'desc')
+        if self.sort_by in self.late_sort_fields:
+            node_list = sorted(
+                node_list, key=attrgetter(self.sort_by),
+                reverse=reverse)
+        context['node_list'] = node_list
+        return context
+
     def get_context_data(self, **kwargs):
         context = super(NodeListView, self).get_context_data(**kwargs)
+        context = self.late_sort(context)
         context.update(get_longpoll_context())
         form_class = self.get_form_class()
         form = self.get_form(form_class)

=== modified file 'src/maasserver/views/tags.py'
--- src/maasserver/views/tags.py	2013-10-07 09:12:40 +0000
+++ src/maasserver/views/tags.py	2014-07-31 14:27:53 +0000
@@ -22,7 +22,10 @@
     Tag,
     )
 from maasserver.views import PaginatedListView
-from maasserver.views.nodes import prefetch_nodes_listing
+from maasserver.views.nodes import (
+    prefetch_nodes_listing,
+    configure_macs,
+    )
 
 
 class TagView(PaginatedListView):
@@ -43,7 +46,8 @@
             user=self.request.user, perm=NODE_PERMISSION.VIEW,
             from_nodes=self.tag.node_set.all())
         nodes = nodes.order_by('-created')
-        return prefetch_nodes_listing(nodes)
+        nodes = prefetch_nodes_listing(nodes)
+        return configure_macs(nodes)
 
     def get_context_data(self, **kwargs):
         context = super(TagView, self).get_context_data(**kwargs)

=== modified file 'src/maasserver/views/tests/test_nodes.py'
--- src/maasserver/views/tests/test_nodes.py	2014-07-18 15:44:55 +0000
+++ src/maasserver/views/tests/test_nodes.py	2014-07-31 14:27:53 +0000
@@ -151,9 +151,19 @@
         response = self.client.get(reverse('node-list'))
         sort_hostname = '?sort=hostname&dir=asc'
         sort_status = '?sort=status&dir=asc'
+        sort_owner = '?sort=owner&dir=asc'
+        sort_cpu_count = '?sort=cpu_count&dir=asc'
+        sort_memory = '?sort=memory&dir=asc'
+        sort_storage = '?sort=storage&dir=asc'
+        sort_primary_mac = '?sort=primary_mac&dir=asc'
         sort_zone = '?sort=zone&dir=asc'
         self.assertIn(sort_hostname, get_content_links(response))
         self.assertIn(sort_status, get_content_links(response))
+        self.assertIn(sort_owner, get_content_links(response))
+        self.assertIn(sort_cpu_count, get_content_links(response))
+        self.assertIn(sort_memory, get_content_links(response))
+        self.assertIn(sort_storage, get_content_links(response))
+        self.assertIn(sort_primary_mac, get_content_links(response))
         self.assertIn(sort_zone, get_content_links(response))
 
     def test_node_list_ignores_unknown_sort_param(self):