launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/metadata-fix-hostname-1.2 into lp:maas.

Commit message:
Return node.fqdn instead of node.hostname in the metadataservice.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1078744 in MAAS: "Nodes are deployed with wrong domain name."
  https://bugs.launchpad.net/maas/+bug/1078744

For more details, see:
https://code.launchpad.net/~rvb/maas/metadata-fix-hostname-1.2/+merge/134303

Return node.fqdn instead of node.hostname in the metadataservice.

(This is a backport of https://code.launchpad.net/~rvb/maas/metadata-fix-hostname/+merge/134302)
-- 
https://code.launchpad.net/~rvb/maas/metadata-fix-hostname-1.2/+merge/134303
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/metadata-fix-hostname-1.2 into lp:maas.

=== modified file 'src/maasserver/api.py'
=== modified file 'src/maasserver/forms.py'
=== modified file 'src/maasserver/templates/maasserver/tag_view.html'
--- src/maasserver/templates/maasserver/tag_view.html	2012-11-08 10:40:48 +0000
+++ src/maasserver/templates/maasserver/tag_view.html	2012-11-14 14:42:22 +0000
@@ -39,9 +39,15 @@
     {% endif %}
   </ul>
   <div id="nodes" class="block first pad-top">
+<<<<<<< TREE
     <h2 class="block first line-top pad-top">{{ paginator.count }} Node{{ paginator.count|pluralize }}</h2>
     {% include "maasserver/nodes_listing.html" with sorting="false" %}
     {% include "maasserver/pagination.html" %}
+=======
+    <h2 class="block first line-top pad-top">{{ paginator.count }} Node{{ paginator.count|pluralize }}</h2>
+    {% include "maasserver/nodes_listing.html" %}
+    {% include "maasserver/pagination.html" %}
+>>>>>>> MERGE-SOURCE
   </div>
 {% endblock %}
 

=== modified file 'src/maasserver/tests/test_api.py'
=== modified file 'src/maasserver/tests/test_forms.py'
=== modified file 'src/maasserver/tests/test_views_tags.py'
--- src/maasserver/tests/test_views_tags.py	2012-11-08 12:58:48 +0000
+++ src/maasserver/tests/test_views_tags.py	2012-11-14 14:42:22 +0000
@@ -20,6 +20,17 @@
 from maasserver.testing.testcase import LoggedInTestCase
 from maasserver.views import tags as tags_views
 from maastesting.matchers import ContainsAll
+<<<<<<< TREE
+=======
+from maasserver.testing import (
+    get_content_links,
+    )
+from maasserver.testing.factory import factory
+from maasserver.testing.testcase import (
+    LoggedInTestCase,
+    )
+from maasserver.views import tags as tags_views
+>>>>>>> MERGE-SOURCE
 
 
 class TagViewsTest(LoggedInTestCase):
@@ -88,6 +99,7 @@
         content_text = doc.cssselect('#content')[0].text_content()
         self.assertIn(node.hostname, content_text)
         self.assertNotIn(node2.hostname, content_text)
+<<<<<<< TREE
 
     def test_view_tag_shows_kernel_params(self):
         tag = factory.make_tag(kernel_opts='--test tag params')
@@ -143,3 +155,49 @@
         self.assertEqual([("first", "."), ("previous", "?page=2")],
             [(a.text.lower(), a.get("href"))
                 for a in expr_page_anchors(page3)])
+=======
+
+    def test_view_tag_paginates_nodes(self):
+        """Listing of nodes with tag is split across multiple pages
+
+        Copy-coded from NodeViewsTest.test_node_list_paginates evilly.
+        """
+        # Set a very small page size to save creating lots of nodes
+        page_size = 2
+        self.patch(tags_views.TagView, 'paginate_by', page_size)
+        tag = factory.make_tag()
+        nodes = [factory.make_node(created="2012-10-12 12:00:%02d" % i)
+            for i in range(page_size * 2 + 1)]
+        for node in nodes:
+            node.tags = [tag]
+        # Order node links with newest first as the view is expected to
+        node_links = [reverse('node-view', args=[node.system_id])
+            for node in reversed(nodes)]
+        expr_node_links = XPath("//div[@id='nodes']/table//a/@href")
+        expr_page_anchors = XPath("//div[@class='pagination']//a")
+        # Fetch first page, should link newest two nodes and page 2
+        response = self.client.get(reverse('tag-view', args=[tag.name]))
+        page1 = fromstring(response.content)
+        self.assertEqual(node_links[:page_size], expr_node_links(page1))
+        self.assertEqual([("next", "?page=2"), ("last", "?page=3")],
+            [(a.text.lower(), a.get("href"))
+                for a in expr_page_anchors(page1)])
+        # Fetch second page, should link next nodes and adjacent pages
+        response = self.client.get(reverse('tag-view', args=[tag.name]),
+            {"page": 2})
+        page2 = fromstring(response.content)
+        self.assertEqual(node_links[page_size:page_size * 2],
+            expr_node_links(page2))
+        self.assertEqual([("first", "."), ("previous", "."),
+                ("next", "?page=3"), ("last", "?page=3")],
+            [(a.text.lower(), a.get("href"))
+                for a in expr_page_anchors(page2)])
+        # Fetch third page, should link oldest node and node list page
+        response = self.client.get(reverse('tag-view', args=[tag.name]),
+            {"page": 3})
+        page3 = fromstring(response.content)
+        self.assertEqual(node_links[page_size * 2:], expr_node_links(page3))
+        self.assertEqual([("first", "."), ("previous", "?page=2")],
+            [(a.text.lower(), a.get("href"))
+                for a in expr_page_anchors(page3)])
+>>>>>>> MERGE-SOURCE

=== modified file 'src/maasserver/views/tags.py'
--- src/maasserver/views/tags.py	2012-11-08 12:58:48 +0000
+++ src/maasserver/views/tags.py	2012-11-14 14:42:22 +0000
@@ -30,6 +30,7 @@
             name=kwargs.get('name', None),
             user=self.request.user,
             to_edit=False)
+<<<<<<< TREE
         return super(TagView, self).get(request, *args, **kwargs)
 
     def get_queryset(self):
@@ -39,6 +40,14 @@
         nodes = nodes.prefetch_related('nodegroup')
         nodes = nodes.prefetch_related('nodegroup__nodegroupinterface_set')
         return nodes
+=======
+        return super(TagView, self).get(request, *args, **kwargs)
+
+    def get_queryset(self):
+        return Tag.objects.get_nodes(
+            self.tag, user=self.request.user, prefetch_mac=True,
+            ).order_by('-created')
+>>>>>>> MERGE-SOURCE
 
     def get_context_data(self, **kwargs):
         context = super(TagView, self).get_context_data(**kwargs)

=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py	2012-11-08 06:34:48 +0000
+++ src/metadataserver/api.py	2012-11-14 14:42:22 +0000
@@ -310,7 +310,7 @@
 
     def local_hostname(self, node, version, item):
         """Produce local-hostname attribute."""
-        return make_text_response(node.hostname)
+        return make_text_response(node.fqdn)
 
     def instance_id(self, node, version, item):
         """Produce instance-id attribute."""

=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py	2012-10-05 14:11:43 +0000
+++ src/metadataserver/tests/test_api.py	2012-11-14 14:42:22 +0000
@@ -20,7 +20,11 @@
 from django.conf import settings
 from django.core.exceptions import PermissionDenied
 from django.core.urlresolvers import reverse
-from maasserver.enum import NODE_STATUS
+from maasserver.enum import (
+    NODE_STATUS,
+    NODEGROUP_STATUS,
+    NODEGROUPINTERFACE_MANAGEMENT,
+    )
 from maasserver.exceptions import (
     MAASAPINotFound,
     Unauthorized,
@@ -220,13 +224,19 @@
         producers = map(handler.get_attribute_producer, handler.fields)
         self.assertNotIn(None, producers)
 
-    def test_meta_data_local_hostname_returns_hostname(self):
+    def test_meta_data_local_hostname_returns_fqdn(self):
+        nodegroup = factory.make_node_group(
+            status=NODEGROUP_STATUS.ACCEPTED,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
         hostname = factory.getRandomString()
-        client = self.make_node_client(factory.make_node(hostname=hostname))
+        domain = factory.getRandomString()
+        node = factory.make_node(
+            hostname='%s.%s' % (hostname, domain), nodegroup=nodegroup)
+        client = self.make_node_client(node)
         url = reverse('metadata-meta-data', args=['latest', 'local-hostname'])
         response = client.get(url)
         self.assertEqual(
-            (httplib.OK, hostname),
+            (httplib.OK, node.fqdn),
             (response.status_code, response.content.decode('ascii')))
         self.assertIn('text/plain', response['Content-Type'])