Merge lp:~rvb/maas/bug-1070765-hostname-1.2 into lp:maas/1.2

[Raphaël Badin]

Backport [r1316..r1318].

Backport [r1316..r1318].

[Raphaël Badin (rvb)]

Self-approving this as it is a straightforward backport.

review: Approve

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-10-30 10:25:55 +0000
+++ src/maasserver/api.py	2012-10-30 15:17:24 +0000
@@ -165,6 +165,7 @@
 from maasserver.utils import (
     absolute_reverse,
     map_enum,
+    strip_domain,
     )
 from maasserver.utils.orm import get_one
 from piston.handler import (
@@ -474,6 +475,12 @@
     model = Node
     fields = DISPLAYED_NODE_FIELDS
 
+    # Override the 'hostname' field so that it returns the FQDN instead as
+    # this is used by Juju to reach that node.
+    @classmethod
+    def hostname(handler, node):
+        return node.fqdn
+
     def read(self, request, system_id):
         """Read a specific Node."""
         return Node.objects.get_node_or_404(
@@ -647,8 +654,15 @@
 class AnonNodesHandler(AnonymousOperationsHandler):
     """Create Nodes."""
     create = read = update = delete = None
+    model = Node
     fields = DISPLAYED_NODE_FIELDS
 
+    # Override the 'hostname' field so that it returns the FQDN instead as
+    # this is used by Juju to reach that node.
+    @classmethod
+    def hostname(handler, node):
+        return node.fqdn
+
     @operation(idempotent=False)
     def new(self, request):
         """Create a new Node.
@@ -827,9 +841,12 @@
             request.user, NODE_PERMISSION.VIEW, ids=match_ids)
         if match_macs is not None:
             nodes = nodes.filter(macaddress__mac_address__in=match_macs)
-        # Prefetch related macaddresses and tags.
+        # Prefetch related macaddresses, tags and nodegroups (plus
+        # related interfaces).
         nodes = nodes.prefetch_related('macaddress_set__node')
         nodes = nodes.prefetch_related('tags')
+        nodes = nodes.prefetch_related('nodegroup')
+        nodes = nodes.prefetch_related('nodegroup__nodegroupinterface_set')
         return nodes.order_by('id')
 
     @operation(idempotent=True)
@@ -1708,7 +1725,7 @@
         preseed_url = compose_preseed_url(node)
         # The node's hostname may include a domain, but we ignore that
         # and use the one from the nodegroup instead.
-        hostname = node.hostname.split('.', 1)[0]
+        hostname = strip_domain(node.hostname)
         domain = node.nodegroup.name
     else:
         try:
 
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-10-30 11:29:20 +0000
+++ src/maasserver/forms.py	2012-10-30 15:17:24 +0000
@@ -31,6 +31,7 @@
 
 import collections
 import json
+import re
 
 from django import forms
 from django.contrib import messages
@@ -82,6 +83,7 @@
 from maasserver.models.nodegroup import NODEGROUP_CLUSTER_NAME_TEMPLATE
 from maasserver.node_action import compile_node_actions
 from maasserver.power_parameters import POWER_TYPE_PARAMETERS
+from maasserver.utils import strip_domain
 from provisioningserver.enum import (
     POWER_TYPE,
     POWER_TYPE_CHOICES,
@@ -334,6 +336,9 @@
         node.nodegroup = form_value
 
 
+IP_BASED_HOSTNAME_REGEXP = re.compile('\d{1,3}-\d{1,3}-\d{1,3}-\d{1,3}')
+
+
 class WithMACAddressesMixin:
     """A form mixin which dynamically adds a MultipleMACAddressField to the
     list of fields.  This mixin also overrides the 'save' method to persist
@@ -389,7 +394,15 @@
         node.save()
         for mac in self.cleaned_data['mac_addresses']:
             node.add_mac_address(mac)
-        if self.cleaned_data['hostname'] == "":
+        hostname = self.cleaned_data['hostname']
+        stripped_hostname = strip_domain(hostname)
+        # Generate a hostname for this node if the provided hostname is
+        # IP-based (because this means that this name comes from a DNS
+        # reverse query to the MAAS DNS) or an empty string.
+        generate_hostname = (
+            hostname == "" or
+            IP_BASED_HOSTNAME_REGEXP.match(stripped_hostname) != None)
+        if generate_hostname:
             node.set_mac_based_hostname(self.cleaned_data['mac_addresses'][0])
         return node
 
 
=== modified file 'src/maasserver/models/dhcplease.py'
--- src/maasserver/models/dhcplease.py	2012-10-09 10:30:10 +0000
+++ src/maasserver/models/dhcplease.py	2012-10-30 15:17:24 +0000
@@ -25,11 +25,7 @@
 from maasserver import DefaultMeta
 from maasserver.fields import MACAddressField
 from maasserver.models.cleansave import CleanSave
-
-
-def strip_domain(hostname):
-    """Return `hostname` with the domain part removed."""
-    return hostname.split('.', 1)[0]
+from maasserver.utils import strip_domain
 
 
 class DHCPLeaseManager(Manager):
 
=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2012-10-26 10:20:19 +0000
+++ src/maasserver/models/node.py	2012-10-30 15:17:24 +0000
@@ -63,7 +63,10 @@
 from maasserver.models.dhcplease import DHCPLease
 from maasserver.models.tag import Tag
 from maasserver.models.timestampedmodel import TimestampedModel
-from maasserver.utils import get_db_state
+from maasserver.utils import (
+    get_db_state,
+    strip_domain,
+    )
 from maasserver.utils.orm import get_first
 from piston.models import Token
 from provisioningserver.enum import (
@@ -487,10 +490,30 @@
 
     def __unicode__(self):
         if self.hostname:
-            return "%s (%s)" % (self.system_id, self.hostname)
+            return "%s (%s)" % (self.system_id, self.fqdn)
         else:
             return self.system_id
 
+    @property
+    def fqdn(self):
+        """Fully qualified domain name for this node.
+
+        If MAAS manages DNS for this node, the domain part of the
+        hostname (if present), is replaced by the domain configured
+        on the cluster controller.
+        If not, simply return the node's hostname.
+        """
+        # Avoid circular imports.
+        from maasserver.dns import is_dns_managed
+        if is_dns_managed(self.nodegroup):
+            # If the hostname field contains a domain, strip it.
+            hostname = strip_domain(self.hostname)
+            # Build the FQDN by using the hostname and nodegroup.name
+            # as the domain name.
+            return '%s.%s' % (hostname, self.nodegroup.name)
+        else:
+            return self.hostname
+
     def tag_names(self):
         # We don't use self.tags.values_list here because this does not
         # take advantage of the cache.
 
=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2012-10-11 10:59:40 +0000
+++ src/maasserver/models/nodegroup.py	2012-10-30 15:17:24 +0000
@@ -31,7 +31,6 @@
 from maasserver.models.nodegroupinterface import NodeGroupInterface
 from maasserver.models.timestampedmodel import TimestampedModel
 from maasserver.refresh_worker import refresh_worker
-from maasserver.utils.orm import get_one
 from piston.models import (
     KEY_SIZE,
     Token,
@@ -196,10 +195,13 @@
         This is a temporary method that should be refactored once we add
         proper support for multiple interfaces on a nodegroup.
         """
-        return get_one(
-            NodeGroupInterface.objects.filter(
-                nodegroup=self).exclude(
-                    management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED))
+        # Iterate over all the interfaces in python instead of doing the
+        # filtering in SQL so that this will use the cached version of
+        # self.nodegroupinterface_set if it is there.
+        for interface in self.nodegroupinterface_set.all():
+            if interface.management != NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED:
+                return interface
+        return None
 
     def ensure_dhcp_key(self):
         """Ensure that this nodegroup has a dhcp key.
 
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-10-30 11:00:20 +0000
+++ src/maasserver/tests/test_api.py	2012-10-30 15:17:24 +0000
@@ -381,6 +381,25 @@
         [diane] = Node.objects.filter(hostname='diane')
         self.assertEqual(architecture, diane.architecture)
 
+    def test_POST_new_generates_hostname_if_ip_based_hostname(self):
+        hostname = '192-168-5-19.domain'
+        response = self.client.post(
+            self.get_uri('nodes/'),
+            {
+                'op': 'new',
+                'hostname': hostname,
+                'architecture': factory.getRandomChoice(ARCHITECTURE_CHOICES),
+                'after_commissioning_action':
+                    NODE_AFTER_COMMISSIONING_ACTION.DEFAULT,
+                'mac_addresses': [factory.getRandomMACAddress()],
+            })
+        parsed_result = json.loads(response.content)
+
+        self.assertEqual(httplib.OK, response.status_code)
+        system_id = parsed_result.get('system_id')
+        node = Node.objects.get(system_id=system_id)
+        self.assertNotEqual(hostname, node.hostname)
+
     def test_POST_new_creates_node_with_power_parameters(self):
         # We're setting power parameters so we disable start_commissioning to
         # prevent anything from attempting to issue power instructions.
@@ -618,6 +637,93 @@
         self.assertItemsEqual(['architecture'], parsed_result)
 
 
+class NodeHostnameTest(APIv10TestMixin, MultipleUsersScenarios, TestCase):
+
+    scenarios = [
+        ('user', dict(userfactory=factory.make_user)),
+        ('admin', dict(userfactory=factory.make_admin)),
+        ]
+
+    def test_GET_list_returns_fqdn_with_domain_name_from_cluster(self):
+        # If DNS management is enabled, the domain part of a hostname
+        # is replaced by the domain name defined on the cluster.
+        hostname_without_domain = factory.make_name('hostname')
+        hostname_with_domain = '%s.%s' % (
+            hostname_without_domain, factory.getRandomString())
+        domain = factory.make_name('domain')
+        nodegroup = factory.make_node_group(
+            status=NODEGROUP_STATUS.ACCEPTED,
+            name=domain,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
+        node = factory.make_node(
+            hostname=hostname_with_domain, nodegroup=nodegroup)
+        expected_hostname = '%s.%s' % (hostname_without_domain, domain)
+        response = self.client.get(self.get_uri('nodes/'), {'op': 'list'})
+        self.assertEqual(httplib.OK, response.status_code, response.content)
+        parsed_result = json.loads(response.content)
+        self.assertItemsEqual(
+            [expected_hostname],
+            [node.get('hostname') for node in parsed_result])
+
+
+class NodeHostnameEnlistmentTest(APIv10TestMixin, MultipleUsersScenarios,
+                                 TestCase):
+
+    scenarios = [
+        ('anon', dict(userfactory=lambda: AnonymousUser())),
+        ('user', dict(userfactory=factory.make_user)),
+        ('admin', dict(userfactory=factory.make_admin)),
+        ]
+
+    def test_created_node_has_domain_from_cluster(self):
+        hostname_without_domain = factory.make_name('hostname')
+        hostname_with_domain = '%s.%s' % (
+            hostname_without_domain, factory.getRandomString())
+        domain = factory.make_name('domain')
+        factory.make_node_group(
+            status=NODEGROUP_STATUS.ACCEPTED,
+            name=domain,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
+        response = self.client.post(
+            self.get_uri('nodes/'),
+            {
+                'op': 'new',
+                'hostname': hostname_with_domain,
+                'architecture': factory.getRandomChoice(ARCHITECTURE_CHOICES),
+                'after_commissioning_action':
+                    NODE_AFTER_COMMISSIONING_ACTION.DEFAULT,
+                'mac_addresses': [factory.getRandomMACAddress()],
+            })
+        self.assertEqual(httplib.OK, response.status_code, response.content)
+        parsed_result = json.loads(response.content)
+        expected_hostname = '%s.%s' % (hostname_without_domain, domain)
+        self.assertEqual(
+            expected_hostname, parsed_result.get('hostname'))
+
+    def test_created_node_gets_domain_from_cluster_appended(self):
+        hostname_without_domain = factory.make_name('hostname')
+        domain = factory.make_name('domain')
+        factory.make_node_group(
+            status=NODEGROUP_STATUS.ACCEPTED,
+            name=domain,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
+        response = self.client.post(
+            self.get_uri('nodes/'),
+            {
+                'op': 'new',
+                'hostname': hostname_without_domain,
+                'architecture': factory.getRandomChoice(ARCHITECTURE_CHOICES),
+                'after_commissioning_action':
+                    NODE_AFTER_COMMISSIONING_ACTION.DEFAULT,
+                'mac_addresses': [factory.getRandomMACAddress()],
+            })
+        self.assertEqual(httplib.OK, response.status_code, response.content)
+        parsed_result = json.loads(response.content)
+        expected_hostname = '%s.%s' % (hostname_without_domain, domain)
+        self.assertEqual(
+            expected_hostname, parsed_result.get('hostname'))
+
+
 class NonAdminEnlistmentAPITest(APIv10TestMixin, MultipleUsersScenarios,
                                 TestCase):
     # Enlistment tests for non-admin users.
@@ -684,6 +790,7 @@
                 'netboot',
                 'power_type',
                 'tag_names',
+                'resource_uri',
             ],
             list(parsed_result))
 
 
=== modified file 'src/maasserver/tests/test_dhcplease.py'
--- src/maasserver/tests/test_dhcplease.py	2012-10-09 10:30:10 +0000
+++ src/maasserver/tests/test_dhcplease.py	2012-10-30 15:17:24 +0000
@@ -14,7 +14,6 @@
 
 from maasserver import dns
 from maasserver.models import DHCPLease
-from maasserver.models.dhcplease import strip_domain
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import TestCase
 from maasserver.utils import ignore_unused
@@ -47,20 +46,6 @@
         self.assertEqual(mac, lease.mac)
 
 
-class TestUtitilies(TestCase):
-
-    def test_strip_domain(self):
-        input_and_results = [
-            ('name.domain',  'name'),
-            ('name', 'name'),
-            ('name.domain.what', 'name'),
-            ('name..domain', 'name'),
-            ]
-        inputs = [input for input, _ in input_and_results]
-        results = [result for _, result in input_and_results]
-        self.assertEqual(results, map(strip_domain, inputs))
-
-
 class TestDHCPLeaseManager(TestCase):
     """Tests for :class:`DHCPLeaseManager`."""
 
 
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py	2012-10-30 10:46:58 +0000
+++ src/maasserver/tests/test_forms.py	2012-10-30 15:17:24 +0000
@@ -112,14 +112,17 @@
         return query_dict
 
     def make_params(self, mac_addresses=None, architecture=None,
-                    nodegroup=None):
+                    hostname=None, nodegroup=None):
         if mac_addresses is None:
             mac_addresses = [factory.getRandomMACAddress()]
         if architecture is None:
             architecture = factory.getRandomEnum(ARCHITECTURE)
+        if hostname is None:
+            hostname = factory.make_name('hostname')
         params = {
             'mac_addresses': mac_addresses,
             'architecture': architecture,
+            'hostname': hostname,
         }
         if nodegroup is not None:
             params['nodegroup'] = nodegroup
@@ -218,6 +221,18 @@
         form.save()
         self.assertEqual(original_nodegroup, reload_object(node).nodegroup)
 
+    def test_form_without_hostname_generates_hostname(self):
+        form = NodeWithMACAddressesForm(self.make_params(hostname=''))
+        node = form.save()
+        self.assertTrue(len(node.hostname) > 0)
+
+    def test_form_with_ip_based_hostname_generates_hostname(self):
+        ip_based_hostname = '192-168-12-10.domain'
+        form = NodeWithMACAddressesForm(
+            self.make_params(hostname=ip_based_hostname))
+        node = form.save()
+        self.assertNotEqual(ip_based_hostname, node.hostname)
+
 
 class TestOptionForm(ConfigForm):
     field1 = forms.CharField(label="Field 1", max_length=10)
 
=== modified file 'src/maasserver/tests/test_node.py'
--- src/maasserver/tests/test_node.py	2012-10-24 14:59:57 +0000
+++ src/maasserver/tests/test_node.py	2012-10-30 15:17:24 +0000
@@ -26,6 +26,8 @@
     NODE_STATUS,
     NODE_STATUS_CHOICES,
     NODE_STATUS_CHOICES_DICT,
+    NODEGROUP_STATUS,
+    NODEGROUPINTERFACE_MANAGEMENT,
     )
 from maasserver.exceptions import NodeStateViolation
 from maasserver.models import (
@@ -570,6 +572,30 @@
         node = reload_object(node)
         self.assertEqual([], list(node.tags.all()))
 
+    def test_fqdn_returns_hostname_if_dns_not_managed(self):
+        nodegroup = factory.make_node_group(
+            name=factory.getRandomString(),
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
+        hostname_with_domain = '%s.%s' % (
+            factory.getRandomString(), factory.getRandomString())
+        node = factory.make_node(
+            nodegroup=nodegroup, hostname=hostname_with_domain)
+        self.assertEqual(hostname_with_domain, node.fqdn)
+
+    def test_fqdn_replaces_hostname_if_dns_is_managed(self):
+        hostname_without_domain = factory.make_name('hostname')
+        hostname_with_domain = '%s.%s' % (
+            hostname_without_domain, factory.getRandomString())
+        domain = factory.make_name('domain')
+        nodegroup = factory.make_node_group(
+            status=NODEGROUP_STATUS.ACCEPTED,
+            name=domain,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
+        node = factory.make_node(
+            hostname=hostname_with_domain, nodegroup=nodegroup)
+        expected_hostname = '%s.%s' % (hostname_without_domain, domain)
+        self.assertEqual(expected_hostname, node.fqdn)
+
 
 class NodeTransitionsTests(TestCase):
     """Test the structure of NODE_TRANSITIONS."""
 
=== modified file 'src/maasserver/utils/__init__.py'
--- src/maasserver/utils/__init__.py	2012-08-24 10:28:29 +0000
+++ src/maasserver/utils/__init__.py	2012-10-30 15:17:24 +0000
@@ -15,6 +15,7 @@
     'get_db_state',
     'ignore_unused',
     'map_enum',
+    'strip_domain',
     ]
 
 from urllib import urlencode
@@ -82,3 +83,8 @@
     if query is not None:
         url += '?%s' % urlencode(query, doseq=True)
     return url
+
+
+def strip_domain(hostname):
+    """Return `hostname` with the domain part removed."""
+    return hostname.split('.', 1)[0]
 
=== modified file 'src/maasserver/utils/tests/test_utils.py'
--- src/maasserver/utils/tests/test_utils.py	2012-06-26 16:36:10 +0000
+++ src/maasserver/utils/tests/test_utils.py	2012-10-30 15:17:24 +0000
@@ -23,6 +23,7 @@
     absolute_reverse,
     get_db_state,
     map_enum,
+    strip_domain,
     )
 from maastesting.testcase import TestCase
 
@@ -104,3 +105,17 @@
             NODE_STATUS_CHOICES, but_not=[status])
         node.status = another_status
         self.assertEqual(status, get_db_state(node, 'status'))
+
+
+class TestStripDomain(TestCase):
+
+    def test_strip_domain(self):
+        input_and_results = [
+            ('name.domain',  'name'),
+            ('name', 'name'),
+            ('name.domain.what', 'name'),
+            ('name..domain', 'name'),
+            ]
+        inputs = [input for input, _ in input_and_results]
+        results = [result for _, result in input_and_results]
+        self.assertEqual(results, map(strip_domain, inputs))