Merge lp:~allenap/maas/preseed-cluster-host into lp:maas

Proposed by Gavin Panella
Status: Merged
Approved by: Andres Rodriguez
Approved revision: 1439
Merged at revision: 1458
Proposed branch: lp:~allenap/maas/preseed-cluster-host
Merge into: lp:maas
Diff against target: 88 lines (+41/-5)
2 files modified
src/maasserver/preseed.py (+8/-1)
src/maasserver/tests/test_preseed.py (+33/-4)
To merge this branch: bzr merge lp:~allenap/maas/preseed-cluster-host
Reviewer Review Type Date Requested Status
Andres Rodriguez (community) Needs Fixing
Julian Edwards (community) Approve
Review via email: mp+149954@code.launchpad.net

Commit message

Include a 'cluster_host' setting in the preseed template rendering context.

This is set to the IP address of the first managed interface of the cluster.

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

I've realised that cluster_host can already be derived in the preseed environment:

  node.get_managed_interface().ip

This is less convenient, but it works right now, and doesn't require backporting.

Revision history for this message
Andres Rodriguez (andreserl) wrote :

Hi Gavin,

I don't think that node.get_managed_interface() exists right? Shouldn't it be nodegroup.get_managed_interface().ip? I'm guessing this is being handled automatically for each of the cluster controllers as in it provides the IP of the cluster controller from which the node is trying to install from right?

Revision history for this message
Andres Rodriguez (andreserl) wrote :

Ok I have tested this in a single node MAAS and it seems to work like a charm. The node.nodegroup.get_managed_interface().ip didn't really work.

Revision history for this message
Andres Rodriguez (andreserl) wrote :

No my bad... this doesn't work unfortunately... we need the IP of the cluster node regardless of whether we are managing an interface or not .... (I think that's why it doesn't work).

Revision history for this message
Julian Edwards (julian-edwards) :
review: Approve
Revision history for this message
Andres Rodriguez (andreserl) wrote :

Forgot to add the needs fixing since it doesn't work :(

review: Needs Fixing
Revision history for this message
Julian Edwards (julian-edwards) wrote :

On 05/03/13 14:20, Andres Rodriguez wrote:
> Review: Needs Fixing
>
> Forgot to add the needs fixing since it doesn't work :(
>

I approved on the basis that the branch itself is fine to land, even
though your particular problem is not fixed.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/preseed.py'
2--- src/maasserver/preseed.py 2012-11-27 10:39:51 +0000
3+++ src/maasserver/preseed.py 2013-02-21 23:39:27 +0000
4@@ -225,7 +225,13 @@
5 Config.objects.get_config('main_archive'))
6 ports_archive_hostname, ports_archive_directory = get_hostname_and_path(
7 Config.objects.get_config('ports_archive'))
8- base_url = nodegroup.maas_url if nodegroup is not None else None
9+ if nodegroup is None:
10+ base_url = None
11+ cluster_host = None
12+ else:
13+ base_url = nodegroup.maas_url
14+ cluster_if = nodegroup.get_managed_interface()
15+ cluster_host = None if cluster_if is None else cluster_if.ip
16 return {
17 'main_archive_hostname': main_archive_hostname,
18 'main_archive_directory': main_archive_directory,
19@@ -236,6 +242,7 @@
20 'server_url': absolute_reverse('nodes_handler', base_url=base_url),
21 'metadata_enlist_url': absolute_reverse('enlist', base_url=base_url),
22 'http_proxy': Config.objects.get_config('http_proxy'),
23+ 'cluster_host': cluster_host,
24 }
25
26
27
28=== modified file 'src/maasserver/tests/test_preseed.py'
29--- src/maasserver/tests/test_preseed.py 2012-11-27 10:26:19 +0000
30+++ src/maasserver/tests/test_preseed.py 2013-02-21 23:39:27 +0000
31@@ -21,6 +21,7 @@
32 from maasserver.enum import (
33 ARCHITECTURE,
34 NODE_STATUS,
35+ NODEGROUPINTERFACE_MANAGEMENT,
36 PRESEED_TYPE,
37 )
38 from maasserver.models import Config
39@@ -327,10 +328,9 @@
40 context = get_preseed_context(release, nodegroup)
41 self.assertItemsEqual(
42 ['release', 'metadata_enlist_url', 'server_host', 'server_url',
43- 'main_archive_hostname', 'main_archive_directory',
44- 'ports_archive_hostname', 'ports_archive_directory',
45- 'http_proxy',
46- ],
47+ 'cluster_host', 'main_archive_hostname', 'main_archive_directory',
48+ 'ports_archive_hostname', 'ports_archive_directory',
49+ 'http_proxy'],
50 context)
51
52 def test_get_preseed_context_archive_refs(self):
53@@ -358,6 +358,35 @@
54 context['ports_archive_directory'],
55 ))
56
57+ def test_preseed_context_cluster_host(self):
58+ # The cluster_host context variable is derived from the nodegroup.
59+ release = factory.getRandomString()
60+ nodegroup = factory.make_node_group(maas_url=factory.getRandomString())
61+ context = get_preseed_context(release, nodegroup)
62+ self.assertIsNotNone(context["cluster_host"])
63+ self.assertEqual(
64+ nodegroup.get_managed_interface().ip,
65+ context["cluster_host"])
66+
67+ def test_preseed_context_null_cluster_host_if_unmanaged(self):
68+ # If the nodegroup has no managed interface recorded, which is
69+ # possible in the data model but would be a bit weird, the
70+ # cluster_host context variable is present, but None.
71+ release = factory.getRandomString()
72+ nodegroup = factory.make_node_group(maas_url=factory.getRandomString())
73+ for interface in nodegroup.nodegroupinterface_set.all():
74+ interface.management = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED
75+ interface.save()
76+ context = get_preseed_context(release, nodegroup)
77+ self.assertIsNone(context["cluster_host"])
78+
79+ def test_preseed_context_null_cluster_host_if_does_not_exist(self):
80+ # If there's no nodegroup, the cluster_host context variable is
81+ # present, but None.
82+ release = factory.getRandomString()
83+ context = get_preseed_context(release)
84+ self.assertIsNone(context["cluster_host"])
85+
86
87 class TestNodePreseedContext(TestCase):
88 """Tests for `get_node_preseed_context`."""