Merge lp:~adam-collard/landscape-client/juju-info-at-registration into lp:~landscape/landscape-client/trunk

Proposed by Adam Collard on 2013-09-26
Status: Merged
Approved by: Adam Collard on 2013-09-27
Approved revision: no longer in the source branch.
Merged at revision: 733
Proposed branch: lp:~adam-collard/landscape-client/juju-info-at-registration
Merge into: lp:~landscape/landscape-client/trunk
Diff against target: 614 lines (+240/-118)
13 files modified
landscape/broker/registration.py (+69/-51)
landscape/broker/tests/helpers.py (+3/-0)
landscape/broker/tests/test_registration.py (+38/-7)
landscape/deployment.py (+5/-0)
landscape/lib/juju.py (+26/-0)
landscape/lib/tests/test_encoding.py (+1/-1)
landscape/lib/tests/test_juju.py (+62/-0)
landscape/message_schemas.py (+10/-4)
landscape/monitor/config.py (+0/-7)
landscape/monitor/jujuinfo.py (+4/-22)
landscape/monitor/tests/test_config.py (+0/-11)
landscape/monitor/tests/test_jujuinfo.py (+9/-12)
landscape/tests/test_deployment.py (+13/-3)
To merge this branch: bzr merge lp:~adam-collard/landscape-client/juju-info-at-registration
Reviewer Review Type Date Requested Status
Björn Tillenius 2013-09-26 Approve on 2013-09-27
Alberto Donato 2013-09-26 Approve on 2013-09-27
Review via email: mp+187836@code.launchpad.net

Commit message

Add Juju information at registration time.

 * Changed schema of JUJU_INFO message to be strict about the keys
 * Re-used schema for REGISTER and REGISTER_CLOUD_VM messages
 * Added l.lib.juju as resting ground for json parsing and error handling
 * Moved juju_filename to main config object, not just specific to monitor

Description of the change

Add Juju information at registration time.

 * Changed schema of JUJU_INFO message to be strict about the keys
 * Re-used schema for REGISTER and REGISTER_CLOUD_VM messages
 * Added l.lib.juju as resting ground for json parsing and error handling
 * Moved juju_filename to main config object, not just specific to monitor

To post a comment you must log in.
Alberto Donato (ack) wrote :

+1, looks good!

#1:
+ juju_registration_info = {}
+ juju_registration_info["environment-uuid"] = juju_info[
+ "environment-uuid"]
+ juju_registration_info["api-addresses"] = juju_info[
+ "api-addresses"].split()
+ juju_registration_info["unit-name"] = juju_info["unit-name"]

I think the juju_info["api-addresses"].split() should me moved inside get_juju_info(), since there are currently two places where the split is performed after getting the info.
As a result this could we written as

+ juju_registration_info = dict(
+ (key, value) for key, value in juju_info
+ if key in ["environment-uuid", "api-addresses", "unit-name"])

#2:
+import os.path

Please use "from os import path" instead.

#3:
+class JujuTest(LandscapeTest):

Tests in this class don't have docstrings.

#4:
+ self.assertEqual("DEAD-BEEF", juju_info["environment-uuid"])
+ self.assertEqual("juju-unit-name", juju_info["unit-name"])
+ self.assertEqual("10.0.3.1:17070", juju_info["api-addresses"])
+ self.assertEqual("127.0.0.1", juju_info["private-address"])

It'd be better to compare juju_info with an expected dict, to ensure there are no extra keys.

review: Approve
Björn Tillenius (bjornt) wrote :

Looks good, nothing major, but still marking it as Needs Fixing,
since [6] might change the diff a bit.

[1]

29 + self._juju_data = self._get_juju_data()

I think it would be cleaner to collect the data when on "run", just
like ec2 data is collected.

[2]

222 + self.assertMessages(self.mstore.get_pending_messages(),
223 + [{"type": "register",
224 + "computer_title": self.config.computer_title,
225 + "account_name": "account_name",
226 + "registration_password": None,
227 + "hostname": socket.getfqdn(),
228 + "vm-info": get_vm_info(),
229 + "tags": None,
230 + "juju-info": {
231 + "environment-uuid": "DEAD-BEEF",
232 + "api-addresses": ["10.0.3.1:17070"],
233 + "unit-name": "juju-unit-name"}
234 + }])

The indentation looks off here. Also, please use valid unit names,
like service/0.

[3]
264 - "tags": None}])
265 + "tags": None,
266 + "juju-info": {
267 + "environment-uuid": "DEAD-BEEF",
268 + "api-addresses": ["10.0.3.1:17070"],
269 + "unit-name": "juju-unit-name"}}])

Indentation is off here as well.

[4]
365 + def test_get_juju_info_sample_data(self):
366 + """L{get_juju_info} works with sample data."""

What does "works with sample data" mean? It's implied that our
code works. Better to explain what the code is expected to do instead.

[5]
384 + self.assertTrue(
385 + "Error attempting to read JSON" in self.logfile.getvalue())

Please use assertIn instead, which gives a better error message on failures.

[6]

435 + "juju-info": KeyDict(juju_data)},
436 + optional=["tags", "vm-info", "public_ipv4", "local_ipv4",
437 + "juju-info"])

I think we shouldn't include juju-info in the register-cloud-vm message.
By default Juju instances won't use that the register-cloud-vm message,
and the plan is to remove it altogether.

review: Needs Fixing
Adam Collard (adam-collard) wrote :

On 27 September 2013 15:32, Björn Tillenius <email address hidden> wrote:

> [1]
>
> 29 + self._juju_data = self._get_juju_data()
>
> I think it would be cleaner to collect the data when on "run", just
> like ec2 data is collected.
>

OK. Done!

>
> [2]
>
>
> 222 + self.assertMessages(self.mstore.get_pending_messages(),
> 223 + [{"type": "register",
> 224 + "computer_title":
> self.config.computer_title,
> 225 + "account_name": "account_name",
> 226 + "registration_password": None,
> 227 + "hostname": socket.getfqdn(),
> 228 + "vm-info": get_vm_info(),
> 229 + "tags": None,
> 230 + "juju-info": {
> 231 + "environment-uuid": "DEAD-BEEF",
> 232 + "api-addresses": ["10.0.3.1:17070"],
> 233 + "unit-name": "juju-unit-name"}
> 234 + }])
>
> The indentation looks off here. Also, please use valid unit names,
> like service/0.
>

Emacs wanted to indent it like that - have shifted things around a bit.

Done!

>
>
> [3]
> 264 - "tags": None}])
> 265 + "tags": None,
> 266 + "juju-info": {
> 267 + "environment-uuid": "DEAD-BEEF",
> 268 + "api-addresses": ["10.0.3.1:17070
> "],
> 269 + "unit-name": "juju-unit-name"}}])
>
> Indentation is off here as well.
>

This became irrelevant because of [6]

>
> [4]
> 365 + def test_get_juju_info_sample_data(self):
> 366 + """L{get_juju_info} works with sample data."""
>
> What does "works with sample data" mean? It's implied that our
> code works. Better to explain what the code is expected to do instead.
>

I've tried re-phrasing it. It really is just "does it work with realistic
data"

>
> [5]
> 384 + self.assertTrue(
> 385 + "Error attempting to read JSON" in
> self.logfile.getvalue())
>
> Please use assertIn instead, which gives a better error message on
> failures.
>

Argh. I thought we didn't have it because client has to work on 2.6, but
mocker implements it. Grr, I had it as assertIn and the other test using
assertIs then carefully undid them. Thanks :)

>
>
> [6]
>
> 435 + "juju-info": KeyDict(juju_data)},
> 436 + optional=["tags", "vm-info", "public_ipv4", "local_ipv4",
> 437 + "juju-info"])
>
> I think we shouldn't include juju-info in the register-cloud-vm message.
> By default Juju instances won't use that the register-cloud-vm message,
> and the plan is to remove it altogether.
>

OK, I wasn't sure what this was so erred on the side of caution and
included it. Removed.

Björn Tillenius (bjornt) wrote :

Thanks, looks good now, +1!

[7]

358 + def test_get_juju_info_sample_data(self):
359 + """L{get_juju_info} parses realistic data properly."""

I might be nitpicking a bit :), but I'd say never use words like
"works", "properly", "as expected", etc. in test docstrings. How about?

  """ L{get_juju_info} parses JSON data from the juju_filename file."""

[8]

I don't see a test for when get_juju_info parses a file having
multiple API endpoints. Is there one?

review: Approve
732. By Björn Tillenius on 2013-09-27

Merge unmet-dependencies-bug-1203855 [f=1203855] [r=tribaal,ack] [a=Björn Tillenius]
Don't omit the package from the package changer result text if it's
broken but we don't know why.

When a package is broken we check certain dependency to see whether they
are satisfied. If all the dependencies we check are satisified, the
package wasn't reported as being broken.

Ideally we should write why the package is broken, but I have been
unable to reproduce the error in the bug report.

733. By Adam Collard on 2013-09-27

Merge juju-info-at-registration [f=1229630] [r=bjornt,ack] [a=Adam Collard]
Add Juju information at registration time.

 * Changed schema of JUJU_INFO message to be strict about the keys
 * Re-used schema for REGISTER and REGISTER_CLOUD_VM messages
 * Added l.lib.juju as resting ground for json parsing and error handling
 * Moved juju_filename to main config object, not just specific to monitor

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'landscape/broker/registration.py'
2--- landscape/broker/registration.py 2013-07-09 15:58:36 +0000
3+++ landscape/broker/registration.py 2013-09-27 14:18:49 +0000
4@@ -5,7 +5,7 @@
5 as a new client without providing any identification credentials, and the
6 server replies with the available registration mechanisms. At this point
7 the machinery in this module will notice that we have no identification
8-credentials yet and that the server accepts regiration messages, so it
9+credentials yet and that the server accepts registration messages, so it
10 will craft an appropriate one and send it out.
11 """
12 import time
13@@ -14,8 +14,11 @@
14
15 from twisted.internet.defer import Deferred
16
17+from landscape.message_schemas import juju_data
18+
19 from landscape.lib.bpickle import loads
20 from landscape.lib.log import log_failure
21+from landscape.lib.juju import get_juju_info
22 from landscape.lib.fetch import fetch, FetchError
23 from landscape.lib.tag import is_valid_tag_list
24 from landscape.lib.network import get_fqdn
25@@ -97,6 +100,7 @@
26 self._pinger = pinger
27 self._message_store = message_store
28 self._reactor.call_on("run", self._fetch_ec2_data)
29+ self._reactor.call_on("run", self._get_juju_data)
30 self._reactor.call_on("pre-exchange", self._handle_pre_exchange)
31 self._reactor.call_on("exchange-done", self._handle_exchange_done)
32 self._exchange.register_message("set-id", self._handle_set_id)
33@@ -107,6 +111,7 @@
34 self._fetch_async = fetch_async
35 self._otp = None
36 self._ec2_data = None
37+ self._juju_data = None
38
39 def should_register(self):
40 id = self._identity
41@@ -134,6 +139,14 @@
42 self._exchange.exchange()
43 return result
44
45+ def _get_juju_data(self):
46+ """Load Juju information."""
47+ juju_info = get_juju_info(self._config)
48+ if juju_info is None:
49+ return None
50+ self._juju_data = dict(
51+ (key, juju_info[key]) for key in juju_data)
52+
53 def _get_data(self, path, accumulate):
54 """
55 Get data at C{path} on the EC2 API endpoint, and add the result to the
56@@ -254,65 +267,70 @@
57 # registration.
58 self._should_register = self.should_register()
59
60- if self._should_register:
61- id = self._identity
62- self._message_store.delete_all_messages()
63- tags = id.tags
64- if not is_valid_tag_list(tags):
65- tags = None
66- logging.error("Invalid tags provided for cloud "
67- "registration.")
68- if self._config.cloud and self._ec2_data is not None:
69- if self._otp:
70- logging.info("Queueing message to register with OTP")
71- message = {"type": "register-cloud-vm",
72- "otp": self._otp,
73- "hostname": get_fqdn(),
74- "account_name": None,
75- "registration_password": None,
76- "tags": tags,
77- "vm-info": get_vm_info()}
78- message.update(self._ec2_data)
79- self._exchange.send(message)
80- elif id.account_name:
81- with_tags = ["", u"and tags %s " % tags][bool(tags)]
82- logging.info(
83- u"Queueing message to register with account %r %s"
84- u"as an EC2 instance." % (id.account_name, with_tags))
85- message = {"type": "register-cloud-vm",
86- "otp": None,
87- "hostname": get_fqdn(),
88- "account_name": id.account_name,
89- "registration_password": \
90- id.registration_key,
91- "tags": tags,
92- "vm-info": get_vm_info()}
93- message.update(self._ec2_data)
94- self._exchange.send(message)
95- else:
96- self._reactor.fire("registration-failed")
97+ if not self._should_register:
98+ return
99+ id = self._identity
100+ self._message_store.delete_all_messages()
101+ tags = id.tags
102+ if not is_valid_tag_list(tags):
103+ tags = None
104+ logging.error("Invalid tags provided for cloud registration.")
105+ if self._config.cloud and self._ec2_data is not None:
106+ if self._otp:
107+ logging.info("Queueing message to register with OTP")
108+ message = {"type": "register-cloud-vm",
109+ "otp": self._otp,
110+ "hostname": get_fqdn(),
111+ "account_name": None,
112+ "registration_password": None,
113+ "tags": tags,
114+ "vm-info": get_vm_info()}
115+ message.update(self._ec2_data)
116+ if self._juju_data is not None:
117+ message["juju-info"] = self._juju_data
118+ self._exchange.send(message)
119 elif id.account_name:
120- with_word = ["without", "with"][bool(id.registration_key)]
121 with_tags = ["", u"and tags %s " % tags][bool(tags)]
122- logging.info(u"Queueing message to register with account %r %s"
123- "%s a password." % (id.account_name, with_tags,
124- with_word))
125- message = {"type": "register",
126- "computer_title": id.computer_title,
127+ logging.info(
128+ u"Queueing message to register with account %r %s"
129+ u"as an EC2 instance." % (id.account_name, with_tags))
130+ message = {"type": "register-cloud-vm",
131+ "otp": None,
132+ "hostname": get_fqdn(),
133 "account_name": id.account_name,
134 "registration_password": id.registration_key,
135- "hostname": get_fqdn(),
136 "tags": tags,
137 "vm-info": get_vm_info()}
138- self._exchange.send(message)
139- elif self._config.provisioning_otp:
140- logging.info(u"Queueing message to register with OTP as a"
141- u" newly provisioned machine.")
142- message = {"type": "register-provisioned-machine",
143- "otp": self._config.provisioning_otp}
144+ message.update(self._ec2_data)
145+ if self._juju_data is not None:
146+ message["juju-info"] = self._juju_data
147 self._exchange.send(message)
148 else:
149 self._reactor.fire("registration-failed")
150+ elif id.account_name:
151+ with_word = ["without", "with"][bool(id.registration_key)]
152+ with_tags = ["", u"and tags %s " % tags][bool(tags)]
153+ logging.info(u"Queueing message to register with account %r %s"
154+ "%s a password." % (id.account_name, with_tags,
155+ with_word))
156+ message = {"type": "register",
157+ "computer_title": id.computer_title,
158+ "account_name": id.account_name,
159+ "registration_password": id.registration_key,
160+ "hostname": get_fqdn(),
161+ "tags": tags,
162+ "vm-info": get_vm_info()}
163+ if self._juju_data is not None:
164+ message["juju-info"] = self._juju_data
165+ self._exchange.send(message)
166+ elif self._config.provisioning_otp:
167+ logging.info(u"Queueing message to register with OTP as a"
168+ u" newly provisioned machine.")
169+ message = {"type": "register-provisioned-machine",
170+ "otp": self._config.provisioning_otp}
171+ self._exchange.send(message)
172+ else:
173+ self._reactor.fire("registration-failed")
174
175 def _handle_set_id(self, message):
176 """Registered handler for the C{"set-id"} event.
177
178=== modified file 'landscape/broker/tests/helpers.py'
179--- landscape/broker/tests/helpers.py 2013-05-07 22:47:06 +0000
180+++ landscape/broker/tests/helpers.py 2013-09-27 14:18:49 +0000
181@@ -127,6 +127,9 @@
182
183 test_case.fetch_func = fetch_async
184 test_case.config.cloud = getattr(test_case, "cloud", False)
185+ if hasattr(test_case, "juju_contents"):
186+ test_case.makeFile(
187+ test_case.juju_contents, path=test_case.config.juju_filename)
188 test_case.handler = RegistrationHandler(
189 test_case.config, test_case.identity, test_case.reactor,
190 test_case.exchanger, test_case.pinger, test_case.mstore,
191
192=== modified file 'landscape/broker/tests/test_registration.py'
193--- landscape/broker/tests/test_registration.py 2013-08-20 23:48:23 +0000
194+++ landscape/broker/tests/test_registration.py 2013-09-27 14:18:49 +0000
195@@ -1,3 +1,4 @@
196+import json
197 import os
198 import logging
199 import pycurl
200@@ -177,7 +178,7 @@
201 """
202 When a computer_title and account_name are available, no
203 secure_id is set, and an exchange is about to happen,
204- queue a registration message.
205+ queue a registration message with VM information.
206 """
207 get_vm_info_mock = self.mocker.replace(get_vm_info)
208 get_vm_info_mock()
209@@ -484,6 +485,38 @@
210 "tags": None}])
211
212
213+class JujuRegistrationHandlerTest(RegistrationHandlerTestBase):
214+
215+ juju_contents = json.dumps({"environment-uuid": "DEAD-BEEF",
216+ "unit-name": "service/0",
217+ "api-addresses": "10.0.3.1:17070",
218+ "private-address": "127.0.0.1"})
219+
220+ def test_juju_information_added_when_present(self):
221+ """
222+ When Juju information is found in $data_dir/juju-info.json,
223+ key parts of it are sent in the registration message.
224+ """
225+ self.mstore.set_accepted_types(["register"])
226+ self.config.account_name = "account_name"
227+ self.reactor.fire("run")
228+ self.reactor.fire("pre-exchange")
229+
230+ self.assertMessages(
231+ self.mstore.get_pending_messages(),
232+ [{"type": "register",
233+ "computer_title": self.config.computer_title,
234+ "account_name": "account_name",
235+ "registration_password": None,
236+ "hostname": socket.getfqdn(),
237+ "vm-info": get_vm_info(),
238+ "tags": None,
239+ "juju-info": {"environment-uuid": "DEAD-BEEF",
240+ "api-addresses": ["10.0.3.1:17070"],
241+ "unit-name": "service/0"}
242+ }])
243+
244+
245 class CloudRegistrationHandlerTest(RegistrationHandlerTestBase):
246
247 cloud = True
248@@ -574,7 +607,6 @@
249 local_ipv4=u"10.0.0.1",
250 public_ipv4=u"10.0.0.2",
251 tags=None)
252-
253 # The get_vm_info() needs to be deferred to the else. If vm-info is
254 # specified in kwargs, get_vm_info() will typically be mocked.
255 if "vm_info" in kwargs:
256@@ -912,7 +944,8 @@
257 "registration_password": u"password",
258 "hostname": socket.getfqdn(),
259 "vm-info": get_vm_info(),
260- "tags": None}])
261+ "tags": None,
262+ }])
263
264 def test_should_register_in_cloud(self):
265 """
266@@ -1157,7 +1190,5 @@
267 self.config.provisioning_otp = ""
268 self.reactor.fire("pre-exchange")
269
270- self.assertMessages([],
271- self.mstore.get_pending_messages())
272- self.assertEqual(u"",
273- self.logfile.getvalue().strip())
274+ self.assertMessages([], self.mstore.get_pending_messages())
275+ self.assertEqual(u"", self.logfile.getvalue().strip())
276
277=== modified file 'landscape/deployment.py'
278--- landscape/deployment.py 2013-09-25 08:50:25 +0000
279+++ landscape/deployment.py 2013-09-27 14:18:49 +0000
280@@ -419,6 +419,11 @@
281 """
282 return os.path.join(self.data_path, "annotations.d")
283
284+ @property
285+ def juju_filename(self):
286+ """Get the path to the Juju JSON file."""
287+ return os.path.join(self.data_path, "juju-info.json")
288+
289
290 def get_versioned_persist(service):
291 """Get a L{Persist} database with upgrade rules applied.
292
293=== added file 'landscape/lib/juju.py'
294--- landscape/lib/juju.py 1970-01-01 00:00:00 +0000
295+++ landscape/lib/juju.py 2013-09-27 14:18:49 +0000
296@@ -0,0 +1,26 @@
297+import json
298+import logging
299+import os.path
300+
301+from landscape.lib.fs import read_file
302+
303+
304+def get_juju_info(config):
305+ """
306+ Returns the Juju info or C{None} if the path referenced from
307+ L{config} is not a file or that file isn't valid JSON.
308+ """
309+ juju_filename = config.juju_filename
310+ if not os.path.isfile(juju_filename):
311+ return None
312+ json_contents = read_file(juju_filename)
313+ try:
314+ juju_info = json.loads(json_contents)
315+ except Exception:
316+ logging.exception(
317+ "Error attempting to read JSON from %s" % juju_filename)
318+ return None
319+ else:
320+ if "api-addresses" in juju_info:
321+ juju_info["api-addresses"] = juju_info["api-addresses"].split()
322+ return juju_info
323
324=== modified file 'landscape/lib/tests/test_encoding.py'
325--- landscape/lib/tests/test_encoding.py 2013-03-20 08:58:22 +0000
326+++ landscape/lib/tests/test_encoding.py 2013-09-27 14:18:49 +0000
327@@ -4,7 +4,7 @@
328 from landscape.lib.encoding import encode_if_needed, encode_dict_if_needed
329
330
331-class EncodingTestTest(LandscapeTest):
332+class EncodingTest(LandscapeTest):
333
334 def test_encode_if_needed_utf_string(self):
335 """
336
337=== added file 'landscape/lib/tests/test_juju.py'
338--- landscape/lib/tests/test_juju.py 1970-01-01 00:00:00 +0000
339+++ landscape/lib/tests/test_juju.py 2013-09-27 14:18:49 +0000
340@@ -0,0 +1,62 @@
341+from collections import namedtuple
342+import json
343+
344+from landscape.tests.helpers import LandscapeTest
345+from landscape.lib.juju import get_juju_info
346+
347+
348+SAMPLE_JUJU_INFO = json.dumps({"environment-uuid": "DEAD-BEEF",
349+ "unit-name": "service/0",
350+ "api-addresses": "10.0.3.1:17070",
351+ "private-address": "127.0.0.1"})
352+
353+
354+class JujuTest(LandscapeTest):
355+
356+ Config = namedtuple("Config", "juju_filename")
357+
358+ def test_get_juju_info_sample_data(self):
359+ """L{get_juju_info} parses JSON data from the juju_filename file."""
360+ stub_config = self.Config(self.makeFile(SAMPLE_JUJU_INFO))
361+ juju_info = get_juju_info(stub_config)
362+ self.assertEqual(
363+ {u"environment-uuid": "DEAD-BEEF",
364+ u"unit-name": "service/0",
365+ u"api-addresses": ["10.0.3.1:17070"],
366+ u"private-address": "127.0.0.1"}, juju_info)
367+
368+ def test_get_juju_info_empty_file(self):
369+ """
370+ If L{get_juju_info} is called with a configuration pointing to
371+ an empty file, it returns C{None}.
372+ """
373+ stub_config = self.Config(self.makeFile(""))
374+ juju_info = get_juju_info(stub_config)
375+ self.log_helper.ignore_errors(ValueError)
376+ self.assertEqual(juju_info, None)
377+ self.assertIn("Error attempting to read JSON", self.logfile.getvalue())
378+
379+ def test_get_juju_info_not_a_file(self):
380+ """
381+ If L{get_juju_info} is called with a configuration pointing to
382+ a directory, it returns C{None}.
383+ """
384+ stub_config = self.Config("/")
385+ juju_info = get_juju_info(stub_config)
386+ self.assertIs(juju_info, None)
387+
388+ def test_get_juju_info_multiple_endpoints(self):
389+ """L{get_juju_info} turns space separated API addresses into a list."""
390+ juju_multiple_endpoints = json.dumps({
391+ "environment-uuid": "DEAD-BEEF",
392+ "unit-name": "service/0",
393+ "api-addresses": "10.0.3.1:17070 10.0.3.2:18080",
394+ "private-address": "127.0.0.1"})
395+
396+ stub_config = self.Config(self.makeFile(juju_multiple_endpoints))
397+ juju_info = get_juju_info(stub_config)
398+ self.assertEqual(
399+ {u"environment-uuid": "DEAD-BEEF",
400+ u"unit-name": "service/0",
401+ u"api-addresses": ["10.0.3.1:17070", "10.0.3.2:18080"],
402+ u"private-address": "127.0.0.1"}, juju_info)
403
404=== modified file 'landscape/message_schemas.py'
405--- landscape/message_schemas.py 2013-09-25 08:43:53 +0000
406+++ landscape/message_schemas.py 2013-09-27 14:18:49 +0000
407@@ -103,8 +103,12 @@
408 HARDWARE_INFO = Message("hardware-info", {
409 "data": utf8})
410
411-JUJU_INFO = Message("juju-info", {
412- "data": Dict(utf8, utf8)})
413+juju_data = {"environment-uuid": utf8,
414+ "api-addresses": List(utf8),
415+ "unit-name": utf8}
416+
417+# The copy is needed because Message mutates the dictionary
418+JUJU_INFO = Message("juju-info", juju_data.copy())
419
420 LOAD_AVERAGE = Message("load-average", {
421 "load-averages": List(Tuple(Int(), Float())),
422@@ -171,9 +175,11 @@
423 "hostname": utf8,
424 "account_name": utf8,
425 "tags": Any(utf8, Constant(None)),
426- "vm-info": String()},
427+ "vm-info": String(),
428+ "juju-info": KeyDict(juju_data)},
429 # hostname wasn't around in old versions
430- optional=["registration_password", "hostname", "tags", "vm-info"])
431+ optional=["registration_password", "hostname", "tags", "vm-info",
432+ "juju-info"])
433
434
435 REGISTER_PROVISIONED_MACHINE = Message(
436
437=== modified file 'landscape/monitor/config.py'
438--- landscape/monitor/config.py 2013-09-25 14:06:05 +0000
439+++ landscape/monitor/config.py 2013-09-27 14:18:49 +0000
440@@ -1,5 +1,3 @@
441-import os.path
442-
443 from landscape.deployment import Configuration
444
445
446@@ -32,8 +30,3 @@
447 if self.monitor_plugins == "ALL":
448 return ALL_PLUGINS
449 return [x.strip() for x in self.monitor_plugins.split(",")]
450-
451- @property
452- def juju_filename(self):
453- """Get the path to the Juju JSON file."""
454- return os.path.join(self.data_path, "juju-info.json")
455
456=== modified file 'landscape/monitor/jujuinfo.py'
457--- landscape/monitor/jujuinfo.py 2013-09-25 14:06:05 +0000
458+++ landscape/monitor/jujuinfo.py 2013-09-27 14:18:49 +0000
459@@ -1,8 +1,6 @@
460-import json
461 import logging
462-import os.path
463
464-from landscape.lib.fs import read_file
465+from landscape.lib.juju import get_juju_info
466 from landscape.monitor.plugin import MonitorPlugin
467
468
469@@ -21,32 +19,16 @@
470 broker.call_if_accepted("juju-info", self.send_juju_message, urgent)
471
472 def send_juju_message(self, urgent=False):
473- message = {}
474- juju_data = self._create_juju_info_message()
475- if juju_data:
476+ message = self._create_juju_info_message()
477+ if message:
478 message["type"] = "juju-info"
479- message["data"] = juju_data
480 logging.info("Queuing message with updated juju info.")
481 self.registry.broker.send_message(message, self._session_id,
482 urgent=urgent)
483
484 def _create_juju_info_message(self):
485- message = self._get_juju_info()
486+ message = get_juju_info(self.registry.config)
487 if message != self._persist.get("juju-info"):
488 self._persist.set("juju-info", message)
489 return message
490 return None
491-
492- def _get_juju_info(self):
493- juju_filename = self.registry.config.juju_filename
494- if not os.path.isfile(juju_filename):
495- return None
496- json_contents = read_file(juju_filename)
497- try:
498- juju_info = json.loads(json_contents)
499- except Exception:
500- logging.exception(
501- "Error attempting to read JSON from %s" % juju_filename)
502- return None
503- else:
504- return juju_info
505
506=== modified file 'landscape/monitor/tests/test_config.py'
507--- landscape/monitor/tests/test_config.py 2013-09-25 14:06:05 +0000
508+++ landscape/monitor/tests/test_config.py 2013-09-27 14:18:49 +0000
509@@ -1,5 +1,3 @@
510-import os.path
511-
512 from landscape.tests.helpers import LandscapeTest
513 from landscape.monitor.config import MonitorConfiguration, ALL_PLUGINS
514
515@@ -32,12 +30,3 @@
516 """
517 self.config.load(["--flush-interval", "123"])
518 self.assertEqual(self.config.flush_interval, 123)
519-
520- def test_juju_filename(self):
521- """
522- Can get path to juju JSON file from config.
523- """
524- self.config.data_path = self.makeDir()
525- self.assertEqual(self.config.juju_filename, os.path.join(
526- self.config.data_path, "juju-info.json"))
527-
528
529=== modified file 'landscape/monitor/tests/test_jujuinfo.py'
530--- landscape/monitor/tests/test_jujuinfo.py 2013-09-25 14:06:05 +0000
531+++ landscape/monitor/tests/test_jujuinfo.py 2013-09-27 14:18:49 +0000
532@@ -30,10 +30,9 @@
533 self.plugin.exchange()
534 message = self.mstore.get_pending_messages()[0]
535 self.assertEqual(message["type"], "juju-info")
536- self.assertEqual(message["data"]["environment-uuid"], "DEAD-BEEF")
537- self.assertEqual(message["data"]["unit-name"], "juju-unit-name")
538- self.assertEqual(message["data"]["api-addresses"], "10.0.3.1:17070")
539- self.assertEqual(message["data"]["private-address"], "127.0.0.1")
540+ self.assertEqual(message["environment-uuid"], "DEAD-BEEF")
541+ self.assertEqual(message["unit-name"], "juju-unit-name")
542+ self.assertEqual(message["api-addresses"], ["10.0.3.1:17070"])
543
544 def test_juju_info_reported_only_once(self):
545 """
546@@ -57,10 +56,9 @@
547 self.plugin.exchange()
548 message = self.mstore.get_pending_messages()[0]
549 self.assertEqual(message["type"], "juju-info")
550- self.assertEqual(message["data"]["environment-uuid"], "DEAD-BEEF")
551- self.assertEqual(message["data"]["unit-name"], "juju-unit-name")
552- self.assertEqual(message["data"]["api-addresses"], "10.0.3.1:17070")
553- self.assertEqual(message["data"]["private-address"], "127.0.0.1")
554+ self.assertEqual(message["environment-uuid"], "DEAD-BEEF")
555+ self.assertEqual(message["unit-name"], "juju-unit-name")
556+ self.assertEqual(message["api-addresses"], ["10.0.3.1:17070"])
557
558 self.makeFile(
559 json.dumps({"environment-uuid": "FEED-BEEF",
560@@ -71,10 +69,9 @@
561 self.plugin.exchange()
562 message = self.mstore.get_pending_messages()[1]
563 self.assertEqual(message["type"], "juju-info")
564- self.assertEqual(message["data"]["environment-uuid"], "FEED-BEEF")
565- self.assertEqual(message["data"]["unit-name"], "changed-unit-name")
566- self.assertEqual(message["data"]["api-addresses"], "10.0.3.2:17070")
567- self.assertEqual(message["data"]["private-address"], "127.0.1.1")
568+ self.assertEqual(message["environment-uuid"], "FEED-BEEF")
569+ self.assertEqual(message["unit-name"], "changed-unit-name")
570+ self.assertEqual(message["api-addresses"], ["10.0.3.2:17070"])
571
572 def test_no_message_with_invalid_json(self):
573 """No Juju message is sent if the JSON file is invalid."""
574
575=== modified file 'landscape/tests/test_deployment.py'
576--- landscape/tests/test_deployment.py 2013-09-25 09:24:17 +0000
577+++ landscape/tests/test_deployment.py 2013-09-27 14:18:49 +0000
578@@ -560,7 +560,7 @@
579
580 If a specific config file is requested, use this instead of defaults.
581
582- If a cmdilne --config option is specified this should take precedence
583+ If a cmdline --config option is specified this should take precedence
584 over either of the former options.
585 """
586 default_filename1 = self.makeFile("")
587@@ -599,8 +599,9 @@
588 The L{Configuration.socket_path} property returns the path to the
589 socket directory.
590 """
591- self.assertEqual(self.config.sockets_path,
592- "/var/lib/landscape/client/sockets")
593+ self.assertEqual(
594+ "/var/lib/landscape/client/sockets",
595+ self.config.sockets_path)
596
597 def test_annotations_path(self):
598 """
599@@ -611,6 +612,15 @@
600 "/var/lib/landscape/client/annotations.d",
601 self.config.annotations_path)
602
603+ def test_juju_filename(self):
604+ """
605+ The L{Configuration.juju_filename} property returns the path to the
606+ juju info file.
607+ """
608+ self.assertEqual(
609+ "/var/lib/landscape/client/juju-info.json",
610+ self.config.juju_filename)
611+
612 def test_clone(self):
613 """The L{Configuration.clone} method clones a configuration."""
614 self.write_config_file()

Subscribers

People subscribed via source and target branches

to all changes: