Merge lp:~frankban/juju-quickstart/new-machine-info into lp:juju-quickstart
- new-machine-info
- Merge into trunk
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email: mp+214317@code.launchpad.net |
Commit message
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/
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!
Francesco Banconi (frankban) wrote : | # |
Brad Crittenden (bac) wrote : | # |
LGTM. Will QA now.
https:/
File quickstart/
https:/
quickstart/
on the EC2 cloud. '
This change looks familiar...
https:/
File quickstart/
https:/
quickstart/
public address is not available.
Two spaces: LXC public
(I know you hate those!)
https:/
quickstart/
instance is proprely returned.
typo: properly
Brad Crittenden (bac) wrote : | # |
- 66. By Francesco Banconi
-
Add debug to the Makefile.
- 67. By Francesco Banconi
-
Changes as per review.
Francesco Banconi (frankban) wrote : | # |
Please take a look.
https:/
File quickstart/
https:/
quickstart/
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:/
File quickstart/
https:/
quickstart/
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:/
quickstart/
instance is proprely returned.
On 2014/04/04 18:24:04, bac wrote:
> typo: properly
Done.
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:/
File quickstart/app.py (right):
https:/
quickstart/
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:/
File quickstart/
https:/
quickstart/
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?
- 68. By Francesco Banconi
-
Changes as per review.
Francesco Banconi (frankban) wrote : | # |
*** 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/
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:/
https:/
File quickstart/app.py (right):
https:/
quickstart/
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:/
File quickstart/
https:/
quickstart/
Francesco Banconi (frankban) wrote : | # |
Thank you both!
Preview Diff
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', '') |
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): models/ envs.py tests/test_ app.py tests/test_ watchers. py watchers. py
A [revision details]
M quickstart/app.py
M quickstart/
M quickstart/
M quickstart/
M quickstart/