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

Proposed by Raphaël Badin on 2012-11-07
Status: Merged
Approved by: Raphaël Badin on 2012-11-08
Approved revision: 1327
Merged at revision: 1335
Proposed branch: lp:~rvb/maas/ui-update-fqdn
Merge into: lp:maas/trunk
Diff against target: 219 lines (+67/-22)
8 files modified
src/maasserver/forms.py (+9/-0)
src/maasserver/templates/maasserver/node_edit.html (+2/-0)
src/maasserver/templates/maasserver/node_view.html (+4/-4)
src/maasserver/templates/maasserver/nodes_listing.html (+24/-12)
src/maasserver/tests/test_views_nodes.py (+17/-0)
src/maasserver/tests/test_views_tags.py (+4/-4)
src/maasserver/views/nodes.py (+3/-1)
src/maasserver/views/tags.py (+4/-1)
To merge this branch: bzr merge lp:~rvb/maas/ui-update-fqdn
Reviewer Review Type Date Requested Status
Julian Edwards (community) 2012-11-07 Approve on 2012-11-08
Review via email: mp+133271@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Julian Edwards (julian-edwards) wrote :

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) wrote :

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) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/forms.py'
2--- src/maasserver/forms.py 2012-11-01 15:13:10 +0000
3+++ src/maasserver/forms.py 2012-11-08 08:36:21 +0000
4@@ -152,6 +152,15 @@
5 initial=ARCHITECTURE.i386,
6 error_messages={'invalid_choice': INVALID_ARCHITECTURE_MESSAGE})
7
8+ hostname = forms.CharField(
9+ label="Host name", required=False, help_text=(
10+ "The FQDN (Fully Qualified Domain Name) is derived from the "
11+ "host name: If the cluster controller for this node is managing "
12+ "DNS then the domain part in the host name (if any) is replaced "
13+ "by the domain defined on the cluster; if the cluster controller "
14+ "does not manage DNS, then the host name as entered will be the "
15+ "FQDN."))
16+
17 class Meta:
18 model = Node
19
20
21=== modified file 'src/maasserver/templates/maasserver/node_edit.html'
22--- src/maasserver/templates/maasserver/node_edit.html 2012-07-10 16:03:27 +0000
23+++ src/maasserver/templates/maasserver/node_edit.html 2012-11-08 08:36:21 +0000
24@@ -30,6 +30,7 @@
25 {% endblock %}
26
27 {% block content %}
28+ <div id="node-edit" class="block size7">
29 <form action="." method="post" class="block auto-width">
30 {% csrf_token %}
31 <ul>
32@@ -62,4 +63,5 @@
33 <input type="submit" value="Save node" class="right" />
34 <a class="link-button" href="{% url 'node-view' node.system_id %}">Cancel</a>
35 </form>
36+ </div>
37 {% endblock %}
38
39=== modified file 'src/maasserver/templates/maasserver/node_view.html'
40--- src/maasserver/templates/maasserver/node_view.html 2012-10-05 17:42:12 +0000
41+++ src/maasserver/templates/maasserver/node_view.html 2012-11-08 08:36:21 +0000
42@@ -1,8 +1,8 @@
43 {% extends "maasserver/base.html" %}
44
45 {% block nav-active-settings %}active{% endblock %}
46-{% block title %}Node: {{ node.hostname }}{% endblock %}
47-{% block page-title %}Node: {{ node.hostname }}{% endblock %}
48+{% block title %}Node: {{ node.fqdn }}{% endblock %}
49+{% block page-title %}Node: {{ node.fqdn }}{% endblock %}
50 {% block layout-modifiers %}sidebar{% endblock %}
51
52 {% block sidebar %}
53@@ -39,8 +39,8 @@
54 {% block content %}
55 <ul class="data-list">
56 <li class="block size3 first">
57- <h4>Hostname</h4>
58- <span>{{ node.hostname }}</span>
59+ <h4><acronym title="Fully Qualified Domain Name">FQDN</acronym></h4>
60+ <span>{{ node.fqdn }}</span>
61 </li>
62 <li class="block size3">
63 <h4>MAC addresses</h4>
64
65=== modified file 'src/maasserver/templates/maasserver/nodes_listing.html'
66--- src/maasserver/templates/maasserver/nodes_listing.html 2012-10-24 12:51:10 +0000
67+++ src/maasserver/templates/maasserver/nodes_listing.html 2012-11-08 08:36:21 +0000
68@@ -2,26 +2,38 @@
69 <table class="list">
70 <thead>
71 <tr>
72- {% if sorting == "true" %}
73- <th><a href="{{ sort_links.hostname }}"
74- class="{{ sort_classes.hostname }}">MAC</a></th>
75- <th><a href="{{ sort_links.status }}"
76- class="{{ sort_classes.status }}">Status</a></th>
77- {% else %}
78- <th>MAC</th>
79+ {% if sorting == "true" %}
80+ <th><a href="{{ sort_links.hostname }}"
81+ class="{{ sort_classes.hostname }}">
82+ <acronym title="Fully Qualified Domain Name">FQDN</acronym>
83+ </a></th>
84+ <th>
85+ <acronym
86+ title="Media Access Control addresses">MAC</acronym>
87+ </th>
88+ <th>
89+ <a href="{{ sort_links.status }}"
90+ class="{{ sort_classes.status }}">Status</a>
91+ </th>
92+ {% else %}
93+ <th><acronym title="Fully Qualified Domain Name">FQDN</acronym></th>
94+ <th><acronym
95+ title="Media Access Control addresses">MAC</acronym></th>
96 <th>Status</th>
97- {% endif %}
98+ {% endif %}
99 </tr>
100 </thead>
101 {% for node in node_list %}
102 <tr class="node {% cycle 'even' 'odd' %}">
103 <td>
104 <a href="{% url 'node-view' node.system_id %}">
105- {% for macaddress in node.macaddress_set.all reversed %}
106- {{ macaddress }}{% if not forloop.last %},{% endif %}
107- {% endfor %}
108+ {{ node.fqdn }}
109 </a>
110- ({{ node.hostname }})
111+ </td>
112+ <td>
113+ {% for macaddress in node.macaddress_set.all reversed %}
114+ {{ macaddress }}{% if not forloop.last %},{% endif %}
115+ {% endfor %}
116 </td>
117 <td>{{ node.display_status }}</td>
118 </tr>
119
120=== modified file 'src/maasserver/tests/test_views_nodes.py'
121--- src/maasserver/tests/test_views_nodes.py 2012-11-01 15:59:22 +0000
122+++ src/maasserver/tests/test_views_nodes.py 2012-11-08 08:36:21 +0000
123@@ -30,6 +30,8 @@
124 ARCHITECTURE_CHOICES,
125 NODE_AFTER_COMMISSIONING_ACTION,
126 NODE_STATUS,
127+ NODEGROUP_STATUS,
128+ NODEGROUPINTERFACE_MANAGEMENT,
129 )
130 from maasserver.exceptions import (
131 InvalidConstraint,
132@@ -186,6 +188,21 @@
133 ('sort', next(fields)),
134 ('dir', next(field_dirs))]))
135
136+ def test_node_list_displays_fqdn_dns_not_managed(self):
137+ nodes = [factory.make_node() for i in range(3)]
138+ response = self.client.get(reverse('node-list'))
139+ node_fqdns = [node.fqdn for node in nodes]
140+ self.assertThat(response.content, ContainsAll(node_fqdns))
141+
142+ def test_node_list_displays_fqdn_dns_managed(self):
143+ nodegroup = factory.make_node_group(
144+ status=NODEGROUP_STATUS.ACCEPTED,
145+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
146+ nodes = [factory.make_node(nodegroup=nodegroup) for i in range(3)]
147+ response = self.client.get(reverse('node-list'))
148+ node_fqdns = [node.fqdn for node in nodes]
149+ self.assertThat(response.content, ContainsAll(node_fqdns))
150+
151 def test_node_list_displays_sorted_list_of_nodes(self):
152 # Nodes are sorted on the node list page, newest first.
153 nodes = [factory.make_node() for i in range(3)]
154
155=== modified file 'src/maasserver/tests/test_views_tags.py'
156--- src/maasserver/tests/test_views_tags.py 2012-11-01 15:59:22 +0000
157+++ src/maasserver/tests/test_views_tags.py 2012-11-08 08:36:21 +0000
158@@ -33,8 +33,8 @@
159 response = self.client.get(tag_link)
160 doc = fromstring(response.content)
161 content_text = doc.cssselect('#content')[0].text_content()
162- self.assertThat(content_text,
163- ContainsAll([tag.comment, tag.definition]))
164+ self.assertThat(
165+ content_text, ContainsAll([tag.comment, tag.definition]))
166
167 def test_view_tag_includes_node_links(self):
168 tag = factory.make_tag()
169@@ -46,8 +46,8 @@
170 response = self.client.get(tag_link)
171 doc = fromstring(response.content)
172 content_text = doc.cssselect('#content')[0].text_content()
173- self.assertThat(content_text,
174- ContainsAll([mac, '(%s)' % node.hostname]))
175+ self.assertThat(
176+ content_text, ContainsAll([mac, '%s' % node.hostname]))
177 self.assertNotIn(node.system_id, content_text)
178 self.assertIn(node_link, get_content_links(response))
179
180
181=== modified file 'src/maasserver/views/nodes.py'
182--- src/maasserver/views/nodes.py 2012-10-24 15:57:18 +0000
183+++ src/maasserver/views/nodes.py 2012-11-08 08:36:21 +0000
184@@ -126,7 +126,7 @@
185 else:
186 order_by = ('-created', )
187
188- # Return the sorted node list
189+ # Return the sorted node list.
190 nodes = Node.objects.get_nodes(
191 user=self.request.user, prefetch_mac=True,
192 perm=NODE_PERMISSION.VIEW,).order_by(*order_by)
193@@ -136,6 +136,8 @@
194 except InvalidConstraint as e:
195 self.query_error = e
196 return Node.objects.none()
197+ nodes = nodes.prefetch_related('nodegroup')
198+ nodes = nodes.prefetch_related('nodegroup__nodegroupinterface_set')
199 return nodes
200
201 def _prepare_sort_links(self):
202
203=== modified file 'src/maasserver/views/tags.py'
204--- src/maasserver/views/tags.py 2012-10-18 11:26:02 +0000
205+++ src/maasserver/views/tags.py 2012-11-08 08:36:21 +0000
206@@ -33,9 +33,12 @@
207 return super(TagView, self).get(request, *args, **kwargs)
208
209 def get_queryset(self):
210- return Tag.objects.get_nodes(
211+ nodes = Tag.objects.get_nodes(
212 self.tag, user=self.request.user, prefetch_mac=True,
213 ).order_by('-created')
214+ nodes = nodes.prefetch_related('nodegroup')
215+ nodes = nodes.prefetch_related('nodegroup__nodegroupinterface_set')
216+ return nodes
217
218 def get_context_data(self, **kwargs):
219 context = super(TagView, self).get_context_data(**kwargs)