Merge lp:~frankban/juju-quickstart/maas-address into lp:juju-quickstart
- maas-address
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 103 |
Proposed branch: | lp:~frankban/juju-quickstart/maas-address |
Merge into: | lp:juju-quickstart |
Diff against target: |
1288 lines (+332/-433) 12 files modified
README.rst (+2/-0) quickstart/__init__.py (+1/-1) quickstart/app.py (+21/-31) quickstart/manage.py (+6/-11) quickstart/settings.py (+3/-0) quickstart/tests/helpers.py (+11/-0) quickstart/tests/test_app.py (+63/-183) quickstart/tests/test_manage.py (+19/-68) quickstart/tests/test_utils.py (+18/-0) quickstart/tests/test_watchers.py (+122/-108) quickstart/utils.py (+14/-0) quickstart/watchers.py (+52/-31) |
To merge this branch: | bzr merge lp:~frankban/juju-quickstart/maas-address |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email: mp+241263@code.launchpad.net |
Commit message
Description of the change
Unit address from the machines watcher only
Only use the mega-watcher for machines to retrieve
the Juju GUI unit address.
This change has several consequences:
- it allows us to apply some logic on how the
right address is chosen. For instance, now
we try to resolve public hostnames before
proceeding, and this should fix the cases
where a cloud dns is not configured on the
machine running quickstart. This is the case
of many maas environments;
- it simplifies parsing the mega-watcher changes;
- more importantly, it breaks compatibility
with very old versions of juju (<1.18), in which
the mega-watcher for machines did not include
machine addresses.
For this reason, quickstart now explicitly
drops support for juju < 1.18.1
(1.18.1 is the version on trusty universe).
This also allows for removing some version
checks in the code, including sudo handling when
calling bootstrap on local envs, several special
cases on the watcher side, and other oddities.
For the reasons above, I bumped the quickstart
version up to 1.5.0.
PS: my apologies for the long diff, hope the code
is still easy to follow. Sorry.
Tests: `make check`
QA:
run quickstart as usual, on local and cloud envs,
check it works properly when run again, etc.
this branch has been already successfully QAed in
a maas environment by Adam (Landscape team).
Francesco Banconi (frankban) wrote : | # |
Brad Crittenden (bac) wrote : | # |
On 2014/11/10 12:30:34, frankban wrote:
> Please take a look.
LGTM. Have not done QA yet.
Brad Crittenden (bac) wrote : | # |
Richard Harding (rharding) wrote : | # |
LGTM ty for the updates!
https:/
File quickstart/app.py (left):
https:/
quickstart/
is this because of the new juju requirement?
Francesco Banconi (frankban) wrote : | # |
Thank you both for the reviews of this long diff!
https:/
File quickstart/app.py (left):
https:/
quickstart/
On 2014/11/10 15:25:20, rharding wrote:
> is this because of the new juju requirement?
Yes! We got rid of very old versions of juju requiring an explicit sudo
on the command line.
Francesco Banconi (frankban) wrote : | # |
*** Submitted:
Unit address from the machines watcher only
Only use the mega-watcher for machines to retrieve
the Juju GUI unit address.
This change has several consequences:
- it allows us to apply some logic on how the
right address is chosen. For instance, now
we try to resolve public hostnames before
proceeding, and this should fix the cases
where a cloud dns is not configured on the
machine running quickstart. This is the case
of many maas environments;
- it simplifies parsing the mega-watcher changes;
- more importantly, it breaks compatibility
with very old versions of juju (<1.18), in which
the mega-watcher for machines did not include
machine addresses.
For this reason, quickstart now explicitly
drops support for juju < 1.18.1
(1.18.1 is the version on trusty universe).
This also allows for removing some version
checks in the code, including sudo handling when
calling bootstrap on local envs, several special
cases on the watcher side, and other oddities.
For the reasons above, I bumped the quickstart
version up to 1.5.0.
PS: my apologies for the long diff, hope the code
is still easy to follow. Sorry.
Tests: `make check`
QA:
run quickstart as usual, on local and cloud envs,
check it works properly when run again, etc.
this branch has been already successfully QAed in
a maas environment by Adam (Landscape team).
R=bac, rharding
CC=
https:/
Preview Diff
1 | === modified file 'README.rst' | |||
2 | --- README.rst 2014-09-01 14:44:39 +0000 | |||
3 | +++ README.rst 2014-11-10 12:30:00 +0000 | |||
4 | @@ -36,6 +36,8 @@ | |||
5 | 36 | Juju Quickstart is available on Ubuntu releases 12.04 LTS (precise), 14.04 LTS | 36 | Juju Quickstart is available on Ubuntu releases 12.04 LTS (precise), 14.04 LTS |
6 | 37 | (trusty), 14.10 (utopic) and on OS X (10.7 and later). | 37 | (trusty), 14.10 (utopic) and on OS X (10.7 and later). |
7 | 38 | 38 | ||
8 | 39 | Starting from version 1.5.0, Juju Quickstart only supports Juju >= 1.18.1. | ||
9 | 40 | |||
10 | 39 | Ubuntu Installation | 41 | Ubuntu Installation |
11 | 40 | ~~~~~~~~~~~~~~~~~~~ | 42 | ~~~~~~~~~~~~~~~~~~~ |
12 | 41 | 43 | ||
13 | 42 | 44 | ||
14 | === modified file 'quickstart/__init__.py' | |||
15 | --- quickstart/__init__.py 2014-10-01 13:43:17 +0000 | |||
16 | +++ quickstart/__init__.py 2014-11-10 12:30:00 +0000 | |||
17 | @@ -45,7 +45,7 @@ | |||
18 | 45 | Once Juju has been installed, the command can also be run as a juju plugin, | 45 | Once Juju has been installed, the command can also be run as a juju plugin, |
19 | 46 | without the hyphen ("juju quickstart"). | 46 | without the hyphen ("juju quickstart"). |
20 | 47 | """ | 47 | """ |
22 | 48 | VERSION = (1, 4, 4) | 48 | VERSION = (1, 5, 0) |
23 | 49 | 49 | ||
24 | 50 | 50 | ||
25 | 51 | def get_version(): | 51 | def get_version(): |
26 | 52 | 52 | ||
27 | === modified file 'quickstart/app.py' | |||
28 | --- quickstart/app.py 2014-08-25 16:57:41 +0000 | |||
29 | +++ quickstart/app.py 2014-11-10 12:30:00 +0000 | |||
30 | @@ -116,6 +116,19 @@ | |||
31 | 116 | return juju_version | 116 | return juju_version |
32 | 117 | 117 | ||
33 | 118 | 118 | ||
34 | 119 | def check_juju_supported(juju_version): | ||
35 | 120 | """Ensure the given Juju version is supported by Quickstart. | ||
36 | 121 | |||
37 | 122 | Raise a ProgramExit if Juju is not supported. | ||
38 | 123 | """ | ||
39 | 124 | if juju_version < settings.JUJU_SUPPORTED_VERSION: | ||
40 | 125 | current = b'.'.join(map(bytes, juju_version)) | ||
41 | 126 | supported = b'.'.join(map(bytes, settings.JUJU_SUPPORTED_VERSION)) | ||
42 | 127 | raise ProgramExit( | ||
43 | 128 | b'the current Juju version ({}) is not supported: ' | ||
44 | 129 | b'please upgrade to Juju >= {}'.format(current, supported)) | ||
45 | 130 | |||
46 | 131 | |||
47 | 119 | def ensure_ssh_keys(): | 132 | def ensure_ssh_keys(): |
48 | 120 | """Ensure that SSH keys are available. | 133 | """Ensure that SSH keys are available. |
49 | 121 | 134 | ||
50 | @@ -163,8 +176,9 @@ | |||
51 | 163 | raise ProgramExit(bytes(err)) | 176 | raise ProgramExit(bytes(err)) |
52 | 164 | 177 | ||
53 | 165 | 178 | ||
56 | 166 | def bootstrap(env_name, juju_command, requires_sudo=False, debug=False, | 179 | def bootstrap( |
57 | 167 | upload_tools=False, upload_series=None, constraints=None): | 180 | env_name, juju_command, debug=False, upload_tools=False, |
58 | 181 | upload_series=None, constraints=None): | ||
59 | 168 | """Bootstrap the Juju environment with the given name. | 182 | """Bootstrap the Juju environment with the given name. |
60 | 169 | 183 | ||
61 | 170 | Do not bootstrap the environment if already bootstrapped. | 184 | Do not bootstrap the environment if already bootstrapped. |
62 | @@ -185,8 +199,6 @@ | |||
63 | 185 | """ | 199 | """ |
64 | 186 | already_bootstrapped = False | 200 | already_bootstrapped = False |
65 | 187 | cmd = [juju_command, 'bootstrap', '-e', env_name] | 201 | cmd = [juju_command, 'bootstrap', '-e', env_name] |
66 | 188 | if requires_sudo: | ||
67 | 189 | cmd.insert(0, 'sudo') | ||
68 | 190 | if debug: | 202 | if debug: |
69 | 191 | cmd.append('--debug') | 203 | cmd.append('--debug') |
70 | 192 | if upload_tools: | 204 | if upload_tools: |
71 | @@ -497,14 +509,10 @@ | |||
72 | 497 | for action, data in unit_changes: | 509 | for action, data in unit_changes: |
73 | 498 | if data['Name'] == unit_name: | 510 | if data['Name'] == unit_name: |
74 | 499 | try: | 511 | try: |
77 | 500 | data = watchers.parse_unit_change( | 512 | unit_status, machine_id = watchers.parse_unit_change( |
78 | 501 | action, data, unit_status, address) | 513 | action, data, unit_status) |
79 | 502 | except ValueError as err: | 514 | except ValueError as err: |
80 | 503 | raise ProgramExit(bytes(err)) | 515 | raise ProgramExit(bytes(err)) |
81 | 504 | unit_status, address, machine_id = data | ||
82 | 505 | if address and unit_status == 'started': | ||
83 | 506 | # We can exit this loop. | ||
84 | 507 | return address | ||
85 | 508 | # The mega-watcher contains a single change for each specific | 516 | # The mega-watcher contains a single change for each specific |
86 | 509 | # unit. For this reason, we can exit the for loop here. | 517 | # unit. For this reason, we can exit the for loop here. |
87 | 510 | break | 518 | break |
88 | @@ -528,12 +536,12 @@ | |||
89 | 528 | action, data, machine_status, address) | 536 | action, data, machine_status, address) |
90 | 529 | except ValueError as err: | 537 | except ValueError as err: |
91 | 530 | raise ProgramExit(bytes(err)) | 538 | raise ProgramExit(bytes(err)) |
92 | 531 | if address and unit_status == 'started': | ||
93 | 532 | # We can exit this loop. | ||
94 | 533 | return address | ||
95 | 534 | # The mega-watcher contains a single change for each specific | 539 | # The mega-watcher contains a single change for each specific |
96 | 535 | # machine. For this reason, we can exit the for loop here. | 540 | # machine. For this reason, we can exit the for loop here. |
97 | 536 | break | 541 | break |
98 | 542 | if address and unit_status == 'started': | ||
99 | 543 | # We can exit this loop. | ||
100 | 544 | return address | ||
101 | 537 | 545 | ||
102 | 538 | 546 | ||
103 | 539 | def deploy_bundle(env, bundle_yaml, bundle_name, bundle_id): | 547 | def deploy_bundle(env, bundle_yaml, bundle_name, bundle_id): |
104 | @@ -549,21 +557,3 @@ | |||
105 | 549 | env.deploy_bundle(bundle_yaml, name=bundle_name, bundle_id=bundle_id) | 557 | env.deploy_bundle(bundle_yaml, name=bundle_name, bundle_id=bundle_id) |
106 | 550 | except jujuclient.EnvError as err: | 558 | except jujuclient.EnvError as err: |
107 | 551 | raise ProgramExit('bad API server response: {}'.format(err.message)) | 559 | raise ProgramExit('bad API server response: {}'.format(err.message)) |
108 | 552 | |||
109 | 553 | |||
110 | 554 | def juju_requires_sudo(env_type, juju_version, customized): | ||
111 | 555 | """Report whether the given Juju version requires "sudo". | ||
112 | 556 | |||
113 | 557 | The env_type argument is the current environment type. | ||
114 | 558 | "e.g. local, ec2, azure" | ||
115 | 559 | |||
116 | 560 | Raise a ProgramExit if a customized version of Juju is being used and Juju | ||
117 | 561 | itself requires "sudo" | ||
118 | 562 | """ | ||
119 | 563 | # If the Juju version is less than 1.17.2 then use sudo for local envs. | ||
120 | 564 | if env_type == 'local' and juju_version < (1, 17, 2): | ||
121 | 565 | if customized: | ||
122 | 566 | raise ProgramExit(b'cannot use a customized Juju when it ' | ||
123 | 567 | 'requires sudo') | ||
124 | 568 | return True | ||
125 | 569 | return False | ||
126 | 570 | 560 | ||
127 | === modified file 'quickstart/manage.py' | |||
128 | --- quickstart/manage.py 2014-08-25 16:57:41 +0000 | |||
129 | +++ quickstart/manage.py 2014-11-10 12:30:00 +0000 | |||
130 | @@ -502,9 +502,7 @@ | |||
131 | 502 | logging.debug('ensuring juju and dependencies are installed') | 502 | logging.debug('ensuring juju and dependencies are installed') |
132 | 503 | juju_version = app.ensure_dependencies( | 503 | juju_version = app.ensure_dependencies( |
133 | 504 | options.distro_only, options.platform, juju_command) | 504 | options.distro_only, options.platform, juju_command) |
137 | 505 | 505 | app.check_juju_supported(juju_version) | |
135 | 506 | requires_sudo = app.juju_requires_sudo( | ||
136 | 507 | options.env_type, juju_version, custom_juju) | ||
138 | 508 | 506 | ||
139 | 509 | logging.debug('ensuring SSH keys are available') | 507 | logging.debug('ensuring SSH keys are available') |
140 | 510 | app.ensure_ssh_keys() | 508 | app.ensure_ssh_keys() |
141 | @@ -513,13 +511,12 @@ | |||
142 | 513 | options.env_name, options.env_type)) | 511 | options.env_name, options.env_type)) |
143 | 514 | if options.env_type == 'local': | 512 | if options.env_type == 'local': |
144 | 515 | # If this is a local environment, notify the user that "sudo" will be | 513 | # If this is a local environment, notify the user that "sudo" will be |
147 | 516 | # required to bootstrap the application, even in newer Juju versions | 514 | # required by Juju to bootstrap the application. |
146 | 517 | # where "sudo" is invoked by juju-core itself. | ||
148 | 518 | print('sudo privileges will be required to bootstrap the environment') | 515 | print('sudo privileges will be required to bootstrap the environment') |
149 | 519 | 516 | ||
150 | 520 | already_bootstrapped, bootstrap_node_series = app.bootstrap( | 517 | already_bootstrapped, bootstrap_node_series = app.bootstrap( |
151 | 521 | options.env_name, juju_command, | 518 | options.env_name, juju_command, |
153 | 522 | requires_sudo=requires_sudo, debug=options.debug, | 519 | debug=options.debug, |
154 | 523 | upload_tools=options.upload_tools, | 520 | upload_tools=options.upload_tools, |
155 | 524 | upload_series=options.upload_series, | 521 | upload_series=options.upload_series, |
156 | 525 | constraints=options.constraints) | 522 | constraints=options.constraints) |
157 | @@ -585,9 +582,7 @@ | |||
158 | 585 | 'Run "juju quickstart -i" if you want to manage\n' | 582 | 'Run "juju quickstart -i" if you want to manage\n' |
159 | 586 | 'or bootstrap your Juju environments using the\n' | 583 | 'or bootstrap your Juju environments using the\n' |
160 | 587 | 'interactive session.\n' | 584 | 'interactive session.\n' |
166 | 588 | 'Run "{sudo}juju destroy-environment {eflag}{env_name} [-y]"\n' | 585 | 'Run "juju destroy-environment {env_name} [-y]"\n' |
167 | 589 | 'to destroy the environment you just bootstrapped.'.format( | 586 | 'to destroy the environment you just bootstrapped.' |
168 | 590 | env_name=options.env_name, | 587 | ''.format(env_name=options.env_name) |
164 | 591 | sudo='sudo ' if requires_sudo else '', | ||
165 | 592 | eflag='-e ' if juju_version < (1, 17, 2) else '') | ||
169 | 593 | ) | 588 | ) |
170 | 594 | 589 | ||
171 | === modified file 'quickstart/settings.py' | |||
172 | --- quickstart/settings.py 2014-10-09 13:14:30 +0000 | |||
173 | +++ quickstart/settings.py 2014-11-10 12:30:00 +0000 | |||
174 | @@ -76,6 +76,9 @@ | |||
175 | 76 | # The set of series supported by the Juju GUI charm. | 76 | # The set of series supported by the Juju GUI charm. |
176 | 77 | JUJU_GUI_SUPPORTED_SERIES = tuple(DEFAULT_CHARM_URLS.keys()) | 77 | JUJU_GUI_SUPPORTED_SERIES = tuple(DEFAULT_CHARM_URLS.keys()) |
177 | 78 | 78 | ||
178 | 79 | # The minimum Juju version supported by Juju Quickstart, | ||
179 | 80 | JUJU_SUPPORTED_VERSION = (1, 18, 1) | ||
180 | 81 | |||
181 | 79 | # The path to the MAAS command line interface. | 82 | # The path to the MAAS command line interface. |
182 | 80 | MAAS_CMD = '/usr/bin/maas' | 83 | MAAS_CMD = '/usr/bin/maas' |
183 | 81 | 84 | ||
184 | 82 | 85 | ||
185 | === modified file 'quickstart/tests/helpers.py' | |||
186 | --- quickstart/tests/helpers.py 2014-09-08 17:49:51 +0000 | |||
187 | +++ quickstart/tests/helpers.py 2014-11-10 12:30:00 +0000 | |||
188 | @@ -195,6 +195,17 @@ | |||
189 | 195 | mock_print = mock.patch('__builtin__.print') | 195 | mock_print = mock.patch('__builtin__.print') |
190 | 196 | 196 | ||
191 | 197 | 197 | ||
192 | 198 | def patch_check_resolvable(error=None): | ||
193 | 199 | """Patch the utils.check_resolvable function to return the given error. | ||
194 | 200 | |||
195 | 201 | This is done so that tests do not try to resolve hostname addresses. | ||
196 | 202 | """ | ||
197 | 203 | return mock.patch( | ||
198 | 204 | 'quickstart.utils.check_resolvable', | ||
199 | 205 | lambda hostname: error, | ||
200 | 206 | ) | ||
201 | 207 | |||
202 | 208 | |||
203 | 198 | class UrlReadTestsMixin(object): | 209 | class UrlReadTestsMixin(object): |
204 | 199 | """Expose a method to mock the quickstart.utils.urlread helper function.""" | 210 | """Expose a method to mock the quickstart.utils.urlread helper function.""" |
205 | 200 | 211 | ||
206 | 201 | 212 | ||
207 | === modified file 'quickstart/tests/test_app.py' | |||
208 | --- quickstart/tests/test_app.py 2014-08-25 16:57:41 +0000 | |||
209 | +++ quickstart/tests/test_app.py 2014-11-10 12:30:00 +0000 | |||
210 | @@ -298,6 +298,28 @@ | |||
211 | 298 | self.assertEqual(3, mock_call.call_count) | 298 | self.assertEqual(3, mock_call.call_count) |
212 | 299 | 299 | ||
213 | 300 | 300 | ||
214 | 301 | class TestCheckJujuSupported(ProgramExitTestsMixin, unittest.TestCase): | ||
215 | 302 | |||
216 | 303 | supported_versions = [(1, 18, 1), (1, 19, 0), (1, 42, 47), (2, 0, 0)] | ||
217 | 304 | unsupported_versions = [(1, 18, 0), (1, 17, 42), (1, 0, 0), (0, 20, 47)] | ||
218 | 305 | |||
219 | 306 | def test_supported(self): | ||
220 | 307 | # No exceptions are raised if the Juju version is supported. | ||
221 | 308 | for version in self.supported_versions: | ||
222 | 309 | app.check_juju_supported(version) | ||
223 | 310 | |||
224 | 311 | def test_not_supported(self): | ||
225 | 312 | # A ProgramExit error is raised if the Juju version is not supported. | ||
226 | 313 | error = ( | ||
227 | 314 | 'the current Juju version ({}.{}.{}) is not supported: ' | ||
228 | 315 | 'please upgrade to Juju >= ' + | ||
229 | 316 | '.'.join(map(str, settings.JUJU_SUPPORTED_VERSION)) | ||
230 | 317 | ) | ||
231 | 318 | for version in self.unsupported_versions: | ||
232 | 319 | with self.assert_program_exit(error.format(*version)): | ||
233 | 320 | app.check_juju_supported(version) | ||
234 | 321 | |||
235 | 322 | |||
236 | 301 | @helpers.mock_print | 323 | @helpers.mock_print |
237 | 302 | class TestEnsureSSHKeys( | 324 | class TestEnsureSSHKeys( |
238 | 303 | helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase): | 325 | helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase): |
239 | @@ -484,19 +506,6 @@ | |||
240 | 484 | ] + self.make_status_calls(1)) | 506 | ] + self.make_status_calls(1)) |
241 | 485 | mock_print.assert_called_once_with(self.status_message) | 507 | mock_print.assert_called_once_with(self.status_message) |
242 | 486 | 508 | ||
243 | 487 | def test_success_local_provider(self, mock_print): | ||
244 | 488 | # The environment is bootstrapped with sudo using the local provider. | ||
245 | 489 | with self.patch_multiple_calls(self.make_side_effects()) as mock_call: | ||
246 | 490 | already_bootstrapped, series = app.bootstrap( | ||
247 | 491 | self.env_name, self.juju_command, requires_sudo=True) | ||
248 | 492 | self.assertFalse(already_bootstrapped) | ||
249 | 493 | self.assertEqual(series, 'hoary') | ||
250 | 494 | mock_call.assert_has_calls([ | ||
251 | 495 | mock.call( | ||
252 | 496 | 'sudo', self.juju_command, 'bootstrap', '-e', self.env_name), | ||
253 | 497 | ] + self.make_status_calls(1)) | ||
254 | 498 | mock_print.assert_called_once_with(self.status_message) | ||
255 | 499 | |||
256 | 500 | def test_success_debug(self, mock_print): | 509 | def test_success_debug(self, mock_print): |
257 | 501 | # The environment is successfully bootstrapped in debug mode. | 510 | # The environment is successfully bootstrapped in debug mode. |
258 | 502 | with self.patch_multiple_calls(self.make_side_effects()) as mock_call: | 511 | with self.patch_multiple_calls(self.make_side_effects()) as mock_call: |
259 | @@ -1275,16 +1284,16 @@ | |||
260 | 1275 | 1284 | ||
261 | 1276 | 1285 | ||
262 | 1277 | @helpers.mock_print | 1286 | @helpers.mock_print |
263 | 1287 | @helpers.patch_check_resolvable() | ||
264 | 1278 | class TestWatch( | 1288 | class TestWatch( |
265 | 1279 | ProgramExitTestsMixin, helpers.ValueErrorTestsMixin, | 1289 | ProgramExitTestsMixin, helpers.ValueErrorTestsMixin, |
266 | 1280 | unittest.TestCase): | 1290 | unittest.TestCase): |
267 | 1281 | 1291 | ||
268 | 1282 | address = 'unit.example.com' | 1292 | address = 'unit.example.com' |
269 | 1283 | machine_pending_call = mock.call('machine 0 provisioning is pending') | 1293 | machine_pending_call = mock.call('machine 0 provisioning is pending') |
271 | 1284 | unit_placed_machine_call = mock.call('unit placed on unit.example.com') | 1294 | unit_placed_call = mock.call('unit placed on {}'.format(address)) |
272 | 1285 | machine_started_call = mock.call('machine 0 is started') | 1295 | machine_started_call = mock.call('machine 0 is started') |
273 | 1286 | unit_pending_call = mock.call('django/42 deployment is pending') | 1296 | unit_pending_call = mock.call('django/42 deployment is pending') |
274 | 1287 | unit_placed_unit_call = mock.call('django/42 placed on {}'.format(address)) | ||
275 | 1288 | unit_installed_call = mock.call('django/42 is installed') | 1297 | unit_installed_call = mock.call('django/42 is installed') |
276 | 1289 | unit_started_call = mock.call('django/42 is ready on machine 0') | 1298 | unit_started_call = mock.call('django/42 is ready on machine 0') |
277 | 1290 | 1299 | ||
278 | @@ -1314,16 +1323,9 @@ | |||
279 | 1314 | }] | 1323 | }] |
280 | 1315 | return 'change', data | 1324 | return 'change', data |
281 | 1316 | 1325 | ||
292 | 1317 | def make_unit_change(self, status, name='django/42', address=None): | 1326 | def make_unit_change(self, status, name='django/42'): |
293 | 1318 | """Create and return a unit change. | 1327 | """Create and return a unit change.""" |
294 | 1319 | 1328 | return 'change', {'MachineId': '0', 'Name': name, 'Status': status} | |
285 | 1320 | If the address argument is None, the change does not include the | ||
286 | 1321 | corresponding address field. | ||
287 | 1322 | """ | ||
288 | 1323 | data = {'MachineId': '0', 'Name': name, 'Status': status} | ||
289 | 1324 | if address is not None: | ||
290 | 1325 | data['PublicAddress'] = address | ||
291 | 1326 | return 'change', data | ||
295 | 1327 | 1329 | ||
296 | 1328 | # The following group of tests exercises both the function return value and | 1330 | # The following group of tests exercises both the function return value and |
297 | 1329 | # the function output, even if the output is handled by sub-functions. | 1331 | # the function output, even if the output is handled by sub-functions. |
298 | @@ -1334,35 +1336,10 @@ | |||
299 | 1334 | # The glorious moments in the unit's life are properly highlighted. | 1336 | # The glorious moments in the unit's life are properly highlighted. |
300 | 1335 | # The machine achievements are also celebrated. | 1337 | # The machine achievements are also celebrated. |
301 | 1336 | env = self.make_env([ | 1338 | env = self.make_env([ |
303 | 1337 | ([self.make_unit_change('pending', address='')], | 1339 | ([self.make_unit_change('pending')], |
304 | 1338 | [self.make_machine_change('pending')]), | 1340 | [self.make_machine_change('pending')]), |
305 | 1339 | ([], [self.make_machine_change('started')]), | 1341 | ([], [self.make_machine_change('started')]), |
306 | 1340 | ([self.make_unit_change('pending', address=self.address)], []), | ||
307 | 1341 | ([self.make_unit_change('installed', address=self.address)], []), | ||
308 | 1342 | ([self.make_unit_change('started', address=self.address)], []), | ||
309 | 1343 | ]) | ||
310 | 1344 | address = app.watch(env, 'django/42') | ||
311 | 1345 | self.assertEqual(self.address, address) | ||
312 | 1346 | self.assertEqual(6, mock_print.call_count) | ||
313 | 1347 | mock_print.assert_has_calls([ | ||
314 | 1348 | self.unit_pending_call, | ||
315 | 1349 | self.machine_pending_call, | ||
316 | 1350 | self.machine_started_call, | ||
317 | 1351 | self.unit_placed_unit_call, | ||
318 | 1352 | self.unit_installed_call, | ||
319 | 1353 | self.unit_started_call, | ||
320 | 1354 | ]) | ||
321 | 1355 | |||
322 | 1356 | def test_unit_life_with_machine_address(self, mock_print): | ||
323 | 1357 | # The glorious moments in the unit's life are properly highlighted. | ||
324 | 1358 | # The machine achievements are also celebrated. | ||
325 | 1359 | # This time the new mega-watcher behavior is simulated, in which | ||
326 | 1360 | # addresses are included in the machine change. | ||
327 | 1361 | env = self.make_env([ | ||
328 | 1362 | ([self.make_unit_change('pending')], | ||
329 | 1363 | [self.make_machine_change('pending', address='')]), | ||
330 | 1364 | ([], [self.make_machine_change('started', address=self.address)]), | 1342 | ([], [self.make_machine_change('started', address=self.address)]), |
331 | 1365 | ([self.make_unit_change('pending')], []), | ||
332 | 1366 | ([self.make_unit_change('installed')], []), | 1343 | ([self.make_unit_change('installed')], []), |
333 | 1367 | ([self.make_unit_change('started')], []), | 1344 | ([self.make_unit_change('started')], []), |
334 | 1368 | ]) | 1345 | ]) |
335 | @@ -1372,8 +1349,8 @@ | |||
336 | 1372 | mock_print.assert_has_calls([ | 1349 | mock_print.assert_has_calls([ |
337 | 1373 | self.unit_pending_call, | 1350 | self.unit_pending_call, |
338 | 1374 | self.machine_pending_call, | 1351 | self.machine_pending_call, |
339 | 1375 | self.unit_placed_machine_call, | ||
340 | 1376 | self.machine_started_call, | 1352 | self.machine_started_call, |
341 | 1353 | self.unit_placed_call, | ||
342 | 1377 | self.unit_installed_call, | 1354 | self.unit_installed_call, |
343 | 1378 | self.unit_started_call, | 1355 | self.unit_started_call, |
344 | 1379 | ]) | 1356 | ]) |
345 | @@ -1382,21 +1359,23 @@ | |||
346 | 1382 | # Strange unit evolutions are handled. | 1359 | # Strange unit evolutions are handled. |
347 | 1383 | env = self.make_env([ | 1360 | env = self.make_env([ |
348 | 1384 | # The unit is first reachable and then pending. The machine starts | 1361 | # The unit is first reachable and then pending. The machine starts |
357 | 1385 | # when the unit is already installed. All of this makes no sense | 1362 | # when the unit is already installed. The machine has an address |
358 | 1386 | # and should never happen, but if it does, we deal with it. | 1363 | # when still provisioning. All of this makes no sense and should |
359 | 1387 | ([self.make_unit_change('pending', address=self.address)], []), | 1364 | # never happen, but if it does, we deal with it. |
360 | 1388 | ([self.make_unit_change('pending', address='')], | 1365 | ([self.make_unit_change('pending')], []), |
361 | 1389 | [self.make_machine_change('pending')]), | 1366 | ([], [self.make_machine_change('pending', address=self.address)]), |
362 | 1390 | ([self.make_unit_change('installed', address=self.address)], []), | 1367 | ([self.make_unit_change('pending')], |
363 | 1391 | ([], [self.make_machine_change('started')]), | 1368 | [self.make_machine_change('pending', address='')]), |
364 | 1392 | ([self.make_unit_change('started', address=self.address)], []), | 1369 | ([self.make_unit_change('installed')], []), |
365 | 1370 | ([], [self.make_machine_change('started', address=self.address)]), | ||
366 | 1371 | ([self.make_unit_change('started')], []), | ||
367 | 1393 | ]) | 1372 | ]) |
368 | 1394 | address = app.watch(env, 'django/42') | 1373 | address = app.watch(env, 'django/42') |
369 | 1395 | self.assertEqual(self.address, address) | 1374 | self.assertEqual(self.address, address) |
370 | 1396 | self.assertEqual(6, mock_print.call_count) | 1375 | self.assertEqual(6, mock_print.call_count) |
371 | 1397 | mock_print.assert_has_calls([ | 1376 | mock_print.assert_has_calls([ |
372 | 1398 | self.unit_placed_unit_call, | ||
373 | 1399 | self.unit_pending_call, | 1377 | self.unit_pending_call, |
374 | 1378 | self.unit_placed_call, | ||
375 | 1400 | self.machine_pending_call, | 1379 | self.machine_pending_call, |
376 | 1401 | self.unit_installed_call, | 1380 | self.unit_installed_call, |
377 | 1402 | self.machine_started_call, | 1381 | self.machine_started_call, |
378 | @@ -1404,22 +1383,8 @@ | |||
379 | 1404 | ]) | 1383 | ]) |
380 | 1405 | 1384 | ||
381 | 1406 | def test_missing_changes(self, mock_print): | 1385 | def test_missing_changes(self, mock_print): |
398 | 1407 | # Only the unit started change is strictly required when the unit | 1386 | # Only the unit started change is strictly required when the machine |
399 | 1408 | # change includes the public address. | 1387 | # change includes the addresses. |
384 | 1409 | env = self.make_env([ | ||
385 | 1410 | ([self.make_unit_change('started', address=self.address)], []), | ||
386 | 1411 | ]) | ||
387 | 1412 | address = app.watch(env, 'django/42') | ||
388 | 1413 | self.assertEqual(self.address, address) | ||
389 | 1414 | self.assertEqual(2, mock_print.call_count) | ||
390 | 1415 | mock_print.assert_has_calls([ | ||
391 | 1416 | self.unit_placed_unit_call, | ||
392 | 1417 | self.unit_started_call, | ||
393 | 1418 | ]) | ||
394 | 1419 | |||
395 | 1420 | def test_missing_changes_with_machine_address(self, mock_print): | ||
396 | 1421 | # When using the new mega-watcher, a machine change including its | ||
397 | 1422 | # public address is also required. | ||
400 | 1423 | env = self.make_env([ | 1388 | env = self.make_env([ |
401 | 1424 | ([self.make_unit_change('started')], []), | 1389 | ([self.make_unit_change('started')], []), |
402 | 1425 | ([], [self.make_machine_change('started', address=self.address)]), | 1390 | ([], [self.make_machine_change('started', address=self.address)]), |
403 | @@ -1429,29 +1394,12 @@ | |||
404 | 1429 | self.assertEqual(3, mock_print.call_count) | 1394 | self.assertEqual(3, mock_print.call_count) |
405 | 1430 | mock_print.assert_has_calls([ | 1395 | mock_print.assert_has_calls([ |
406 | 1431 | self.unit_started_call, | 1396 | self.unit_started_call, |
408 | 1432 | self.unit_placed_machine_call, | 1397 | self.unit_placed_call, |
409 | 1433 | self.machine_started_call, | 1398 | self.machine_started_call, |
410 | 1434 | ]) | 1399 | ]) |
411 | 1435 | 1400 | ||
412 | 1436 | def test_ignored_machine_changes(self, mock_print): | 1401 | def test_ignored_machine_changes(self, mock_print): |
413 | 1437 | # All machine changes are ignored until the application knows what | 1402 | # All machine changes are ignored until the application knows what |
414 | 1438 | # machine the unit belongs to. | ||
415 | 1439 | env = self.make_env([ | ||
416 | 1440 | ([], [self.make_machine_change('pending')]), | ||
417 | 1441 | ([], [self.make_machine_change('started')]), | ||
418 | 1442 | ([self.make_unit_change('started', address=self.address)], []), | ||
419 | 1443 | ]) | ||
420 | 1444 | address = app.watch(env, 'django/42') | ||
421 | 1445 | self.assertEqual(self.address, address) | ||
422 | 1446 | # No machine related messages have been printed. | ||
423 | 1447 | self.assertEqual(2, mock_print.call_count) | ||
424 | 1448 | mock_print.assert_has_calls([ | ||
425 | 1449 | self.unit_placed_unit_call, | ||
426 | 1450 | self.unit_started_call, | ||
427 | 1451 | ]) | ||
428 | 1452 | |||
429 | 1453 | def test_ignored_machine_changes_with_machine_address(self, mock_print): | ||
430 | 1454 | # All machine changes are ignored until the application knows what | ||
431 | 1455 | # machine the unit belongs to. When the above happens, previously | 1403 | # machine the unit belongs to. When the above happens, previously |
432 | 1456 | # collected machine changes are still parsed in the case the address | 1404 | # collected machine changes are still parsed in the case the address |
433 | 1457 | # is not yet known. | 1405 | # is not yet known. |
434 | @@ -1468,7 +1416,7 @@ | |||
435 | 1468 | self.assertEqual(3, mock_print.call_count) | 1416 | self.assertEqual(3, mock_print.call_count) |
436 | 1469 | mock_print.assert_has_calls([ | 1417 | mock_print.assert_has_calls([ |
437 | 1470 | self.unit_started_call, | 1418 | self.unit_started_call, |
439 | 1471 | self.unit_placed_machine_call, | 1419 | self.unit_placed_call, |
440 | 1472 | self.machine_started_call, | 1420 | self.machine_started_call, |
441 | 1473 | ]) | 1421 | ]) |
442 | 1474 | 1422 | ||
443 | @@ -1477,20 +1425,6 @@ | |||
444 | 1477 | # This happens, e.g., when executing Quickstart a second time, and both | 1425 | # This happens, e.g., when executing Quickstart a second time, and both |
445 | 1478 | # the unit and the machine are already started. | 1426 | # the unit and the machine are already started. |
446 | 1479 | env = self.make_env([ | 1427 | env = self.make_env([ |
447 | 1480 | ([self.make_unit_change('started', address=self.address)], | ||
448 | 1481 | [self.make_machine_change('started')]), | ||
449 | 1482 | ]) | ||
450 | 1483 | address = app.watch(env, 'django/42') | ||
451 | 1484 | self.assertEqual(self.address, address) | ||
452 | 1485 | self.assertEqual(2, mock_print.call_count) | ||
453 | 1486 | |||
454 | 1487 | def test_unit_already_deployed_with_machine_address(self, mock_print): | ||
455 | 1488 | # Simulate the unit we are observing has been already deployed. | ||
456 | 1489 | # This happens, e.g., when executing Quickstart a second time, and both | ||
457 | 1490 | # the unit and the machine are already started. | ||
458 | 1491 | # This time the new mega-watcher behavior is simulated, in which | ||
459 | 1492 | # addresses are included in the machine change. | ||
460 | 1493 | env = self.make_env([ | ||
461 | 1494 | ([self.make_unit_change('started')], | 1428 | ([self.make_unit_change('started')], |
462 | 1495 | [self.make_machine_change('started', address=self.address)]), | 1429 | [self.make_machine_change('started', address=self.address)]), |
463 | 1496 | ]) | 1430 | ]) |
464 | @@ -1499,7 +1433,7 @@ | |||
465 | 1499 | self.assertEqual(3, mock_print.call_count) | 1433 | self.assertEqual(3, mock_print.call_count) |
466 | 1500 | mock_print.assert_has_calls([ | 1434 | mock_print.assert_has_calls([ |
467 | 1501 | self.unit_started_call, | 1435 | self.unit_started_call, |
469 | 1502 | self.unit_placed_machine_call, | 1436 | self.unit_placed_call, |
470 | 1503 | self.machine_started_call, | 1437 | self.machine_started_call, |
471 | 1504 | ]) | 1438 | ]) |
472 | 1505 | 1439 | ||
473 | @@ -1509,31 +1443,6 @@ | |||
474 | 1509 | # environment type: the unit is deployed on the bootstrap node, which | 1443 | # environment type: the unit is deployed on the bootstrap node, which |
475 | 1510 | # is assumed to be started. | 1444 | # is assumed to be started. |
476 | 1511 | env = self.make_env([ | 1445 | env = self.make_env([ |
477 | 1512 | ([self.make_unit_change('pending', address='')], | ||
478 | 1513 | [self.make_machine_change('started')]), | ||
479 | 1514 | ([self.make_unit_change('pending', address=self.address)], []), | ||
480 | 1515 | ([self.make_unit_change('installed', address=self.address)], []), | ||
481 | 1516 | ([self.make_unit_change('started', address=self.address)], []), | ||
482 | 1517 | ]) | ||
483 | 1518 | address = app.watch(env, 'django/42') | ||
484 | 1519 | self.assertEqual(self.address, address) | ||
485 | 1520 | self.assertEqual(5, mock_print.call_count) | ||
486 | 1521 | mock_print.assert_has_calls([ | ||
487 | 1522 | self.unit_pending_call, | ||
488 | 1523 | self.machine_started_call, | ||
489 | 1524 | self.unit_placed_unit_call, | ||
490 | 1525 | self.unit_installed_call, | ||
491 | 1526 | self.unit_started_call, | ||
492 | 1527 | ]) | ||
493 | 1528 | |||
494 | 1529 | def test_machine_already_started_with_machine_address(self, mock_print): | ||
495 | 1530 | # Simulate the unit is being deployed on an already started machine. | ||
496 | 1531 | # This happens, e.g., when running Quickstart on a non-local | ||
497 | 1532 | # environment type: the unit is deployed on the bootstrap node, which | ||
498 | 1533 | # is assumed to be started. | ||
499 | 1534 | # This time the new mega-watcher behavior is simulated, in which | ||
500 | 1535 | # addresses are included in the machine change. | ||
501 | 1536 | env = self.make_env([ | ||
502 | 1537 | ([self.make_unit_change('pending')], | 1446 | ([self.make_unit_change('pending')], |
503 | 1538 | [self.make_machine_change('started', address=self.address)]), | 1447 | [self.make_machine_change('started', address=self.address)]), |
504 | 1539 | ([self.make_unit_change('pending')], []), | 1448 | ([self.make_unit_change('pending')], []), |
505 | @@ -1545,7 +1454,7 @@ | |||
506 | 1545 | self.assertEqual(5, mock_print.call_count) | 1454 | self.assertEqual(5, mock_print.call_count) |
507 | 1546 | mock_print.assert_has_calls([ | 1455 | mock_print.assert_has_calls([ |
508 | 1547 | self.unit_pending_call, | 1456 | self.unit_pending_call, |
510 | 1548 | self.unit_placed_machine_call, | 1457 | self.unit_placed_call, |
511 | 1549 | self.machine_started_call, | 1458 | self.machine_started_call, |
512 | 1550 | self.unit_installed_call, | 1459 | self.unit_installed_call, |
513 | 1551 | self.unit_started_call, | 1460 | self.unit_started_call, |
514 | @@ -1555,24 +1464,26 @@ | |||
515 | 1555 | # Changes to units or machines we are not observing are ignored. Also | 1464 | # Changes to units or machines we are not observing are ignored. Also |
516 | 1556 | # ensure that repeated changes to a single entity are ignored, even if | 1465 | # ensure that repeated changes to a single entity are ignored, even if |
517 | 1557 | # they are unlikely to happen. | 1466 | # they are unlikely to happen. |
521 | 1558 | pending_unit_change = self.make_unit_change('pending', address='') | 1467 | pending_unit_change = self.make_unit_change('pending') |
522 | 1559 | started_unit_change = self.make_unit_change( | 1468 | started_unit_change = self.make_unit_change('started') |
523 | 1560 | 'started', address=self.address) | 1469 | pending_machine_change = self.make_machine_change('pending') |
524 | 1561 | env = self.make_env([ | 1470 | env = self.make_env([ |
526 | 1562 | # Add a repeated change. | 1471 | # Add a repeated unit change. |
527 | 1563 | ([pending_unit_change, pending_unit_change], | 1472 | ([pending_unit_change, pending_unit_change], |
529 | 1564 | [self.make_machine_change('pending')]), | 1473 | [pending_machine_change]), |
530 | 1565 | # Add extraneous unit and machine changes. | 1474 | # Add extraneous unit and machine changes. |
531 | 1566 | ([self.make_unit_change('pending', name='haproxy/0')], | 1475 | ([self.make_unit_change('pending', name='haproxy/0')], |
532 | 1567 | [self.make_machine_change('pending', name='42')]), | 1476 | [self.make_machine_change('pending', name='42')]), |
533 | 1477 | # Add a repeated machine change. | ||
534 | 1478 | ([], [pending_machine_change, pending_machine_change]), | ||
535 | 1568 | # Add a change to an extraneous machine. | 1479 | # Add a change to an extraneous machine. |
536 | 1569 | ([], [self.make_machine_change('started', name='42'), | 1480 | ([], [self.make_machine_change('started', name='42'), |
538 | 1570 | self.make_machine_change('started')]), | 1481 | self.make_machine_change('started', address=self.address)]), |
539 | 1571 | # Add a change to an extraneous unit. | 1482 | # Add a change to an extraneous unit. |
540 | 1572 | ([self.make_unit_change('started', name='haproxy/0'), | 1483 | ([self.make_unit_change('started', name='haproxy/0'), |
544 | 1573 | self.make_unit_change('pending', address=self.address)], []), | 1484 | self.make_unit_change('pending')], []), |
545 | 1574 | ([self.make_unit_change('installed', address=self.address)], []), | 1485 | ([self.make_unit_change('installed')], []), |
546 | 1575 | # Add another repeated change. | 1486 | # Add another repeated unit change. |
547 | 1576 | ([started_unit_change, started_unit_change], []), | 1487 | ([started_unit_change, started_unit_change], []), |
548 | 1577 | ]) | 1488 | ]) |
549 | 1578 | address = app.watch(env, 'django/42') | 1489 | address = app.watch(env, 'django/42') |
550 | @@ -1581,8 +1492,8 @@ | |||
551 | 1581 | mock_print.assert_has_calls([ | 1492 | mock_print.assert_has_calls([ |
552 | 1582 | self.unit_pending_call, | 1493 | self.unit_pending_call, |
553 | 1583 | self.machine_pending_call, | 1494 | self.machine_pending_call, |
554 | 1495 | self.unit_placed_call, | ||
555 | 1584 | self.machine_started_call, | 1496 | self.machine_started_call, |
556 | 1585 | self.unit_placed_unit_call, | ||
557 | 1586 | self.unit_installed_call, | 1497 | self.unit_installed_call, |
558 | 1587 | self.unit_started_call, | 1498 | self.unit_started_call, |
559 | 1588 | ]) | 1499 | ]) |
560 | @@ -1590,7 +1501,7 @@ | |||
561 | 1590 | def test_api_error(self, mock_print): | 1501 | def test_api_error(self, mock_print): |
562 | 1591 | # A ProgramExit is raised if an error occurs in one of the API calls. | 1502 | # A ProgramExit is raised if an error occurs in one of the API calls. |
563 | 1592 | env = self.make_env([ | 1503 | env = self.make_env([ |
565 | 1593 | ([self.make_unit_change('pending', address='')], []), | 1504 | ([self.make_unit_change('pending')], []), |
566 | 1594 | self.make_env_error('next returned an error'), | 1505 | self.make_env_error('next returned an error'), |
567 | 1595 | ]) | 1506 | ]) |
568 | 1596 | expected = 'bad API server response: next returned an error' | 1507 | expected = 'bad API server response: next returned an error' |
569 | @@ -1602,14 +1513,13 @@ | |||
570 | 1602 | def test_other_errors(self, mock_print): | 1513 | def test_other_errors(self, mock_print): |
571 | 1603 | # Any other errors occurred during the process are not trapped. | 1514 | # Any other errors occurred during the process are not trapped. |
572 | 1604 | env = self.make_env([ | 1515 | env = self.make_env([ |
574 | 1605 | ([self.make_unit_change('installed', address=self.address)], []), | 1516 | ([self.make_unit_change('installed')], []), |
575 | 1606 | ValueError('explode!'), | 1517 | ValueError('explode!'), |
576 | 1607 | ]) | 1518 | ]) |
577 | 1608 | with self.assert_value_error('explode!'): | 1519 | with self.assert_value_error('explode!'): |
578 | 1609 | app.watch(env, 'django/42') | 1520 | app.watch(env, 'django/42') |
582 | 1610 | self.assertEqual(2, mock_print.call_count) | 1521 | self.assertEqual(1, mock_print.call_count) |
583 | 1611 | mock_print.assert_has_calls([ | 1522 | mock_print.assert_has_calls([self.unit_installed_call]) |
581 | 1612 | self.unit_placed_unit_call, self.unit_installed_call]) | ||
584 | 1613 | 1523 | ||
585 | 1614 | def test_machine_status_error(self, mock_print): | 1524 | def test_machine_status_error(self, mock_print): |
586 | 1615 | # A ProgramExit is raised if an the machine is found in an error state. | 1525 | # A ProgramExit is raised if an the machine is found in an error state. |
587 | @@ -1622,8 +1532,7 @@ | |||
588 | 1622 | # The unit pending change is required to make the function know which | 1532 | # The unit pending change is required to make the function know which |
589 | 1623 | # machine to observe. | 1533 | # machine to observe. |
590 | 1624 | env = self.make_env([ | 1534 | env = self.make_env([ |
593 | 1625 | ([self.make_unit_change('pending', address='')], | 1535 | ([self.make_unit_change('pending')], [change_machine_error]), |
592 | 1626 | [change_machine_error]), | ||
594 | 1627 | ]) | 1536 | ]) |
595 | 1628 | expected = 'machine 0 is in an error state: error: oddities' | 1537 | expected = 'machine 0 is in an error state: error: oddities' |
596 | 1629 | with self.assert_program_exit(expected): | 1538 | with self.assert_program_exit(expected): |
597 | @@ -1677,32 +1586,3 @@ | |||
598 | 1677 | with self.assertRaises(ValueError) as context_manager: | 1586 | with self.assertRaises(ValueError) as context_manager: |
599 | 1678 | app.deploy_bundle(env, self.yaml, self.name, None) | 1587 | app.deploy_bundle(env, self.yaml, self.name, None) |
600 | 1679 | self.assertIs(error, context_manager.exception) | 1588 | self.assertIs(error, context_manager.exception) |
601 | 1680 | |||
602 | 1681 | |||
603 | 1682 | class TestJujuRequiresSudo(ProgramExitTestsMixin, unittest.TestCase): | ||
604 | 1683 | no_sudo_versions = [ | ||
605 | 1684 | (1, 17, 2), (1, 17, 10), (1, 18, 0), (1, 18, 2), (2, 16, 1)] | ||
606 | 1685 | sudo_versions = [(0, 7, 9), (1, 0, 0), (1, 16, 42), (1, 17, 0), (1, 17, 1)] | ||
607 | 1686 | |||
608 | 1687 | def test_non_local_returns_false(self): | ||
609 | 1688 | # A non-local provider does not require sudo. | ||
610 | 1689 | actual = app.juju_requires_sudo('aws', None, None) | ||
611 | 1690 | self.assertFalse(actual) | ||
612 | 1691 | |||
613 | 1692 | def test_local_old_juju_returns_true(self): | ||
614 | 1693 | # A juju previous to 1.17.2 requires sudo. | ||
615 | 1694 | for version in self.sudo_versions: | ||
616 | 1695 | self.assertTrue(app.juju_requires_sudo('local', version, False), | ||
617 | 1696 | version) | ||
618 | 1697 | |||
619 | 1698 | def test_local_newer_juju_returns_false(self): | ||
620 | 1699 | # A juju 1.17.2 and newer does not require sudo. | ||
621 | 1700 | for version in self.no_sudo_versions: | ||
622 | 1701 | self.assertFalse(app.juju_requires_sudo('local', version, False), | ||
623 | 1702 | version) | ||
624 | 1703 | |||
625 | 1704 | def test_raises_programexit(self): | ||
626 | 1705 | for version in self.sudo_versions: | ||
627 | 1706 | with self.assert_program_exit('cannot use a customized Juju when ' | ||
628 | 1707 | 'it requires sudo'): | ||
629 | 1708 | app.juju_requires_sudo('local', version, True) | ||
630 | 1709 | 1589 | ||
631 | === modified file 'quickstart/tests/test_manage.py' | |||
632 | --- quickstart/tests/test_manage.py 2014-08-25 16:57:41 +0000 | |||
633 | +++ quickstart/tests/test_manage.py 2014-11-10 12:30:00 +0000 | |||
634 | @@ -795,7 +795,6 @@ | |||
635 | 795 | mock_app.watch.return_value = '1.2.3.4' | 795 | mock_app.watch.return_value = '1.2.3.4' |
636 | 796 | # Make mock_app.create_auth_token return a fake authentication token. | 796 | # Make mock_app.create_auth_token return a fake authentication token. |
637 | 797 | mock_app.create_auth_token.return_value = 'AUTHTOKEN' | 797 | mock_app.create_auth_token.return_value = 'AUTHTOKEN' |
638 | 798 | mock_app.juju_requires_sudo.return_value = False | ||
639 | 799 | options = self.make_options() | 798 | options = self.make_options() |
640 | 800 | with mock.patch('quickstart.manage.platform_support.get_juju_command', | 799 | with mock.patch('quickstart.manage.platform_support.get_juju_command', |
641 | 801 | side_effect=[(self.juju_command, False)]): | 800 | side_effect=[(self.juju_command, False)]): |
642 | @@ -803,8 +802,9 @@ | |||
643 | 803 | mock_app.ensure_dependencies.assert_called() | 802 | mock_app.ensure_dependencies.assert_called() |
644 | 804 | mock_app.ensure_ssh_keys.assert_called() | 803 | mock_app.ensure_ssh_keys.assert_called() |
645 | 805 | mock_app.bootstrap.assert_called_once_with( | 804 | mock_app.bootstrap.assert_called_once_with( |
648 | 806 | options.env_name, self.juju_command, requires_sudo=False, | 805 | options.env_name, self.juju_command, |
649 | 807 | debug=options.debug, upload_tools=options.upload_tools, | 806 | debug=options.debug, |
650 | 807 | upload_tools=options.upload_tools, | ||
651 | 808 | upload_series=options.upload_series, | 808 | upload_series=options.upload_series, |
652 | 809 | constraints=options.constraints) | 809 | constraints=options.constraints) |
653 | 810 | mock_app.get_api_url.assert_called_once_with( | 810 | mock_app.get_api_url.assert_called_once_with( |
654 | @@ -879,68 +879,19 @@ | |||
655 | 879 | # where to deploy the charm, the service and unit data. | 879 | # where to deploy the charm, the service and unit data. |
656 | 880 | mock_app.check_environment.return_value = ( | 880 | mock_app.check_environment.return_value = ( |
657 | 881 | 'cs:precise/juju-gui-42', '0', None, None) | 881 | 'cs:precise/juju-gui-42', '0', None, None) |
720 | 882 | mock_app.juju_requires_sudo.return_value = False | 882 | for version in versions: |
721 | 883 | for version in versions: | 883 | mock_app.ensure_dependencies.return_value = version |
722 | 884 | mock_app.ensure_dependencies.return_value = version | 884 | with mock.patch( |
723 | 885 | with mock.patch( | 885 | 'quickstart.manage.platform_support.get_juju_command', |
724 | 886 | 'quickstart.manage.platform_support.get_juju_command', | 886 | side_effect=[(self.juju_command, False)]): |
725 | 887 | side_effect=[(self.juju_command, False)]): | 887 | manage.run(options) |
726 | 888 | manage.run(options) | 888 | mock_app.bootstrap.assert_called_once_with( |
727 | 889 | mock_app.bootstrap.assert_called_once_with( | 889 | options.env_name, self.juju_command, |
728 | 890 | options.env_name, self.juju_command, requires_sudo=False, | 890 | debug=options.debug, |
729 | 891 | debug=options.debug, upload_tools=options.upload_tools, | 891 | upload_tools=options.upload_tools, |
730 | 892 | upload_series=options.upload_series, | 892 | upload_series=options.upload_series, |
731 | 893 | constraints=options.constraints) | 893 | constraints=options.constraints) |
732 | 894 | mock_app.bootstrap.reset_mock() | 894 | mock_app.bootstrap.reset_mock() |
671 | 895 | |||
672 | 896 | def test_local_provider_requiring_sudo(self, mock_app, mock_open): | ||
673 | 897 | # The application correctly handles working with local providers when | ||
674 | 898 | # Juju requires an external "sudo" call to bootstrap the environment. | ||
675 | 899 | # Sudo privileges are required if the Juju version is < 1.17.2. | ||
676 | 900 | options = self.make_options(env_type='local') | ||
677 | 901 | versions = [(0, 7, 9), (1, 0, 0), (1, 16, 42), (1, 17, 0), (1, 17, 1)] | ||
678 | 902 | # Make mock_app.bootstrap return the already_bootstrapped flag and the | ||
679 | 903 | # bootstrap node series. | ||
680 | 904 | mock_app.bootstrap.return_value = (True, 'trusty') | ||
681 | 905 | # Make mock_app.check_environment return the charm URL, the machine | ||
682 | 906 | # where to deploy the charm, the service and unit data. | ||
683 | 907 | mock_app.check_environment.return_value = ( | ||
684 | 908 | 'cs:trusty/juju-gui-42', '0', None, None) | ||
685 | 909 | mock_app.juju_requires_sudo.return_value = True | ||
686 | 910 | for version in versions: | ||
687 | 911 | mock_app.ensure_dependencies.return_value = version | ||
688 | 912 | with mock.patch( | ||
689 | 913 | 'quickstart.manage.platform_support.get_juju_command', | ||
690 | 914 | side_effect=[(self.juju_command, False)]): | ||
691 | 915 | manage.run(options) | ||
692 | 916 | mock_app.bootstrap.assert_called_once_with( | ||
693 | 917 | options.env_name, self.juju_command, requires_sudo=True, | ||
694 | 918 | debug=options.debug, upload_tools=options.upload_tools, | ||
695 | 919 | upload_series=options.upload_series, | ||
696 | 920 | constraints=options.constraints) | ||
697 | 921 | mock_app.bootstrap.reset_mock() | ||
698 | 922 | |||
699 | 923 | def test_no_local_no_sudo(self, mock_app, mock_open): | ||
700 | 924 | # Sudo privileges are never required for non-local environments. | ||
701 | 925 | options = self.make_options(env_type='ec2') | ||
702 | 926 | mock_app.ensure_dependencies.return_value = (1, 14, 0) | ||
703 | 927 | # Make mock_app.bootstrap return the already_bootstrapped flag and the | ||
704 | 928 | # bootstrap node series. | ||
705 | 929 | mock_app.bootstrap.return_value = (True, 'precise') | ||
706 | 930 | # Make mock_app.check_environment return the charm URL, the machine | ||
707 | 931 | # where to deploy the charm, the service and unit data. | ||
708 | 932 | mock_app.check_environment.return_value = ( | ||
709 | 933 | 'cs:precise/juju-gui-42', '0', None, None) | ||
710 | 934 | mock_app.juju_requires_sudo.return_value = False | ||
711 | 935 | with mock.patch('quickstart.manage.platform_support.get_juju_command', | ||
712 | 936 | side_effect=[(self.juju_command, False)]): | ||
713 | 937 | manage.run(options) | ||
714 | 938 | mock_app.bootstrap.assert_called_once_with( | ||
715 | 939 | options.env_name, self.juju_command, | ||
716 | 940 | requires_sudo=False, | ||
717 | 941 | debug=options.debug, upload_tools=options.upload_tools, | ||
718 | 942 | upload_series=options.upload_series, | ||
719 | 943 | constraints=options.constraints) | ||
733 | 944 | 895 | ||
734 | 945 | def test_no_browser(self, mock_app, mock_open): | 896 | def test_no_browser(self, mock_app, mock_open): |
735 | 946 | # It is possible to avoid opening the GUI in the browser. | 897 | # It is possible to avoid opening the GUI in the browser. |
736 | @@ -1015,14 +966,14 @@ | |||
737 | 1015 | mock_app.bootstrap.return_value = (True, 'precise') | 966 | mock_app.bootstrap.return_value = (True, 'precise') |
738 | 1016 | mock_app.check_environment.return_value = ( | 967 | mock_app.check_environment.return_value = ( |
739 | 1017 | 'cs:precise/juju-gui-42', '0', None, None) | 968 | 'cs:precise/juju-gui-42', '0', None, None) |
740 | 1018 | mock_app.juju_requires_sudo.return_value = False | ||
741 | 1019 | options = self.make_options(env_type='aws') | 969 | options = self.make_options(env_type='aws') |
742 | 1020 | juju_command = 'some/devel/path/juju' | 970 | juju_command = 'some/devel/path/juju' |
743 | 1021 | with mock.patch('os.environ', {'JUJU': juju_command}): | 971 | with mock.patch('os.environ', {'JUJU': juju_command}): |
744 | 1022 | manage.run(options) | 972 | manage.run(options) |
745 | 1023 | mock_app.bootstrap.assert_called_once_with( | 973 | mock_app.bootstrap.assert_called_once_with( |
748 | 1024 | options.env_name, juju_command, requires_sudo=False, | 974 | options.env_name, juju_command, |
749 | 1025 | debug=options.debug, upload_tools=options.upload_tools, | 975 | debug=options.debug, |
750 | 976 | upload_tools=options.upload_tools, | ||
751 | 1026 | upload_series=options.upload_series, | 977 | upload_series=options.upload_series, |
752 | 1027 | constraints=options.constraints) | 978 | constraints=options.constraints) |
753 | 1028 | mock_app.get_api_url.assert_called_once_with( | 979 | mock_app.get_api_url.assert_called_once_with( |
754 | 1029 | 980 | ||
755 | === modified file 'quickstart/tests/test_utils.py' | |||
756 | --- quickstart/tests/test_utils.py 2014-08-22 16:38:23 +0000 | |||
757 | +++ quickstart/tests/test_utils.py 2014-11-10 12:30:00 +0000 | |||
758 | @@ -156,6 +156,24 @@ | |||
759 | 156 | utils.call('echo', 'we are the borg!') | 156 | utils.call('echo', 'we are the borg!') |
760 | 157 | 157 | ||
761 | 158 | 158 | ||
762 | 159 | class TestCheckResolvable(unittest.TestCase): | ||
763 | 160 | |||
764 | 161 | def test_resolvable(self): | ||
765 | 162 | # None is returned if the hostname can be resolved. | ||
766 | 163 | expected_log = 'example.com resolved to 1.2.3.4' | ||
767 | 164 | with helpers.assert_logs([expected_log], level='debug'): | ||
768 | 165 | with mock.patch('socket.gethostbyname', return_value='1.2.3.4'): | ||
769 | 166 | error = utils.check_resolvable('example.com') | ||
770 | 167 | self.assertIsNone(error) | ||
771 | 168 | |||
772 | 169 | def test_not_resolvable(self): | ||
773 | 170 | # An error message is returned if the hostname cannot be resolved. | ||
774 | 171 | exception = socket.gaierror('bad wolf') | ||
775 | 172 | with mock.patch('socket.gethostbyname', side_effect=exception): | ||
776 | 173 | error = utils.check_resolvable('example.com') | ||
777 | 174 | self.assertEqual('bad wolf', error) | ||
778 | 175 | |||
779 | 176 | |||
780 | 159 | @mock.patch('__builtin__.print', mock.Mock()) | 177 | @mock.patch('__builtin__.print', mock.Mock()) |
781 | 160 | class TestParseGuiCharmUrl(unittest.TestCase): | 178 | class TestParseGuiCharmUrl(unittest.TestCase): |
782 | 161 | 179 | ||
783 | 162 | 180 | ||
784 | === modified file 'quickstart/tests/test_watchers.py' | |||
785 | --- quickstart/tests/test_watchers.py 2014-07-04 13:09:11 +0000 | |||
786 | +++ quickstart/tests/test_watchers.py 2014-11-10 12:30:00 +0000 | |||
787 | @@ -63,23 +63,29 @@ | |||
788 | 63 | 63 | ||
789 | 64 | class TestRetrievePublicAddress(unittest.TestCase): | 64 | class TestRetrievePublicAddress(unittest.TestCase): |
790 | 65 | 65 | ||
791 | 66 | def resolver(self, hostname): | ||
792 | 67 | """A fake resolver returning no errors.""" | ||
793 | 68 | return None | ||
794 | 69 | |||
795 | 66 | def test_empty_addresses(self): | 70 | def test_empty_addresses(self): |
796 | 67 | # None is returned if there are no available addresses. | 71 | # None is returned if there are no available addresses. |
798 | 68 | self.assertIsNone(watchers.retrieve_public_adddress([])) | 72 | address = watchers.retrieve_public_adddress([], self.resolver) |
799 | 73 | self.assertIsNone(address) | ||
800 | 69 | 74 | ||
801 | 70 | def test_cloud_address_not_found(self): | 75 | def test_cloud_address_not_found(self): |
802 | 71 | # None is returned if a cloud machine public address is not available. | 76 | # None is returned if a cloud machine public address is not available. |
803 | 72 | addresses = [ | 77 | addresses = [ |
804 | 73 | {'NetworkName': '', | 78 | {'NetworkName': '', |
808 | 74 | 'Scope': 'local-cloud', | 79 | 'Scope': 'public', |
809 | 75 | 'Type': 'hostname', | 80 | 'Type': 'ipv6', |
810 | 76 | 'Value': 'eu-west-1.example.internal'}, | 81 | 'Value': 'fe80::92b8:d0ff:fe94:8f8c'}, |
811 | 77 | {'NetworkName': '', | 82 | {'NetworkName': '', |
812 | 78 | 'Scope': 'local-cloud', | 83 | 'Scope': 'local-cloud', |
815 | 79 | 'Type': 'ipv4', | 84 | 'Type': 'ipv6', |
816 | 80 | 'Value': '10.42.47.10'}, | 85 | 'Value': 'fe80::216:3eff:fefd:787e'}, |
817 | 81 | ] | 86 | ] |
819 | 82 | self.assertIsNone(watchers.retrieve_public_adddress(addresses)) | 87 | address = watchers.retrieve_public_adddress(addresses, self.resolver) |
820 | 88 | self.assertIsNone(address) | ||
821 | 83 | 89 | ||
822 | 84 | def test_container_address_not_found(self): | 90 | def test_container_address_not_found(self): |
823 | 85 | # None is returned if an LXC public address is not available. | 91 | # None is returned if an LXC public address is not available. |
824 | @@ -89,7 +95,8 @@ | |||
825 | 89 | 'Type': 'ipv6', | 95 | 'Type': 'ipv6', |
826 | 90 | 'Value': 'fe80::216:3eff:fefd:787e', | 96 | 'Value': 'fe80::216:3eff:fefd:787e', |
827 | 91 | }] | 97 | }] |
829 | 92 | self.assertIsNone(watchers.retrieve_public_adddress(addresses)) | 98 | address = watchers.retrieve_public_adddress(addresses, self.resolver) |
830 | 99 | self.assertIsNone(address) | ||
831 | 93 | 100 | ||
832 | 94 | def test_empty_public_address(self): | 101 | def test_empty_public_address(self): |
833 | 95 | # None is returned if the public address has no value. | 102 | # None is returned if the public address has no value. |
834 | @@ -103,17 +110,20 @@ | |||
835 | 103 | 'Type': 'ipv4', | 110 | 'Type': 'ipv4', |
836 | 104 | 'Value': ''}, | 111 | 'Value': ''}, |
837 | 105 | ] | 112 | ] |
839 | 106 | self.assertIsNone(watchers.retrieve_public_adddress(addresses)) | 113 | address = watchers.retrieve_public_adddress(addresses, self.resolver) |
840 | 114 | self.assertIsNone(address) | ||
841 | 107 | 115 | ||
842 | 108 | def test_cloud_addresses(self): | 116 | def test_cloud_addresses(self): |
843 | 109 | # The public address of a cloud machine is properly returned. | 117 | # The public address of a cloud machine is properly returned. |
846 | 110 | public_address = watchers.retrieve_public_adddress(cloud_addresses) | 118 | address = watchers.retrieve_public_adddress( |
847 | 111 | self.assertEqual('eu-west-1.compute.example.com', public_address) | 119 | cloud_addresses, self.resolver) |
848 | 120 | self.assertEqual('eu-west-1.compute.example.com', address) | ||
849 | 112 | 121 | ||
850 | 113 | def test_container_addresses(self): | 122 | def test_container_addresses(self): |
851 | 114 | # The public address of an LXC instance is properly returned. | 123 | # The public address of an LXC instance is properly returned. |
854 | 115 | public_address = watchers.retrieve_public_adddress(container_addresses) | 124 | address = watchers.retrieve_public_adddress( |
855 | 116 | self.assertEqual('10.0.3.42', public_address) | 125 | container_addresses, self.resolver) |
856 | 126 | self.assertEqual('10.0.3.42', address) | ||
857 | 117 | 127 | ||
858 | 118 | def test_old_juju_version(self): | 128 | def test_old_juju_version(self): |
859 | 119 | # The public address is properly returned when using Juju < 1.20. | 129 | # The public address is properly returned when using Juju < 1.20. |
860 | @@ -129,11 +139,32 @@ | |||
861 | 129 | 'Type': 'hostname', | 139 | 'Type': 'hostname', |
862 | 130 | 'Value': 'eu-west-1.example.internal'}, | 140 | 'Value': 'eu-west-1.example.internal'}, |
863 | 131 | ] | 141 | ] |
869 | 132 | public_address = watchers.retrieve_public_adddress(addresses) | 142 | address = watchers.retrieve_public_adddress(addresses, self.resolver) |
870 | 133 | self.assertEqual('eu-west-1.compute.example.com', public_address) | 143 | self.assertEqual('eu-west-1.compute.example.com', address) |
871 | 134 | 144 | ||
872 | 135 | def test_last_unknown_address(self): | 145 | def test_local_cloud_address(self): |
873 | 136 | # If the scope of multiple addresses is unknown, the last one is taken. | 146 | # If there are no available public addresses the first ipv4 address |
874 | 147 | # with cloud-local scope is returned. | ||
875 | 148 | addresses = [ | ||
876 | 149 | {'NetworkName': '', | ||
877 | 150 | 'Scope': 'local-cloud', | ||
878 | 151 | 'Type': 'hostname', | ||
879 | 152 | 'Value': 'eu-west-1.example.internal'}, | ||
880 | 153 | {'NetworkName': '', | ||
881 | 154 | 'Scope': 'local-cloud', | ||
882 | 155 | 'Type': 'ipv4', | ||
883 | 156 | 'Value': '10.42.47.10'}, | ||
884 | 157 | {'NetworkName': '', | ||
885 | 158 | 'Scope': 'local-cloud', | ||
886 | 159 | 'Type': 'ipv4', | ||
887 | 160 | 'Value': '1.2.3.4'}, | ||
888 | 161 | ] | ||
889 | 162 | address = watchers.retrieve_public_adddress(addresses, self.resolver) | ||
890 | 163 | self.assertEqual('10.42.47.10', address) | ||
891 | 164 | |||
892 | 165 | def test_unknown_address(self): | ||
893 | 166 | # If there are no available public or local-cloud addresses, the first | ||
894 | 167 | # address with unknown scope is returned. | ||
895 | 137 | addresses = [ | 168 | addresses = [ |
896 | 138 | {'NetworkName': '', | 169 | {'NetworkName': '', |
897 | 139 | 'Scope': '', | 170 | 'Scope': '', |
898 | @@ -144,8 +175,52 @@ | |||
899 | 144 | 'Type': 'ipv4', | 175 | 'Type': 'ipv4', |
900 | 145 | 'Value': '10.0.3.47'}, | 176 | 'Value': '10.0.3.47'}, |
901 | 146 | ] | 177 | ] |
904 | 147 | public_address = watchers.retrieve_public_adddress(addresses) | 178 | address = watchers.retrieve_public_adddress(addresses, self.resolver) |
905 | 148 | self.assertEqual('10.0.3.47', public_address) | 179 | self.assertEqual('10.0.3.42', address) |
906 | 180 | |||
907 | 181 | def test_preferred_fallback_address(self): | ||
908 | 182 | # If there are no available public addresses the first fallback address | ||
909 | 183 | # in the list is returned. | ||
910 | 184 | addresses = [ | ||
911 | 185 | {'NetworkName': '', | ||
912 | 186 | 'Scope': '', | ||
913 | 187 | 'Type': 'ipv4', | ||
914 | 188 | 'Value': '10.0.3.47'}, | ||
915 | 189 | {'NetworkName': '', | ||
916 | 190 | 'Scope': 'local-cloud', | ||
917 | 191 | 'Type': 'ipv4', | ||
918 | 192 | 'Value': '10.42.47.10'}, | ||
919 | 193 | |||
920 | 194 | ] | ||
921 | 195 | address = watchers.retrieve_public_adddress(addresses, self.resolver) | ||
922 | 196 | self.assertEqual('10.0.3.47', address) | ||
923 | 197 | # Now test with a reversed order. | ||
924 | 198 | address = watchers.retrieve_public_adddress( | ||
925 | 199 | reversed(addresses), self.resolver) | ||
926 | 200 | self.assertEqual('10.42.47.10', address) | ||
927 | 201 | |||
928 | 202 | def test_unresolvable_public_address(self): | ||
929 | 203 | # None is returned if a public cloud is found but it is not resolvable. | ||
930 | 204 | addresses = [ | ||
931 | 205 | {'NetworkName': '', | ||
932 | 206 | 'Scope': 'public', | ||
933 | 207 | 'Type': 'hostname', | ||
934 | 208 | 'Value': 'eu-west-1.example.internal'}, | ||
935 | 209 | ] | ||
936 | 210 | expected_warning = ( | ||
937 | 211 | 'cannot resolve public eu-west-1.example.internal address, ' | ||
938 | 212 | 'looking for another candidate: bad wolf') | ||
939 | 213 | with helpers.assert_logs([expected_warning], level='warn'): | ||
940 | 214 | address = watchers.retrieve_public_adddress( | ||
941 | 215 | addresses, lambda hostname: 'bad wolf') | ||
942 | 216 | self.assertIsNone(address) | ||
943 | 217 | |||
944 | 218 | def test_unresolvable_public_address_fallback(self): | ||
945 | 219 | # If there are no resolvable public addresses the first ipv4 address | ||
946 | 220 | # with cloud-local scope is returned. | ||
947 | 221 | address = watchers.retrieve_public_adddress( | ||
948 | 222 | cloud_addresses, lambda hostname: 'bad wolf') | ||
949 | 223 | self.assertEqual('10.42.47.10', address) | ||
950 | 149 | 224 | ||
951 | 150 | 225 | ||
952 | 151 | class TestParseMachineChange(helpers.ValueErrorTestsMixin, unittest.TestCase): | 226 | class TestParseMachineChange(helpers.ValueErrorTestsMixin, unittest.TestCase): |
953 | @@ -205,8 +280,12 @@ | |||
954 | 205 | # A message is printed to stdout when the machine obtains a public | 280 | # A message is printed to stdout when the machine obtains a public |
955 | 206 | # address. | 281 | # address. |
956 | 207 | data = {'Addresses': cloud_addresses, 'Id': '1', 'Status': 'pending'} | 282 | data = {'Addresses': cloud_addresses, 'Id': '1', 'Status': 'pending'} |
959 | 208 | status, address = watchers.parse_machine_change( | 283 | # Patch the hostname resolver used to check hostname addresses. |
960 | 209 | 'change', data, 'pending', '') | 284 | # Note that resolve error cases are properly exercised by the |
961 | 285 | # watchers.retrieve_public_adddress tests. | ||
962 | 286 | with helpers.patch_check_resolvable(): | ||
963 | 287 | status, address = watchers.parse_machine_change( | ||
964 | 288 | 'change', data, 'pending', '') | ||
965 | 210 | self.assertEqual('pending', status) | 289 | self.assertEqual('pending', status) |
966 | 211 | self.assertEqual('eu-west-1.compute.example.com', address) | 290 | self.assertEqual('eu-west-1.compute.example.com', address) |
967 | 212 | mock_print.assert_called_once_with( | 291 | mock_print.assert_called_once_with( |
968 | @@ -248,8 +327,8 @@ | |||
969 | 248 | # A ValueError is raised if the change represents a unit removal. | 327 | # A ValueError is raised if the change represents a unit removal. |
970 | 249 | data = {'Name': 'django/42', 'Status': 'started'} | 328 | data = {'Name': 'django/42', 'Status': 'started'} |
971 | 250 | with self.assert_value_error('django/42 unexpectedly removed'): | 329 | with self.assert_value_error('django/42 unexpectedly removed'): |
974 | 251 | # The last two arguments are the current status and address. | 330 | # The last argument is the current status. |
975 | 252 | watchers.parse_unit_change('remove', data, '', '') | 331 | watchers.parse_unit_change('remove', data, '') |
976 | 253 | 332 | ||
977 | 254 | def test_unit_error(self): | 333 | def test_unit_error(self): |
978 | 255 | # A ValueError is raised if the unit is in an error state. | 334 | # A ValueError is raised if the unit is in an error state. |
979 | @@ -260,120 +339,55 @@ | |||
980 | 260 | } | 339 | } |
981 | 261 | expected_error = 'django/0 is in an error state: start error: bad wolf' | 340 | expected_error = 'django/0 is in an error state: start error: bad wolf' |
982 | 262 | with self.assert_value_error(expected_error): | 341 | with self.assert_value_error(expected_error): |
1004 | 263 | # The last two arguments are the current status and address. | 342 | # The last argument is the current status. |
1005 | 264 | watchers.parse_unit_change('change', data, '', '') | 343 | watchers.parse_unit_change('change', data, '') |
985 | 265 | |||
986 | 266 | @helpers.mock_print | ||
987 | 267 | def test_address_notified(self, mock_print): | ||
988 | 268 | # A message is printed to stdout when the unit obtains a public | ||
989 | 269 | # address. The function returns the status, the new address and the | ||
990 | 270 | # machine identifier. | ||
991 | 271 | data = { | ||
992 | 272 | 'Name': 'haproxy/2', | ||
993 | 273 | 'Status': 'pending', | ||
994 | 274 | 'PublicAddress': 'haproxy2.example.com', | ||
995 | 275 | 'MachineId': '42', | ||
996 | 276 | } | ||
997 | 277 | status, address, machine_id = watchers.parse_unit_change( | ||
998 | 278 | 'change', data, 'pending', '') | ||
999 | 279 | self.assertEqual('pending', status) | ||
1000 | 280 | self.assertEqual('haproxy2.example.com', address) | ||
1001 | 281 | self.assertEqual('42', machine_id) | ||
1002 | 282 | mock_print.assert_called_once_with( | ||
1003 | 283 | 'haproxy/2 placed on haproxy2.example.com') | ||
1006 | 284 | 344 | ||
1007 | 285 | @helpers.mock_print | 345 | @helpers.mock_print |
1008 | 286 | def test_pending_status_notified(self, mock_print): | 346 | def test_pending_status_notified(self, mock_print): |
1009 | 287 | # A message is printed to stdout when the unit changes its status to | 347 | # A message is printed to stdout when the unit changes its status to |
1017 | 288 | # "pending". The function returns the new status, the address and the | 348 | # "pending". The function returns the new status and the machine |
1018 | 289 | # machine identifier. The last two values are empty strings if the unit | 349 | # identifier, which is empty if the unit has not yet been assigned to a |
1019 | 290 | # has not yet been assigned to a machine. | 350 | # machine. |
1020 | 291 | data = {'Name': 'django/1', 'Status': 'pending', 'PublicAddress': ''} | 351 | data = {'Name': 'django/1', 'Status': 'pending'} |
1021 | 292 | # The last two arguments are the current status and address. | 352 | # The last argument is the current status. |
1022 | 293 | status, address, machine_id = watchers.parse_unit_change( | 353 | status, machine_id = watchers.parse_unit_change('change', data, '') |
1016 | 294 | 'change', data, '', '') | ||
1023 | 295 | self.assertEqual('pending', status) | 354 | self.assertEqual('pending', status) |
1024 | 296 | self.assertEqual('', address) | ||
1025 | 297 | self.assertEqual('', machine_id) | 355 | self.assertEqual('', machine_id) |
1026 | 298 | mock_print.assert_called_once_with('django/1 deployment is pending') | 356 | mock_print.assert_called_once_with('django/1 deployment is pending') |
1027 | 299 | 357 | ||
1028 | 300 | @helpers.mock_print | 358 | @helpers.mock_print |
1029 | 301 | def test_installed_status_notified(self, mock_print): | 359 | def test_installed_status_notified(self, mock_print): |
1030 | 302 | # A message is printed to stdout when the unit changes its status to | 360 | # A message is printed to stdout when the unit changes its status to |
1041 | 303 | # "installed". The function returns the new status, the address and the | 361 | # "installed". |
1042 | 304 | # machine identifier. | 362 | data = {'Name': 'django/42', 'Status': 'installed', 'MachineId': '1'} |
1043 | 305 | data = { | 363 | status, machine_id = watchers.parse_unit_change( |
1044 | 306 | 'Name': 'django/42', | 364 | 'change', data, 'pending') |
1035 | 307 | 'Status': 'installed', | ||
1036 | 308 | 'PublicAddress': 'django42.example.com', | ||
1037 | 309 | 'MachineId': '1', | ||
1038 | 310 | } | ||
1039 | 311 | status, address, machine_id = watchers.parse_unit_change( | ||
1040 | 312 | 'change', data, 'pending', 'django42.example.com') | ||
1045 | 313 | self.assertEqual('installed', status) | 365 | self.assertEqual('installed', status) |
1046 | 314 | self.assertEqual('django42.example.com', address) | ||
1047 | 315 | self.assertEqual('1', machine_id) | 366 | self.assertEqual('1', machine_id) |
1048 | 316 | mock_print.assert_called_once_with('django/42 is installed') | 367 | mock_print.assert_called_once_with('django/42 is installed') |
1049 | 317 | 368 | ||
1050 | 318 | @helpers.mock_print | 369 | @helpers.mock_print |
1051 | 319 | def test_started_status_notified(self, mock_print): | 370 | def test_started_status_notified(self, mock_print): |
1052 | 320 | # A message is printed to stdout when the unit changes its status to | 371 | # A message is printed to stdout when the unit changes its status to |
1063 | 321 | # "started". The function returns the new status, the address and the | 372 | # "started". |
1064 | 322 | # machine identifier. | 373 | data = {'Name': 'wordpress/0', 'Status': 'started', 'MachineId': '0'} |
1065 | 323 | data = { | 374 | # The last argument is the current status. |
1066 | 324 | 'Name': 'wordpress/0', | 375 | status, machine_id = watchers.parse_unit_change('change', data, '') |
1057 | 325 | 'Status': 'started', | ||
1058 | 326 | 'PublicAddress': 'wordpress0.example.com', | ||
1059 | 327 | 'MachineId': '0', | ||
1060 | 328 | } | ||
1061 | 329 | status, address, machine_id = watchers.parse_unit_change( | ||
1062 | 330 | 'change', data, '', 'wordpress0.example.com') | ||
1067 | 331 | self.assertEqual('started', status) | 376 | self.assertEqual('started', status) |
1068 | 332 | self.assertEqual('wordpress0.example.com', address) | ||
1069 | 333 | self.assertEqual('0', machine_id) | 377 | self.assertEqual('0', machine_id) |
1070 | 334 | mock_print.assert_called_once_with('wordpress/0 is ready on machine 0') | 378 | mock_print.assert_called_once_with('wordpress/0 is ready on machine 0') |
1071 | 335 | 379 | ||
1072 | 336 | @helpers.mock_print | 380 | @helpers.mock_print |
1073 | 337 | def test_both_status_and_address_notified(self, mock_print): | ||
1074 | 338 | # Both status and public address changes are notified if required. | ||
1075 | 339 | data = { | ||
1076 | 340 | 'Name': 'django/0', | ||
1077 | 341 | 'Status': 'started', | ||
1078 | 342 | 'PublicAddress': 'django42.example.com', | ||
1079 | 343 | 'MachineId': '0', | ||
1080 | 344 | } | ||
1081 | 345 | # The last two arguments are the current status and address. | ||
1082 | 346 | watchers.parse_unit_change('change', data, '', '') | ||
1083 | 347 | self.assertEqual(2, mock_print.call_count) | ||
1084 | 348 | mock_print.assert_has_calls([ | ||
1085 | 349 | mock.call('django/0 placed on django42.example.com'), | ||
1086 | 350 | mock.call('django/0 is ready on machine 0'), | ||
1087 | 351 | ]) | ||
1088 | 352 | |||
1089 | 353 | @helpers.mock_print | ||
1090 | 354 | def test_status_not_changed(self, mock_print): | 381 | def test_status_not_changed(self, mock_print): |
1091 | 355 | # If the status in the unit change and the given current status are the | 382 | # If the status in the unit change and the given current status are the |
1092 | 356 | # same value, nothing is printed and the current values are returned. | 383 | # same value, nothing is printed and the current values are returned. |
1096 | 357 | data = {'Name': 'django/1', 'Status': 'pending', 'PublicAddress': ''} | 384 | data = {'Name': 'django/1', 'Status': 'pending'} |
1097 | 358 | status, address, machine_id = watchers.parse_unit_change( | 385 | status, machine_id = watchers.parse_unit_change( |
1098 | 359 | 'change', data, 'pending', '') | 386 | 'change', data, 'pending') |
1099 | 360 | self.assertEqual('pending', status) | 387 | self.assertEqual('pending', status) |
1100 | 361 | self.assertEqual('', address) | ||
1101 | 362 | self.assertEqual('', machine_id) | 388 | self.assertEqual('', machine_id) |
1102 | 363 | self.assertFalse(mock_print.called) | 389 | self.assertFalse(mock_print.called) |
1103 | 364 | 390 | ||
1104 | 365 | @helpers.mock_print | ||
1105 | 366 | def test_address_not_available(self, mock_print): | ||
1106 | 367 | # An empty address is returned when the public address field is not | ||
1107 | 368 | # included in the change data. | ||
1108 | 369 | data = {'Name': 'haproxy/2', 'Status': 'pending', 'MachineId': '42'} | ||
1109 | 370 | status, address, machine_id = watchers.parse_unit_change( | ||
1110 | 371 | 'change', data, 'pending', '') | ||
1111 | 372 | self.assertEqual('pending', status) | ||
1112 | 373 | self.assertEqual('', address) | ||
1113 | 374 | self.assertEqual('42', machine_id) | ||
1114 | 375 | self.assertFalse(mock_print.called) | ||
1115 | 376 | |||
1116 | 377 | 391 | ||
1117 | 378 | class TestUnitMachineChanges(unittest.TestCase): | 392 | class TestUnitMachineChanges(unittest.TestCase): |
1118 | 379 | 393 | ||
1119 | 380 | 394 | ||
1120 | === modified file 'quickstart/utils.py' | |||
1121 | --- quickstart/utils.py 2014-08-22 18:32:15 +0000 | |||
1122 | +++ quickstart/utils.py 2014-11-10 12:30:00 +0000 | |||
1123 | @@ -105,6 +105,20 @@ | |||
1124 | 105 | return retcode, output.decode('utf-8'), error.decode('utf-8') | 105 | return retcode, output.decode('utf-8'), error.decode('utf-8') |
1125 | 106 | 106 | ||
1126 | 107 | 107 | ||
1127 | 108 | def check_resolvable(hostname): | ||
1128 | 109 | """Check that the hostname can be resolved to a numeric IP address. | ||
1129 | 110 | |||
1130 | 111 | Return an error message if the address cannot be resolved. | ||
1131 | 112 | """ | ||
1132 | 113 | try: | ||
1133 | 114 | address = socket.gethostbyname(hostname) | ||
1134 | 115 | except socket.error as err: | ||
1135 | 116 | return bytes(err).decode('utf-8') | ||
1136 | 117 | logging.debug('{} resolved to {}'.format( | ||
1137 | 118 | hostname, address.decode('utf-8'))) | ||
1138 | 119 | return None | ||
1139 | 120 | |||
1140 | 121 | |||
1141 | 108 | def parse_gui_charm_url(charm_url): | 122 | def parse_gui_charm_url(charm_url): |
1142 | 109 | """Parse the given charm URL. | 123 | """Parse the given charm URL. |
1143 | 110 | 124 | ||
1144 | 111 | 125 | ||
1145 | === modified file 'quickstart/watchers.py' | |||
1146 | --- quickstart/watchers.py 2014-07-04 13:09:11 +0000 | |||
1147 | +++ quickstart/watchers.py 2014-11-10 12:30:00 +0000 | |||
1148 | @@ -21,15 +21,25 @@ | |||
1149 | 21 | unicode_literals, | 21 | unicode_literals, |
1150 | 22 | ) | 22 | ) |
1151 | 23 | 23 | ||
1153 | 24 | 24 | import logging | |
1154 | 25 | |||
1155 | 26 | from quickstart import utils | ||
1156 | 27 | |||
1157 | 28 | |||
1158 | 29 | IPV4_ADDRESS = 'ipv4' | ||
1159 | 25 | IPV6_ADDRESS = 'ipv6' | 30 | IPV6_ADDRESS = 'ipv6' |
1165 | 26 | NETWORK_PUBLIC = 'public' | 31 | SCOPE_CLOUD_LOCAL = 'local-cloud' |
1166 | 27 | NETWORK_UNKNOWN = '' | 32 | SCOPE_PUBLIC = 'public' |
1167 | 28 | 33 | SCOPE_UNKNOWN = '' | |
1168 | 29 | 34 | ||
1169 | 30 | def retrieve_public_adddress(addresses): | 35 | |
1170 | 36 | def retrieve_public_adddress(addresses, hostname_resolver): | ||
1171 | 31 | """Parse the given addresses and return a public address if available. | 37 | """Parse the given addresses and return a public address if available. |
1172 | 32 | 38 | ||
1173 | 39 | Use the given hostname_resolver callable to ensure a candidate address can | ||
1174 | 40 | be resolved. The given function must accept an hostname/ip address and | ||
1175 | 41 | return None if the address is resolvable, or an error string otherwise. | ||
1176 | 42 | |||
1177 | 33 | The addresses argument is a list of address dictionaries. | 43 | The addresses argument is a list of address dictionaries. |
1178 | 34 | Cloud addresses look like the following: | 44 | Cloud addresses look like the following: |
1179 | 35 | 45 | ||
1180 | @@ -73,8 +83,8 @@ | |||
1181 | 73 | found, this function returns None. | 83 | found, this function returns None. |
1182 | 74 | """ | 84 | """ |
1183 | 75 | # This implementation reflects how the public address is retrieved in Juju: | 85 | # This implementation reflects how the public address is retrieved in Juju: |
1186 | 76 | # see juju-core/instance/address.go:SelectPublicAddress. | 86 | # see juju-core/network/address.go:SelectPublicAddress. |
1187 | 77 | public_address = None | 87 | fallback_address = None |
1188 | 78 | for address in addresses: | 88 | for address in addresses: |
1189 | 79 | value = address['Value'] | 89 | value = address['Value'] |
1190 | 80 | # Exclude empty values and ipv6 addresses. | 90 | # Exclude empty values and ipv6 addresses. |
1191 | @@ -82,14 +92,33 @@ | |||
1192 | 82 | # The address scope was called "NetworkScope" prior to juju 1.20.0. | 92 | # The address scope was called "NetworkScope" prior to juju 1.20.0. |
1193 | 83 | scope = address.get('Scope', address.get('NetworkScope')) | 93 | scope = address.get('Scope', address.get('NetworkScope')) |
1194 | 84 | # If the scope is public then we have found the address. | 94 | # If the scope is public then we have found the address. |
1203 | 85 | if scope == NETWORK_PUBLIC: | 95 | if scope == SCOPE_PUBLIC: |
1204 | 86 | return value | 96 | error = hostname_resolver(value) |
1205 | 87 | # If the scope is unknown then store the value. This way the last | 97 | if error is None: |
1206 | 88 | # address with unknown scope will be returned, and we are able to | 98 | return value |
1207 | 89 | # return the right LXC address. | 99 | # If the address is not resolvable, fall back to the next |
1208 | 90 | if scope == NETWORK_UNKNOWN: | 100 | # candidate, whether it is another public address, a |
1209 | 91 | public_address = value | 101 | # local-cloud one or an unknown one. The local-cloud case, for |
1210 | 92 | return public_address | 102 | # instance, would happen when using the maas provider and the |
1211 | 103 | # maas DNS server is not configured locally. | ||
1212 | 104 | logging.warn( | ||
1213 | 105 | 'cannot resolve public {} address, looking for another ' | ||
1214 | 106 | 'candidate: {}'.format(value, error)) | ||
1215 | 107 | if fallback_address is not None: | ||
1216 | 108 | # If we already have a fallback address then we can jump to the | ||
1217 | 109 | # next candidate. | ||
1218 | 110 | continue | ||
1219 | 111 | address_type = address.get('Type') | ||
1220 | 112 | if scope == SCOPE_CLOUD_LOCAL and address_type == IPV4_ADDRESS: | ||
1221 | 113 | # Use the local cloud scoped address as fallback value: the | ||
1222 | 114 | # current environment is probably maas and the public address | ||
1223 | 115 | # of the instance is not resolvable by local DNS. | ||
1224 | 116 | fallback_address = value | ||
1225 | 117 | elif scope == SCOPE_UNKNOWN: | ||
1226 | 118 | # Use the unknown scoped address as fallback value: the current | ||
1227 | 119 | # environment is probably local. | ||
1228 | 120 | fallback_address = value | ||
1229 | 121 | return fallback_address | ||
1230 | 93 | 122 | ||
1231 | 94 | 123 | ||
1232 | 95 | def parse_machine_change(action, data, current_status, address): | 124 | def parse_machine_change(action, data, current_status, address): |
1233 | @@ -120,7 +149,8 @@ | |||
1234 | 120 | # of units hosted by a specific machine. | 149 | # of units hosted by a specific machine. |
1235 | 121 | if not address: | 150 | if not address: |
1236 | 122 | addresses = data.get('Addresses', []) | 151 | addresses = data.get('Addresses', []) |
1238 | 123 | public_address = retrieve_public_adddress(addresses) | 152 | public_address = retrieve_public_adddress( |
1239 | 153 | addresses, utils.check_resolvable) | ||
1240 | 124 | if public_address is not None: | 154 | if public_address is not None: |
1241 | 125 | address = public_address | 155 | address = public_address |
1242 | 126 | print('unit placed on {}'.format(address)) | 156 | print('unit placed on {}'.format(address)) |
1243 | @@ -134,16 +164,16 @@ | |||
1244 | 134 | return status, address | 164 | return status, address |
1245 | 135 | 165 | ||
1246 | 136 | 166 | ||
1248 | 137 | def parse_unit_change(action, data, current_status, address): | 167 | def parse_unit_change(action, data, current_status): |
1249 | 138 | """Parse the given unit change. | 168 | """Parse the given unit change. |
1250 | 139 | 169 | ||
1251 | 140 | The change is represented by the given action/data pair. | 170 | The change is represented by the given action/data pair. |
1254 | 141 | Also receive the last known unit status and address, which can be empty | 171 | Also receive the last known unit status which can be empty if the unit |
1255 | 142 | strings if those pieces of information are unknown. | 172 | status is not yet known. |
1256 | 143 | 173 | ||
1257 | 144 | Output a human readable message each time a relevant change is found. | 174 | Output a human readable message each time a relevant change is found. |
1258 | 145 | 175 | ||
1260 | 146 | Return the unit status, address and machine identifier. | 176 | Return the unit status and the identifier of the machine hosting the unit. |
1261 | 147 | Raise a ValueError if the service unit is removed or in an error state. | 177 | Raise a ValueError if the service unit is removed or in an error state. |
1262 | 148 | """ | 178 | """ |
1263 | 149 | unit_name = data['Name'] | 179 | unit_name = data['Name'] |
1264 | @@ -157,15 +187,6 @@ | |||
1265 | 157 | msg = '{} is in an error state: {}: {}'.format( | 187 | msg = '{} is in an error state: {}: {}'.format( |
1266 | 158 | unit_name, status, data['StatusInfo']) | 188 | unit_name, status, data['StatusInfo']) |
1267 | 159 | raise ValueError(msg.encode('utf-8')) | 189 | raise ValueError(msg.encode('utf-8')) |
1268 | 160 | # Notify when the unit becomes reachable. Up to juju-core 1.18, the | ||
1269 | 161 | # mega-watcher for units includes the public address for each unit. This | ||
1270 | 162 | # info is likely to be deprecated in favor of addresses as included in the | ||
1271 | 163 | # mega-watcher for machines, but we still try to retrieve the address here | ||
1272 | 164 | # for backward compatibility. | ||
1273 | 165 | if not address: | ||
1274 | 166 | address = data.get('PublicAddress', '') | ||
1275 | 167 | if address: | ||
1276 | 168 | print('{} placed on {}'.format(unit_name, address)) | ||
1277 | 169 | # Notify status changes. | 190 | # Notify status changes. |
1278 | 170 | if status != current_status: | 191 | if status != current_status: |
1279 | 171 | if status == 'pending': | 192 | if status == 'pending': |
1280 | @@ -175,7 +196,7 @@ | |||
1281 | 175 | elif status == 'started': | 196 | elif status == 'started': |
1282 | 176 | print('{} is ready on machine {}'.format( | 197 | print('{} is ready on machine {}'.format( |
1283 | 177 | unit_name, data['MachineId'])) | 198 | unit_name, data['MachineId'])) |
1285 | 178 | return status, address, data.get('MachineId', '') | 199 | return status, data.get('MachineId', '') |
1286 | 179 | 200 | ||
1287 | 180 | 201 | ||
1288 | 181 | def unit_machine_changes(changeset): | 202 | def unit_machine_changes(changeset): |
Reviewers: mp+241263_ code.launchpad. net,
Message:
Please take a look.
Description:
Unit address from the machines watcher only
Only use the mega-watcher for machines to retrieve
the Juju GUI unit address.
This change has several consequences:
- it allows us to apply some logic on how the
right address is chosen. For instance, now
we try to resolve public hostnames before
proceeding, and this should fix the cases
where a cloud dns is not configured on the
machine running quickstart. This is the case
of many maas environments;
- it simplifies parsing the mega-watcher changes;
- more importantly, it breaks compatibility
with very old versions of juju (<1.18), in which
the mega-watcher for machines did not include
machine addresses.
For this reason, quickstart now explicitly
drops support for juju < 1.18.1
(1.18.1 is the version on trusty universe).
This also allows for removing some version
checks in the code, including sudo handling when
calling bootstrap on local envs, several special
cases on the watcher side, and other oddities.
For the reasons above, I bumped the quickstart
version up to 1.5.0.
PS: my apologies for the long diff, hope the code
is still easy to follow. Sorry.
Tests: `make check`
QA:
run quickstart as usual, on local and cloud envs,
check it works properly when run again, etc.
this branch has been already successfully QAed in
a maas environment by Adam (Landscape team).
https:/ /code.launchpad .net/~frankban/ juju-quickstart /maas-address/ +merge/ 241263
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/174790043/
Affected files (+334, -433 lines): __init_ _.py manage. py settings. py tests/helpers. py tests/test_ app.py tests/test_ manage. py tests/test_ utils.py tests/test_ watchers. py watchers. py
M README.rst
A [revision details]
M quickstart/
M quickstart/app.py
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/utils.py
M quickstart/