launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13895
[Merge] lp:~rvb/maas/bug-1070765-hostname-1.2 into lp:maas/1.2
Raphaël Badin has proposed merging lp:~rvb/maas/bug-1070765-hostname-1.2 into lp:maas/1.2.
Commit message:
Backport [r1316..r1318].
Requested reviews:
MAAS Maintainers (maas-maintainers)
Related bugs:
Bug #1070765 in MAAS: "DNS forward zone ends up with nonsensical entries"
https://bugs.launchpad.net/maas/+bug/1070765
For more details, see:
https://code.launchpad.net/~rvb/maas/bug-1070765-hostname-1.2/+merge/132139
Backport [r1316..r1318].
--
https://code.launchpad.net/~rvb/maas/bug-1070765-hostname-1.2/+merge/132139
Your team MAAS Maintainers is requested to review the proposed merge of lp:~rvb/maas/bug-1070765-hostname-1.2 into lp:maas/1.2.
=== 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))
Follow ups
