Merge lp:~rvb/maas/ui-update-fqdn into lp:~maas-committers/maas/trunk

[Raphaël Badin]

Display the FQDN instead of the hostname on the node listing pages.

This is the final branch in a series of branches aiming at replacing the FQDN (Fully Qualified Domain Name) of nodes where we had the hostname previously.

This branch is the UI part (the API part was merged a few days ago, see https://code.launchpad.net/~rvb/maas/use-fqdn/+merge/131860): it changes the node listing page so that it displays the FQDN instead of the hostname. I also added the display of the FQDN on the node view page and added a note on the node edit page to explain how the FQDN is derived from the hostname.

Before: http://people.canonical.com/~rvb/old-node-listing.png
After: http://people.canonical.com/~rvb/node-listing.png

[Julian Edwards (julian-edwards)]

Hmm this might be needs-fixing, but I'll let you explain yourself - where are the tests for the changed node_view?
I'll approve for now on the basis you either know that it's tested implicitly elsewhere or that you'll add a test.

Some other minor points:

9 + label="Hostname", help_text=(

s/Hostname/Host name/

ie two words (in multiple places)

14 + "does not manage DNS, then the hostname is the FQDN."))

I'd say: "the host name as entered will be the FQDN."

58 + <h4><acronym title="Fully Qualified Domain Name">FQDN</acronym></h4>

I had no idea that tag even existed!

review: Approve

[Raphaël Badin (rvb)]

Thanks for the review!

> Hmm this might be needs-fixing, but I'll let you explain yourself - where are
> the tests for the changed node_view?
> I'll approve for now on the basis you either know that it's tested implicitly
> elsewhere or that you'll add a test.

That page is indeed massively tested already but I added 2 tests to make sure that the FQDN is properly displayed.

> Some other minor points:
>
> 9 + label="Hostname", help_text=(
>
> s/Hostname/Host name/
>
> ie two words (in multiple places)
>
> 14 + "does not manage DNS, then the hostname is the FQDN."))

Done.

> I'd say: "the host name as entered will be the FQDN."

Done.

[MAAS Lander (maas-lander)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-11-01 15:13:10 +0000
+++ src/maasserver/forms.py	2012-11-08 08:36:21 +0000
@@ -152,6 +152,15 @@
         initial=ARCHITECTURE.i386,
         error_messages={'invalid_choice': INVALID_ARCHITECTURE_MESSAGE})
 
+    hostname = forms.CharField(
+        label="Host name", required=False, help_text=(
+            "The FQDN (Fully Qualified Domain Name) is derived from the "
+            "host name: If the cluster controller for this node is managing "
+            "DNS then the domain part in the host name (if any) is replaced "
+            "by the domain defined on the cluster; if the cluster controller "
+            "does not manage DNS, then the host name as entered will be the "
+            "FQDN."))
+
     class Meta:
         model = Node
 

=== modified file 'src/maasserver/templates/maasserver/node_edit.html'
--- src/maasserver/templates/maasserver/node_edit.html	2012-07-10 16:03:27 +0000
+++ src/maasserver/templates/maasserver/node_edit.html	2012-11-08 08:36:21 +0000
@@ -30,6 +30,7 @@
 {% endblock %}
 
 {% block content %}
+  <div id="node-edit" class="block size7">
   <form action="." method="post" class="block auto-width">
     {% csrf_token %}
     <ul>
@@ -62,4 +63,5 @@
     <input type="submit" value="Save node" class="right" />
     <a class="link-button" href="{% url 'node-view' node.system_id %}">Cancel</a>
   </form>
+  </div>
 {% endblock %}

=== modified file 'src/maasserver/templates/maasserver/node_view.html'
--- src/maasserver/templates/maasserver/node_view.html	2012-10-05 17:42:12 +0000
+++ src/maasserver/templates/maasserver/node_view.html	2012-11-08 08:36:21 +0000
@@ -1,8 +1,8 @@
 {% extends "maasserver/base.html" %}
 
 {% block nav-active-settings %}active{% endblock %}
-{% block title %}Node: {{ node.hostname }}{% endblock %}
-{% block page-title %}Node: {{ node.hostname }}{% endblock %}
+{% block title %}Node: {{ node.fqdn }}{% endblock %}
+{% block page-title %}Node: {{ node.fqdn }}{% endblock %}
 {% block layout-modifiers %}sidebar{% endblock %}
 
 {% block sidebar %}
@@ -39,8 +39,8 @@
 {% block content %}
   <ul class="data-list">
     <li class="block size3 first">
-      <h4>Hostname</h4>
-      <span>{{ node.hostname }}</span>
+      <h4><acronym title="Fully Qualified Domain Name">FQDN</acronym></h4>
+        <span>{{ node.fqdn }}</span>
     </li>
     <li class="block size3">
       <h4>MAC addresses</h4>

=== modified file 'src/maasserver/templates/maasserver/nodes_listing.html'
--- src/maasserver/templates/maasserver/nodes_listing.html	2012-10-24 12:51:10 +0000
+++ src/maasserver/templates/maasserver/nodes_listing.html	2012-11-08 08:36:21 +0000
@@ -2,26 +2,38 @@
   <table class="list">
     <thead>
       <tr>
-	{% if sorting == "true" %}
-        <th><a href="{{ sort_links.hostname }}"
-	       class="{{ sort_classes.hostname }}">MAC</a></th>
-        <th><a href="{{ sort_links.status }}"
-	       class="{{ sort_classes.status }}">Status</a></th>
-	{% else %}
-        <th>MAC</th>
+      {% if sorting == "true" %}
+      <th><a href="{{ sort_links.hostname }}"
+             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>
+        <a href="{{ sort_links.status }}"
+	       class="{{ sort_classes.status }}">Status</a>
+      </th>
+      {% else %}
+        <th><acronym title="Fully Qualified Domain Name">FQDN</acronym></th>
+        <th><acronym
+          title="Media Access Control addresses">MAC</acronym></th>
         <th>Status</th>
-	{% endif %}
+      {% endif %}
       </tr>
     </thead>
     {% for node in node_list %}
       <tr class="node {% cycle 'even' 'odd' %}">
         <td>
           <a href="{% url 'node-view' node.system_id %}">
-            {% for macaddress in node.macaddress_set.all reversed %}
-              {{ macaddress }}{% if not forloop.last %},{% endif %}
-            {% endfor %}
+          {{ node.fqdn }}
           </a>
-          ({{ node.hostname }})
+        </td>
+        <td>
+          {% for macaddress in node.macaddress_set.all reversed %}
+            {{ macaddress }}{% if not forloop.last %},{% endif %}
+          {% endfor %}
         </td>
         <td>{{ node.display_status }}</td>
       </tr>

=== modified file 'src/maasserver/tests/test_views_nodes.py'
--- src/maasserver/tests/test_views_nodes.py	2012-11-01 15:59:22 +0000
+++ src/maasserver/tests/test_views_nodes.py	2012-11-08 08:36:21 +0000
@@ -30,6 +30,8 @@
     ARCHITECTURE_CHOICES,
     NODE_AFTER_COMMISSIONING_ACTION,
     NODE_STATUS,
+    NODEGROUP_STATUS,
+    NODEGROUPINTERFACE_MANAGEMENT,
     )
 from maasserver.exceptions import (
     InvalidConstraint,
@@ -186,6 +188,21 @@
                     ('sort', next(fields)),
                     ('dir', next(field_dirs))]))
 
+    def test_node_list_displays_fqdn_dns_not_managed(self):
+        nodes = [factory.make_node() for i in range(3)]
+        response = self.client.get(reverse('node-list'))
+        node_fqdns = [node.fqdn for node in nodes]
+        self.assertThat(response.content, ContainsAll(node_fqdns))
+
+    def test_node_list_displays_fqdn_dns_managed(self):
+        nodegroup = factory.make_node_group(
+            status=NODEGROUP_STATUS.ACCEPTED,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
+        nodes = [factory.make_node(nodegroup=nodegroup) for i in range(3)]
+        response = self.client.get(reverse('node-list'))
+        node_fqdns = [node.fqdn for node in nodes]
+        self.assertThat(response.content, ContainsAll(node_fqdns))
+
     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)]

=== modified file 'src/maasserver/tests/test_views_tags.py'
--- src/maasserver/tests/test_views_tags.py	2012-11-01 15:59:22 +0000
+++ src/maasserver/tests/test_views_tags.py	2012-11-08 08:36:21 +0000
@@ -33,8 +33,8 @@
         response = self.client.get(tag_link)
         doc = fromstring(response.content)
         content_text = doc.cssselect('#content')[0].text_content()
-        self.assertThat(content_text,
-                        ContainsAll([tag.comment, tag.definition]))
+        self.assertThat(
+            content_text, ContainsAll([tag.comment, tag.definition]))
 
     def test_view_tag_includes_node_links(self):
         tag = factory.make_tag()
@@ -46,8 +46,8 @@
         response = self.client.get(tag_link)
         doc = fromstring(response.content)
         content_text = doc.cssselect('#content')[0].text_content()
-        self.assertThat(content_text,
-                        ContainsAll([mac, '(%s)' % node.hostname]))
+        self.assertThat(
+            content_text, ContainsAll([mac, '%s' % node.hostname]))
         self.assertNotIn(node.system_id, content_text)
         self.assertIn(node_link, get_content_links(response))
 

=== modified file 'src/maasserver/views/nodes.py'
--- src/maasserver/views/nodes.py	2012-10-24 15:57:18 +0000
+++ src/maasserver/views/nodes.py	2012-11-08 08:36:21 +0000
@@ -126,7 +126,7 @@
         else:
             order_by = ('-created', )
 
-        # Return the sorted node list
+        # Return the sorted node list.
         nodes = Node.objects.get_nodes(
             user=self.request.user, prefetch_mac=True,
             perm=NODE_PERMISSION.VIEW,).order_by(*order_by)
@@ -136,6 +136,8 @@
             except InvalidConstraint as e:
                 self.query_error = e
                 return Node.objects.none()
+        nodes = nodes.prefetch_related('nodegroup')
+        nodes = nodes.prefetch_related('nodegroup__nodegroupinterface_set')
         return nodes
 
     def _prepare_sort_links(self):

=== modified file 'src/maasserver/views/tags.py'
--- src/maasserver/views/tags.py	2012-10-18 11:26:02 +0000
+++ src/maasserver/views/tags.py	2012-11-08 08:36:21 +0000
@@ -33,9 +33,12 @@
         return super(TagView, self).get(request, *args, **kwargs)
 
     def get_queryset(self):
-        return Tag.objects.get_nodes(
+        nodes = Tag.objects.get_nodes(
             self.tag, user=self.request.user, prefetch_mac=True,
             ).order_by('-created')
+        nodes = nodes.prefetch_related('nodegroup')
+        nodes = nodes.prefetch_related('nodegroup__nodegroupinterface_set')
+        return nodes
 
     def get_context_data(self, **kwargs):
         context = super(TagView, self).get_context_data(**kwargs)