Merge lp:~frankban/juju-quickstart/maas-address into lp:juju-quickstart

Proposed by Francesco Banconi
Status: Merged
Merged at revision: 103
Proposed branch: lp:~frankban/juju-quickstart/maas-address
Merge into: lp:juju-quickstart
Diff against target: 1288 lines (+332/-433)
12 files modified
README.rst (+2/-0)
quickstart/__init__.py (+1/-1)
quickstart/app.py (+21/-31)
quickstart/manage.py (+6/-11)
quickstart/settings.py (+3/-0)
quickstart/tests/helpers.py (+11/-0)
quickstart/tests/test_app.py (+63/-183)
quickstart/tests/test_manage.py (+19/-68)
quickstart/tests/test_utils.py (+18/-0)
quickstart/tests/test_watchers.py (+122/-108)
quickstart/utils.py (+14/-0)
quickstart/watchers.py (+52/-31)
To merge this branch: bzr merge lp:~frankban/juju-quickstart/maas-address
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+241263@code.launchpad.net

Description of the change

Unit address from the machines watcher only

Only use the mega-watcher for machines to retrieve
the Juju GUI unit address.
This change has several consequences:
- it allows us to apply some logic on how the
  right address is chosen. For instance, now
  we try to resolve public hostnames before
  proceeding, and this should fix the cases
  where a cloud dns is not configured on the
  machine running quickstart. This is the case
  of many maas environments;
- it simplifies parsing the mega-watcher changes;
- more importantly, it breaks compatibility
  with very old versions of juju (<1.18), in which
  the mega-watcher for machines did not include
  machine addresses.

For this reason, quickstart now explicitly
drops support for juju < 1.18.1
(1.18.1 is the version on trusty universe).

This also allows for removing some version
checks in the code, including sudo handling when
calling bootstrap on local envs, several special
cases on the watcher side, and other oddities.

For the reasons above, I bumped the quickstart
version up to 1.5.0.

PS: my apologies for the long diff, hope the code
is still easy to follow. Sorry.

Tests: `make check`

QA:
run quickstart as usual, on local and cloud envs,
check it works properly when run again, etc.
this branch has been already successfully QAed in
a maas environment by Adam (Landscape team).

https://codereview.appspot.com/174790043/

To post a comment you must log in.
Revision history for this message
Francesco Banconi (frankban) wrote :

Reviewers: mp+241263_code.launchpad.net,

Message:
Please take a look.

Description:
Unit address from the machines watcher only

Only use the mega-watcher for machines to retrieve
the Juju GUI unit address.
This change has several consequences:
- it allows us to apply some logic on how the
   right address is chosen. For instance, now
   we try to resolve public hostnames before
   proceeding, and this should fix the cases
   where a cloud dns is not configured on the
   machine running quickstart. This is the case
   of many maas environments;
- it simplifies parsing the mega-watcher changes;
- more importantly, it breaks compatibility
   with very old versions of juju (<1.18), in which
   the mega-watcher for machines did not include
   machine addresses.

For this reason, quickstart now explicitly
drops support for juju < 1.18.1
(1.18.1 is the version on trusty universe).

This also allows for removing some version
checks in the code, including sudo handling when
calling bootstrap on local envs, several special
cases on the watcher side, and other oddities.

For the reasons above, I bumped the quickstart
version up to 1.5.0.

PS: my apologies for the long diff, hope the code
is still easy to follow. Sorry.

Tests: `make check`

QA:
run quickstart as usual, on local and cloud envs,
check it works properly when run again, etc.
this branch has been already successfully QAed in
a maas environment by Adam (Landscape team).

https://code.launchpad.net/~frankban/juju-quickstart/maas-address/+merge/241263

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/174790043/

Affected files (+334, -433 lines):
   M README.rst
   A [revision details]
   M quickstart/__init__.py
   M quickstart/app.py
   M quickstart/manage.py
   M quickstart/settings.py
   M quickstart/tests/helpers.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_manage.py
   M quickstart/tests/test_utils.py
   M quickstart/tests/test_watchers.py
   M quickstart/utils.py
   M quickstart/watchers.py

Revision history for this message
Brad Crittenden (bac) wrote :

On 2014/11/10 12:30:34, frankban wrote:
> Please take a look.

LGTM. Have not done QA yet.

https://codereview.appspot.com/174790043/

Revision history for this message
Brad Crittenden (bac) wrote :
Revision history for this message
Richard Harding (rharding) wrote :

LGTM ty for the updates!

https://codereview.appspot.com/174790043/diff/1/quickstart/app.py
File quickstart/app.py (left):

https://codereview.appspot.com/174790043/diff/1/quickstart/app.py#oldcode188
quickstart/app.py:188: if requires_sudo:
is this because of the new juju requirement?

https://codereview.appspot.com/174790043/

Revision history for this message
Francesco Banconi (frankban) wrote :

Thank you both for the reviews of this long diff!

https://codereview.appspot.com/174790043/diff/1/quickstart/app.py
File quickstart/app.py (left):

https://codereview.appspot.com/174790043/diff/1/quickstart/app.py#oldcode188
quickstart/app.py:188: if requires_sudo:
On 2014/11/10 15:25:20, rharding wrote:
> is this because of the new juju requirement?

Yes! We got rid of very old versions of juju requiring an explicit sudo
on the command line.

https://codereview.appspot.com/174790043/

Revision history for this message
Francesco Banconi (frankban) wrote :

*** Submitted:

Unit address from the machines watcher only

Only use the mega-watcher for machines to retrieve
the Juju GUI unit address.
This change has several consequences:
- it allows us to apply some logic on how the
   right address is chosen. For instance, now
   we try to resolve public hostnames before
   proceeding, and this should fix the cases
   where a cloud dns is not configured on the
   machine running quickstart. This is the case
   of many maas environments;
- it simplifies parsing the mega-watcher changes;
- more importantly, it breaks compatibility
   with very old versions of juju (<1.18), in which
   the mega-watcher for machines did not include
   machine addresses.

For this reason, quickstart now explicitly
drops support for juju < 1.18.1
(1.18.1 is the version on trusty universe).

This also allows for removing some version
checks in the code, including sudo handling when
calling bootstrap on local envs, several special
cases on the watcher side, and other oddities.

For the reasons above, I bumped the quickstart
version up to 1.5.0.

PS: my apologies for the long diff, hope the code
is still easy to follow. Sorry.

Tests: `make check`

QA:
run quickstart as usual, on local and cloud envs,
check it works properly when run again, etc.
this branch has been already successfully QAed in
a maas environment by Adam (Landscape team).

R=bac, rharding
CC=
https://codereview.appspot.com/174790043

https://codereview.appspot.com/174790043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'README.rst'
2--- README.rst 2014-09-01 14:44:39 +0000
3+++ README.rst 2014-11-10 12:30:00 +0000
4@@ -36,6 +36,8 @@
5 Juju Quickstart is available on Ubuntu releases 12.04 LTS (precise), 14.04 LTS
6 (trusty), 14.10 (utopic) and on OS X (10.7 and later).
7
8+Starting from version 1.5.0, Juju Quickstart only supports Juju >= 1.18.1.
9+
10 Ubuntu Installation
11 ~~~~~~~~~~~~~~~~~~~
12
13
14=== modified file 'quickstart/__init__.py'
15--- quickstart/__init__.py 2014-10-01 13:43:17 +0000
16+++ quickstart/__init__.py 2014-11-10 12:30:00 +0000
17@@ -45,7 +45,7 @@
18 Once Juju has been installed, the command can also be run as a juju plugin,
19 without the hyphen ("juju quickstart").
20 """
21-VERSION = (1, 4, 4)
22+VERSION = (1, 5, 0)
23
24
25 def get_version():
26
27=== modified file 'quickstart/app.py'
28--- quickstart/app.py 2014-08-25 16:57:41 +0000
29+++ quickstart/app.py 2014-11-10 12:30:00 +0000
30@@ -116,6 +116,19 @@
31 return juju_version
32
33
34+def check_juju_supported(juju_version):
35+ """Ensure the given Juju version is supported by Quickstart.
36+
37+ Raise a ProgramExit if Juju is not supported.
38+ """
39+ if juju_version < settings.JUJU_SUPPORTED_VERSION:
40+ current = b'.'.join(map(bytes, juju_version))
41+ supported = b'.'.join(map(bytes, settings.JUJU_SUPPORTED_VERSION))
42+ raise ProgramExit(
43+ b'the current Juju version ({}) is not supported: '
44+ b'please upgrade to Juju >= {}'.format(current, supported))
45+
46+
47 def ensure_ssh_keys():
48 """Ensure that SSH keys are available.
49
50@@ -163,8 +176,9 @@
51 raise ProgramExit(bytes(err))
52
53
54-def bootstrap(env_name, juju_command, requires_sudo=False, debug=False,
55- upload_tools=False, upload_series=None, constraints=None):
56+def bootstrap(
57+ env_name, juju_command, debug=False, upload_tools=False,
58+ upload_series=None, constraints=None):
59 """Bootstrap the Juju environment with the given name.
60
61 Do not bootstrap the environment if already bootstrapped.
62@@ -185,8 +199,6 @@
63 """
64 already_bootstrapped = False
65 cmd = [juju_command, 'bootstrap', '-e', env_name]
66- if requires_sudo:
67- cmd.insert(0, 'sudo')
68 if debug:
69 cmd.append('--debug')
70 if upload_tools:
71@@ -497,14 +509,10 @@
72 for action, data in unit_changes:
73 if data['Name'] == unit_name:
74 try:
75- data = watchers.parse_unit_change(
76- action, data, unit_status, address)
77+ unit_status, machine_id = watchers.parse_unit_change(
78+ action, data, unit_status)
79 except ValueError as err:
80 raise ProgramExit(bytes(err))
81- unit_status, address, machine_id = data
82- if address and unit_status == 'started':
83- # We can exit this loop.
84- return address
85 # The mega-watcher contains a single change for each specific
86 # unit. For this reason, we can exit the for loop here.
87 break
88@@ -528,12 +536,12 @@
89 action, data, machine_status, address)
90 except ValueError as err:
91 raise ProgramExit(bytes(err))
92- if address and unit_status == 'started':
93- # We can exit this loop.
94- return address
95 # The mega-watcher contains a single change for each specific
96 # machine. For this reason, we can exit the for loop here.
97 break
98+ if address and unit_status == 'started':
99+ # We can exit this loop.
100+ return address
101
102
103 def deploy_bundle(env, bundle_yaml, bundle_name, bundle_id):
104@@ -549,21 +557,3 @@
105 env.deploy_bundle(bundle_yaml, name=bundle_name, bundle_id=bundle_id)
106 except jujuclient.EnvError as err:
107 raise ProgramExit('bad API server response: {}'.format(err.message))
108-
109-
110-def juju_requires_sudo(env_type, juju_version, customized):
111- """Report whether the given Juju version requires "sudo".
112-
113- The env_type argument is the current environment type.
114- "e.g. local, ec2, azure"
115-
116- Raise a ProgramExit if a customized version of Juju is being used and Juju
117- itself requires "sudo"
118- """
119- # If the Juju version is less than 1.17.2 then use sudo for local envs.
120- if env_type == 'local' and juju_version < (1, 17, 2):
121- if customized:
122- raise ProgramExit(b'cannot use a customized Juju when it '
123- 'requires sudo')
124- return True
125- return False
126
127=== modified file 'quickstart/manage.py'
128--- quickstart/manage.py 2014-08-25 16:57:41 +0000
129+++ quickstart/manage.py 2014-11-10 12:30:00 +0000
130@@ -502,9 +502,7 @@
131 logging.debug('ensuring juju and dependencies are installed')
132 juju_version = app.ensure_dependencies(
133 options.distro_only, options.platform, juju_command)
134-
135- requires_sudo = app.juju_requires_sudo(
136- options.env_type, juju_version, custom_juju)
137+ app.check_juju_supported(juju_version)
138
139 logging.debug('ensuring SSH keys are available')
140 app.ensure_ssh_keys()
141@@ -513,13 +511,12 @@
142 options.env_name, options.env_type))
143 if options.env_type == 'local':
144 # If this is a local environment, notify the user that "sudo" will be
145- # required to bootstrap the application, even in newer Juju versions
146- # where "sudo" is invoked by juju-core itself.
147+ # required by Juju to bootstrap the application.
148 print('sudo privileges will be required to bootstrap the environment')
149
150 already_bootstrapped, bootstrap_node_series = app.bootstrap(
151 options.env_name, juju_command,
152- requires_sudo=requires_sudo, debug=options.debug,
153+ debug=options.debug,
154 upload_tools=options.upload_tools,
155 upload_series=options.upload_series,
156 constraints=options.constraints)
157@@ -585,9 +582,7 @@
158 'Run "juju quickstart -i" if you want to manage\n'
159 'or bootstrap your Juju environments using the\n'
160 'interactive session.\n'
161- 'Run "{sudo}juju destroy-environment {eflag}{env_name} [-y]"\n'
162- 'to destroy the environment you just bootstrapped.'.format(
163- env_name=options.env_name,
164- sudo='sudo ' if requires_sudo else '',
165- eflag='-e ' if juju_version < (1, 17, 2) else '')
166+ 'Run "juju destroy-environment {env_name} [-y]"\n'
167+ 'to destroy the environment you just bootstrapped.'
168+ ''.format(env_name=options.env_name)
169 )
170
171=== modified file 'quickstart/settings.py'
172--- quickstart/settings.py 2014-10-09 13:14:30 +0000
173+++ quickstart/settings.py 2014-11-10 12:30:00 +0000
174@@ -76,6 +76,9 @@
175 # The set of series supported by the Juju GUI charm.
176 JUJU_GUI_SUPPORTED_SERIES = tuple(DEFAULT_CHARM_URLS.keys())
177
178+# The minimum Juju version supported by Juju Quickstart,
179+JUJU_SUPPORTED_VERSION = (1, 18, 1)
180+
181 # The path to the MAAS command line interface.
182 MAAS_CMD = '/usr/bin/maas'
183
184
185=== modified file 'quickstart/tests/helpers.py'
186--- quickstart/tests/helpers.py 2014-09-08 17:49:51 +0000
187+++ quickstart/tests/helpers.py 2014-11-10 12:30:00 +0000
188@@ -195,6 +195,17 @@
189 mock_print = mock.patch('__builtin__.print')
190
191
192+def patch_check_resolvable(error=None):
193+ """Patch the utils.check_resolvable function to return the given error.
194+
195+ This is done so that tests do not try to resolve hostname addresses.
196+ """
197+ return mock.patch(
198+ 'quickstart.utils.check_resolvable',
199+ lambda hostname: error,
200+ )
201+
202+
203 class UrlReadTestsMixin(object):
204 """Expose a method to mock the quickstart.utils.urlread helper function."""
205
206
207=== modified file 'quickstart/tests/test_app.py'
208--- quickstart/tests/test_app.py 2014-08-25 16:57:41 +0000
209+++ quickstart/tests/test_app.py 2014-11-10 12:30:00 +0000
210@@ -298,6 +298,28 @@
211 self.assertEqual(3, mock_call.call_count)
212
213
214+class TestCheckJujuSupported(ProgramExitTestsMixin, unittest.TestCase):
215+
216+ supported_versions = [(1, 18, 1), (1, 19, 0), (1, 42, 47), (2, 0, 0)]
217+ unsupported_versions = [(1, 18, 0), (1, 17, 42), (1, 0, 0), (0, 20, 47)]
218+
219+ def test_supported(self):
220+ # No exceptions are raised if the Juju version is supported.
221+ for version in self.supported_versions:
222+ app.check_juju_supported(version)
223+
224+ def test_not_supported(self):
225+ # A ProgramExit error is raised if the Juju version is not supported.
226+ error = (
227+ 'the current Juju version ({}.{}.{}) is not supported: '
228+ 'please upgrade to Juju >= ' +
229+ '.'.join(map(str, settings.JUJU_SUPPORTED_VERSION))
230+ )
231+ for version in self.unsupported_versions:
232+ with self.assert_program_exit(error.format(*version)):
233+ app.check_juju_supported(version)
234+
235+
236 @helpers.mock_print
237 class TestEnsureSSHKeys(
238 helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):
239@@ -484,19 +506,6 @@
240 ] + self.make_status_calls(1))
241 mock_print.assert_called_once_with(self.status_message)
242
243- def test_success_local_provider(self, mock_print):
244- # The environment is bootstrapped with sudo using the local provider.
245- with self.patch_multiple_calls(self.make_side_effects()) as mock_call:
246- already_bootstrapped, series = app.bootstrap(
247- self.env_name, self.juju_command, requires_sudo=True)
248- self.assertFalse(already_bootstrapped)
249- self.assertEqual(series, 'hoary')
250- mock_call.assert_has_calls([
251- mock.call(
252- 'sudo', self.juju_command, 'bootstrap', '-e', self.env_name),
253- ] + self.make_status_calls(1))
254- mock_print.assert_called_once_with(self.status_message)
255-
256 def test_success_debug(self, mock_print):
257 # The environment is successfully bootstrapped in debug mode.
258 with self.patch_multiple_calls(self.make_side_effects()) as mock_call:
259@@ -1275,16 +1284,16 @@
260
261
262 @helpers.mock_print
263+@helpers.patch_check_resolvable()
264 class TestWatch(
265 ProgramExitTestsMixin, helpers.ValueErrorTestsMixin,
266 unittest.TestCase):
267
268 address = 'unit.example.com'
269 machine_pending_call = mock.call('machine 0 provisioning is pending')
270- unit_placed_machine_call = mock.call('unit placed on unit.example.com')
271+ unit_placed_call = mock.call('unit placed on {}'.format(address))
272 machine_started_call = mock.call('machine 0 is started')
273 unit_pending_call = mock.call('django/42 deployment is pending')
274- unit_placed_unit_call = mock.call('django/42 placed on {}'.format(address))
275 unit_installed_call = mock.call('django/42 is installed')
276 unit_started_call = mock.call('django/42 is ready on machine 0')
277
278@@ -1314,16 +1323,9 @@
279 }]
280 return 'change', data
281
282- def make_unit_change(self, status, name='django/42', address=None):
283- """Create and return a unit change.
284-
285- If the address argument is None, the change does not include the
286- corresponding address field.
287- """
288- data = {'MachineId': '0', 'Name': name, 'Status': status}
289- if address is not None:
290- data['PublicAddress'] = address
291- return 'change', data
292+ def make_unit_change(self, status, name='django/42'):
293+ """Create and return a unit change."""
294+ return 'change', {'MachineId': '0', 'Name': name, 'Status': status}
295
296 # The following group of tests exercises both the function return value and
297 # the function output, even if the output is handled by sub-functions.
298@@ -1334,35 +1336,10 @@
299 # The glorious moments in the unit's life are properly highlighted.
300 # The machine achievements are also celebrated.
301 env = self.make_env([
302- ([self.make_unit_change('pending', address='')],
303+ ([self.make_unit_change('pending')],
304 [self.make_machine_change('pending')]),
305 ([], [self.make_machine_change('started')]),
306- ([self.make_unit_change('pending', address=self.address)], []),
307- ([self.make_unit_change('installed', address=self.address)], []),
308- ([self.make_unit_change('started', address=self.address)], []),
309- ])
310- address = app.watch(env, 'django/42')
311- self.assertEqual(self.address, address)
312- self.assertEqual(6, mock_print.call_count)
313- mock_print.assert_has_calls([
314- self.unit_pending_call,
315- self.machine_pending_call,
316- self.machine_started_call,
317- self.unit_placed_unit_call,
318- self.unit_installed_call,
319- self.unit_started_call,
320- ])
321-
322- def test_unit_life_with_machine_address(self, mock_print):
323- # The glorious moments in the unit's life are properly highlighted.
324- # The machine achievements are also celebrated.
325- # This time the new mega-watcher behavior is simulated, in which
326- # addresses are included in the machine change.
327- env = self.make_env([
328- ([self.make_unit_change('pending')],
329- [self.make_machine_change('pending', address='')]),
330 ([], [self.make_machine_change('started', address=self.address)]),
331- ([self.make_unit_change('pending')], []),
332 ([self.make_unit_change('installed')], []),
333 ([self.make_unit_change('started')], []),
334 ])
335@@ -1372,8 +1349,8 @@
336 mock_print.assert_has_calls([
337 self.unit_pending_call,
338 self.machine_pending_call,
339- self.unit_placed_machine_call,
340 self.machine_started_call,
341+ self.unit_placed_call,
342 self.unit_installed_call,
343 self.unit_started_call,
344 ])
345@@ -1382,21 +1359,23 @@
346 # Strange unit evolutions are handled.
347 env = self.make_env([
348 # The unit is first reachable and then pending. The machine starts
349- # when the unit is already installed. All of this makes no sense
350- # and should never happen, but if it does, we deal with it.
351- ([self.make_unit_change('pending', address=self.address)], []),
352- ([self.make_unit_change('pending', address='')],
353- [self.make_machine_change('pending')]),
354- ([self.make_unit_change('installed', address=self.address)], []),
355- ([], [self.make_machine_change('started')]),
356- ([self.make_unit_change('started', address=self.address)], []),
357+ # when the unit is already installed. The machine has an address
358+ # when still provisioning. All of this makes no sense and should
359+ # never happen, but if it does, we deal with it.
360+ ([self.make_unit_change('pending')], []),
361+ ([], [self.make_machine_change('pending', address=self.address)]),
362+ ([self.make_unit_change('pending')],
363+ [self.make_machine_change('pending', address='')]),
364+ ([self.make_unit_change('installed')], []),
365+ ([], [self.make_machine_change('started', address=self.address)]),
366+ ([self.make_unit_change('started')], []),
367 ])
368 address = app.watch(env, 'django/42')
369 self.assertEqual(self.address, address)
370 self.assertEqual(6, mock_print.call_count)
371 mock_print.assert_has_calls([
372- self.unit_placed_unit_call,
373 self.unit_pending_call,
374+ self.unit_placed_call,
375 self.machine_pending_call,
376 self.unit_installed_call,
377 self.machine_started_call,
378@@ -1404,22 +1383,8 @@
379 ])
380
381 def test_missing_changes(self, mock_print):
382- # Only the unit started change is strictly required when the unit
383- # change includes the public address.
384- env = self.make_env([
385- ([self.make_unit_change('started', address=self.address)], []),
386- ])
387- address = app.watch(env, 'django/42')
388- self.assertEqual(self.address, address)
389- self.assertEqual(2, mock_print.call_count)
390- mock_print.assert_has_calls([
391- self.unit_placed_unit_call,
392- self.unit_started_call,
393- ])
394-
395- def test_missing_changes_with_machine_address(self, mock_print):
396- # When using the new mega-watcher, a machine change including its
397- # public address is also required.
398+ # Only the unit started change is strictly required when the machine
399+ # change includes the addresses.
400 env = self.make_env([
401 ([self.make_unit_change('started')], []),
402 ([], [self.make_machine_change('started', address=self.address)]),
403@@ -1429,29 +1394,12 @@
404 self.assertEqual(3, mock_print.call_count)
405 mock_print.assert_has_calls([
406 self.unit_started_call,
407- self.unit_placed_machine_call,
408+ self.unit_placed_call,
409 self.machine_started_call,
410 ])
411
412 def test_ignored_machine_changes(self, mock_print):
413 # All machine changes are ignored until the application knows what
414- # machine the unit belongs to.
415- env = self.make_env([
416- ([], [self.make_machine_change('pending')]),
417- ([], [self.make_machine_change('started')]),
418- ([self.make_unit_change('started', address=self.address)], []),
419- ])
420- address = app.watch(env, 'django/42')
421- self.assertEqual(self.address, address)
422- # No machine related messages have been printed.
423- self.assertEqual(2, mock_print.call_count)
424- mock_print.assert_has_calls([
425- self.unit_placed_unit_call,
426- self.unit_started_call,
427- ])
428-
429- def test_ignored_machine_changes_with_machine_address(self, mock_print):
430- # All machine changes are ignored until the application knows what
431 # machine the unit belongs to. When the above happens, previously
432 # collected machine changes are still parsed in the case the address
433 # is not yet known.
434@@ -1468,7 +1416,7 @@
435 self.assertEqual(3, mock_print.call_count)
436 mock_print.assert_has_calls([
437 self.unit_started_call,
438- self.unit_placed_machine_call,
439+ self.unit_placed_call,
440 self.machine_started_call,
441 ])
442
443@@ -1477,20 +1425,6 @@
444 # This happens, e.g., when executing Quickstart a second time, and both
445 # the unit and the machine are already started.
446 env = self.make_env([
447- ([self.make_unit_change('started', address=self.address)],
448- [self.make_machine_change('started')]),
449- ])
450- address = app.watch(env, 'django/42')
451- self.assertEqual(self.address, address)
452- self.assertEqual(2, mock_print.call_count)
453-
454- def test_unit_already_deployed_with_machine_address(self, mock_print):
455- # Simulate the unit we are observing has been already deployed.
456- # This happens, e.g., when executing Quickstart a second time, and both
457- # the unit and the machine are already started.
458- # This time the new mega-watcher behavior is simulated, in which
459- # addresses are included in the machine change.
460- env = self.make_env([
461 ([self.make_unit_change('started')],
462 [self.make_machine_change('started', address=self.address)]),
463 ])
464@@ -1499,7 +1433,7 @@
465 self.assertEqual(3, mock_print.call_count)
466 mock_print.assert_has_calls([
467 self.unit_started_call,
468- self.unit_placed_machine_call,
469+ self.unit_placed_call,
470 self.machine_started_call,
471 ])
472
473@@ -1509,31 +1443,6 @@
474 # environment type: the unit is deployed on the bootstrap node, which
475 # is assumed to be started.
476 env = self.make_env([
477- ([self.make_unit_change('pending', address='')],
478- [self.make_machine_change('started')]),
479- ([self.make_unit_change('pending', address=self.address)], []),
480- ([self.make_unit_change('installed', address=self.address)], []),
481- ([self.make_unit_change('started', address=self.address)], []),
482- ])
483- address = app.watch(env, 'django/42')
484- self.assertEqual(self.address, address)
485- self.assertEqual(5, mock_print.call_count)
486- mock_print.assert_has_calls([
487- self.unit_pending_call,
488- self.machine_started_call,
489- self.unit_placed_unit_call,
490- self.unit_installed_call,
491- self.unit_started_call,
492- ])
493-
494- def test_machine_already_started_with_machine_address(self, mock_print):
495- # Simulate the unit is being deployed on an already started machine.
496- # This happens, e.g., when running Quickstart on a non-local
497- # environment type: the unit is deployed on the bootstrap node, which
498- # is assumed to be started.
499- # This time the new mega-watcher behavior is simulated, in which
500- # addresses are included in the machine change.
501- env = self.make_env([
502 ([self.make_unit_change('pending')],
503 [self.make_machine_change('started', address=self.address)]),
504 ([self.make_unit_change('pending')], []),
505@@ -1545,7 +1454,7 @@
506 self.assertEqual(5, mock_print.call_count)
507 mock_print.assert_has_calls([
508 self.unit_pending_call,
509- self.unit_placed_machine_call,
510+ self.unit_placed_call,
511 self.machine_started_call,
512 self.unit_installed_call,
513 self.unit_started_call,
514@@ -1555,24 +1464,26 @@
515 # Changes to units or machines we are not observing are ignored. Also
516 # ensure that repeated changes to a single entity are ignored, even if
517 # they are unlikely to happen.
518- pending_unit_change = self.make_unit_change('pending', address='')
519- started_unit_change = self.make_unit_change(
520- 'started', address=self.address)
521+ pending_unit_change = self.make_unit_change('pending')
522+ started_unit_change = self.make_unit_change('started')
523+ pending_machine_change = self.make_machine_change('pending')
524 env = self.make_env([
525- # Add a repeated change.
526+ # Add a repeated unit change.
527 ([pending_unit_change, pending_unit_change],
528- [self.make_machine_change('pending')]),
529+ [pending_machine_change]),
530 # Add extraneous unit and machine changes.
531 ([self.make_unit_change('pending', name='haproxy/0')],
532 [self.make_machine_change('pending', name='42')]),
533+ # Add a repeated machine change.
534+ ([], [pending_machine_change, pending_machine_change]),
535 # Add a change to an extraneous machine.
536 ([], [self.make_machine_change('started', name='42'),
537- self.make_machine_change('started')]),
538+ self.make_machine_change('started', address=self.address)]),
539 # Add a change to an extraneous unit.
540 ([self.make_unit_change('started', name='haproxy/0'),
541- self.make_unit_change('pending', address=self.address)], []),
542- ([self.make_unit_change('installed', address=self.address)], []),
543- # Add another repeated change.
544+ self.make_unit_change('pending')], []),
545+ ([self.make_unit_change('installed')], []),
546+ # Add another repeated unit change.
547 ([started_unit_change, started_unit_change], []),
548 ])
549 address = app.watch(env, 'django/42')
550@@ -1581,8 +1492,8 @@
551 mock_print.assert_has_calls([
552 self.unit_pending_call,
553 self.machine_pending_call,
554+ self.unit_placed_call,
555 self.machine_started_call,
556- self.unit_placed_unit_call,
557 self.unit_installed_call,
558 self.unit_started_call,
559 ])
560@@ -1590,7 +1501,7 @@
561 def test_api_error(self, mock_print):
562 # A ProgramExit is raised if an error occurs in one of the API calls.
563 env = self.make_env([
564- ([self.make_unit_change('pending', address='')], []),
565+ ([self.make_unit_change('pending')], []),
566 self.make_env_error('next returned an error'),
567 ])
568 expected = 'bad API server response: next returned an error'
569@@ -1602,14 +1513,13 @@
570 def test_other_errors(self, mock_print):
571 # Any other errors occurred during the process are not trapped.
572 env = self.make_env([
573- ([self.make_unit_change('installed', address=self.address)], []),
574+ ([self.make_unit_change('installed')], []),
575 ValueError('explode!'),
576 ])
577 with self.assert_value_error('explode!'):
578 app.watch(env, 'django/42')
579- self.assertEqual(2, mock_print.call_count)
580- mock_print.assert_has_calls([
581- self.unit_placed_unit_call, self.unit_installed_call])
582+ self.assertEqual(1, mock_print.call_count)
583+ mock_print.assert_has_calls([self.unit_installed_call])
584
585 def test_machine_status_error(self, mock_print):
586 # A ProgramExit is raised if an the machine is found in an error state.
587@@ -1622,8 +1532,7 @@
588 # The unit pending change is required to make the function know which
589 # machine to observe.
590 env = self.make_env([
591- ([self.make_unit_change('pending', address='')],
592- [change_machine_error]),
593+ ([self.make_unit_change('pending')], [change_machine_error]),
594 ])
595 expected = 'machine 0 is in an error state: error: oddities'
596 with self.assert_program_exit(expected):
597@@ -1677,32 +1586,3 @@
598 with self.assertRaises(ValueError) as context_manager:
599 app.deploy_bundle(env, self.yaml, self.name, None)
600 self.assertIs(error, context_manager.exception)
601-
602-
603-class TestJujuRequiresSudo(ProgramExitTestsMixin, unittest.TestCase):
604- no_sudo_versions = [
605- (1, 17, 2), (1, 17, 10), (1, 18, 0), (1, 18, 2), (2, 16, 1)]
606- sudo_versions = [(0, 7, 9), (1, 0, 0), (1, 16, 42), (1, 17, 0), (1, 17, 1)]
607-
608- def test_non_local_returns_false(self):
609- # A non-local provider does not require sudo.
610- actual = app.juju_requires_sudo('aws', None, None)
611- self.assertFalse(actual)
612-
613- def test_local_old_juju_returns_true(self):
614- # A juju previous to 1.17.2 requires sudo.
615- for version in self.sudo_versions:
616- self.assertTrue(app.juju_requires_sudo('local', version, False),
617- version)
618-
619- def test_local_newer_juju_returns_false(self):
620- # A juju 1.17.2 and newer does not require sudo.
621- for version in self.no_sudo_versions:
622- self.assertFalse(app.juju_requires_sudo('local', version, False),
623- version)
624-
625- def test_raises_programexit(self):
626- for version in self.sudo_versions:
627- with self.assert_program_exit('cannot use a customized Juju when '
628- 'it requires sudo'):
629- app.juju_requires_sudo('local', version, True)
630
631=== modified file 'quickstart/tests/test_manage.py'
632--- quickstart/tests/test_manage.py 2014-08-25 16:57:41 +0000
633+++ quickstart/tests/test_manage.py 2014-11-10 12:30:00 +0000
634@@ -795,7 +795,6 @@
635 mock_app.watch.return_value = '1.2.3.4'
636 # Make mock_app.create_auth_token return a fake authentication token.
637 mock_app.create_auth_token.return_value = 'AUTHTOKEN'
638- mock_app.juju_requires_sudo.return_value = False
639 options = self.make_options()
640 with mock.patch('quickstart.manage.platform_support.get_juju_command',
641 side_effect=[(self.juju_command, False)]):
642@@ -803,8 +802,9 @@
643 mock_app.ensure_dependencies.assert_called()
644 mock_app.ensure_ssh_keys.assert_called()
645 mock_app.bootstrap.assert_called_once_with(
646- options.env_name, self.juju_command, requires_sudo=False,
647- debug=options.debug, upload_tools=options.upload_tools,
648+ options.env_name, self.juju_command,
649+ debug=options.debug,
650+ upload_tools=options.upload_tools,
651 upload_series=options.upload_series,
652 constraints=options.constraints)
653 mock_app.get_api_url.assert_called_once_with(
654@@ -879,68 +879,19 @@
655 # where to deploy the charm, the service and unit data.
656 mock_app.check_environment.return_value = (
657 'cs:precise/juju-gui-42', '0', None, None)
658- mock_app.juju_requires_sudo.return_value = False
659- for version in versions:
660- mock_app.ensure_dependencies.return_value = version
661- with mock.patch(
662- 'quickstart.manage.platform_support.get_juju_command',
663- side_effect=[(self.juju_command, False)]):
664- manage.run(options)
665- mock_app.bootstrap.assert_called_once_with(
666- options.env_name, self.juju_command, requires_sudo=False,
667- debug=options.debug, upload_tools=options.upload_tools,
668- upload_series=options.upload_series,
669- constraints=options.constraints)
670- mock_app.bootstrap.reset_mock()
671-
672- def test_local_provider_requiring_sudo(self, mock_app, mock_open):
673- # The application correctly handles working with local providers when
674- # Juju requires an external "sudo" call to bootstrap the environment.
675- # Sudo privileges are required if the Juju version is < 1.17.2.
676- options = self.make_options(env_type='local')
677- versions = [(0, 7, 9), (1, 0, 0), (1, 16, 42), (1, 17, 0), (1, 17, 1)]
678- # Make mock_app.bootstrap return the already_bootstrapped flag and the
679- # bootstrap node series.
680- mock_app.bootstrap.return_value = (True, 'trusty')
681- # Make mock_app.check_environment return the charm URL, the machine
682- # where to deploy the charm, the service and unit data.
683- mock_app.check_environment.return_value = (
684- 'cs:trusty/juju-gui-42', '0', None, None)
685- mock_app.juju_requires_sudo.return_value = True
686- for version in versions:
687- mock_app.ensure_dependencies.return_value = version
688- with mock.patch(
689- 'quickstart.manage.platform_support.get_juju_command',
690- side_effect=[(self.juju_command, False)]):
691- manage.run(options)
692- mock_app.bootstrap.assert_called_once_with(
693- options.env_name, self.juju_command, requires_sudo=True,
694- debug=options.debug, upload_tools=options.upload_tools,
695- upload_series=options.upload_series,
696- constraints=options.constraints)
697- mock_app.bootstrap.reset_mock()
698-
699- def test_no_local_no_sudo(self, mock_app, mock_open):
700- # Sudo privileges are never required for non-local environments.
701- options = self.make_options(env_type='ec2')
702- mock_app.ensure_dependencies.return_value = (1, 14, 0)
703- # Make mock_app.bootstrap return the already_bootstrapped flag and the
704- # bootstrap node series.
705- mock_app.bootstrap.return_value = (True, 'precise')
706- # Make mock_app.check_environment return the charm URL, the machine
707- # where to deploy the charm, the service and unit data.
708- mock_app.check_environment.return_value = (
709- 'cs:precise/juju-gui-42', '0', None, None)
710- mock_app.juju_requires_sudo.return_value = False
711- with mock.patch('quickstart.manage.platform_support.get_juju_command',
712- side_effect=[(self.juju_command, False)]):
713- manage.run(options)
714- mock_app.bootstrap.assert_called_once_with(
715- options.env_name, self.juju_command,
716- requires_sudo=False,
717- debug=options.debug, upload_tools=options.upload_tools,
718- upload_series=options.upload_series,
719- constraints=options.constraints)
720+ for version in versions:
721+ mock_app.ensure_dependencies.return_value = version
722+ with mock.patch(
723+ 'quickstart.manage.platform_support.get_juju_command',
724+ side_effect=[(self.juju_command, False)]):
725+ manage.run(options)
726+ mock_app.bootstrap.assert_called_once_with(
727+ options.env_name, self.juju_command,
728+ debug=options.debug,
729+ upload_tools=options.upload_tools,
730+ upload_series=options.upload_series,
731+ constraints=options.constraints)
732+ mock_app.bootstrap.reset_mock()
733
734 def test_no_browser(self, mock_app, mock_open):
735 # It is possible to avoid opening the GUI in the browser.
736@@ -1015,14 +966,14 @@
737 mock_app.bootstrap.return_value = (True, 'precise')
738 mock_app.check_environment.return_value = (
739 'cs:precise/juju-gui-42', '0', None, None)
740- mock_app.juju_requires_sudo.return_value = False
741 options = self.make_options(env_type='aws')
742 juju_command = 'some/devel/path/juju'
743 with mock.patch('os.environ', {'JUJU': juju_command}):
744 manage.run(options)
745 mock_app.bootstrap.assert_called_once_with(
746- options.env_name, juju_command, requires_sudo=False,
747- debug=options.debug, upload_tools=options.upload_tools,
748+ options.env_name, juju_command,
749+ debug=options.debug,
750+ upload_tools=options.upload_tools,
751 upload_series=options.upload_series,
752 constraints=options.constraints)
753 mock_app.get_api_url.assert_called_once_with(
754
755=== modified file 'quickstart/tests/test_utils.py'
756--- quickstart/tests/test_utils.py 2014-08-22 16:38:23 +0000
757+++ quickstart/tests/test_utils.py 2014-11-10 12:30:00 +0000
758@@ -156,6 +156,24 @@
759 utils.call('echo', 'we are the borg!')
760
761
762+class TestCheckResolvable(unittest.TestCase):
763+
764+ def test_resolvable(self):
765+ # None is returned if the hostname can be resolved.
766+ expected_log = 'example.com resolved to 1.2.3.4'
767+ with helpers.assert_logs([expected_log], level='debug'):
768+ with mock.patch('socket.gethostbyname', return_value='1.2.3.4'):
769+ error = utils.check_resolvable('example.com')
770+ self.assertIsNone(error)
771+
772+ def test_not_resolvable(self):
773+ # An error message is returned if the hostname cannot be resolved.
774+ exception = socket.gaierror('bad wolf')
775+ with mock.patch('socket.gethostbyname', side_effect=exception):
776+ error = utils.check_resolvable('example.com')
777+ self.assertEqual('bad wolf', error)
778+
779+
780 @mock.patch('__builtin__.print', mock.Mock())
781 class TestParseGuiCharmUrl(unittest.TestCase):
782
783
784=== modified file 'quickstart/tests/test_watchers.py'
785--- quickstart/tests/test_watchers.py 2014-07-04 13:09:11 +0000
786+++ quickstart/tests/test_watchers.py 2014-11-10 12:30:00 +0000
787@@ -63,23 +63,29 @@
788
789 class TestRetrievePublicAddress(unittest.TestCase):
790
791+ def resolver(self, hostname):
792+ """A fake resolver returning no errors."""
793+ return None
794+
795 def test_empty_addresses(self):
796 # None is returned if there are no available addresses.
797- self.assertIsNone(watchers.retrieve_public_adddress([]))
798+ address = watchers.retrieve_public_adddress([], self.resolver)
799+ self.assertIsNone(address)
800
801 def test_cloud_address_not_found(self):
802 # None is returned if a cloud machine public address is not available.
803 addresses = [
804 {'NetworkName': '',
805- 'Scope': 'local-cloud',
806- 'Type': 'hostname',
807- 'Value': 'eu-west-1.example.internal'},
808+ 'Scope': 'public',
809+ 'Type': 'ipv6',
810+ 'Value': 'fe80::92b8:d0ff:fe94:8f8c'},
811 {'NetworkName': '',
812 'Scope': 'local-cloud',
813- 'Type': 'ipv4',
814- 'Value': '10.42.47.10'},
815+ 'Type': 'ipv6',
816+ 'Value': 'fe80::216:3eff:fefd:787e'},
817 ]
818- self.assertIsNone(watchers.retrieve_public_adddress(addresses))
819+ address = watchers.retrieve_public_adddress(addresses, self.resolver)
820+ self.assertIsNone(address)
821
822 def test_container_address_not_found(self):
823 # None is returned if an LXC public address is not available.
824@@ -89,7 +95,8 @@
825 'Type': 'ipv6',
826 'Value': 'fe80::216:3eff:fefd:787e',
827 }]
828- self.assertIsNone(watchers.retrieve_public_adddress(addresses))
829+ address = watchers.retrieve_public_adddress(addresses, self.resolver)
830+ self.assertIsNone(address)
831
832 def test_empty_public_address(self):
833 # None is returned if the public address has no value.
834@@ -103,17 +110,20 @@
835 'Type': 'ipv4',
836 'Value': ''},
837 ]
838- self.assertIsNone(watchers.retrieve_public_adddress(addresses))
839+ address = watchers.retrieve_public_adddress(addresses, self.resolver)
840+ self.assertIsNone(address)
841
842 def test_cloud_addresses(self):
843 # The public address of a cloud machine is properly returned.
844- public_address = watchers.retrieve_public_adddress(cloud_addresses)
845- self.assertEqual('eu-west-1.compute.example.com', public_address)
846+ address = watchers.retrieve_public_adddress(
847+ cloud_addresses, self.resolver)
848+ self.assertEqual('eu-west-1.compute.example.com', address)
849
850 def test_container_addresses(self):
851 # The public address of an LXC instance is properly returned.
852- public_address = watchers.retrieve_public_adddress(container_addresses)
853- self.assertEqual('10.0.3.42', public_address)
854+ address = watchers.retrieve_public_adddress(
855+ container_addresses, self.resolver)
856+ self.assertEqual('10.0.3.42', address)
857
858 def test_old_juju_version(self):
859 # The public address is properly returned when using Juju < 1.20.
860@@ -129,11 +139,32 @@
861 'Type': 'hostname',
862 'Value': 'eu-west-1.example.internal'},
863 ]
864- public_address = watchers.retrieve_public_adddress(addresses)
865- self.assertEqual('eu-west-1.compute.example.com', public_address)
866-
867- def test_last_unknown_address(self):
868- # If the scope of multiple addresses is unknown, the last one is taken.
869+ address = watchers.retrieve_public_adddress(addresses, self.resolver)
870+ self.assertEqual('eu-west-1.compute.example.com', address)
871+
872+ def test_local_cloud_address(self):
873+ # If there are no available public addresses the first ipv4 address
874+ # with cloud-local scope is returned.
875+ addresses = [
876+ {'NetworkName': '',
877+ 'Scope': 'local-cloud',
878+ 'Type': 'hostname',
879+ 'Value': 'eu-west-1.example.internal'},
880+ {'NetworkName': '',
881+ 'Scope': 'local-cloud',
882+ 'Type': 'ipv4',
883+ 'Value': '10.42.47.10'},
884+ {'NetworkName': '',
885+ 'Scope': 'local-cloud',
886+ 'Type': 'ipv4',
887+ 'Value': '1.2.3.4'},
888+ ]
889+ address = watchers.retrieve_public_adddress(addresses, self.resolver)
890+ self.assertEqual('10.42.47.10', address)
891+
892+ def test_unknown_address(self):
893+ # If there are no available public or local-cloud addresses, the first
894+ # address with unknown scope is returned.
895 addresses = [
896 {'NetworkName': '',
897 'Scope': '',
898@@ -144,8 +175,52 @@
899 'Type': 'ipv4',
900 'Value': '10.0.3.47'},
901 ]
902- public_address = watchers.retrieve_public_adddress(addresses)
903- self.assertEqual('10.0.3.47', public_address)
904+ address = watchers.retrieve_public_adddress(addresses, self.resolver)
905+ self.assertEqual('10.0.3.42', address)
906+
907+ def test_preferred_fallback_address(self):
908+ # If there are no available public addresses the first fallback address
909+ # in the list is returned.
910+ addresses = [
911+ {'NetworkName': '',
912+ 'Scope': '',
913+ 'Type': 'ipv4',
914+ 'Value': '10.0.3.47'},
915+ {'NetworkName': '',
916+ 'Scope': 'local-cloud',
917+ 'Type': 'ipv4',
918+ 'Value': '10.42.47.10'},
919+
920+ ]
921+ address = watchers.retrieve_public_adddress(addresses, self.resolver)
922+ self.assertEqual('10.0.3.47', address)
923+ # Now test with a reversed order.
924+ address = watchers.retrieve_public_adddress(
925+ reversed(addresses), self.resolver)
926+ self.assertEqual('10.42.47.10', address)
927+
928+ def test_unresolvable_public_address(self):
929+ # None is returned if a public cloud is found but it is not resolvable.
930+ addresses = [
931+ {'NetworkName': '',
932+ 'Scope': 'public',
933+ 'Type': 'hostname',
934+ 'Value': 'eu-west-1.example.internal'},
935+ ]
936+ expected_warning = (
937+ 'cannot resolve public eu-west-1.example.internal address, '
938+ 'looking for another candidate: bad wolf')
939+ with helpers.assert_logs([expected_warning], level='warn'):
940+ address = watchers.retrieve_public_adddress(
941+ addresses, lambda hostname: 'bad wolf')
942+ self.assertIsNone(address)
943+
944+ def test_unresolvable_public_address_fallback(self):
945+ # If there are no resolvable public addresses the first ipv4 address
946+ # with cloud-local scope is returned.
947+ address = watchers.retrieve_public_adddress(
948+ cloud_addresses, lambda hostname: 'bad wolf')
949+ self.assertEqual('10.42.47.10', address)
950
951
952 class TestParseMachineChange(helpers.ValueErrorTestsMixin, unittest.TestCase):
953@@ -205,8 +280,12 @@
954 # A message is printed to stdout when the machine obtains a public
955 # address.
956 data = {'Addresses': cloud_addresses, 'Id': '1', 'Status': 'pending'}
957- status, address = watchers.parse_machine_change(
958- 'change', data, 'pending', '')
959+ # Patch the hostname resolver used to check hostname addresses.
960+ # Note that resolve error cases are properly exercised by the
961+ # watchers.retrieve_public_adddress tests.
962+ with helpers.patch_check_resolvable():
963+ status, address = watchers.parse_machine_change(
964+ 'change', data, 'pending', '')
965 self.assertEqual('pending', status)
966 self.assertEqual('eu-west-1.compute.example.com', address)
967 mock_print.assert_called_once_with(
968@@ -248,8 +327,8 @@
969 # A ValueError is raised if the change represents a unit removal.
970 data = {'Name': 'django/42', 'Status': 'started'}
971 with self.assert_value_error('django/42 unexpectedly removed'):
972- # The last two arguments are the current status and address.
973- watchers.parse_unit_change('remove', data, '', '')
974+ # The last argument is the current status.
975+ watchers.parse_unit_change('remove', data, '')
976
977 def test_unit_error(self):
978 # A ValueError is raised if the unit is in an error state.
979@@ -260,120 +339,55 @@
980 }
981 expected_error = 'django/0 is in an error state: start error: bad wolf'
982 with self.assert_value_error(expected_error):
983- # The last two arguments are the current status and address.
984- watchers.parse_unit_change('change', data, '', '')
985-
986- @helpers.mock_print
987- def test_address_notified(self, mock_print):
988- # A message is printed to stdout when the unit obtains a public
989- # address. The function returns the status, the new address and the
990- # machine identifier.
991- data = {
992- 'Name': 'haproxy/2',
993- 'Status': 'pending',
994- 'PublicAddress': 'haproxy2.example.com',
995- 'MachineId': '42',
996- }
997- status, address, machine_id = watchers.parse_unit_change(
998- 'change', data, 'pending', '')
999- self.assertEqual('pending', status)
1000- self.assertEqual('haproxy2.example.com', address)
1001- self.assertEqual('42', machine_id)
1002- mock_print.assert_called_once_with(
1003- 'haproxy/2 placed on haproxy2.example.com')
1004+ # The last argument is the current status.
1005+ watchers.parse_unit_change('change', data, '')
1006
1007 @helpers.mock_print
1008 def test_pending_status_notified(self, mock_print):
1009 # A message is printed to stdout when the unit changes its status to
1010- # "pending". The function returns the new status, the address and the
1011- # machine identifier. The last two values are empty strings if the unit
1012- # has not yet been assigned to a machine.
1013- data = {'Name': 'django/1', 'Status': 'pending', 'PublicAddress': ''}
1014- # The last two arguments are the current status and address.
1015- status, address, machine_id = watchers.parse_unit_change(
1016- 'change', data, '', '')
1017+ # "pending". The function returns the new status and the machine
1018+ # identifier, which is empty if the unit has not yet been assigned to a
1019+ # machine.
1020+ data = {'Name': 'django/1', 'Status': 'pending'}
1021+ # The last argument is the current status.
1022+ status, machine_id = watchers.parse_unit_change('change', data, '')
1023 self.assertEqual('pending', status)
1024- self.assertEqual('', address)
1025 self.assertEqual('', machine_id)
1026 mock_print.assert_called_once_with('django/1 deployment is pending')
1027
1028 @helpers.mock_print
1029 def test_installed_status_notified(self, mock_print):
1030 # A message is printed to stdout when the unit changes its status to
1031- # "installed". The function returns the new status, the address and the
1032- # machine identifier.
1033- data = {
1034- 'Name': 'django/42',
1035- 'Status': 'installed',
1036- 'PublicAddress': 'django42.example.com',
1037- 'MachineId': '1',
1038- }
1039- status, address, machine_id = watchers.parse_unit_change(
1040- 'change', data, 'pending', 'django42.example.com')
1041+ # "installed".
1042+ data = {'Name': 'django/42', 'Status': 'installed', 'MachineId': '1'}
1043+ status, machine_id = watchers.parse_unit_change(
1044+ 'change', data, 'pending')
1045 self.assertEqual('installed', status)
1046- self.assertEqual('django42.example.com', address)
1047 self.assertEqual('1', machine_id)
1048 mock_print.assert_called_once_with('django/42 is installed')
1049
1050 @helpers.mock_print
1051 def test_started_status_notified(self, mock_print):
1052 # A message is printed to stdout when the unit changes its status to
1053- # "started". The function returns the new status, the address and the
1054- # machine identifier.
1055- data = {
1056- 'Name': 'wordpress/0',
1057- 'Status': 'started',
1058- 'PublicAddress': 'wordpress0.example.com',
1059- 'MachineId': '0',
1060- }
1061- status, address, machine_id = watchers.parse_unit_change(
1062- 'change', data, '', 'wordpress0.example.com')
1063+ # "started".
1064+ data = {'Name': 'wordpress/0', 'Status': 'started', 'MachineId': '0'}
1065+ # The last argument is the current status.
1066+ status, machine_id = watchers.parse_unit_change('change', data, '')
1067 self.assertEqual('started', status)
1068- self.assertEqual('wordpress0.example.com', address)
1069 self.assertEqual('0', machine_id)
1070 mock_print.assert_called_once_with('wordpress/0 is ready on machine 0')
1071
1072 @helpers.mock_print
1073- def test_both_status_and_address_notified(self, mock_print):
1074- # Both status and public address changes are notified if required.
1075- data = {
1076- 'Name': 'django/0',
1077- 'Status': 'started',
1078- 'PublicAddress': 'django42.example.com',
1079- 'MachineId': '0',
1080- }
1081- # The last two arguments are the current status and address.
1082- watchers.parse_unit_change('change', data, '', '')
1083- self.assertEqual(2, mock_print.call_count)
1084- mock_print.assert_has_calls([
1085- mock.call('django/0 placed on django42.example.com'),
1086- mock.call('django/0 is ready on machine 0'),
1087- ])
1088-
1089- @helpers.mock_print
1090 def test_status_not_changed(self, mock_print):
1091 # If the status in the unit change and the given current status are the
1092 # same value, nothing is printed and the current values are returned.
1093- data = {'Name': 'django/1', 'Status': 'pending', 'PublicAddress': ''}
1094- status, address, machine_id = watchers.parse_unit_change(
1095- 'change', data, 'pending', '')
1096+ data = {'Name': 'django/1', 'Status': 'pending'}
1097+ status, machine_id = watchers.parse_unit_change(
1098+ 'change', data, 'pending')
1099 self.assertEqual('pending', status)
1100- self.assertEqual('', address)
1101 self.assertEqual('', machine_id)
1102 self.assertFalse(mock_print.called)
1103
1104- @helpers.mock_print
1105- def test_address_not_available(self, mock_print):
1106- # An empty address is returned when the public address field is not
1107- # included in the change data.
1108- data = {'Name': 'haproxy/2', 'Status': 'pending', 'MachineId': '42'}
1109- status, address, machine_id = watchers.parse_unit_change(
1110- 'change', data, 'pending', '')
1111- self.assertEqual('pending', status)
1112- self.assertEqual('', address)
1113- self.assertEqual('42', machine_id)
1114- self.assertFalse(mock_print.called)
1115-
1116
1117 class TestUnitMachineChanges(unittest.TestCase):
1118
1119
1120=== modified file 'quickstart/utils.py'
1121--- quickstart/utils.py 2014-08-22 18:32:15 +0000
1122+++ quickstart/utils.py 2014-11-10 12:30:00 +0000
1123@@ -105,6 +105,20 @@
1124 return retcode, output.decode('utf-8'), error.decode('utf-8')
1125
1126
1127+def check_resolvable(hostname):
1128+ """Check that the hostname can be resolved to a numeric IP address.
1129+
1130+ Return an error message if the address cannot be resolved.
1131+ """
1132+ try:
1133+ address = socket.gethostbyname(hostname)
1134+ except socket.error as err:
1135+ return bytes(err).decode('utf-8')
1136+ logging.debug('{} resolved to {}'.format(
1137+ hostname, address.decode('utf-8')))
1138+ return None
1139+
1140+
1141 def parse_gui_charm_url(charm_url):
1142 """Parse the given charm URL.
1143
1144
1145=== modified file 'quickstart/watchers.py'
1146--- quickstart/watchers.py 2014-07-04 13:09:11 +0000
1147+++ quickstart/watchers.py 2014-11-10 12:30:00 +0000
1148@@ -21,15 +21,25 @@
1149 unicode_literals,
1150 )
1151
1152-
1153+import logging
1154+
1155+from quickstart import utils
1156+
1157+
1158+IPV4_ADDRESS = 'ipv4'
1159 IPV6_ADDRESS = 'ipv6'
1160-NETWORK_PUBLIC = 'public'
1161-NETWORK_UNKNOWN = ''
1162-
1163-
1164-def retrieve_public_adddress(addresses):
1165+SCOPE_CLOUD_LOCAL = 'local-cloud'
1166+SCOPE_PUBLIC = 'public'
1167+SCOPE_UNKNOWN = ''
1168+
1169+
1170+def retrieve_public_adddress(addresses, hostname_resolver):
1171 """Parse the given addresses and return a public address if available.
1172
1173+ Use the given hostname_resolver callable to ensure a candidate address can
1174+ be resolved. The given function must accept an hostname/ip address and
1175+ return None if the address is resolvable, or an error string otherwise.
1176+
1177 The addresses argument is a list of address dictionaries.
1178 Cloud addresses look like the following:
1179
1180@@ -73,8 +83,8 @@
1181 found, this function returns None.
1182 """
1183 # This implementation reflects how the public address is retrieved in Juju:
1184- # see juju-core/instance/address.go:SelectPublicAddress.
1185- public_address = None
1186+ # see juju-core/network/address.go:SelectPublicAddress.
1187+ fallback_address = None
1188 for address in addresses:
1189 value = address['Value']
1190 # Exclude empty values and ipv6 addresses.
1191@@ -82,14 +92,33 @@
1192 # The address scope was called "NetworkScope" prior to juju 1.20.0.
1193 scope = address.get('Scope', address.get('NetworkScope'))
1194 # If the scope is public then we have found the address.
1195- if scope == NETWORK_PUBLIC:
1196- return value
1197- # If the scope is unknown then store the value. This way the last
1198- # address with unknown scope will be returned, and we are able to
1199- # return the right LXC address.
1200- if scope == NETWORK_UNKNOWN:
1201- public_address = value
1202- return public_address
1203+ if scope == SCOPE_PUBLIC:
1204+ error = hostname_resolver(value)
1205+ if error is None:
1206+ return value
1207+ # If the address is not resolvable, fall back to the next
1208+ # candidate, whether it is another public address, a
1209+ # local-cloud one or an unknown one. The local-cloud case, for
1210+ # instance, would happen when using the maas provider and the
1211+ # maas DNS server is not configured locally.
1212+ logging.warn(
1213+ 'cannot resolve public {} address, looking for another '
1214+ 'candidate: {}'.format(value, error))
1215+ if fallback_address is not None:
1216+ # If we already have a fallback address then we can jump to the
1217+ # next candidate.
1218+ continue
1219+ address_type = address.get('Type')
1220+ if scope == SCOPE_CLOUD_LOCAL and address_type == IPV4_ADDRESS:
1221+ # Use the local cloud scoped address as fallback value: the
1222+ # current environment is probably maas and the public address
1223+ # of the instance is not resolvable by local DNS.
1224+ fallback_address = value
1225+ elif scope == SCOPE_UNKNOWN:
1226+ # Use the unknown scoped address as fallback value: the current
1227+ # environment is probably local.
1228+ fallback_address = value
1229+ return fallback_address
1230
1231
1232 def parse_machine_change(action, data, current_status, address):
1233@@ -120,7 +149,8 @@
1234 # of units hosted by a specific machine.
1235 if not address:
1236 addresses = data.get('Addresses', [])
1237- public_address = retrieve_public_adddress(addresses)
1238+ public_address = retrieve_public_adddress(
1239+ addresses, utils.check_resolvable)
1240 if public_address is not None:
1241 address = public_address
1242 print('unit placed on {}'.format(address))
1243@@ -134,16 +164,16 @@
1244 return status, address
1245
1246
1247-def parse_unit_change(action, data, current_status, address):
1248+def parse_unit_change(action, data, current_status):
1249 """Parse the given unit change.
1250
1251 The change is represented by the given action/data pair.
1252- Also receive the last known unit status and address, which can be empty
1253- strings if those pieces of information are unknown.
1254+ Also receive the last known unit status which can be empty if the unit
1255+ status is not yet known.
1256
1257 Output a human readable message each time a relevant change is found.
1258
1259- Return the unit status, address and machine identifier.
1260+ Return the unit status and the identifier of the machine hosting the unit.
1261 Raise a ValueError if the service unit is removed or in an error state.
1262 """
1263 unit_name = data['Name']
1264@@ -157,15 +187,6 @@
1265 msg = '{} is in an error state: {}: {}'.format(
1266 unit_name, status, data['StatusInfo'])
1267 raise ValueError(msg.encode('utf-8'))
1268- # Notify when the unit becomes reachable. Up to juju-core 1.18, the
1269- # mega-watcher for units includes the public address for each unit. This
1270- # info is likely to be deprecated in favor of addresses as included in the
1271- # mega-watcher for machines, but we still try to retrieve the address here
1272- # for backward compatibility.
1273- if not address:
1274- address = data.get('PublicAddress', '')
1275- if address:
1276- print('{} placed on {}'.format(unit_name, address))
1277 # Notify status changes.
1278 if status != current_status:
1279 if status == 'pending':
1280@@ -175,7 +196,7 @@
1281 elif status == 'started':
1282 print('{} is ready on machine {}'.format(
1283 unit_name, data['MachineId']))
1284- return status, address, data.get('MachineId', '')
1285+ return status, data.get('MachineId', '')
1286
1287
1288 def unit_machine_changes(changeset):

Subscribers

People subscribed via source and target branches