Merge lp:~frankban/juju-quickstart/new-machine-info into lp:juju-quickstart

Proposed by Francesco Banconi
Status: Merged
Merged at revision: 62
Proposed branch: lp:~frankban/juju-quickstart/new-machine-info
Merge into: lp:juju-quickstart
Diff against target: 943 lines (+504/-140)
6 files modified
Makefile (+2/-1)
quickstart/app.py (+19/-7)
quickstart/models/envs.py (+3/-3)
quickstart/tests/test_app.py (+209/-109)
quickstart/tests/test_watchers.py (+179/-11)
quickstart/watchers.py (+92/-9)
To merge this branch: bzr merge lp:~frankban/juju-quickstart/new-machine-info
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+214317@code.launchpad.net

Description of the change

Support MachineInfo addresses.

juju-core 1.18 introduces a change in the mega-watcher:
the MachineInfo includes the public/private addresses
for each machine/container in the environment.
This will be the preferred way to retrieve entity
addresses in future versions of juju-core, which
might also discard the public address field in
the UnitInfo.

This branch updates quickstart so that it can work
in both scenarios: for backward compatibility, the address
is retrieved trying to parse both the unit and the machine
info, without assuming the corresponding fields to be
always included.

This required some testing and documentation efforts,
resulting in a diff longer than usual: sorry about that.

Tests: `make check`.

QA:
Use juju 1.16 (current stable).
In the steps below the command to run is:
`.venv/bin/python juju-quickstart -e {ENV_NAME}`.

1) Bootstrap a local environment with quickstart, ensure
   the quickstart process completes correctly, the juju-gui
   address is retrieved, the GUI is opened. Also ensure
   the user messages showed on stdout make sense.
2) Execute quickstart again, with the local environment already
   bootstrapped. Ensure the process completes correctly,
   and user messages are sane.
3) Destroy the local environment.
4) Bootstrap an ec2 environment with quickstart, ensure
   the quickstart process completes correctly, the juju-gui
   address is retrieved, the GUI is opened. Also ensure
   the user messages showed on stdout make sense.
5) Execute quickstart again, with the ec2 environment already
   bootstrapped. Ensure the process completes correctly,
   and user messages are sane.
6) Destroy the ec2 environment.

Use juju 1.18. This must be compiled from the juju-core 1.18 branch,
which can be found in `lp:juju-core/1.18`.

7) Edit the quickstart/settings.py file included in this branch:
   set `JUJU_CMD` to point to the juju 1.18 path.
8) Follow steps 1) to 6) again, in order to check that
   quickstart works well also with Juju 1.18.

Done, thank you!

https://codereview.appspot.com/84630043/

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

Reviewers: mp+214317_code.launchpad.net,

Message:
Please take a look.

Description:
Support MachineInfo addresses.

juju-core 1.18 introduces a change in the mega-watcher:
the MachineInfo includes the public/private addresses
for each machine/container in the environment.
This will be the preferred way to retrieve entity
addresses in future versions of juju-core, which
might also discard the public address field in
the UnitInfo.

This branch updates quickstart so that it can work
in both scenarios: for backward compatibility, the address
is retrieved trying to parse both the unit and the machine
info, without assuming the corresponding fields to be
always included.

This required some testing and documentation efforts,
resulting in a diff longer than usual: sorry about that.

Tests: `make check`.

QA:
Use juju 1.16 (current stable).
In the steps below the command to run is:
`.venv/bin/python juju-quickstart -e {ENV_NAME}`.

1) Bootstrap a local environment with quickstart, ensure
    the quickstart process completes correctly, the juju-gui
    address is retrieved, the GUI is opened. Also ensure
    the user messages showed on stdout make sense.
2) Execute quickstart again, with the local environment already
    bootstrapped. Ensure the process completes correctly,
    and user messages are sane.
3) Destroy the local environment.
4) Bootstrap an ec2 environment with quickstart, ensure
    the quickstart process completes correctly, the juju-gui
    address is retrieved, the GUI is opened. Also ensure
    the user messages showed on stdout make sense.
5) Execute quickstart again, with the ec2 environment already
    bootstrapped. Ensure the process completes correctly,
    and user messages are sane.
6) Destroy the ec2 environment.

Use juju 1.18. This must be compiled from the juju-core 1.18 branch,
which can be found in `lp:juju-core/1.18`.

7) Edit the quickstart/settings.py file included in this branch:
    set `JUJU_CMD` to point to the juju 1.18 path.
8) Follow steps 1) to 6) again, in order to check that
    quickstart works well also with Juju 1.18.

Done, thank you!

https://code.launchpad.net/~frankban/juju-quickstart/new-machine-info/+merge/214317

(do not edit description out of merge proposal)

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

Affected files (+500, -139 lines):
   A [revision details]
   M quickstart/app.py
   M quickstart/models/envs.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_watchers.py
   M quickstart/watchers.py

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

LGTM. Will QA now.

https://codereview.appspot.com/84630043/diff/1/quickstart/models/envs.py
File quickstart/models/envs.py (right):

https://codereview.appspot.com/84630043/diff/1/quickstart/models/envs.py#newcode444
quickstart/models/envs.py:444: 'The ec2 provider enables you to run Juju
on the EC2 cloud. '
This change looks familiar...

https://codereview.appspot.com/84630043/diff/1/quickstart/tests/test_watchers.py
File quickstart/tests/test_watchers.py (right):

https://codereview.appspot.com/84630043/diff/1/quickstart/tests/test_watchers.py#newcode85
quickstart/tests/test_watchers.py:85: # None is returned if an LXC
public address is not available.
Two spaces: LXC public

(I know you hate those!)

https://codereview.appspot.com/84630043/diff/1/quickstart/tests/test_watchers.py#newcode114
quickstart/tests/test_watchers.py:114: # The public address of an LXC
instance is proprely returned.
typo: properly

https://codereview.appspot.com/84630043/

Revision history for this message
Brad Crittenden (bac) wrote :
66. By Francesco Banconi

Add debug to the Makefile.

67. By Francesco Banconi

Changes as per review.

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

Please take a look.

https://codereview.appspot.com/84630043/diff/1/quickstart/models/envs.py
File quickstart/models/envs.py (right):

https://codereview.appspot.com/84630043/diff/1/quickstart/models/envs.py#newcode444
quickstart/models/envs.py:444: 'The ec2 provider enables you to run Juju
on the EC2 cloud. '
On 2014/04/04 18:24:04, bac wrote:
> This change looks familiar...

Yeah, I really wanted to have these typos fixed in trusty.

https://codereview.appspot.com/84630043/diff/1/quickstart/tests/test_watchers.py
File quickstart/tests/test_watchers.py (right):

https://codereview.appspot.com/84630043/diff/1/quickstart/tests/test_watchers.py#newcode85
quickstart/tests/test_watchers.py:85: # None is returned if an LXC
public address is not available.
On 2014/04/04 18:24:04, bac wrote:
> Two spaces: LXC public

> (I know you hate those!)

Indeed! Thank you for spotting this.

https://codereview.appspot.com/84630043/diff/1/quickstart/tests/test_watchers.py#newcode114
quickstart/tests/test_watchers.py:114: # The public address of an LXC
instance is proprely returned.
On 2014/04/04 18:24:04, bac wrote:
> typo: properly

Done.

https://codereview.appspot.com/84630043/

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

LGTM, couple of questions/comments. I think I follow the first one now.
If only you could order files in review to line up better.

https://codereview.appspot.com/84630043/diff/20001/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/84630043/diff/20001/quickstart/app.py#newcode420
quickstart/app.py:420: collected_machine_changes = []
Can we use something like a hash keyed by machine_id (or something else
since it might not have an id per below) and then just update the
changes/state as it comes in to not need to sort/reverse/etc everything.
Just always keep the latest info for the key? I'm guessing not, but
curious as to the process.

https://codereview.appspot.com/84630043/diff/20001/quickstart/tests/test_watchers.py
File quickstart/tests/test_watchers.py (right):

https://codereview.appspot.com/84630043/diff/20001/quickstart/tests/test_watchers.py#newcode52
quickstart/tests/test_watchers.py:52: lxc_addresses = [
these can be kvm as well correct? Should these be tested as part of this
or is there no difference other than the three chars?

https://codereview.appspot.com/84630043/

68. By Francesco Banconi

Changes as per review.

Revision history for this message
Francesco Banconi (frankban) wrote :
Download full text (3.5 KiB)

*** Submitted:

Support MachineInfo addresses.

juju-core 1.18 introduces a change in the mega-watcher:
the MachineInfo includes the public/private addresses
for each machine/container in the environment.
This will be the preferred way to retrieve entity
addresses in future versions of juju-core, which
might also discard the public address field in
the UnitInfo.

This branch updates quickstart so that it can work
in both scenarios: for backward compatibility, the address
is retrieved trying to parse both the unit and the machine
info, without assuming the corresponding fields to be
always included.

This required some testing and documentation efforts,
resulting in a diff longer than usual: sorry about that.

Tests: `make check`.

QA:
Use juju 1.16 (current stable).
In the steps below the command to run is:
`.venv/bin/python juju-quickstart -e {ENV_NAME}`.

1) Bootstrap a local environment with quickstart, ensure
    the quickstart process completes correctly, the juju-gui
    address is retrieved, the GUI is opened. Also ensure
    the user messages showed on stdout make sense.
2) Execute quickstart again, with the local environment already
    bootstrapped. Ensure the process completes correctly,
    and user messages are sane.
3) Destroy the local environment.
4) Bootstrap an ec2 environment with quickstart, ensure
    the quickstart process completes correctly, the juju-gui
    address is retrieved, the GUI is opened. Also ensure
    the user messages showed on stdout make sense.
5) Execute quickstart again, with the ec2 environment already
    bootstrapped. Ensure the process completes correctly,
    and user messages are sane.
6) Destroy the ec2 environment.

Use juju 1.18. This must be compiled from the juju-core 1.18 branch,
which can be found in `lp:juju-core/1.18`.

7) Edit the quickstart/settings.py file included in this branch:
    set `JUJU_CMD` to point to the juju 1.18 path.
8) Follow steps 1) to 6) again, in order to check that
    quickstart works well also with Juju 1.18.

Done, thank you!

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

https://codereview.appspot.com/84630043/diff/20001/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/84630043/diff/20001/quickstart/app.py#newcode420
quickstart/app.py:420: collected_machine_changes = []
On 2014/04/07 12:57:51, rharding wrote:
> Can we use something like a hash keyed by machine_id (or something
else since it
> might not have an id per below) and then just update the changes/state
as it
> comes in to not need to sort/reverse/etc everything. Just always keep
the latest
> info for the key? I'm guessing not, but curious as to the process.

That can be an option, but I preferred to just collect changes rather
than parse them here and update a mutable data structure.
Since the current approach has already been QA'd, I'd be inclined to
preserve this code, at least for this branch.

https://codereview.appspot.com/84630043/diff/20001/quickstart/tests/test_watchers.py
File quickstart/tests/test_watchers.py (right):

https://codereview.appspot.com/84630043/diff/20001/quickstart/tests/test_watchers.py#newcode52
quickstart/tests/test_watcher...

Read more...

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile'
2--- Makefile 2014-01-31 21:00:06 +0000
3+++ Makefile 2014-04-07 13:24:46 +0000
4@@ -85,4 +85,5 @@
5 --with-coverage --cover-package=quickstart quickstart
6 @rm .coverage
7
8-.PHONY: all clean check help install lint release run setup source sysdeps test
9+.PHONY: all clean check debug help install lint release run setup source \
10+ sysdeps test
11
12=== modified file 'quickstart/app.py'
13--- quickstart/app.py 2014-03-27 10:38:01 +0000
14+++ quickstart/app.py 2014-04-07 13:24:46 +0000
15@@ -417,6 +417,7 @@
16 either the service unit or the machine is removed or in error.
17 """
18 address = unit_status = machine_id = machine_status = ''
19+ collected_machine_changes = []
20 watcher = env.watch_changes(watchers.unit_machine_changes)
21 while True:
22 try:
23@@ -424,7 +425,7 @@
24 except jujuclient.EnvError as err:
25 raise ProgramExit(
26 'bad API server response: {}'.format(err.message))
27- # Process unit changes:
28+ # Process unit changes.
29 for action, data in unit_changes:
30 if data['Name'] == unit_name:
31 try:
32@@ -440,17 +441,28 @@
33 # unit. For this reason, we can exit the for loop here.
34 break
35 if not machine_id:
36- # No need to process machine changes: we still don't know what
37- # machine the unit belongs to.
38+ # No need to process machine changes: we don't know what machine
39+ # the unit belongs to. However, machine changes are collected so
40+ # that they can be parsed later.
41+ collected_machine_changes.extend(machine_changes)
42 continue
43- # Process machine changes.
44- for action, data in machine_changes:
45+ # Process machine changes. Since relevant info can also be found
46+ # in previously collected changes, add those to the current changes,
47+ # in reverse order so that more complete info comes first.
48+ all_machine_changes = machine_changes + list(
49+ reversed(collected_machine_changes))
50+ # At this point we can discard collected changes.
51+ collected_machine_changes = []
52+ for action, data in all_machine_changes:
53 if data['Id'] == machine_id:
54 try:
55- machine_status = watchers.parse_machine_change(
56- action, data, machine_status)
57+ machine_status, address = watchers.parse_machine_change(
58+ action, data, machine_status, address)
59 except ValueError as err:
60 raise ProgramExit(bytes(err))
61+ if address and unit_status == 'started':
62+ # We can exit this loop.
63+ return address
64 # The mega-watcher contains a single change for each specific
65 # machine. For this reason, we can exit the for loop here.
66 break
67
68=== modified file 'quickstart/models/envs.py'
69--- quickstart/models/envs.py 2014-01-28 20:35:42 +0000
70+++ quickstart/models/envs.py 2014-04-07 13:24:46 +0000
71@@ -441,7 +441,7 @@
72 env_type_db['ec2'] = {
73 'label': 'Amazon EC2',
74 'description': (
75- 'The ec2 provider enable you to run Juju on the EC2 cloud. '
76+ 'The ec2 provider enables you to run Juju on the EC2 cloud. '
77 'This process requires you to have an Amazon Web Services (AWS) '
78 'account. If you have not signed up for one yet, it can obtained '
79 'at http://aws.amazon.com. '
80@@ -481,7 +481,7 @@
81 env_type_db['openstack'] = {
82 'label': 'OpenStack (or HP Public Cloud)',
83 'description': (
84- 'The openstack provider enable you to run Juju on OpenStack '
85+ 'The openstack provider enables you to run Juju on OpenStack '
86 'private and public clouds. Use this also if you want to '
87 'set up Juju on HP Public Cloud. See '
88 'https://juju.ubuntu.com/docs/config-openstack.html and '
89@@ -546,7 +546,7 @@
90 env_type_db['azure'] = {
91 'label': 'Windows Azure',
92 'description': (
93- 'The azure provider enable you to run Juju on Windows Azure. '
94+ 'The azure provider enables you to run Juju on Windows Azure. '
95 'It requires you to have an Windows Azure account. If you have '
96 'not signed up for one yet, it can obtained at '
97 'http://www.windowsazure.com/. See '
98
99=== modified file 'quickstart/tests/test_app.py'
100--- quickstart/tests/test_app.py 2014-03-27 11:00:36 +0000
101+++ quickstart/tests/test_app.py 2014-04-07 13:24:46 +0000
102@@ -1037,43 +1037,11 @@
103 unittest.TestCase):
104
105 address = 'unit.example.com'
106- change_machine_pending = ('change', {
107- 'Id': '0',
108- 'Status': 'pending',
109- })
110- change_machine_started = ('change', {
111- 'Id': '0',
112- 'Status': 'started',
113- })
114- change_unit_pending = ('change', {
115- 'MachineId': '0',
116- 'Name': 'django/42',
117- 'PublicAddress': '',
118- 'Status': 'pending',
119- })
120- change_unit_reachable = ('change', {
121- 'MachineId': '0',
122- 'Name': 'django/42',
123- 'PublicAddress': address,
124- 'Status': 'pending',
125- })
126- change_unit_installed = ('change', {
127- 'MachineId': '0',
128- 'Name': 'django/42',
129- 'PublicAddress': address,
130- 'Status': 'installed',
131- })
132- change_unit_started = ('change', {
133- 'MachineId': '0',
134- 'Name': 'django/42',
135- 'PublicAddress': address,
136- 'Status': 'started',
137- })
138 machine_pending_call = mock.call('machine 0 provisioning is pending')
139+ unit_placed_machine_call = mock.call('unit placed on unit.example.com')
140 machine_started_call = mock.call('machine 0 is started')
141 unit_pending_call = mock.call('django/42 deployment is pending')
142- unit_reachable_call = mock.call(
143- 'setting up django/42 on {}'.format(address))
144+ unit_placed_unit_call = mock.call('django/42 placed on {}'.format(address))
145 unit_installed_call = mock.call('django/42 is installed')
146 unit_started_call = mock.call('django/42 is ready on machine 0')
147
148@@ -1087,6 +1055,33 @@
149 env.watch_changes().next.side_effect = changes
150 return env
151
152+ def make_machine_change(self, status, name='0', address=None):
153+ """Create and return a machine change.
154+
155+ If the address argument is None, the change does not include the
156+ corresponding address field.
157+ """
158+ data = {'Id': name, 'Status': status}
159+ if address is not None:
160+ data['Addresses'] = [{
161+ 'NetworkName': '',
162+ 'NetworkScope': 'public',
163+ 'Type': 'hostname',
164+ 'Value': address,
165+ }]
166+ return 'change', data
167+
168+ def make_unit_change(self, status, name='django/42', address=None):
169+ """Create and return a unit change.
170+
171+ If the address argument is None, the change does not include the
172+ corresponding address field.
173+ """
174+ data = {'MachineId': '0', 'Name': name, 'Status': status}
175+ if address is not None:
176+ data['PublicAddress'] = address
177+ return 'change', data
178+
179 # The following group of tests exercises both the function return value and
180 # the function output, even if the output is handled by sub-functions.
181 # This is done to simulate the different user experiences of observing the
182@@ -1096,20 +1091,46 @@
183 # The glorious moments in the unit's life are properly highlighted.
184 # The machine achievements are also celebrated.
185 env = self.make_env([
186- ([self.change_unit_pending], [self.change_machine_pending]),
187- ([], [self.change_machine_started]),
188- ([self.change_unit_reachable], []),
189- ([self.change_unit_installed], []),
190- ([self.change_unit_started], []),
191- ])
192- address = app.watch(env, 'django/42')
193- self.assertEqual(self.address, address)
194- self.assertEqual(6, mock_print.call_count)
195- mock_print.assert_has_calls([
196- self.unit_pending_call,
197- self.machine_pending_call,
198- self.machine_started_call,
199- self.unit_reachable_call,
200+ ([self.make_unit_change('pending', address='')],
201+ [self.make_machine_change('pending')]),
202+ ([], [self.make_machine_change('started')]),
203+ ([self.make_unit_change('pending', address=self.address)], []),
204+ ([self.make_unit_change('installed', address=self.address)], []),
205+ ([self.make_unit_change('started', address=self.address)], []),
206+ ])
207+ address = app.watch(env, 'django/42')
208+ self.assertEqual(self.address, address)
209+ self.assertEqual(6, mock_print.call_count)
210+ mock_print.assert_has_calls([
211+ self.unit_pending_call,
212+ self.machine_pending_call,
213+ self.machine_started_call,
214+ self.unit_placed_unit_call,
215+ self.unit_installed_call,
216+ self.unit_started_call,
217+ ])
218+
219+ def test_unit_life_with_machine_address(self, mock_print):
220+ # The glorious moments in the unit's life are properly highlighted.
221+ # The machine achievements are also celebrated.
222+ # This time the new mega-watcher behavior is simulated, in which
223+ # addresses are included in the machine change.
224+ env = self.make_env([
225+ ([self.make_unit_change('pending')],
226+ [self.make_machine_change('pending', address='')]),
227+ ([], [self.make_machine_change('started', address=self.address)]),
228+ ([self.make_unit_change('pending')], []),
229+ ([self.make_unit_change('installed')], []),
230+ ([self.make_unit_change('started')], []),
231+ ])
232+ address = app.watch(env, 'django/42')
233+ self.assertEqual(self.address, address)
234+ self.assertEqual(6, mock_print.call_count)
235+ mock_print.assert_has_calls([
236+ self.unit_pending_call,
237+ self.machine_pending_call,
238+ self.unit_placed_machine_call,
239+ self.machine_started_call,
240 self.unit_installed_call,
241 self.unit_started_call,
242 ])
243@@ -1120,17 +1141,18 @@
244 # The unit is first reachable and then pending. The machine starts
245 # when the unit is already installed. All of this makes no sense
246 # and should never happen, but if it does, we deal with it.
247- ([self.change_unit_reachable], []),
248- ([self.change_unit_pending], [self.change_machine_pending]),
249- ([self.change_unit_installed], []),
250- ([], [self.change_machine_started]),
251- ([self.change_unit_started], []),
252+ ([self.make_unit_change('pending', address=self.address)], []),
253+ ([self.make_unit_change('pending', address='')],
254+ [self.make_machine_change('pending')]),
255+ ([self.make_unit_change('installed', address=self.address)], []),
256+ ([], [self.make_machine_change('started')]),
257+ ([self.make_unit_change('started', address=self.address)], []),
258 ])
259 address = app.watch(env, 'django/42')
260 self.assertEqual(self.address, address)
261 self.assertEqual(6, mock_print.call_count)
262 mock_print.assert_has_calls([
263- self.unit_reachable_call,
264+ self.unit_placed_unit_call,
265 self.unit_pending_call,
266 self.machine_pending_call,
267 self.unit_installed_call,
268@@ -1139,31 +1161,72 @@
269 ])
270
271 def test_missing_changes(self, mock_print):
272- # Only the unit started change is strictly required.
273- env = self.make_env([([self.change_unit_started], [])])
274+ # Only the unit started change is strictly required when the unit
275+ # change includes the public address.
276+ env = self.make_env([
277+ ([self.make_unit_change('started', address=self.address)], []),
278+ ])
279 address = app.watch(env, 'django/42')
280 self.assertEqual(self.address, address)
281 self.assertEqual(2, mock_print.call_count)
282 mock_print.assert_has_calls([
283- self.unit_reachable_call,
284- self.unit_started_call,
285+ self.unit_placed_unit_call,
286+ self.unit_started_call,
287+ ])
288+
289+ def test_missing_changes_with_machine_address(self, mock_print):
290+ # When using the new mega-watcher, a machine change including its
291+ # public address is also required.
292+ env = self.make_env([
293+ ([self.make_unit_change('started')], []),
294+ ([], [self.make_machine_change('started', address=self.address)]),
295+ ])
296+ address = app.watch(env, 'django/42')
297+ self.assertEqual(self.address, address)
298+ self.assertEqual(3, mock_print.call_count)
299+ mock_print.assert_has_calls([
300+ self.unit_started_call,
301+ self.unit_placed_machine_call,
302+ self.machine_started_call,
303 ])
304
305 def test_ignored_machine_changes(self, mock_print):
306 # All machine changes are ignored until the application knows what
307 # machine the unit belongs to.
308 env = self.make_env([
309- ([], [self.change_machine_pending]),
310- ([], [self.change_machine_started]),
311- ([self.change_unit_started], []),
312+ ([], [self.make_machine_change('pending')]),
313+ ([], [self.make_machine_change('started')]),
314+ ([self.make_unit_change('started', address=self.address)], []),
315 ])
316 address = app.watch(env, 'django/42')
317 self.assertEqual(self.address, address)
318 # No machine related messages have been printed.
319 self.assertEqual(2, mock_print.call_count)
320 mock_print.assert_has_calls([
321- self.unit_reachable_call,
322- self.unit_started_call,
323+ self.unit_placed_unit_call,
324+ self.unit_started_call,
325+ ])
326+
327+ def test_ignored_machine_changes_with_machine_address(self, mock_print):
328+ # All machine changes are ignored until the application knows what
329+ # machine the unit belongs to. When the above happens, previously
330+ # collected machine changes are still parsed in the case the address
331+ # is not yet known.
332+ env = self.make_env([
333+ ([], [self.make_machine_change('pending')]),
334+ ([],
335+ [self.make_machine_change('installed', address=self.address)]),
336+ ([], [self.make_machine_change('started', address=self.address)]),
337+ ([self.make_unit_change('started')], []),
338+ ])
339+ address = app.watch(env, 'django/42')
340+ self.assertEqual(self.address, address)
341+ # No machine related messages have been printed.
342+ self.assertEqual(3, mock_print.call_count)
343+ mock_print.assert_has_calls([
344+ self.unit_started_call,
345+ self.unit_placed_machine_call,
346+ self.machine_started_call,
347 ])
348
349 def test_unit_already_deployed(self, mock_print):
350@@ -1171,30 +1234,76 @@
351 # This happens, e.g., when executing Quickstart a second time, and both
352 # the unit and the machine are already started.
353 env = self.make_env([
354- ([self.change_unit_started], [self.change_machine_started]),
355+ ([self.make_unit_change('started', address=self.address)],
356+ [self.make_machine_change('started')]),
357 ])
358 address = app.watch(env, 'django/42')
359 self.assertEqual(self.address, address)
360 self.assertEqual(2, mock_print.call_count)
361
362+ def test_unit_already_deployed_with_machine_address(self, mock_print):
363+ # Simulate the unit we are observing has been already deployed.
364+ # This happens, e.g., when executing Quickstart a second time, and both
365+ # the unit and the machine are already started.
366+ # This time the new mega-watcher behavior is simulated, in which
367+ # addresses are included in the machine change.
368+ env = self.make_env([
369+ ([self.make_unit_change('started')],
370+ [self.make_machine_change('started', address=self.address)]),
371+ ])
372+ address = app.watch(env, 'django/42')
373+ self.assertEqual(self.address, address)
374+ self.assertEqual(3, mock_print.call_count)
375+ mock_print.assert_has_calls([
376+ self.unit_started_call,
377+ self.unit_placed_machine_call,
378+ self.machine_started_call,
379+ ])
380+
381 def test_machine_already_started(self, mock_print):
382 # Simulate the unit is being deployed on an already started machine.
383 # This happens, e.g., when running Quickstart on a non-local
384 # environment type: the unit is deployed on the bootstrap node, which
385 # is assumed to be started.
386 env = self.make_env([
387- ([self.change_unit_pending], [self.change_machine_started]),
388- ([self.change_unit_reachable], []),
389- ([self.change_unit_installed], []),
390- ([self.change_unit_started], []),
391- ])
392- address = app.watch(env, 'django/42')
393- self.assertEqual(self.address, address)
394- self.assertEqual(5, mock_print.call_count)
395- mock_print.assert_has_calls([
396- self.unit_pending_call,
397- self.machine_started_call,
398- self.unit_reachable_call,
399+ ([self.make_unit_change('pending', address='')],
400+ [self.make_machine_change('started')]),
401+ ([self.make_unit_change('pending', address=self.address)], []),
402+ ([self.make_unit_change('installed', address=self.address)], []),
403+ ([self.make_unit_change('started', address=self.address)], []),
404+ ])
405+ address = app.watch(env, 'django/42')
406+ self.assertEqual(self.address, address)
407+ self.assertEqual(5, mock_print.call_count)
408+ mock_print.assert_has_calls([
409+ self.unit_pending_call,
410+ self.machine_started_call,
411+ self.unit_placed_unit_call,
412+ self.unit_installed_call,
413+ self.unit_started_call,
414+ ])
415+
416+ def test_machine_already_started_with_machine_address(self, mock_print):
417+ # Simulate the unit is being deployed on an already started machine.
418+ # This happens, e.g., when running Quickstart on a non-local
419+ # environment type: the unit is deployed on the bootstrap node, which
420+ # is assumed to be started.
421+ # This time the new mega-watcher behavior is simulated, in which
422+ # addresses are included in the machine change.
423+ env = self.make_env([
424+ ([self.make_unit_change('pending')],
425+ [self.make_machine_change('started', address=self.address)]),
426+ ([self.make_unit_change('pending')], []),
427+ ([self.make_unit_change('installed')], []),
428+ ([self.make_unit_change('started')], []),
429+ ])
430+ address = app.watch(env, 'django/42')
431+ self.assertEqual(self.address, address)
432+ self.assertEqual(5, mock_print.call_count)
433+ mock_print.assert_has_calls([
434+ self.unit_pending_call,
435+ self.unit_placed_machine_call,
436+ self.machine_started_call,
437 self.unit_installed_call,
438 self.unit_started_call,
439 ])
440@@ -1203,38 +1312,25 @@
441 # Changes to units or machines we are not observing are ignored. Also
442 # ensure that repeated changes to a single entity are ignored, even if
443 # they are unlikely to happen.
444- change_another_machine_pending = ('change', {
445- 'Id': '42',
446- 'Status': 'pending',
447- })
448- change_another_machine_started = ('change', {
449- 'Id': '1',
450- 'Status': 'started',
451- })
452- change_another_unit_pending = ('change', {
453- 'MachineId': '0',
454- 'Name': 'haproxy/0',
455- 'Status': 'pending',
456- })
457- change_another_unit_started = ('change', {
458- 'MachineId': '0',
459- 'Name': 'haproxy/0',
460- 'Status': 'started',
461- })
462+ pending_unit_change = self.make_unit_change('pending', address='')
463+ started_unit_change = self.make_unit_change(
464+ 'started', address=self.address)
465 env = self.make_env([
466 # Add a repeated change.
467- ([self.change_unit_pending, self.change_unit_pending],
468- [self.change_machine_pending]),
469+ ([pending_unit_change, pending_unit_change],
470+ [self.make_machine_change('pending')]),
471 # Add extraneous unit and machine changes.
472- ([change_another_unit_pending], [change_another_machine_pending]),
473+ ([self.make_unit_change('pending', name='haproxy/0')],
474+ [self.make_machine_change('pending', name='42')]),
475 # Add a change to an extraneous machine.
476- ([], [change_another_machine_started,
477- self.change_machine_started]),
478+ ([], [self.make_machine_change('started', name='42'),
479+ self.make_machine_change('started')]),
480 # Add a change to an extraneous unit.
481- ([change_another_unit_started, self.change_unit_reachable], []),
482- ([self.change_unit_installed], []),
483+ ([self.make_unit_change('started', name='haproxy/0'),
484+ self.make_unit_change('pending', address=self.address)], []),
485+ ([self.make_unit_change('installed', address=self.address)], []),
486 # Add another repeated change.
487- ([self.change_unit_started, self.change_unit_started], []),
488+ ([started_unit_change, started_unit_change], []),
489 ])
490 address = app.watch(env, 'django/42')
491 self.assertEqual(self.address, address)
492@@ -1243,7 +1339,7 @@
493 self.unit_pending_call,
494 self.machine_pending_call,
495 self.machine_started_call,
496- self.unit_reachable_call,
497+ self.unit_placed_unit_call,
498 self.unit_installed_call,
499 self.unit_started_call,
500 ])
501@@ -1251,7 +1347,7 @@
502 def test_api_error(self, mock_print):
503 # A ProgramExit is raised if an error occurs in one of the API calls.
504 env = self.make_env([
505- ([self.change_unit_pending], []),
506+ ([self.make_unit_change('pending', address='')], []),
507 self.make_env_error('next returned an error'),
508 ])
509 expected = 'bad API server response: next returned an error'
510@@ -1262,13 +1358,15 @@
511
512 def test_other_errors(self, mock_print):
513 # Any other errors occurred during the process are not trapped.
514- error = ValueError('explode!')
515- env = self.make_env([([self.change_unit_installed], []), error])
516+ env = self.make_env([
517+ ([self.make_unit_change('installed', address=self.address)], []),
518+ ValueError('explode!'),
519+ ])
520 with self.assert_value_error('explode!'):
521 app.watch(env, 'django/42')
522 self.assertEqual(2, mock_print.call_count)
523 mock_print.assert_has_calls([
524- self.unit_reachable_call, self.unit_installed_call])
525+ self.unit_placed_unit_call, self.unit_installed_call])
526
527 def test_machine_status_error(self, mock_print):
528 # A ProgramExit is raised if an the machine is found in an error state.
529@@ -1277,10 +1375,12 @@
530 'Status': 'error',
531 'StatusInfo': 'oddities',
532 })
533+ self.make_machine_change('error')
534 # The unit pending change is required to make the function know which
535 # machine to observe.
536- env = self.make_env([(
537- [self.change_unit_pending], [change_machine_error]),
538+ env = self.make_env([
539+ ([self.make_unit_change('pending', address='')],
540+ [change_machine_error]),
541 ])
542 expected = 'machine 0 is in an error state: error: oddities'
543 with self.assert_program_exit(expected):
544
545=== modified file 'quickstart/tests/test_watchers.py'
546--- quickstart/tests/test_watchers.py 2014-01-17 13:54:51 +0000
547+++ quickstart/tests/test_watchers.py 2014-04-07 13:24:46 +0000
548@@ -26,46 +26,202 @@
549 from quickstart.tests import helpers
550
551
552+# Define addresses to be used in tests.
553+cloud_addresses = [
554+ {'NetworkName': '',
555+ 'NetworkScope': 'public',
556+ 'Type': 'hostname',
557+ 'Value': 'eu-west-1.compute.example.com'},
558+ {'NetworkName': '',
559+ 'NetworkScope': 'local-cloud',
560+ 'Type': 'hostname',
561+ 'Value': 'eu-west-1.example.internal'},
562+ {'NetworkName': '',
563+ 'NetworkScope': 'public',
564+ 'Type': 'ipv4',
565+ 'Value': '444.222.444.222'},
566+ {'NetworkName': '',
567+ 'NetworkScope': 'local-cloud',
568+ 'Type': 'ipv4',
569+ 'Value': '10.42.47.10'},
570+ {'NetworkName': '',
571+ 'NetworkScope': '',
572+ 'Type': 'ipv6',
573+ 'Value': 'fe80::92b8:d0ff:fe94:8f8c'},
574+]
575+container_addresses = [
576+ {'NetworkName': '',
577+ 'NetworkScope': '',
578+ 'Type': 'ipv4',
579+ 'Value': '10.0.3.42'},
580+ {'NetworkName': '',
581+ 'NetworkScope': '',
582+ 'Type': 'ipv6',
583+ 'Value': 'fe80::216:3eff:fefd:787e'},
584+]
585+
586+
587+class TestRetrievePublicAddress(unittest.TestCase):
588+
589+ def test_empty_addresses(self):
590+ # None is returned if there are no available addresses.
591+ self.assertIsNone(watchers.retrieve_public_adddress([]))
592+
593+ def test_cloud_address_not_found(self):
594+ # None is returned if a cloud machine public address is not available.
595+ addresses = [
596+ {'NetworkName': '',
597+ 'NetworkScope': 'local-cloud',
598+ 'Type': 'hostname',
599+ 'Value': 'eu-west-1.example.internal'},
600+ {'NetworkName': '',
601+ 'NetworkScope': 'local-cloud',
602+ 'Type': 'ipv4',
603+ 'Value': '10.42.47.10'},
604+ ]
605+ self.assertIsNone(watchers.retrieve_public_adddress(addresses))
606+
607+ def test_container_address_not_found(self):
608+ # None is returned if an LXC public address is not available.
609+ addresses = [{
610+ 'NetworkName': '',
611+ 'NetworkScope': '',
612+ 'Type': 'ipv6',
613+ 'Value': 'fe80::216:3eff:fefd:787e',
614+ }]
615+ self.assertIsNone(watchers.retrieve_public_adddress(addresses))
616+
617+ def test_empty_public_address(self):
618+ # None is returned if the public address has no value.
619+ addresses = [
620+ {'NetworkName': '',
621+ 'NetworkScope': 'local-cloud',
622+ 'Type': 'hostname',
623+ 'Value': 'eu-west-1.example.internal'},
624+ {'NetworkName': '',
625+ 'NetworkScope': 'public',
626+ 'Type': 'ipv4',
627+ 'Value': ''},
628+ ]
629+ self.assertIsNone(watchers.retrieve_public_adddress(addresses))
630+
631+ def test_cloud_addresses(self):
632+ # The public address of a cloud machine is properly returned.
633+ public_address = watchers.retrieve_public_adddress(cloud_addresses)
634+ self.assertEqual('eu-west-1.compute.example.com', public_address)
635+
636+ def test_container_addresses(self):
637+ # The public address of an LXC instance is properly returned.
638+ public_address = watchers.retrieve_public_adddress(container_addresses)
639+ self.assertEqual('10.0.3.42', public_address)
640+
641+ def test_last_unknown_address(self):
642+ # If the scope of multiple addresses is unknown, the last one is taken.
643+ addresses = [
644+ {'NetworkName': '',
645+ 'NetworkScope': '',
646+ 'Type': 'ipv4',
647+ 'Value': '10.0.3.42'},
648+ {'NetworkName': '',
649+ 'NetworkScope': '',
650+ 'Type': 'ipv4',
651+ 'Value': '10.0.3.47'},
652+ ]
653+ public_address = watchers.retrieve_public_adddress(addresses)
654+ self.assertEqual('10.0.3.47', public_address)
655+
656+
657 class TestParseMachineChange(helpers.ValueErrorTestsMixin, unittest.TestCase):
658
659 def test_machine_removed(self):
660 # A ValueError is raised if the change represents a machine removal.
661- data = {'Id': '1', 'Status': 'started'}
662+ data = {'Addresses': [], 'Id': '1', 'Status': 'started'}
663 with self.assert_value_error('machine 1 unexpectedly removed'):
664- watchers.parse_machine_change('remove', data, '')
665+ watchers.parse_machine_change('remove', data, '', '')
666
667 def test_machine_error(self):
668 # A ValueError is raised if the machine is in an error state.
669- data = {'Id': '1', 'Status': 'error', 'StatusInfo': 'bad wolf'}
670+ data = {
671+ 'Addresses': [],
672+ 'Id': '1',
673+ 'Status': 'error',
674+ 'StatusInfo': 'bad wolf',
675+ }
676 expected_error = 'machine 1 is in an error state: error: bad wolf'
677 with self.assert_value_error(expected_error):
678- watchers.parse_machine_change('change', data, '')
679+ watchers.parse_machine_change('change', data, '', '')
680
681 @helpers.mock_print
682 def test_pending_status_notified(self, mock_print):
683 # A message is printed to stdout when the machine changes its status
684 # to "pending". The new status is also returned by the function.
685- data = {'Id': '1', 'Status': 'pending'}
686- status = watchers.parse_machine_change('change', data, '')
687+ data = {'Addresses': [], 'Id': '1', 'Status': 'pending'}
688+ status, address = watchers.parse_machine_change('change', data, '', '')
689 self.assertEqual('pending', status)
690+ self.assertEqual('', address)
691 mock_print.assert_called_once_with('machine 1 provisioning is pending')
692
693 @helpers.mock_print
694 def test_started_status_notified(self, mock_print):
695 # A message is printed to stdout when the machine changes its status
696 # to "started". The new status is also returned by the function.
697- data = {'Id': '42', 'Status': 'started'}
698- status = watchers.parse_machine_change('change', data, 'pending')
699+ data = {'Addresses': [], 'Id': '42', 'Status': 'started'}
700+ status, address = watchers.parse_machine_change(
701+ 'change', data, 'pending', '')
702 self.assertEqual('started', status)
703+ self.assertEqual('', address)
704 mock_print.assert_called_once_with('machine 42 is started')
705
706 @helpers.mock_print
707 def test_status_not_changed(self, mock_print):
708 # If the status in the machine change and the given current status are
709 # the same value, nothing is printed and the status is returned.
710+ data = {'Addresses': [], 'Id': '47', 'Status': 'pending'}
711+ status, address = watchers.parse_machine_change(
712+ 'change', data, 'pending', '')
713+ self.assertEqual('pending', status)
714+ self.assertEqual('', address)
715+ self.assertFalse(mock_print.called)
716+
717+ @helpers.mock_print
718+ def test_address_notified(self, mock_print):
719+ # A message is printed to stdout when the machine obtains a public
720+ # address.
721+ data = {'Addresses': cloud_addresses, 'Id': '1', 'Status': 'pending'}
722+ status, address = watchers.parse_machine_change(
723+ 'change', data, 'pending', '')
724+ self.assertEqual('pending', status)
725+ self.assertEqual('eu-west-1.compute.example.com', address)
726+ mock_print.assert_called_once_with(
727+ 'unit placed on eu-west-1.compute.example.com')
728+
729+ @helpers.mock_print
730+ def test_both_status_and_address_notified(self, mock_print):
731+ # Both status and public address changes are notified if required.
732+ data = {
733+ 'Addresses': container_addresses,
734+ 'Id': '0',
735+ 'Status': 'started',
736+ }
737+ status, address = watchers.parse_machine_change(
738+ 'change', data, 'pending', '')
739+ self.assertEqual('started', status)
740+ self.assertEqual('10.0.3.42', address)
741+ self.assertEqual(2, mock_print.call_count)
742+ mock_print.assert_has_calls([
743+ mock.call('unit placed on 10.0.3.42'),
744+ mock.call('machine 0 is started'),
745+ ])
746+
747+ @helpers.mock_print
748+ def test_address_not_available(self, mock_print):
749+ # An empty address is returned when the addresses field is not
750+ # included in the change data.
751 data = {'Id': '47', 'Status': 'pending'}
752- status = watchers.parse_machine_change('change', data, 'pending')
753+ status, address = watchers.parse_machine_change(
754+ 'change', data, 'pending', '')
755 self.assertEqual('pending', status)
756+ self.assertEqual('', address)
757 self.assertFalse(mock_print.called)
758
759
760@@ -107,7 +263,7 @@
761 self.assertEqual('haproxy2.example.com', address)
762 self.assertEqual('42', machine_id)
763 mock_print.assert_called_once_with(
764- 'setting up haproxy/2 on haproxy2.example.com')
765+ 'haproxy/2 placed on haproxy2.example.com')
766
767 @helpers.mock_print
768 def test_pending_status_notified(self, mock_print):
769@@ -173,7 +329,7 @@
770 watchers.parse_unit_change('change', data, '', '')
771 self.assertEqual(2, mock_print.call_count)
772 mock_print.assert_has_calls([
773- mock.call('setting up django/0 on django42.example.com'),
774+ mock.call('django/0 placed on django42.example.com'),
775 mock.call('django/0 is ready on machine 0'),
776 ])
777
778@@ -189,6 +345,18 @@
779 self.assertEqual('', machine_id)
780 self.assertFalse(mock_print.called)
781
782+ @helpers.mock_print
783+ def test_address_not_available(self, mock_print):
784+ # An empty address is returned when the public address field is not
785+ # included in the change data.
786+ data = {'Name': 'haproxy/2', 'Status': 'pending', 'MachineId': '42'}
787+ status, address, machine_id = watchers.parse_unit_change(
788+ 'change', data, 'pending', '')
789+ self.assertEqual('pending', status)
790+ self.assertEqual('', address)
791+ self.assertEqual('42', machine_id)
792+ self.assertFalse(mock_print.called)
793+
794
795 class TestUnitMachineChanges(unittest.TestCase):
796
797
798=== modified file 'quickstart/watchers.py'
799--- quickstart/watchers.py 2014-01-17 10:35:36 +0000
800+++ quickstart/watchers.py 2014-04-07 13:24:46 +0000
801@@ -22,16 +22,85 @@
802 )
803
804
805-def parse_machine_change(action, data, current_status):
806+IPV6_ADDRESS = 'ipv6'
807+NETWORK_PUBLIC = 'public'
808+NETWORK_UNKNOWN = ''
809+
810+
811+def retrieve_public_adddress(addresses):
812+ """Parse the given addresses and return a public address if available.
813+
814+ The addresses argument is a list of address dictionaries.
815+ Cloud addresses look like the following:
816+
817+ [
818+ {'NetworkName': '',
819+ 'NetworkScope': 'public',
820+ 'Type': 'hostname',
821+ 'Value': 'eu-west-1.compute.example.com'},
822+ {'NetworkName': '',
823+ 'NetworkScope': 'local-cloud',
824+ 'Type': 'hostname',
825+ 'Value': 'eu-west-1.example.internal'},
826+ {'NetworkName': '',
827+ 'NetworkScope': 'public',
828+ 'Type': 'ipv4',
829+ 'Value': '444.222.444.222'},
830+ {'NetworkName': '',
831+ 'NetworkScope': 'local-cloud',
832+ 'Type': 'ipv4',
833+ 'Value': '10.42.47.10'},
834+ {'NetworkName': '',
835+ 'NetworkScope': '',
836+ 'Type': 'ipv6',
837+ 'Value': 'fe80::92b8:d0ff:fe94:8f8c'},
838+ ]
839+
840+ When using the local provider, LXC addresses are like the following:
841+
842+ [
843+ {'NetworkName': '',
844+ 'NetworkScope': '',
845+ 'Type': 'ipv4',
846+ 'Value': '10.0.3.42'},
847+ {'NetworkName': '',
848+ 'NetworkScope': '',
849+ 'Type': 'ipv6',
850+ 'Value': 'fe80::216:3eff:fefd:787e'},
851+ ]
852+
853+ If the addresses list is empty, or if no public/reachable addresses can be
854+ found, this function returns None.
855+ """
856+ # This implementation reflects how the public address is retrieved in Juju:
857+ # see juju-core/instance/address.go:SelectPublicAddress.
858+ public_address = None
859+ for address in addresses:
860+ value = address['Value']
861+ # Exclude empty values and ipv6 addresses.
862+ if value and (address['Type'] != IPV6_ADDRESS):
863+ scope = address['NetworkScope']
864+ # If the scope is public then we have found the address.
865+ if scope == NETWORK_PUBLIC:
866+ return value
867+ # If the scope is unknown then store the value. This way the last
868+ # address with unknown scope will be returned, and we are able to
869+ # return the right LXC address.
870+ if scope == NETWORK_UNKNOWN:
871+ public_address = value
872+ return public_address
873+
874+
875+def parse_machine_change(action, data, current_status, address):
876 """Parse the given machine change.
877
878 The change is represented by the given action/data pair.
879- Also receive the last known machine status, which can be an empty string
880- if the information is unknown.
881+ Also receive the last known machine status and address, which can be empty
882+ strings if those pieces of information are unknown.
883
884 Output a human readable message each time a relevant change is found.
885
886- Return the machine status.
887+ Return the machine status and the address.
888 Raise a ValueError if the machine is removed or in an error state.
889 """
890 machine_id = data['Id']
891@@ -44,6 +113,16 @@
892 msg = 'machine {} is in an error state: {}: {}'.format(
893 machine_id, status, data['StatusInfo'])
894 raise ValueError(msg.encode('utf-8'))
895+ # Notify when the machine becomes reachable. Starting from juju-core 1.18,
896+ # the mega-watcher for machines includes addresses for each machine. This
897+ # info is the preferred source where to look to retrieve the public address
898+ # of units hosted by a specific machine.
899+ if not address:
900+ addresses = data.get('Addresses', [])
901+ public_address = retrieve_public_adddress(addresses)
902+ if public_address is not None:
903+ address = public_address
904+ print('unit placed on {}'.format(address))
905 # Notify status changes.
906 if status != current_status:
907 if status == 'pending':
908@@ -51,7 +130,7 @@
909 machine_id))
910 elif status == 'started':
911 print('machine {} is started'.format(machine_id))
912- return status
913+ return status, address
914
915
916 def parse_unit_change(action, data, current_status, address):
917@@ -77,18 +156,22 @@
918 msg = '{} is in an error state: {}: {}'.format(
919 unit_name, status, data['StatusInfo'])
920 raise ValueError(msg.encode('utf-8'))
921- # Notify when the unit becomes reachable.
922+ # Notify when the unit becomes reachable. Up to juju-core 1.18, the
923+ # mega-watcher for units includes the public address for each unit. This
924+ # info is likely to be deprecated in favor of addresses as included in the
925+ # mega-watcher for machines, but we still try to retrieve the address here
926+ # for backward compatibility.
927 if not address:
928- address = data['PublicAddress']
929+ address = data.get('PublicAddress', '')
930 if address:
931- print('setting up {} on {}'.format(unit_name, address))
932+ print('{} placed on {}'.format(unit_name, address))
933 # Notify status changes.
934 if status != current_status:
935 if status == 'pending':
936 print('{} deployment is pending'.format(unit_name))
937 elif status == 'installed':
938 print('{} is installed'.format(unit_name))
939- elif address and status == 'started':
940+ elif status == 'started':
941 print('{} is ready on machine {}'.format(
942 unit_name, data['MachineId']))
943 return status, address, data.get('MachineId', '')

Subscribers

People subscribed via source and target branches