Merge lp:~rvb/maas/use-fqdn into lp:maas/trunk

Proposed by Raphaël Badin on 2012-10-29
Status: Merged
Approved by: Raphaël Badin on 2012-10-30
Approved revision: 1310
Merged at revision: 1318
Proposed branch: lp:~rvb/maas/use-fqdn
Merge into: lp:maas/trunk
Prerequisite: lp:~rvb/maas/add-fqdn
Diff against target: 194 lines (+113/-7)
4 files modified
src/maasserver/api.py (+17/-1)
src/maasserver/models/node.py (+1/-1)
src/maasserver/models/nodegroup.py (+7/-5)
src/maasserver/tests/test_api.py (+88/-0)
To merge this branch: bzr merge lp:~rvb/maas/use-fqdn
Reviewer Review Type Date Requested Status
Julian Edwards (community) Approve on 2012-10-30
Jeroen T. Vermeulen 2012-10-29 Pending
Review via email: mp+131860@code.launchpad.net

Commit message

Use node.fqdn in the api in lieu of the hostname.

Description of the change

Use node.fqdn in the api in lieu of the hostname.

= Notes =

You might wonder why I had to do "model = Node" in AnonNodesHandler: this is to make sure that this handler is registered as the node handler used by anon requests so that the hostname replacement trick also works for anon requests. The side effect is that resource_uri is now included in the anon registration response but I think this is an improvement.

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

As discussed, add a test to show that having a hostname without the domain appended still returns the hostname + domain name from the nodegroup.

cheers!

review: Approve
lp:~rvb/maas/use-fqdn updated on 2012-10-30
1310. By Raphaël Badin on 2012-10-30

Add test.

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:10:26 +0000
3+++ src/maasserver/api.py 2012-10-30 10:10:26 +0000
4@@ -478,6 +478,12 @@
5 model = Node
6 fields = DISPLAYED_NODE_FIELDS
7
8+ # Override the 'hostname' field so that it returns the FQDN instead as
9+ # this is used by Juju to reach that node.
10+ @classmethod
11+ def hostname(handler, node):
12+ return node.fqdn
13+
14 def read(self, request, system_id):
15 """Read a specific Node."""
16 return Node.objects.get_node_or_404(
17@@ -651,8 +657,15 @@
18 class AnonNodesHandler(AnonymousOperationsHandler):
19 """Anonymous access to Nodes."""
20 create = read = update = delete = None
21+ model = Node
22 fields = DISPLAYED_NODE_FIELDS
23
24+ # Override the 'hostname' field so that it returns the FQDN instead as
25+ # this is used by Juju to reach that node.
26+ @classmethod
27+ def hostname(handler, node):
28+ return node.fqdn
29+
30 @operation(idempotent=False)
31 def new(self, request):
32 """Create a new Node.
33@@ -876,9 +889,12 @@
34 request.user, NODE_PERMISSION.VIEW, ids=match_ids)
35 if match_macs is not None:
36 nodes = nodes.filter(macaddress__mac_address__in=match_macs)
37- # Prefetch related macaddresses and tags.
38+ # Prefetch related macaddresses, tags and nodegroups (plus
39+ # related interfaces).
40 nodes = nodes.prefetch_related('macaddress_set__node')
41 nodes = nodes.prefetch_related('tags')
42+ nodes = nodes.prefetch_related('nodegroup')
43+ nodes = nodes.prefetch_related('nodegroup__nodegroupinterface_set')
44 return nodes.order_by('id')
45
46 @operation(idempotent=True)
47
48=== modified file 'src/maasserver/models/node.py'
49--- src/maasserver/models/node.py 2012-10-30 10:10:26 +0000
50+++ src/maasserver/models/node.py 2012-10-30 10:10:26 +0000
51@@ -490,7 +490,7 @@
52
53 def __unicode__(self):
54 if self.hostname:
55- return "%s (%s)" % (self.system_id, self.hostname)
56+ return "%s (%s)" % (self.system_id, self.fqdn)
57 else:
58 return self.system_id
59
60
61=== modified file 'src/maasserver/models/nodegroup.py'
62--- src/maasserver/models/nodegroup.py 2012-10-11 10:59:40 +0000
63+++ src/maasserver/models/nodegroup.py 2012-10-30 10:10:26 +0000
64@@ -31,7 +31,6 @@
65 from maasserver.models.nodegroupinterface import NodeGroupInterface
66 from maasserver.models.timestampedmodel import TimestampedModel
67 from maasserver.refresh_worker import refresh_worker
68-from maasserver.utils.orm import get_one
69 from piston.models import (
70 KEY_SIZE,
71 Token,
72@@ -196,10 +195,13 @@
73 This is a temporary method that should be refactored once we add
74 proper support for multiple interfaces on a nodegroup.
75 """
76- return get_one(
77- NodeGroupInterface.objects.filter(
78- nodegroup=self).exclude(
79- management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED))
80+ # Iterate over all the interfaces in python instead of doing the
81+ # filtering in SQL so that this will use the cached version of
82+ # self.nodegroupinterface_set if it is there.
83+ for interface in self.nodegroupinterface_set.all():
84+ if interface.management != NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED:
85+ return interface
86+ return None
87
88 def ensure_dhcp_key(self):
89 """Ensure that this nodegroup has a dhcp key.
90
91=== modified file 'src/maasserver/tests/test_api.py'
92--- src/maasserver/tests/test_api.py 2012-10-30 10:10:26 +0000
93+++ src/maasserver/tests/test_api.py 2012-10-30 10:10:26 +0000
94@@ -637,6 +637,93 @@
95 self.assertItemsEqual(['architecture'], parsed_result)
96
97
98+class NodeHostnameTest(APIv10TestMixin, MultipleUsersScenarios, TestCase):
99+
100+ scenarios = [
101+ ('user', dict(userfactory=factory.make_user)),
102+ ('admin', dict(userfactory=factory.make_admin)),
103+ ]
104+
105+ def test_GET_list_returns_fqdn_with_domain_name_from_cluster(self):
106+ # If DNS management is enabled, the domain part of a hostname
107+ # is replaced by the domain name defined on the cluster.
108+ hostname_without_domain = factory.make_name('hostname')
109+ hostname_with_domain = '%s.%s' % (
110+ hostname_without_domain, factory.getRandomString())
111+ domain = factory.make_name('domain')
112+ nodegroup = factory.make_node_group(
113+ status=NODEGROUP_STATUS.ACCEPTED,
114+ name=domain,
115+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
116+ node = factory.make_node(
117+ hostname=hostname_with_domain, nodegroup=nodegroup)
118+ expected_hostname = '%s.%s' % (hostname_without_domain, domain)
119+ response = self.client.get(self.get_uri('nodes/'), {'op': 'list'})
120+ self.assertEqual(httplib.OK, response.status_code, response.content)
121+ parsed_result = json.loads(response.content)
122+ self.assertItemsEqual(
123+ [expected_hostname],
124+ [node.get('hostname') for node in parsed_result])
125+
126+
127+class NodeHostnameEnlistmentTest(APIv10TestMixin, MultipleUsersScenarios,
128+ TestCase):
129+
130+ scenarios = [
131+ ('anon', dict(userfactory=lambda: AnonymousUser())),
132+ ('user', dict(userfactory=factory.make_user)),
133+ ('admin', dict(userfactory=factory.make_admin)),
134+ ]
135+
136+ def test_created_node_has_domain_from_cluster(self):
137+ hostname_without_domain = factory.make_name('hostname')
138+ hostname_with_domain = '%s.%s' % (
139+ hostname_without_domain, factory.getRandomString())
140+ domain = factory.make_name('domain')
141+ factory.make_node_group(
142+ status=NODEGROUP_STATUS.ACCEPTED,
143+ name=domain,
144+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
145+ response = self.client.post(
146+ self.get_uri('nodes/'),
147+ {
148+ 'op': 'new',
149+ 'hostname': hostname_with_domain,
150+ 'architecture': factory.getRandomChoice(ARCHITECTURE_CHOICES),
151+ 'after_commissioning_action':
152+ NODE_AFTER_COMMISSIONING_ACTION.DEFAULT,
153+ 'mac_addresses': [factory.getRandomMACAddress()],
154+ })
155+ self.assertEqual(httplib.OK, response.status_code, response.content)
156+ parsed_result = json.loads(response.content)
157+ expected_hostname = '%s.%s' % (hostname_without_domain, domain)
158+ self.assertEqual(
159+ expected_hostname, parsed_result.get('hostname'))
160+
161+ def test_created_node_gets_domain_from_cluster_appended(self):
162+ hostname_without_domain = factory.make_name('hostname')
163+ domain = factory.make_name('domain')
164+ factory.make_node_group(
165+ status=NODEGROUP_STATUS.ACCEPTED,
166+ name=domain,
167+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
168+ response = self.client.post(
169+ self.get_uri('nodes/'),
170+ {
171+ 'op': 'new',
172+ 'hostname': hostname_without_domain,
173+ 'architecture': factory.getRandomChoice(ARCHITECTURE_CHOICES),
174+ 'after_commissioning_action':
175+ NODE_AFTER_COMMISSIONING_ACTION.DEFAULT,
176+ 'mac_addresses': [factory.getRandomMACAddress()],
177+ })
178+ self.assertEqual(httplib.OK, response.status_code, response.content)
179+ parsed_result = json.loads(response.content)
180+ expected_hostname = '%s.%s' % (hostname_without_domain, domain)
181+ self.assertEqual(
182+ expected_hostname, parsed_result.get('hostname'))
183+
184+
185 class NonAdminEnlistmentAPITest(APIv10TestMixin, MultipleUsersScenarios,
186 TestCase):
187 # Enlistment tests for non-admin users.
188@@ -703,6 +790,7 @@
189 'netboot',
190 'power_type',
191 'tag_names',
192+ 'resource_uri',
193 ],
194 list(parsed_result))
195