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

Proposed by Raphaël Badin on 2012-10-30
Status: Merged
Approved by: Raphaël Badin on 2012-10-30
Approved revision: 1281
Merged at revision: 1280
Proposed branch: lp:~rvb/maas/bug-1070765-hostname-1.2
Merge into: lp:maas/1.2
Diff against target: 507 lines (+236/-31)
11 files modified
src/maasserver/api.py (+19/-2)
src/maasserver/forms.py (+14/-1)
src/maasserver/models/dhcplease.py (+1/-5)
src/maasserver/models/node.py (+25/-2)
src/maasserver/models/nodegroup.py (+7/-5)
src/maasserver/tests/test_api.py (+107/-0)
src/maasserver/tests/test_dhcplease.py (+0/-15)
src/maasserver/tests/test_forms.py (+16/-1)
src/maasserver/tests/test_node.py (+26/-0)
src/maasserver/utils/__init__.py (+6/-0)
src/maasserver/utils/tests/test_utils.py (+15/-0)
To merge this branch: bzr merge lp:~rvb/maas/bug-1070765-hostname-1.2
Reviewer Review Type Date Requested Status
Raphaël Badin (community) Approve on 2012-10-30
Review via email: mp+132139@code.launchpad.net

Commit message

Backport [r1316..r1318].

Description of the change

Backport [r1316..r1318].

To post a comment you must log in.
Raphaël Badin (rvb) wrote :

Self-approving this as it is a straightforward backport.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/api.py'
2--- src/maasserver/api.py 2012-10-30 10:25:55 +0000
3+++ src/maasserver/api.py 2012-10-30 15:17:24 +0000
4@@ -165,6 +165,7 @@
5 from maasserver.utils import (
6 absolute_reverse,
7 map_enum,
8+ strip_domain,
9 )
10 from maasserver.utils.orm import get_one
11 from piston.handler import (
12@@ -474,6 +475,12 @@
13 model = Node
14 fields = DISPLAYED_NODE_FIELDS
15
16+ # Override the 'hostname' field so that it returns the FQDN instead as
17+ # this is used by Juju to reach that node.
18+ @classmethod
19+ def hostname(handler, node):
20+ return node.fqdn
21+
22 def read(self, request, system_id):
23 """Read a specific Node."""
24 return Node.objects.get_node_or_404(
25@@ -647,8 +654,15 @@
26 class AnonNodesHandler(AnonymousOperationsHandler):
27 """Create Nodes."""
28 create = read = update = delete = None
29+ model = Node
30 fields = DISPLAYED_NODE_FIELDS
31
32+ # Override the 'hostname' field so that it returns the FQDN instead as
33+ # this is used by Juju to reach that node.
34+ @classmethod
35+ def hostname(handler, node):
36+ return node.fqdn
37+
38 @operation(idempotent=False)
39 def new(self, request):
40 """Create a new Node.
41@@ -827,9 +841,12 @@
42 request.user, NODE_PERMISSION.VIEW, ids=match_ids)
43 if match_macs is not None:
44 nodes = nodes.filter(macaddress__mac_address__in=match_macs)
45- # Prefetch related macaddresses and tags.
46+ # Prefetch related macaddresses, tags and nodegroups (plus
47+ # related interfaces).
48 nodes = nodes.prefetch_related('macaddress_set__node')
49 nodes = nodes.prefetch_related('tags')
50+ nodes = nodes.prefetch_related('nodegroup')
51+ nodes = nodes.prefetch_related('nodegroup__nodegroupinterface_set')
52 return nodes.order_by('id')
53
54 @operation(idempotent=True)
55@@ -1708,7 +1725,7 @@
56 preseed_url = compose_preseed_url(node)
57 # The node's hostname may include a domain, but we ignore that
58 # and use the one from the nodegroup instead.
59- hostname = node.hostname.split('.', 1)[0]
60+ hostname = strip_domain(node.hostname)
61 domain = node.nodegroup.name
62 else:
63 try:
64
65=== modified file 'src/maasserver/forms.py'
66--- src/maasserver/forms.py 2012-10-30 11:29:20 +0000
67+++ src/maasserver/forms.py 2012-10-30 15:17:24 +0000
68@@ -31,6 +31,7 @@
69
70 import collections
71 import json
72+import re
73
74 from django import forms
75 from django.contrib import messages
76@@ -82,6 +83,7 @@
77 from maasserver.models.nodegroup import NODEGROUP_CLUSTER_NAME_TEMPLATE
78 from maasserver.node_action import compile_node_actions
79 from maasserver.power_parameters import POWER_TYPE_PARAMETERS
80+from maasserver.utils import strip_domain
81 from provisioningserver.enum import (
82 POWER_TYPE,
83 POWER_TYPE_CHOICES,
84@@ -334,6 +336,9 @@
85 node.nodegroup = form_value
86
87
88+IP_BASED_HOSTNAME_REGEXP = re.compile('\d{1,3}-\d{1,3}-\d{1,3}-\d{1,3}')
89+
90+
91 class WithMACAddressesMixin:
92 """A form mixin which dynamically adds a MultipleMACAddressField to the
93 list of fields. This mixin also overrides the 'save' method to persist
94@@ -389,7 +394,15 @@
95 node.save()
96 for mac in self.cleaned_data['mac_addresses']:
97 node.add_mac_address(mac)
98- if self.cleaned_data['hostname'] == "":
99+ hostname = self.cleaned_data['hostname']
100+ stripped_hostname = strip_domain(hostname)
101+ # Generate a hostname for this node if the provided hostname is
102+ # IP-based (because this means that this name comes from a DNS
103+ # reverse query to the MAAS DNS) or an empty string.
104+ generate_hostname = (
105+ hostname == "" or
106+ IP_BASED_HOSTNAME_REGEXP.match(stripped_hostname) != None)
107+ if generate_hostname:
108 node.set_mac_based_hostname(self.cleaned_data['mac_addresses'][0])
109 return node
110
111
112=== modified file 'src/maasserver/models/dhcplease.py'
113--- src/maasserver/models/dhcplease.py 2012-10-09 10:30:10 +0000
114+++ src/maasserver/models/dhcplease.py 2012-10-30 15:17:24 +0000
115@@ -25,11 +25,7 @@
116 from maasserver import DefaultMeta
117 from maasserver.fields import MACAddressField
118 from maasserver.models.cleansave import CleanSave
119-
120-
121-def strip_domain(hostname):
122- """Return `hostname` with the domain part removed."""
123- return hostname.split('.', 1)[0]
124+from maasserver.utils import strip_domain
125
126
127 class DHCPLeaseManager(Manager):
128
129=== modified file 'src/maasserver/models/node.py'
130--- src/maasserver/models/node.py 2012-10-26 10:20:19 +0000
131+++ src/maasserver/models/node.py 2012-10-30 15:17:24 +0000
132@@ -63,7 +63,10 @@
133 from maasserver.models.dhcplease import DHCPLease
134 from maasserver.models.tag import Tag
135 from maasserver.models.timestampedmodel import TimestampedModel
136-from maasserver.utils import get_db_state
137+from maasserver.utils import (
138+ get_db_state,
139+ strip_domain,
140+ )
141 from maasserver.utils.orm import get_first
142 from piston.models import Token
143 from provisioningserver.enum import (
144@@ -487,10 +490,30 @@
145
146 def __unicode__(self):
147 if self.hostname:
148- return "%s (%s)" % (self.system_id, self.hostname)
149+ return "%s (%s)" % (self.system_id, self.fqdn)
150 else:
151 return self.system_id
152
153+ @property
154+ def fqdn(self):
155+ """Fully qualified domain name for this node.
156+
157+ If MAAS manages DNS for this node, the domain part of the
158+ hostname (if present), is replaced by the domain configured
159+ on the cluster controller.
160+ If not, simply return the node's hostname.
161+ """
162+ # Avoid circular imports.
163+ from maasserver.dns import is_dns_managed
164+ if is_dns_managed(self.nodegroup):
165+ # If the hostname field contains a domain, strip it.
166+ hostname = strip_domain(self.hostname)
167+ # Build the FQDN by using the hostname and nodegroup.name
168+ # as the domain name.
169+ return '%s.%s' % (hostname, self.nodegroup.name)
170+ else:
171+ return self.hostname
172+
173 def tag_names(self):
174 # We don't use self.tags.values_list here because this does not
175 # take advantage of the cache.
176
177=== modified file 'src/maasserver/models/nodegroup.py'
178--- src/maasserver/models/nodegroup.py 2012-10-11 10:59:40 +0000
179+++ src/maasserver/models/nodegroup.py 2012-10-30 15:17:24 +0000
180@@ -31,7 +31,6 @@
181 from maasserver.models.nodegroupinterface import NodeGroupInterface
182 from maasserver.models.timestampedmodel import TimestampedModel
183 from maasserver.refresh_worker import refresh_worker
184-from maasserver.utils.orm import get_one
185 from piston.models import (
186 KEY_SIZE,
187 Token,
188@@ -196,10 +195,13 @@
189 This is a temporary method that should be refactored once we add
190 proper support for multiple interfaces on a nodegroup.
191 """
192- return get_one(
193- NodeGroupInterface.objects.filter(
194- nodegroup=self).exclude(
195- management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED))
196+ # Iterate over all the interfaces in python instead of doing the
197+ # filtering in SQL so that this will use the cached version of
198+ # self.nodegroupinterface_set if it is there.
199+ for interface in self.nodegroupinterface_set.all():
200+ if interface.management != NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED:
201+ return interface
202+ return None
203
204 def ensure_dhcp_key(self):
205 """Ensure that this nodegroup has a dhcp key.
206
207=== modified file 'src/maasserver/tests/test_api.py'
208--- src/maasserver/tests/test_api.py 2012-10-30 11:00:20 +0000
209+++ src/maasserver/tests/test_api.py 2012-10-30 15:17:24 +0000
210@@ -381,6 +381,25 @@
211 [diane] = Node.objects.filter(hostname='diane')
212 self.assertEqual(architecture, diane.architecture)
213
214+ def test_POST_new_generates_hostname_if_ip_based_hostname(self):
215+ hostname = '192-168-5-19.domain'
216+ response = self.client.post(
217+ self.get_uri('nodes/'),
218+ {
219+ 'op': 'new',
220+ 'hostname': hostname,
221+ 'architecture': factory.getRandomChoice(ARCHITECTURE_CHOICES),
222+ 'after_commissioning_action':
223+ NODE_AFTER_COMMISSIONING_ACTION.DEFAULT,
224+ 'mac_addresses': [factory.getRandomMACAddress()],
225+ })
226+ parsed_result = json.loads(response.content)
227+
228+ self.assertEqual(httplib.OK, response.status_code)
229+ system_id = parsed_result.get('system_id')
230+ node = Node.objects.get(system_id=system_id)
231+ self.assertNotEqual(hostname, node.hostname)
232+
233 def test_POST_new_creates_node_with_power_parameters(self):
234 # We're setting power parameters so we disable start_commissioning to
235 # prevent anything from attempting to issue power instructions.
236@@ -618,6 +637,93 @@
237 self.assertItemsEqual(['architecture'], parsed_result)
238
239
240+class NodeHostnameTest(APIv10TestMixin, MultipleUsersScenarios, TestCase):
241+
242+ scenarios = [
243+ ('user', dict(userfactory=factory.make_user)),
244+ ('admin', dict(userfactory=factory.make_admin)),
245+ ]
246+
247+ def test_GET_list_returns_fqdn_with_domain_name_from_cluster(self):
248+ # If DNS management is enabled, the domain part of a hostname
249+ # is replaced by the domain name defined on the cluster.
250+ hostname_without_domain = factory.make_name('hostname')
251+ hostname_with_domain = '%s.%s' % (
252+ hostname_without_domain, factory.getRandomString())
253+ domain = factory.make_name('domain')
254+ nodegroup = factory.make_node_group(
255+ status=NODEGROUP_STATUS.ACCEPTED,
256+ name=domain,
257+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
258+ node = factory.make_node(
259+ hostname=hostname_with_domain, nodegroup=nodegroup)
260+ expected_hostname = '%s.%s' % (hostname_without_domain, domain)
261+ response = self.client.get(self.get_uri('nodes/'), {'op': 'list'})
262+ self.assertEqual(httplib.OK, response.status_code, response.content)
263+ parsed_result = json.loads(response.content)
264+ self.assertItemsEqual(
265+ [expected_hostname],
266+ [node.get('hostname') for node in parsed_result])
267+
268+
269+class NodeHostnameEnlistmentTest(APIv10TestMixin, MultipleUsersScenarios,
270+ TestCase):
271+
272+ scenarios = [
273+ ('anon', dict(userfactory=lambda: AnonymousUser())),
274+ ('user', dict(userfactory=factory.make_user)),
275+ ('admin', dict(userfactory=factory.make_admin)),
276+ ]
277+
278+ def test_created_node_has_domain_from_cluster(self):
279+ hostname_without_domain = factory.make_name('hostname')
280+ hostname_with_domain = '%s.%s' % (
281+ hostname_without_domain, factory.getRandomString())
282+ domain = factory.make_name('domain')
283+ factory.make_node_group(
284+ status=NODEGROUP_STATUS.ACCEPTED,
285+ name=domain,
286+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
287+ response = self.client.post(
288+ self.get_uri('nodes/'),
289+ {
290+ 'op': 'new',
291+ 'hostname': hostname_with_domain,
292+ 'architecture': factory.getRandomChoice(ARCHITECTURE_CHOICES),
293+ 'after_commissioning_action':
294+ NODE_AFTER_COMMISSIONING_ACTION.DEFAULT,
295+ 'mac_addresses': [factory.getRandomMACAddress()],
296+ })
297+ self.assertEqual(httplib.OK, response.status_code, response.content)
298+ parsed_result = json.loads(response.content)
299+ expected_hostname = '%s.%s' % (hostname_without_domain, domain)
300+ self.assertEqual(
301+ expected_hostname, parsed_result.get('hostname'))
302+
303+ def test_created_node_gets_domain_from_cluster_appended(self):
304+ hostname_without_domain = factory.make_name('hostname')
305+ domain = factory.make_name('domain')
306+ factory.make_node_group(
307+ status=NODEGROUP_STATUS.ACCEPTED,
308+ name=domain,
309+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
310+ response = self.client.post(
311+ self.get_uri('nodes/'),
312+ {
313+ 'op': 'new',
314+ 'hostname': hostname_without_domain,
315+ 'architecture': factory.getRandomChoice(ARCHITECTURE_CHOICES),
316+ 'after_commissioning_action':
317+ NODE_AFTER_COMMISSIONING_ACTION.DEFAULT,
318+ 'mac_addresses': [factory.getRandomMACAddress()],
319+ })
320+ self.assertEqual(httplib.OK, response.status_code, response.content)
321+ parsed_result = json.loads(response.content)
322+ expected_hostname = '%s.%s' % (hostname_without_domain, domain)
323+ self.assertEqual(
324+ expected_hostname, parsed_result.get('hostname'))
325+
326+
327 class NonAdminEnlistmentAPITest(APIv10TestMixin, MultipleUsersScenarios,
328 TestCase):
329 # Enlistment tests for non-admin users.
330@@ -684,6 +790,7 @@
331 'netboot',
332 'power_type',
333 'tag_names',
334+ 'resource_uri',
335 ],
336 list(parsed_result))
337
338
339=== modified file 'src/maasserver/tests/test_dhcplease.py'
340--- src/maasserver/tests/test_dhcplease.py 2012-10-09 10:30:10 +0000
341+++ src/maasserver/tests/test_dhcplease.py 2012-10-30 15:17:24 +0000
342@@ -14,7 +14,6 @@
343
344 from maasserver import dns
345 from maasserver.models import DHCPLease
346-from maasserver.models.dhcplease import strip_domain
347 from maasserver.testing.factory import factory
348 from maasserver.testing.testcase import TestCase
349 from maasserver.utils import ignore_unused
350@@ -47,20 +46,6 @@
351 self.assertEqual(mac, lease.mac)
352
353
354-class TestUtitilies(TestCase):
355-
356- def test_strip_domain(self):
357- input_and_results = [
358- ('name.domain', 'name'),
359- ('name', 'name'),
360- ('name.domain.what', 'name'),
361- ('name..domain', 'name'),
362- ]
363- inputs = [input for input, _ in input_and_results]
364- results = [result for _, result in input_and_results]
365- self.assertEqual(results, map(strip_domain, inputs))
366-
367-
368 class TestDHCPLeaseManager(TestCase):
369 """Tests for :class:`DHCPLeaseManager`."""
370
371
372=== modified file 'src/maasserver/tests/test_forms.py'
373--- src/maasserver/tests/test_forms.py 2012-10-30 10:46:58 +0000
374+++ src/maasserver/tests/test_forms.py 2012-10-30 15:17:24 +0000
375@@ -112,14 +112,17 @@
376 return query_dict
377
378 def make_params(self, mac_addresses=None, architecture=None,
379- nodegroup=None):
380+ hostname=None, nodegroup=None):
381 if mac_addresses is None:
382 mac_addresses = [factory.getRandomMACAddress()]
383 if architecture is None:
384 architecture = factory.getRandomEnum(ARCHITECTURE)
385+ if hostname is None:
386+ hostname = factory.make_name('hostname')
387 params = {
388 'mac_addresses': mac_addresses,
389 'architecture': architecture,
390+ 'hostname': hostname,
391 }
392 if nodegroup is not None:
393 params['nodegroup'] = nodegroup
394@@ -218,6 +221,18 @@
395 form.save()
396 self.assertEqual(original_nodegroup, reload_object(node).nodegroup)
397
398+ def test_form_without_hostname_generates_hostname(self):
399+ form = NodeWithMACAddressesForm(self.make_params(hostname=''))
400+ node = form.save()
401+ self.assertTrue(len(node.hostname) > 0)
402+
403+ def test_form_with_ip_based_hostname_generates_hostname(self):
404+ ip_based_hostname = '192-168-12-10.domain'
405+ form = NodeWithMACAddressesForm(
406+ self.make_params(hostname=ip_based_hostname))
407+ node = form.save()
408+ self.assertNotEqual(ip_based_hostname, node.hostname)
409+
410
411 class TestOptionForm(ConfigForm):
412 field1 = forms.CharField(label="Field 1", max_length=10)
413
414=== modified file 'src/maasserver/tests/test_node.py'
415--- src/maasserver/tests/test_node.py 2012-10-24 14:59:57 +0000
416+++ src/maasserver/tests/test_node.py 2012-10-30 15:17:24 +0000
417@@ -26,6 +26,8 @@
418 NODE_STATUS,
419 NODE_STATUS_CHOICES,
420 NODE_STATUS_CHOICES_DICT,
421+ NODEGROUP_STATUS,
422+ NODEGROUPINTERFACE_MANAGEMENT,
423 )
424 from maasserver.exceptions import NodeStateViolation
425 from maasserver.models import (
426@@ -570,6 +572,30 @@
427 node = reload_object(node)
428 self.assertEqual([], list(node.tags.all()))
429
430+ def test_fqdn_returns_hostname_if_dns_not_managed(self):
431+ nodegroup = factory.make_node_group(
432+ name=factory.getRandomString(),
433+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
434+ hostname_with_domain = '%s.%s' % (
435+ factory.getRandomString(), factory.getRandomString())
436+ node = factory.make_node(
437+ nodegroup=nodegroup, hostname=hostname_with_domain)
438+ self.assertEqual(hostname_with_domain, node.fqdn)
439+
440+ def test_fqdn_replaces_hostname_if_dns_is_managed(self):
441+ hostname_without_domain = factory.make_name('hostname')
442+ hostname_with_domain = '%s.%s' % (
443+ hostname_without_domain, factory.getRandomString())
444+ domain = factory.make_name('domain')
445+ nodegroup = factory.make_node_group(
446+ status=NODEGROUP_STATUS.ACCEPTED,
447+ name=domain,
448+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
449+ node = factory.make_node(
450+ hostname=hostname_with_domain, nodegroup=nodegroup)
451+ expected_hostname = '%s.%s' % (hostname_without_domain, domain)
452+ self.assertEqual(expected_hostname, node.fqdn)
453+
454
455 class NodeTransitionsTests(TestCase):
456 """Test the structure of NODE_TRANSITIONS."""
457
458=== modified file 'src/maasserver/utils/__init__.py'
459--- src/maasserver/utils/__init__.py 2012-08-24 10:28:29 +0000
460+++ src/maasserver/utils/__init__.py 2012-10-30 15:17:24 +0000
461@@ -15,6 +15,7 @@
462 'get_db_state',
463 'ignore_unused',
464 'map_enum',
465+ 'strip_domain',
466 ]
467
468 from urllib import urlencode
469@@ -82,3 +83,8 @@
470 if query is not None:
471 url += '?%s' % urlencode(query, doseq=True)
472 return url
473+
474+
475+def strip_domain(hostname):
476+ """Return `hostname` with the domain part removed."""
477+ return hostname.split('.', 1)[0]
478
479=== modified file 'src/maasserver/utils/tests/test_utils.py'
480--- src/maasserver/utils/tests/test_utils.py 2012-06-26 16:36:10 +0000
481+++ src/maasserver/utils/tests/test_utils.py 2012-10-30 15:17:24 +0000
482@@ -23,6 +23,7 @@
483 absolute_reverse,
484 get_db_state,
485 map_enum,
486+ strip_domain,
487 )
488 from maastesting.testcase import TestCase
489
490@@ -104,3 +105,17 @@
491 NODE_STATUS_CHOICES, but_not=[status])
492 node.status = another_status
493 self.assertEqual(status, get_db_state(node, 'status'))
494+
495+
496+class TestStripDomain(TestCase):
497+
498+ def test_strip_domain(self):
499+ input_and_results = [
500+ ('name.domain', 'name'),
501+ ('name', 'name'),
502+ ('name.domain.what', 'name'),
503+ ('name..domain', 'name'),
504+ ]
505+ inputs = [input for input, _ in input_and_results]
506+ results = [result for _, result in input_and_results]
507+ self.assertEqual(results, map(strip_domain, inputs))

Subscribers

People subscribed via source and target branches

to status/vote changes: