MAAS

Merge lp:~rvb/maas/1081701-c-bis into lp:~maas-committers/maas/trunk

  1. 1081701-c-bis
  2. Merge into trunk
Proposed by Raphaël Badin
Status: Merged
Approved by: Raphaël Badin
Approved revision: no longer in the source branch.
Merged at revision: 1381
Proposed branch: lp:~rvb/maas/1081701-c-bis
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 357 lines (+77/-39)
8 files modified
src/maasserver/api.py (+4/-4)
src/maasserver/preseed.py (+23/-17)
src/maasserver/tests/test_api.py (+26/-2)
src/maasserver/tests/test_dns.py (+2/-2)
src/maasserver/tests/test_preseed.py (+11/-9)
src/maasserver/tests/test_server_address.py (+3/-3)
src/maastesting/factory.py (+7/-0)
src/metadataserver/api.py (+1/-2)
To merge this branch: bzr merge lp:~rvb/maas/1081701-c-bis
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+136357@code.launchpad.net

Commit message

Fix the value of 'server_host' in the preseed.

Description of the change

This fixes a bug in the preseed generation. I just spotted that the preseed was still using "server_host = get_maas_facing_server_host()" to populate the 'server_host' field instead of "server_host = get_maas_facing_server_host(nodegroup)".

This was not caught by the tests in place for a stupid reason: "self.assertThat(preseed, MatchesAll(*[Contains(ng_url), Not(Contains(maas_url))]))" did not caught the problem because the value of 'server_host' is the hostname part of 'maas_url' but since it went trough urlparse.urlparse, it was lowercased. To avoid that kind of problem in the future, I created a utility method to create hostname (factory.make_hostname) which will return lowercase hostnames.

To fix that I had to refactor the preseed code a bit: instead of passing the url around, we pass the nodegroup. I think it's all for the best: extracting maas_url from nodegroup is done only in the preseed code.

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) :
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-11-26 16:50:14 +0000
3+++ src/maasserver/api.py 2012-11-27 16:03:23 +0000
4@@ -1630,7 +1630,8 @@
5 # The node's hostname may include a domain, but we ignore that
6 # and use the one from the nodegroup instead.
7 hostname = strip_domain(node.hostname)
8- domain = node.nodegroup.name
9+ nodegroup = node.nodegroup
10+ domain = nodegroup.name
11 else:
12 try:
13 pxelinux_arch = request.GET['arch']
14@@ -1665,8 +1666,7 @@
15 subarch = pxelinux_subarch
16
17 nodegroup = find_nodegroup(request)
18- base_url = nodegroup.maas_url if nodegroup is not None else None
19- preseed_url = compose_enlistment_preseed_url(base_url=base_url)
20+ preseed_url = compose_enlistment_preseed_url(nodegroup=nodegroup)
21 hostname = 'maas-enlist'
22 domain = Config.objects.get_config('enlistment_domain')
23
24@@ -1683,7 +1683,7 @@
25 extra_kernel_opts = None
26
27 purpose = get_boot_purpose(node)
28- server_address = get_maas_facing_server_address()
29+ server_address = get_maas_facing_server_address(nodegroup=nodegroup)
30 cluster_address = get_mandatory_param(request.GET, "local")
31
32 params = KernelParameters(
33
34=== modified file 'src/maasserver/preseed.py'
35--- src/maasserver/preseed.py 2012-11-23 14:35:53 +0000
36+++ src/maasserver/preseed.py 2012-11-27 16:03:23 +0000
37@@ -38,24 +38,26 @@
38 GENERIC_FILENAME = 'generic'
39
40
41-def get_enlist_preseed(base_url=''):
42+def get_enlist_preseed(nodegroup=None):
43 """Return the enlistment preseed.
44
45+ :param nodegroup: The nodegroup used to generate the preseed.
46 :return: The rendered preseed string.
47 :rtype: basestring.
48 """
49 return render_enlistment_preseed(
50- PRESEED_TYPE.ENLIST, base_url=base_url)
51-
52-
53-def get_enlist_userdata(base_url=''):
54+ PRESEED_TYPE.ENLIST, nodegroup=nodegroup)
55+
56+
57+def get_enlist_userdata(nodegroup=None):
58 """Return the enlistment preseed.
59
60+ :param nodegroup: The nodegroup used to generate the preseed.
61 :return: The rendered enlistment user-data string.
62 :rtype: basestring.
63 """
64 return render_enlistment_preseed(
65- PRESEED_TYPE.ENLIST_USERDATA, base_url=base_url)
66+ PRESEED_TYPE.ENLIST_USERDATA, nodegroup=nodegroup)
67
68
69 def get_preseed(node):
70@@ -209,21 +211,21 @@
71 return parsed_url.hostname, parsed_url.path
72
73
74-def get_preseed_context(release='', base_url=''):
75+def get_preseed_context(release='', nodegroup=None):
76 """Return the node-independent context dictionary to be used to render
77 preseed templates.
78
79- :param node: See `get_preseed_filenames`.
80- :param prefix: See `get_preseed_filenames`.
81 :param release: See `get_preseed_filenames`.
82+ :param nodegroup: The nodegroup used to generate the preseed.
83 :return: The context dictionary.
84 :rtype: dict.
85 """
86- server_host = get_maas_facing_server_host()
87+ server_host = get_maas_facing_server_host(nodegroup=nodegroup)
88 main_archive_hostname, main_archive_directory = get_hostname_and_path(
89 Config.objects.get_config('main_archive'))
90 ports_archive_hostname, ports_archive_directory = get_hostname_and_path(
91 Config.objects.get_config('ports_archive'))
92+ base_url = nodegroup.maas_url if nodegroup is not None else None
93 return {
94 'main_archive_hostname': main_archive_hostname,
95 'main_archive_directory': main_archive_directory,
96@@ -242,7 +244,6 @@
97 preseed templates.
98
99 :param node: See `get_preseed_filenames`.
100- :param prefix: See `get_preseed_filenames`.
101 :param release: See `get_preseed_filenames`.
102 :return: The context dictionary.
103 :rtype: dict.
104@@ -261,16 +262,17 @@
105 }
106
107
108-def render_enlistment_preseed(prefix, release='', base_url=''):
109+def render_enlistment_preseed(prefix, release='', nodegroup=None):
110 """Return the enlistment preseed.
111
112 :param prefix: See `get_preseed_filenames`.
113 :param release: See `get_preseed_filenames`.
114+ :param nodegroup: The nodegroup used to generate the preseed.
115 :return: The rendered preseed string.
116 :rtype: basestring.
117 """
118 template = load_preseed_template(None, prefix, release)
119- context = get_preseed_context(release, base_url=base_url)
120+ context = get_preseed_context(release, nodegroup=nodegroup)
121 return template.substitute(**context)
122
123
124@@ -284,15 +286,19 @@
125 :rtype: basestring.
126 """
127 template = load_preseed_template(node, prefix, release)
128- base_url = node.nodegroup.maas_url
129- context = get_preseed_context(release, base_url=base_url)
130+ nodegroup = node.nodegroup
131+ context = get_preseed_context(release, nodegroup=nodegroup)
132 context.update(get_node_preseed_context(node, release))
133 return template.substitute(**context)
134
135
136-def compose_enlistment_preseed_url(base_url=''):
137- """Compose enlistment preseed URL."""
138+def compose_enlistment_preseed_url(nodegroup=None):
139+ """Compose enlistment preseed URL.
140+
141+ :param nodegroup: The nodegroup used to generate the preseed.
142+ """
143 # Always uses the latest version of the metadata API.
144+ base_url = nodegroup.maas_url if nodegroup is not None else None
145 version = 'latest'
146 return absolute_reverse(
147 'metadata-enlist-preseed', args=[version],
148
149=== modified file 'src/maasserver/tests/test_api.py'
150--- src/maasserver/tests/test_api.py 2012-11-27 09:51:35 +0000
151+++ src/maasserver/tests/test_api.py 2012-11-27 16:03:23 +0000
152@@ -48,7 +48,10 @@
153 EnvironmentVariableFixture,
154 Fixture,
155 )
156-from maasserver import api
157+from maasserver import (
158+ api,
159+ server_address,
160+ )
161 from maasserver.api import (
162 describe,
163 DISPLAYED_NODEGROUP_FIELDS,
164@@ -3438,9 +3441,11 @@
165
166 def test_pxeconfig_enlistment_preseed_url_detects_request_origin(self):
167 self.silence_get_ephemeral_name()
168- ng_url = 'http://%s' % factory.make_name('host')
169+ hostname = factory.make_hostname()
170+ ng_url = 'http://%s' % hostname
171 network = IPNetwork("10.1.1/24")
172 ip = factory.getRandomIPInNetwork(network)
173+ self.patch(server_address, 'gethostbyname', Mock(return_value=ip))
174 factory.make_node_group(maas_url=ng_url, network=network)
175 params = self.get_default_params()
176
177@@ -3451,6 +3456,24 @@
178 json.loads(response.content)["preseed_url"],
179 StartsWith(ng_url))
180
181+ def test_pxeconfig_enlistment_log_host_url_detects_request_origin(self):
182+ self.silence_get_ephemeral_name()
183+ hostname = factory.make_hostname()
184+ ng_url = 'http://%s' % hostname
185+ network = IPNetwork("10.1.1/24")
186+ ip = factory.getRandomIPInNetwork(network)
187+ mock = self.patch(
188+ server_address, 'gethostbyname', Mock(return_value=ip))
189+ factory.make_node_group(maas_url=ng_url, network=network)
190+ params = self.get_default_params()
191+
192+ # Simulate that the request targets ip by setting 'SERVER_NAME'.
193+ response = self.client.get(
194+ reverse('pxeconfig'), params, SERVER_NAME=ip)
195+ self.assertEqual(
196+ (ip, hostname),
197+ (json.loads(response.content)["log_host"], mock.call_args[0][0]))
198+
199 def test_pxeconfig_has_preseed_url_for_known_node(self):
200 params = self.get_mac_params()
201 node = MACAddress.objects.get(mac_address=params['mac']).node
202@@ -3463,6 +3486,7 @@
203 ng_url = 'http://%s' % factory.make_name('host')
204 network = IPNetwork("10.1.1/24")
205 ip = factory.getRandomIPInNetwork(network)
206+ self.patch(server_address, 'gethostbyname', Mock(return_value=ip))
207 nodegroup = factory.make_node_group(maas_url=ng_url, network=network)
208 params = self.get_mac_params()
209 node = MACAddress.objects.get(mac_address=params['mac']).node
210
211=== modified file 'src/maasserver/tests/test_dns.py'
212--- src/maasserver/tests/test_dns.py 2012-11-26 14:41:53 +0000
213+++ src/maasserver/tests/test_dns.py 2012-11-27 16:03:23 +0000
214@@ -91,7 +91,7 @@
215 ip = factory.getRandomIPAddress()
216 resolver = FakeMethod(result=ip)
217 self.patch(server_address, 'gethostbyname', resolver)
218- hostname = factory.getRandomString().lower()
219+ hostname = factory.make_hostname()
220 self.patch_DEFAULT_MAAS_URL_with_random_values(hostname=hostname)
221 self.assertEqual(
222 (ip, [(hostname, )]),
223@@ -118,7 +118,7 @@
224 ip = factory.getRandomIPAddress()
225 resolver = FakeMethod(result=ip)
226 self.patch(server_address, 'gethostbyname', resolver)
227- hostname = factory.getRandomString().lower()
228+ hostname = factory.make_hostname()
229 maas_url = 'http://%s' % hostname
230 nodegroup = factory.make_node_group(maas_url=maas_url)
231 self.assertEqual(
232
233=== modified file 'src/maasserver/tests/test_preseed.py'
234--- src/maasserver/tests/test_preseed.py 2012-11-23 14:35:53 +0000
235+++ src/maasserver/tests/test_preseed.py 2012-11-27 16:03:23 +0000
236@@ -323,7 +323,8 @@
237
238 def test_get_preseed_context_contains_keys(self):
239 release = factory.getRandomString()
240- context = get_preseed_context(None, release)
241+ nodegroup = factory.make_node_group(maas_url=factory.getRandomString())
242+ context = get_preseed_context(release, nodegroup)
243 self.assertItemsEqual(
244 ['release', 'metadata_enlist_url', 'server_host', 'server_url',
245 'main_archive_hostname', 'main_archive_directory',
246@@ -339,8 +340,8 @@
247 ports_archive = make_url('ports_archive')
248 Config.objects.set_config('main_archive', main_archive)
249 Config.objects.set_config('ports_archive', ports_archive)
250- context = get_preseed_context(
251- factory.make_node(), factory.getRandomString())
252+ nodegroup = factory.make_node_group(maas_url=factory.getRandomString())
253+ context = get_preseed_context(factory.make_node(), nodegroup)
254 parsed_main_archive = urlparse(main_archive)
255 parsed_ports_archive = urlparse(ports_archive)
256 self.assertEqual(
257@@ -402,9 +403,9 @@
258 self.assertIsInstance(preseed, str)
259
260 def test_get_preseed_uses_nodegroup_maas_url(self):
261- ng_url = 'http://%s' % factory.make_name('host')
262+ ng_url = 'http://%s' % factory.make_hostname()
263 ng = factory.make_node_group(maas_url=ng_url)
264- maas_url = 'http://%s' % factory.make_name('host')
265+ maas_url = 'http://%s' % factory.make_hostname()
266 node = factory.make_node(
267 nodegroup=ng, status=NODE_STATUS.COMMISSIONING)
268 self.patch(settings, 'DEFAULT_MAAS_URL', maas_url)
269@@ -423,11 +424,12 @@
270 self.assertIsInstance(preseed, str)
271
272 def test_get_preseed_uses_nodegroup_maas_url(self):
273- ng_url = 'http://%s' % factory.make_name('host')
274- maas_url = 'http://%s' % factory.make_name('host')
275+ ng_url = 'http://%s' % factory.make_hostname()
276+ maas_url = 'http://%s' % factory.make_hostname()
277 self.patch(settings, 'DEFAULT_MAAS_URL', maas_url)
278+ nodegroup = factory.make_node_group(maas_url=ng_url)
279 preseed = render_enlistment_preseed(
280- PRESEED_TYPE.ENLIST, "precise", base_url=ng_url)
281+ PRESEED_TYPE.ENLIST, "precise", nodegroup=nodegroup)
282 self.assertThat(
283 preseed, MatchesAll(*[Contains(ng_url), Not(Contains(maas_url))]))
284
285@@ -461,7 +463,7 @@
286 class TestPreseedProxy(TestCase):
287
288 def test_preseed_uses_default_proxy(self):
289- server_host = factory.getRandomString().lower()
290+ server_host = factory.make_hostname()
291 url = 'http://%s:%d/%s' % (
292 server_host, factory.getRandomPort(), factory.getRandomString())
293 self.patch(settings, 'DEFAULT_MAAS_URL', url)
294
295=== modified file 'src/maasserver/tests/test_server_address.py'
296--- src/maasserver/tests/test_server_address.py 2012-11-26 14:41:53 +0000
297+++ src/maasserver/tests/test_server_address.py 2012-11-27 16:03:23 +0000
298@@ -24,7 +24,7 @@
299 class TestServerAddress(TestCase):
300
301 def make_hostname(self):
302- return '%s.example.com' % factory.make_name('host').lower()
303+ return '%s.example.com' % factory.make_hostname()
304
305 def set_DEFAULT_MAAS_URL(self, hostname=None, with_port=False):
306 """Patch DEFAULT_MAAS_URL to be a (partly) random URL."""
307@@ -49,7 +49,7 @@
308 self.assertEqual(ip, server_address.get_maas_facing_server_host())
309
310 def test_get_maas_facing_server_host_returns_nodegroup_maas_url(self):
311- hostname = factory.make_name('host').lower()
312+ hostname = factory.make_hostname()
313 maas_url = 'http://%s' % hostname
314 nodegroup = factory.make_node_group(maas_url=maas_url)
315 self.assertEqual(
316@@ -82,7 +82,7 @@
317 ip = factory.getRandomIPAddress()
318 resolver = FakeMethod(result=ip)
319 self.patch(server_address, 'gethostbyname', resolver)
320- hostname = self.make_hostname().lower()
321+ hostname = self.make_hostname()
322 self.set_DEFAULT_MAAS_URL(hostname=hostname)
323 self.assertEqual(
324 (ip, [(hostname, )]),
325
326=== modified file 'src/maastesting/factory.py'
327--- src/maastesting/factory.py 2012-09-10 14:50:56 +0000
328+++ src/maastesting/factory.py 2012-11-27 16:03:23 +0000
329@@ -148,6 +148,13 @@
330 return sep.join(
331 filter(None, [prefix, self.getRandomString(size=size)]))
332
333+ def make_hostname(self, prefix='host', *args, **kwargs):
334+ """Generate a random hostname.
335+
336+ The returned hostname is lowercase because python's urlparse
337+ implicitely lowercases the hostnames."""
338+ return self.make_name(prefix=prefix, *args, **kwargs).lower()
339+
340 def make_names(self, *prefixes):
341 """Generate random names.
342
343
344=== modified file 'src/metadataserver/api.py'
345--- src/metadataserver/api.py 2012-11-26 08:52:01 +0000
346+++ src/metadataserver/api.py 2012-11-27 16:03:23 +0000
347@@ -395,9 +395,8 @@
348 def get_enlist_preseed(self, request, version=None):
349 """Render and return a preseed script for enlistment."""
350 nodegroup = find_nodegroup(request)
351- base_url = nodegroup.maas_url if nodegroup is not None else None
352 return HttpResponse(
353- get_enlist_preseed(base_url=base_url), mimetype="text/plain")
354+ get_enlist_preseed(nodegroup=nodegroup), mimetype="text/plain")
355
356 @operation(idempotent=True)
357 def get_preseed(self, request, version=None, system_id=None):