Merge lp:~jtv/maas/bug-1086239 into lp:maas/trunk

Proposed by Jeroen T. Vermeulen on 2012-12-11
Status: Merged
Approved by: Julian Edwards on 2012-12-12
Approved revision: 1399
Merged at revision: 1397
Proposed branch: lp:~jtv/maas/bug-1086239
Merge into: lp:maas/trunk
Diff against target: 393 lines (+111/-68)
14 files modified
etc/democeleryconfig_cluster.py (+0/-4)
services/cluster-worker/run (+5/-0)
services/pserv/run (+5/-0)
src/provisioningserver/auth.py (+0/-8)
src/provisioningserver/boot_images.py (+5/-5)
src/provisioningserver/cluster_config.py (+44/-0)
src/provisioningserver/dhcp/leases.py (+2/-2)
src/provisioningserver/dhcp/tests/test_leases.py (+0/-9)
src/provisioningserver/start_cluster_controller.py (+1/-8)
src/provisioningserver/tags.py (+2/-6)
src/provisioningserver/tests/test_auth.py (+0/-12)
src/provisioningserver/tests/test_boot_images.py (+0/-10)
src/provisioningserver/tests/test_cluster_config.py (+46/-0)
src/provisioningserver/tftp.py (+1/-4)
To merge this branch: bzr merge lp:~jtv/maas/bug-1086239
Reviewer Review Type Date Requested Status
Gavin Panella (community) 2012-12-11 Approve on 2012-12-11
Review via email: mp+139172@code.launchpad.net

Commit message

Take cluster-uuid setting from environment, not celery config.

Description of the change

This builds on a multi-step change agreed with Julian. The relevant part is:
 1 Duplicate the CLUSTER_UUID setting, from Celery config to maas_cluster.conf.
 2 Make pserv's upstart job source maas_cluster.conf.
 3 Have get_cluster_uuid read the setting from the environment instead of from celery.
 4 Clean up the duplicated setting.

What you see here is step 3. It also puts get_cluster_uuid in a saner place. That function was "borrowed" from a place where nobody should be importing it from. Not so bad for a provisional fix, but this is the cleanup.

Finally, the cluster uuid is no longer one of the "worker secrets" that the region controller sends to the clusters. So this also removes some of the remnants of that old situation. In particular, "I don't know my cluster uuid yet" is no longer a valid reason for cluster controllers to be unable to perform tasks. Now, if the uuid is not set, either installation failed in some unusual way or we're running something that needs the information without sourcing maas_cluster.conf. There may be some nooks and crannies where this may still need doing, so I tried to make the failure a helpful one.

All of this work is to be backported to the 1.2 branch.

Jeroen

To post a comment you must log in.
Gavin Panella (allenap) :
review: Approve
MAAS Lander (maas-lander) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/maas-merger-trunk/129/console reported an error when processing this lp:~jtv/maas/bug-1086239 branch.
Not merging it.

Jeroen T. Vermeulen (jtv) wrote :

Weird Jenkins failure. I'll try again. I also snuck in a small change for the part I forgot to cover: running a demo cluster controller in a source branch.

MAAS Lander (maas-lander) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/maas-merger-trunk/130/console reported an error when processing this lp:~jtv/maas/bug-1086239 branch.
Not merging it.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'etc/democeleryconfig_cluster.py'
2--- etc/democeleryconfig_cluster.py 2012-10-23 12:48:31 +0000
3+++ etc/democeleryconfig_cluster.py 2012-12-11 13:35:25 +0000
4@@ -26,10 +26,6 @@
5
6 import_settings(democeleryconfig_common)
7
8-# Set a fixed CLUSTER_UUID. In production, this is taken from
9-# maas_local_celeryconfig.
10-CLUSTER_UUID = "adfd3977-f251-4f2c-8d61-745dbd690bfc"
11-
12 CELERYD_LOG_FILE = os.path.join(
13 DEV_ROOT_DIRECTORY, 'logs/cluster-worker/current')
14
15
16=== modified file 'services/cluster-worker/run'
17--- services/cluster-worker/run 2012-10-03 11:39:37 +0000
18+++ services/cluster-worker/run 2012-12-11 13:35:25 +0000
19@@ -16,6 +16,11 @@
20
21 export PYTHONPATH=etc/:src/
22 export CELERY_CONFIG_MODULE=democeleryconfig_cluster
23+
24+# Set a fixed cluster UUID. This is identical for all cluster services
25+# running in the demo setup: they're all the same cluster.
26+export CLUSTER_UUID="adfd3977-f251-4f2c-8d61-745dbd690bfc"
27+
28 script="$(readlink -f bin/maas-provision)"
29 exec fghack "${script}" start-cluster-controller \
30 http://0.0.0.0:5240/ -u "$(id -un)" -g "$(id -gn)"
31
32=== modified file 'services/pserv/run'
33--- services/pserv/run 2012-09-26 14:16:33 +0000
34+++ services/pserv/run 2012-12-11 13:35:25 +0000
35@@ -17,5 +17,10 @@
36 # Exec the Provisioning Server.
37 script="$(readlink -f bin/twistd.pserv)"
38 config="$(readlink -f etc/pserv.yaml)"
39+
40+# Set a fixed cluster UUID. This is identical for all cluster services
41+# running in the demo setup: they're all the same cluster.
42+export CLUSTER_UUID="adfd3977-f251-4f2c-8d61-745dbd690bfc"
43+
44 exec $(command -v authbind && echo --deep) \
45 "${script}" --nodaemon --pidfile="" maas-pserv --config-file "${config}"
46
47=== modified file 'src/provisioningserver/auth.py'
48--- src/provisioningserver/auth.py 2012-11-23 01:42:03 +0000
49+++ src/provisioningserver/auth.py 2012-12-11 13:35:25 +0000
50@@ -13,13 +13,10 @@
51 __all__ = [
52 'get_recorded_api_credentials',
53 'get_recorded_nodegroup_uuid',
54- 'get_recorded_maas_url',
55 'record_api_credentials',
56 'record_nodegroup_uuid',
57 ]
58
59-import os
60-
61 from apiclient.creds import convert_string_to_tuple
62 from provisioningserver import cache
63
64@@ -30,11 +27,6 @@
65 NODEGROUP_UUID_CACHE_KEY = 'nodegroup_uuid'
66
67
68-def get_recorded_maas_url():
69- """Return the base URL for the MAAS server."""
70- return os.environ.get("MAAS_URL")
71-
72-
73 def record_api_credentials(api_credentials):
74 """Update the recorded API credentials.
75
76
77=== modified file 'src/provisioningserver/boot_images.py'
78--- src/provisioningserver/boot_images.py 2012-11-16 11:25:07 +0000
79+++ src/provisioningserver/boot_images.py 2012-12-11 13:35:25 +0000
80@@ -26,13 +26,13 @@
81 MAASDispatcher,
82 MAASOAuth,
83 )
84-from provisioningserver.auth import (
85- get_recorded_api_credentials,
86- get_recorded_maas_url,
87+from provisioningserver.auth import get_recorded_api_credentials
88+from provisioningserver.cluster_config import (
89+ get_cluster_uuid,
90+ get_maas_url,
91 )
92 from provisioningserver.config import Config
93 from provisioningserver.pxe import tftppath
94-from provisioningserver.start_cluster_controller import get_cluster_uuid
95
96
97 logger = getLogger(__name__)
98@@ -44,7 +44,7 @@
99 :return: Tuple of cached items: (maas_url, api_credentials). Either may
100 be None if the information has not been received from the server yet.
101 """
102- maas_url = get_recorded_maas_url()
103+ maas_url = get_maas_url()
104 if maas_url is None:
105 logger.debug("Not reporting boot images: don't have API URL yet.")
106 api_credentials = get_recorded_api_credentials()
107
108=== added file 'src/provisioningserver/cluster_config.py'
109--- src/provisioningserver/cluster_config.py 1970-01-01 00:00:00 +0000
110+++ src/provisioningserver/cluster_config.py 2012-12-11 13:35:25 +0000
111@@ -0,0 +1,44 @@
112+# Copyright 2012 Canonical Ltd. This software is licensed under the
113+# GNU Affero General Public License version 3 (see the file LICENSE).
114+
115+"""Accessors for cluster configuration as set in `maas_cluster.conf`."""
116+
117+from __future__ import (
118+ absolute_import,
119+ print_function,
120+ unicode_literals,
121+ )
122+
123+__metaclass__ = type
124+__all__ = [
125+ 'get_cluster_uuid',
126+ 'get_maas_url',
127+ ]
128+
129+from os import environ
130+
131+
132+def get_cluster_variable(var):
133+ """Obtain the given environment variable from maas_cluster.conf.
134+
135+ If the variable is not set, it probably means that whatever script
136+ started the current process neglected to run maas_cluster.conf.
137+ In that case, fail helpfully but utterly.
138+ """
139+ value = environ.get(var)
140+ if value is None:
141+ raise AssertionError(
142+ "%s is not set. This probably means that the script which "
143+ "started this program failed to source maas_cluster.conf."
144+ % var)
145+ return value
146+
147+
148+def get_cluster_uuid():
149+ """Return the `CLUSTER_UUID` setting."""
150+ return get_cluster_variable('CLUSTER_UUID')
151+
152+
153+def get_maas_url():
154+ """Return the `MAAS_URL` setting."""
155+ return get_cluster_variable('MAAS_URL')
156
157=== modified file 'src/provisioningserver/dhcp/leases.py'
158--- src/provisioningserver/dhcp/leases.py 2012-11-16 11:25:07 +0000
159+++ src/provisioningserver/dhcp/leases.py 2012-12-11 13:35:25 +0000
160@@ -49,9 +49,9 @@
161 from provisioningserver import cache
162 from provisioningserver.auth import (
163 get_recorded_api_credentials,
164- get_recorded_maas_url,
165 get_recorded_nodegroup_uuid,
166 )
167+from provisioningserver.cluster_config import get_maas_url
168 from provisioningserver.dhcp.leases_parser import parse_leases
169
170
171@@ -150,7 +150,7 @@
172 """Send lease updates to the server API."""
173 # Items that the server must have sent us before we can do this.
174 knowledge = {
175- 'maas_url': get_recorded_maas_url(),
176+ 'maas_url': get_maas_url(),
177 'api_credentials': get_recorded_api_credentials(),
178 'nodegroup_uuid': get_recorded_nodegroup_uuid(),
179 }
180
181=== modified file 'src/provisioningserver/dhcp/tests/test_leases.py'
182--- src/provisioningserver/dhcp/tests/test_leases.py 2012-11-26 02:24:46 +0000
183+++ src/provisioningserver/dhcp/tests/test_leases.py 2012-12-11 13:35:25 +0000
184@@ -328,15 +328,6 @@
185 (get_write_time(leases_file), {params['ip']: params['mac']}),
186 parse_leases_file())
187
188- def test_send_leases_does_nothing_without_maas_url(self):
189- self.patch(MAASClient, 'post', FakeMethod())
190- self.set_lease_state()
191- self.set_items_needed_for_lease_update()
192- self.clear_maas_url()
193- leases = factory.make_random_leases()
194- send_leases(leases)
195- self.assertEqual([], MAASClient.post.calls)
196-
197 def test_send_leases_does_nothing_without_credentials(self):
198 self.patch(MAASClient, 'post', FakeMethod())
199 self.set_items_needed_for_lease_update()
200
201=== modified file 'src/provisioningserver/start_cluster_controller.py'
202--- src/provisioningserver/start_cluster_controller.py 2012-11-23 01:42:03 +0000
203+++ src/provisioningserver/start_cluster_controller.py 2012-12-11 13:35:25 +0000
204@@ -32,6 +32,7 @@
205 MAASDispatcher,
206 NoAuth,
207 )
208+from provisioningserver.cluster_config import get_cluster_uuid
209 from provisioningserver.network import discover_networks
210
211
212@@ -65,14 +66,6 @@
213 return MAASClient(NoAuth(), MAASDispatcher(), server_url)
214
215
216-def get_cluster_uuid():
217- """Read this cluster's UUID from the config."""
218- # Import this lazily. It reads config as a side effect, which can
219- # produce warnings.
220- from celery.app import app_or_default
221- return app_or_default().conf.CLUSTER_UUID
222-
223-
224 def register(server_url):
225 """Request Rabbit connection details from the domain controller.
226
227
228=== modified file 'src/provisioningserver/tags.py'
229--- src/provisioningserver/tags.py 2012-11-16 11:25:07 +0000
230+++ src/provisioningserver/tags.py 2012-12-11 13:35:25 +0000
231@@ -30,9 +30,9 @@
232 from lxml import etree
233 from provisioningserver.auth import (
234 get_recorded_api_credentials,
235- get_recorded_maas_url,
236 get_recorded_nodegroup_uuid,
237 )
238+from provisioningserver.cluster_config import get_maas_url
239 import simplejson as json
240
241
242@@ -51,10 +51,6 @@
243
244 :return: (client, nodegroup_uuid)
245 """
246- maas_url = get_recorded_maas_url()
247- if maas_url is None:
248- logger.error("Not updating tags: don't have API URL yet.")
249- return None, None
250 api_credentials = get_recorded_api_credentials()
251 if api_credentials is None:
252 logger.error("Not updating tags: don't have API key yet.")
253@@ -64,7 +60,7 @@
254 logger.error("Not updating tags: don't have UUID yet.")
255 return None, None
256 client = MAASClient(MAASOAuth(*api_credentials), MAASDispatcher(),
257- maas_url)
258+ get_maas_url())
259 return client, nodegroup_uuid
260
261
262
263=== modified file 'src/provisioningserver/tests/test_auth.py'
264--- src/provisioningserver/tests/test_auth.py 2012-11-26 14:41:53 +0000
265+++ src/provisioningserver/tests/test_auth.py 2012-12-11 13:35:25 +0000
266@@ -14,8 +14,6 @@
267
268 from apiclient.creds import convert_tuple_to_string
269 from apiclient.testing.credentials import make_api_credentials
270-from fixtures import EnvironmentVariableFixture
271-from maastesting.factory import factory
272 from provisioningserver import (
273 auth,
274 cache,
275@@ -38,13 +36,3 @@
276
277 def test_get_recorded_api_credentials_returns_None_without_creds(self):
278 self.assertIsNone(auth.get_recorded_api_credentials())
279-
280- def test_get_recorded_nodegroup_uuid_vs_record_nodegroup_uuid(self):
281- nodegroup_uuid = factory.make_name('nodegroupuuid')
282- auth.record_nodegroup_uuid(nodegroup_uuid)
283- self.assertEqual(nodegroup_uuid, auth.get_recorded_nodegroup_uuid())
284-
285- def test_get_recorded_maas_url_uses_environment_override(self):
286- required_url = factory.make_name("MAAS_URL")
287- self.useFixture(EnvironmentVariableFixture("MAAS_URL", required_url))
288- self.assertEqual(required_url, auth.get_recorded_maas_url())
289
290=== modified file 'src/provisioningserver/tests/test_boot_images.py'
291--- src/provisioningserver/tests/test_boot_images.py 2012-11-26 02:24:46 +0000
292+++ src/provisioningserver/tests/test_boot_images.py 2012-12-11 13:35:25 +0000
293@@ -12,7 +12,6 @@
294 __metaclass__ = type
295 __all__ = []
296
297-import os
298 import json
299
300 from apiclient.maas_client import MAASClient
301@@ -46,15 +45,6 @@
302 self.assertIs(sentinel.uuid, kwargs["nodegroup"])
303 self.assertItemsEqual([image], json.loads(kwargs['images']))
304
305- def test_does_nothing_without_maas_url(self):
306- os.environ.pop("MAAS_URL") # Ensure nothing is set.
307- self.set_api_credentials()
308- self.patch(
309- tftppath, 'list_boot_images',
310- Mock(return_value=make_boot_image_params()))
311- boot_images.report_to_server()
312- self.assertItemsEqual([], tftppath.list_boot_images.call_args_list)
313-
314 def test_does_nothing_without_credentials(self):
315 self.set_maas_url()
316 self.patch(
317
318=== added file 'src/provisioningserver/tests/test_cluster_config.py'
319--- src/provisioningserver/tests/test_cluster_config.py 1970-01-01 00:00:00 +0000
320+++ src/provisioningserver/tests/test_cluster_config.py 2012-12-11 13:35:25 +0000
321@@ -0,0 +1,46 @@
322+# Copyright 2012 Canonical Ltd. This software is licensed under the
323+# GNU Affero General Public License version 3 (see the file LICENSE).
324+
325+"""Tests for `provisioningserver.cluster_config`."""
326+
327+from __future__ import (
328+ absolute_import,
329+ print_function,
330+ unicode_literals,
331+ )
332+
333+__metaclass__ = type
334+__all__ = []
335+
336+from fixtures import EnvironmentVariableFixture
337+from maastesting.factory import factory
338+from maastesting.testcase import TestCase
339+from provisioningserver.cluster_config import (
340+ get_cluster_uuid,
341+ get_cluster_variable,
342+ get_maas_url,
343+ )
344+
345+
346+class TestClusterConfig(TestCase):
347+
348+ def test_get_cluster_variable_reads_env(self):
349+ var = factory.make_name('variable')
350+ value = factory.make_name('value')
351+ self.useFixture(EnvironmentVariableFixture(var, value))
352+ self.assertEqual(value, get_cluster_variable(var))
353+
354+ def test_get_cluster_variable_fails_if_not_set(self):
355+ self.assertRaises(
356+ AssertionError,
357+ get_cluster_variable, factory.make_name('nonexistent-variable'))
358+
359+ def test_get_cluster_uuid_reads_CLUSTER_UUID(self):
360+ uuid = factory.make_name('uuid')
361+ self.useFixture(EnvironmentVariableFixture('CLUSTER_UUID', uuid))
362+ self.assertEqual(uuid, get_cluster_uuid())
363+
364+ def test_get_maas_url_reads_MAAS_URL(self):
365+ maas_url = factory.make_name('maas_url')
366+ self.useFixture(EnvironmentVariableFixture('MAAS_URL', maas_url))
367+ self.assertEqual(maas_url, get_maas_url())
368
369=== modified file 'src/provisioningserver/tftp.py'
370--- src/provisioningserver/tftp.py 2012-12-04 05:02:05 +0000
371+++ src/provisioningserver/tftp.py 2012-12-11 13:35:25 +0000
372@@ -25,10 +25,10 @@
373 urlparse,
374 )
375
376+from provisioningserver.cluster_config import get_cluster_uuid
377 from provisioningserver.enum import ARP_HTYPE
378 from provisioningserver.kernel_opts import KernelParameters
379 from provisioningserver.pxe.config import render_pxe_config
380-from provisioningserver.start_cluster_controller import get_cluster_uuid
381 from provisioningserver.utils import deferred
382 from tftp.backend import (
383 FilesystemSynchronousBackend,
384@@ -218,9 +218,6 @@
385 params["local"] = local_host
386 remote_host, remote_port = get("remote", (None, None))
387 params["remote"] = remote_host
388- # XXX 2012-12-04 JeroenVermeulen bug=1086239: move
389- # get_cluster_uuid to a properly reusable location, and
390- # implement it without the celery import (and side effects).
391 params["cluster_uuid"] = get_cluster_uuid()
392 d = self.get_config_reader(params)
393 d.addErrback(self.get_page_errback, file_name)