launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14566
[Merge] lp:~rvb/maas/1081701-c-bis into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/1081701-c-bis into lp:maas.
Commit message:
Fix the value of 'server_host' in the preseed.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/1081701-c-bis/+merge/136357
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.
--
https://code.launchpad.net/~rvb/maas/1081701-c-bis/+merge/136357
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/1081701-c-bis into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-11-26 16:50:14 +0000
+++ src/maasserver/api.py 2012-11-27 10:28:26 +0000
@@ -1665,8 +1665,7 @@
subarch = pxelinux_subarch
nodegroup = find_nodegroup(request)
- base_url = nodegroup.maas_url if nodegroup is not None else None
- preseed_url = compose_enlistment_preseed_url(base_url=base_url)
+ preseed_url = compose_enlistment_preseed_url(nodegroup=nodegroup)
hostname = 'maas-enlist'
domain = Config.objects.get_config('enlistment_domain')
=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py 2012-11-23 14:35:53 +0000
+++ src/maasserver/preseed.py 2012-11-27 10:28:26 +0000
@@ -38,24 +38,24 @@
GENERIC_FILENAME = 'generic'
-def get_enlist_preseed(base_url=''):
+def get_enlist_preseed(nodegroup=None):
"""Return the enlistment preseed.
:return: The rendered preseed string.
:rtype: basestring.
"""
return render_enlistment_preseed(
- PRESEED_TYPE.ENLIST, base_url=base_url)
-
-
-def get_enlist_userdata(base_url=''):
+ PRESEED_TYPE.ENLIST, nodegroup=nodegroup)
+
+
+def get_enlist_userdata(nodegroup=None):
"""Return the enlistment preseed.
:return: The rendered enlistment user-data string.
:rtype: basestring.
"""
return render_enlistment_preseed(
- PRESEED_TYPE.ENLIST_USERDATA, base_url=base_url)
+ PRESEED_TYPE.ENLIST_USERDATA, nodegroup=nodegroup)
def get_preseed(node):
@@ -209,7 +209,7 @@
return parsed_url.hostname, parsed_url.path
-def get_preseed_context(release='', base_url=''):
+def get_preseed_context(release='', nodegroup=None):
"""Return the node-independent context dictionary to be used to render
preseed templates.
@@ -219,11 +219,12 @@
:return: The context dictionary.
:rtype: dict.
"""
- server_host = get_maas_facing_server_host()
+ server_host = get_maas_facing_server_host(nodegroup=nodegroup)
main_archive_hostname, main_archive_directory = get_hostname_and_path(
Config.objects.get_config('main_archive'))
ports_archive_hostname, ports_archive_directory = get_hostname_and_path(
Config.objects.get_config('ports_archive'))
+ base_url = nodegroup.maas_url if nodegroup is not None else None
return {
'main_archive_hostname': main_archive_hostname,
'main_archive_directory': main_archive_directory,
@@ -261,7 +262,7 @@
}
-def render_enlistment_preseed(prefix, release='', base_url=''):
+def render_enlistment_preseed(prefix, release='', nodegroup=None):
"""Return the enlistment preseed.
:param prefix: See `get_preseed_filenames`.
@@ -270,7 +271,7 @@
:rtype: basestring.
"""
template = load_preseed_template(None, prefix, release)
- context = get_preseed_context(release, base_url=base_url)
+ context = get_preseed_context(release, nodegroup=nodegroup)
return template.substitute(**context)
@@ -284,15 +285,16 @@
:rtype: basestring.
"""
template = load_preseed_template(node, prefix, release)
- base_url = node.nodegroup.maas_url
- context = get_preseed_context(release, base_url=base_url)
+ nodegroup = node.nodegroup
+ context = get_preseed_context(release, nodegroup=nodegroup)
context.update(get_node_preseed_context(node, release))
return template.substitute(**context)
-def compose_enlistment_preseed_url(base_url=''):
+def compose_enlistment_preseed_url(nodegroup=None):
"""Compose enlistment preseed URL."""
# Always uses the latest version of the metadata API.
+ base_url = nodegroup.maas_url if nodegroup is not None else None
version = 'latest'
return absolute_reverse(
'metadata-enlist-preseed', args=[version],
=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py 2012-11-26 14:41:53 +0000
+++ src/maasserver/tests/test_dns.py 2012-11-27 10:28:26 +0000
@@ -91,7 +91,7 @@
ip = factory.getRandomIPAddress()
resolver = FakeMethod(result=ip)
self.patch(server_address, 'gethostbyname', resolver)
- hostname = factory.getRandomString().lower()
+ hostname = factory.make_hostname()
self.patch_DEFAULT_MAAS_URL_with_random_values(hostname=hostname)
self.assertEqual(
(ip, [(hostname, )]),
@@ -118,7 +118,7 @@
ip = factory.getRandomIPAddress()
resolver = FakeMethod(result=ip)
self.patch(server_address, 'gethostbyname', resolver)
- hostname = factory.getRandomString().lower()
+ hostname = factory.make_hostname()
maas_url = 'http://%s' % hostname
nodegroup = factory.make_node_group(maas_url=maas_url)
self.assertEqual(
=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py 2012-11-23 14:35:53 +0000
+++ src/maasserver/tests/test_preseed.py 2012-11-27 10:28:26 +0000
@@ -323,7 +323,8 @@
def test_get_preseed_context_contains_keys(self):
release = factory.getRandomString()
- context = get_preseed_context(None, release)
+ nodegroup = factory.make_node_group(maas_url=factory.getRandomString())
+ context = get_preseed_context(release, nodegroup)
self.assertItemsEqual(
['release', 'metadata_enlist_url', 'server_host', 'server_url',
'main_archive_hostname', 'main_archive_directory',
@@ -339,8 +340,8 @@
ports_archive = make_url('ports_archive')
Config.objects.set_config('main_archive', main_archive)
Config.objects.set_config('ports_archive', ports_archive)
- context = get_preseed_context(
- factory.make_node(), factory.getRandomString())
+ nodegroup = factory.make_node_group(maas_url=factory.getRandomString())
+ context = get_preseed_context(factory.make_node(), nodegroup)
parsed_main_archive = urlparse(main_archive)
parsed_ports_archive = urlparse(ports_archive)
self.assertEqual(
@@ -402,9 +403,9 @@
self.assertIsInstance(preseed, str)
def test_get_preseed_uses_nodegroup_maas_url(self):
- ng_url = 'http://%s' % factory.make_name('host')
+ ng_url = 'http://%s' % factory.make_hostname()
ng = factory.make_node_group(maas_url=ng_url)
- maas_url = 'http://%s' % factory.make_name('host')
+ maas_url = 'http://%s' % factory.make_hostname()
node = factory.make_node(
nodegroup=ng, status=NODE_STATUS.COMMISSIONING)
self.patch(settings, 'DEFAULT_MAAS_URL', maas_url)
@@ -423,11 +424,12 @@
self.assertIsInstance(preseed, str)
def test_get_preseed_uses_nodegroup_maas_url(self):
- ng_url = 'http://%s' % factory.make_name('host')
- maas_url = 'http://%s' % factory.make_name('host')
+ ng_url = 'http://%s' % factory.make_hostname()
+ maas_url = 'http://%s' % factory.make_hostname()
self.patch(settings, 'DEFAULT_MAAS_URL', maas_url)
+ nodegroup = factory.make_node_group(maas_url=ng_url)
preseed = render_enlistment_preseed(
- PRESEED_TYPE.ENLIST, "precise", base_url=ng_url)
+ PRESEED_TYPE.ENLIST, "precise", nodegroup=nodegroup)
self.assertThat(
preseed, MatchesAll(*[Contains(ng_url), Not(Contains(maas_url))]))
@@ -461,7 +463,7 @@
class TestPreseedProxy(TestCase):
def test_preseed_uses_default_proxy(self):
- server_host = factory.getRandomString().lower()
+ server_host = factory.make_hostname()
url = 'http://%s:%d/%s' % (
server_host, factory.getRandomPort(), factory.getRandomString())
self.patch(settings, 'DEFAULT_MAAS_URL', url)
=== modified file 'src/maasserver/tests/test_server_address.py'
--- src/maasserver/tests/test_server_address.py 2012-11-26 14:41:53 +0000
+++ src/maasserver/tests/test_server_address.py 2012-11-27 10:28:26 +0000
@@ -24,7 +24,7 @@
class TestServerAddress(TestCase):
def make_hostname(self):
- return '%s.example.com' % factory.make_name('host').lower()
+ return '%s.example.com' % factory.make_hostname()
def set_DEFAULT_MAAS_URL(self, hostname=None, with_port=False):
"""Patch DEFAULT_MAAS_URL to be a (partly) random URL."""
@@ -49,7 +49,7 @@
self.assertEqual(ip, server_address.get_maas_facing_server_host())
def test_get_maas_facing_server_host_returns_nodegroup_maas_url(self):
- hostname = factory.make_name('host').lower()
+ hostname = factory.make_hostname()
maas_url = 'http://%s' % hostname
nodegroup = factory.make_node_group(maas_url=maas_url)
self.assertEqual(
@@ -82,7 +82,7 @@
ip = factory.getRandomIPAddress()
resolver = FakeMethod(result=ip)
self.patch(server_address, 'gethostbyname', resolver)
- hostname = self.make_hostname().lower()
+ hostname = self.make_hostname()
self.set_DEFAULT_MAAS_URL(hostname=hostname)
self.assertEqual(
(ip, [(hostname, )]),
=== modified file 'src/maastesting/factory.py'
--- src/maastesting/factory.py 2012-09-10 14:50:56 +0000
+++ src/maastesting/factory.py 2012-11-27 10:28:26 +0000
@@ -148,6 +148,13 @@
return sep.join(
filter(None, [prefix, self.getRandomString(size=size)]))
+ def make_hostname(self, prefix='host', *args, **kwargs):
+ """Generate a random hostname.
+
+ The returned hostname is lowercase because python's urlparse
+ implicitely lowercases the hostnames."""
+ return self.make_name(prefix=prefix, *args, **kwargs).lower()
+
def make_names(self, *prefixes):
"""Generate random names.
=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py 2012-11-26 08:52:01 +0000
+++ src/metadataserver/api.py 2012-11-27 10:28:26 +0000
@@ -395,9 +395,8 @@
def get_enlist_preseed(self, request, version=None):
"""Render and return a preseed script for enlistment."""
nodegroup = find_nodegroup(request)
- base_url = nodegroup.maas_url if nodegroup is not None else None
return HttpResponse(
- get_enlist_preseed(base_url=base_url), mimetype="text/plain")
+ get_enlist_preseed(nodegroup=nodegroup), mimetype="text/plain")
@operation(idempotent=True)
def get_preseed(self, request, version=None, system_id=None):
