Merge lp:~frankban/juju-quickstart/idempotent-feature into lp:juju-quickstart
- idempotent-feature
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 15 |
Proposed branch: | lp:~frankban/juju-quickstart/idempotent-feature |
Merge into: | lp:juju-quickstart |
Diff against target: |
1203 lines (+639/-182) 10 files modified
quickstart/__init__.py (+1/-1) quickstart/app.py (+110/-32) quickstart/juju.py (+28/-23) quickstart/manage.py (+15/-6) quickstart/tests/helpers.py (+30/-0) quickstart/tests/test_app.py (+234/-62) quickstart/tests/test_juju.py (+119/-55) quickstart/tests/test_manage.py (+4/-3) quickstart/tests/test_utils.py (+73/-0) quickstart/utils.py (+25/-0) |
To merge this branch: | bzr merge lp:~frankban/juju-quickstart/idempotent-feature |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email:
|
Commit message
Description of the change
Make quickstart idempotent.
- do not bootstrap an environment if already bootstrapped;
- do not deploy the GUI if already there.
Sorry, the diff is long, but there are a lot of tests.
Tests: make check
QA:
- .venv/bin/python juju-quickstart -e ec2
and ensure the GUI is correctly deployed;
- .venv/bin/python juju-quickstart -e ec2
again, to check it recognizes that env is
already bootstrapped and the
GUI unit is already there;
In the following steps, sometimes the browser
can get confused about certs, wss conections
etc. If the GUI is not loading correctly,
try harder, use incognito mode, change the
browser.
- juju destroy-service -e ec2 juju-gui;
- .venv/bin/python juju-quickstart -e ec2
again, to check the service and the unit
are correctly re-deployed;
Incidentally the step above, in the case it
succeeds, also demonstrates that the GUI can
safely be redeployed in the same machine: I
wasn't sure about this and this means we are
cleaning up things correctly in our charm, yay!
- juju unexpose -e ec2 juju-gui;
- .venv/bin/python juju-quickstart -e ec2
again, to check that the service is
properly re-exposed;
- juju destroy-unit -e ec2 juju-gui/0;
- .venv/bin/python juju-quickstart -e ec2
again, to check that the unit is
re-added on the existing service
(this time it should be named juju-gui/1);
- juju destroy-service -e ec2 juju-gui;
- juju deploy -e ec2 juju-gui (if juju exits with a
"service already exists" error, retry after a while);
- .venv/bin/python juju-quickstart -e ec2 \
bundle:
The last command, executed right after juju-deploy should
also demonstrates that incidentally quickstart
can also be used to watch an already running
deployment, and that a bundle can still be deployed;
Final check:
- .venv/bin/python juju-quickstart -e ec2;
just to ensure quickstart is not surprised
that the unit is not in the bootstrap node
(i.e. you should see "juju-gui/0 is ready on machine 1").
Thanks a lot for testing all of this.
I added a card to automate the QA above with
a collection of functional tests.
Remember to destroy your ec2 environment.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Francesco Banconi (frankban) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Gary Poster (gary) wrote : | # |
LGTM with a few trivials and a small. QA good. Very nice
functionality. Thank you!
https:/
File quickstart/app.py (right):
https:/
quickstart/
weak. We are relying on
Could you file a Juju Core bug, add juju-quickstart as an affected
project, and add the bug number to this comment, please?
It looks like Wm prefers --format to get machine readable output, so
maybe that's a way forward?
https:/
quickstart/
the obtained one, we
typo: ...rather than...
https:/
quickstart/
It took me a few seconds of thinking to realize why you didn't just
return True here. Low priority, but might be nice to say something
explanatory. Example:
# Juju is bootstapped, but we don't know if it is ready yet. Fall
# through to the next block for that check.
https:/
quickstart/
error response.
After reading docstring, I am not clear on three things.
(1) why is the provider type important here?
(2) why is the already_
(3) what happens if the GUI charm exists with a different name? what
happens if the given name is not a GUI charm?
Maybe we don't need to answer all of these in the docstring, but seemed
like a hole as I read it.
The code below answers #1 and #2; I have a comment below about #3.
https:/
quickstart/
Ah! It is an optimization.
I thought about suggesting some different names for the argument,
focused on the local behavior of enabling checking for services and
units rather than the contextual idea that we are already bootstrapped.
E.g., s/already_
the local function perspective, you could even argue that you should
only control disabling the behavior, since this is an
optimization--e.g., s/already_
prevent_
In the end, I'm not suggesting this because I don't like any of the
other concrete options I considered. At best, this would have been
bikeshedding, anyway.
https:/
quickstart/
Maybe we should warn the user if the actual charm_url != the given
charm_url, and warn louder if the charm id doesn't match
/\/juju-gui-\d+$/ ?
https:/
quickstart/
provider.
Ah-ha!
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Gary Poster (gary) wrote : | # |
One QA note.
The end of your instructions said this:
-------
Final check:
- .venv/bin/python juju-quickstart -e ec2;
just to ensure quickstart is not surprised
that the unit is not in the bootstrap node
(i.e. you should see "juju-gui/0 is ready on machine 1").
-------
I saw it on machine 0 (ec2), which I expected given the immediately
previous instruction to destroy the service. If this is in fact a
surprise and broken...I reported it. :-)
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Richard Harding (rharding) wrote : | # |
LGTM and qa-ok.
I'm with Gary on the is_bootstrapped being a bit confusing sometimes in
the inner workings.
I also get a bit confused with the 'charm_url' as I forget that in this
context that's ALWAYS the Gui charm. As features grow and things like
deploying bundles come into the picture, I wonder if a
s/charm_
https:/
File quickstart/app.py (right):
https:/
quickstart/
detail. Definitely we
jenv
- 23. By Francesco Banconi
-
Changes as per review.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Francesco Banconi (frankban) wrote : | # |
*** Submitted:
Make quickstart idempotent.
- do not bootstrap an environment if already bootstrapped;
- do not deploy the GUI if already there.
Sorry, the diff is long, but there are a lot of tests.
Tests: make check
QA:
- .venv/bin/python juju-quickstart -e ec2
and ensure the GUI is correctly deployed;
- .venv/bin/python juju-quickstart -e ec2
again, to check it recognizes that env is
already bootstrapped and the
GUI unit is already there;
In the following steps, sometimes the browser
can get confused about certs, wss conections
etc. If the GUI is not loading correctly,
try harder, use incognito mode, change the
browser.
- juju destroy-service -e ec2 juju-gui;
- .venv/bin/python juju-quickstart -e ec2
again, to check the service and the unit
are correctly re-deployed;
Incidentally the step above, in the case it
succeeds, also demonstrates that the GUI can
safely be redeployed in the same machine: I
wasn't sure about this and this means we are
cleaning up things correctly in our charm, yay!
- juju unexpose -e ec2 juju-gui;
- .venv/bin/python juju-quickstart -e ec2
again, to check that the service is
properly re-exposed;
- juju destroy-unit -e ec2 juju-gui/0;
- .venv/bin/python juju-quickstart -e ec2
again, to check that the unit is
re-added on the existing service
(this time it should be named juju-gui/1);
- juju destroy-service -e ec2 juju-gui;
- juju deploy -e ec2 juju-gui (if juju exits with a
"service already exists" error, retry after a while);
- .venv/bin/python juju-quickstart -e ec2 \
bundle:
The last command, executed right after juju-deploy should
also demonstrates that incidentally quickstart
can also be used to watch an already running
deployment, and that a bundle can still be deployed;
Final check:
- .venv/bin/python juju-quickstart -e ec2;
just to ensure quickstart is not surprised
that the unit is not in the bootstrap node
(i.e. you should see "juju-gui/0 is ready on machine 1").
Thanks a lot for testing all of this.
I added a card to automate the QA above with
a collection of functional tests.
Remember to destroy your ec2 environment.
R=gary.poster, rharding
CC=
https:/
https:/
File quickstart/app.py (right):
https:/
quickstart/
weak. We are relying on
On 2013/11/18 15:02:19, gary.poster wrote:
> Could you file a Juju Core bug, add juju-quickstart as an affected
project, and
> add the bug number to this comment, please?
> It looks like Wm prefers --format to get machine readable output, so
maybe
> that's a way forward?
Done. Filed bug 1252322.
https:/
quickstart/
detail. Definitely we
On 2013/11/18 15:42:56, rharding wrote:
> jenv
Done.
https:/
quickstart/
the obtai...
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Francesco Banconi (frankban) wrote : | # |
Thank you both for the great reviews!
Preview Diff
1 | === modified file 'quickstart/__init__.py' |
2 | --- quickstart/__init__.py 2013-11-06 10:17:08 +0000 |
3 | +++ quickstart/__init__.py 2013-11-18 16:54:22 +0000 |
4 | @@ -19,7 +19,7 @@ |
5 | that it can be managed using a Web interface (the Juju GUI). |
6 | """ |
7 | |
8 | -VERSION = (0, 3, 0) |
9 | +VERSION = (0, 4, 0) |
10 | |
11 | |
12 | def get_version(): |
13 | |
14 | === modified file 'quickstart/app.py' |
15 | --- quickstart/app.py 2013-11-13 12:23:36 +0000 |
16 | +++ quickstart/app.py 2013-11-18 16:54:22 +0000 |
17 | @@ -47,17 +47,41 @@ |
18 | def bootstrap(env_name, debug=False): |
19 | """Bootstrap the Juju environment with the given name. |
20 | |
21 | - If debug is True, bootstrap the environment passing the --debug flag. |
22 | - |
23 | - Return when the bootstrap node is ready. |
24 | + Do not bootstrap the environment if already bootstrapped. |
25 | + |
26 | + Return True without errors if the environment is already bootstrapped. |
27 | + Return False otherwise. Only return when the bootstrap node is ready. |
28 | + |
29 | + If debug is True and the environment not bootstrapped, execute the |
30 | + bootstrap command passing the --debug flag. |
31 | + |
32 | Raise a ProgramExit if any error occurs in the bootstrap process. |
33 | """ |
34 | + already_bootstrapped = False |
35 | cmd = ['juju', 'bootstrap', '-e', env_name] |
36 | if debug: |
37 | cmd.append('--debug') |
38 | retcode, _, error = utils.call(*cmd) |
39 | if retcode: |
40 | - raise ProgramExit(error) |
41 | + # XXX frankban 2013-11-13 bug 1252322: the check below is weak. We are |
42 | + # relying on an error message in order to decide if the environment is |
43 | + # already bootstrapped. Other possibilities include checking if the |
44 | + # jenv file is present (in ~/.juju/environments/) and, if so, check the |
45 | + # juju status. Unfortunately this is also prone to errors, because a |
46 | + # jenv file can be there but the environment not really bootstrapped or |
47 | + # functional (e.g. sync-tools was used, or a previous bootstrap failed, |
48 | + # or the user terminated machines from the ec2 panel, etc.). Moreover |
49 | + # jenv files seems to be an internal juju-core detail. Definitely we |
50 | + # need to find a better way, but for now the "asking forgiveness" |
51 | + # approach feels like the best compromise we have. Also note that, |
52 | + # rather than comparing the expected error with the obtained one, we |
53 | + # search in the error in order to support bootstrap --debug. |
54 | + if 'environment is already bootstrapped' not in error: |
55 | + # We exit if the error is not "already bootstrapped". |
56 | + raise ProgramExit(error) |
57 | + # Juju is bootstrapped, but we don't know if it is ready yet. Fall |
58 | + # through to the next block for that check. |
59 | + already_bootstrapped = True |
60 | # Call "juju status" multiple times until the bootstrap node is ready. |
61 | for _ in range(5): |
62 | retcode, output, error = utils.call( |
63 | @@ -70,7 +94,7 @@ |
64 | except ValueError: |
65 | continue |
66 | if agent_state == 'started': |
67 | - return |
68 | + return already_bootstrapped |
69 | details = ''.join(filter(None, [output, error])).strip() |
70 | raise ProgramExit('the state server is not ready:\n{}'.format(details)) |
71 | |
72 | @@ -112,34 +136,88 @@ |
73 | return env |
74 | |
75 | |
76 | -def deploy_gui(env, service_name, charm_url=None): |
77 | +def deploy_gui( |
78 | + env, service_name, machine, charm_url=None, check_preexisting=False): |
79 | """Deploy and expose the given service, reusing the bootstrap node. |
80 | |
81 | - Receive an authenticated Juju Environment instance, the name of the service |
82 | - and the optional Juju GUI charm URL, e.g. cs:~juju-gui/precise/juju-gui-42. |
83 | - If the charm URL is not provided, the function tries to retrieve it from |
84 | - charmworld. In this case a default charm URL is used if charmworld is not |
85 | - available. |
86 | - |
87 | + Only deploy the service if not already present in the environment. |
88 | + Do not add a unit to the service if the unit is already there. |
89 | + |
90 | + Receive an authenticated Juju Environment instance, the name of the |
91 | + service, the machine where to deploy to (or None for a new machine), |
92 | + the optional Juju GUI charm URL (e.g. cs:~juju-gui/precise/juju-gui-42), |
93 | + and a flag (check_preexisting) that can be set to True in order to make |
94 | + the function check for a pre-existing service and/or unit. |
95 | + |
96 | + If the charm URL is not provided, and the service is not already deployed, |
97 | + the function tries to retrieve it from charmworld. In this case a default |
98 | + charm URL is used if charmworld is not available. |
99 | + |
100 | + Return the name of the first running unit belonging to the given service. |
101 | Raise a ProgramExit if the API server returns an error response. |
102 | """ |
103 | - if charm_url is None: |
104 | - try: |
105 | - charm_url = utils.get_charm_url() |
106 | - except (IOError, ValueError) as err: |
107 | - msg = 'unable to retrieve the Juju GUI charm URL from the API: {}' |
108 | - logging.warn(msg.format(err)) |
109 | - charm_url = settings.DEFAULT_CHARM_URL |
110 | - print('charm URL: {}'.format(charm_url)) |
111 | - try: |
112 | - env.deploy(service_name, charm_url, to=0) |
113 | - env.expose(service_name) |
114 | - except jujuclient.EnvError as err: |
115 | - raise ProgramExit('bad API server response: {}'.format(err.message)) |
116 | - |
117 | - |
118 | -def watch(env, service_name): |
119 | - """Start watching the first unit belonging to the given service. |
120 | + service_data, unit_data = None, None |
121 | + if check_preexisting: |
122 | + # The service and/or the unit can be already in the environment. |
123 | + try: |
124 | + status = env.get_status() |
125 | + except jujuclient.EnvError as err: |
126 | + raise ProgramExit('bad API response: {}'.format(err.message)) |
127 | + service_data, unit_data = utils.get_service_info(status, service_name) |
128 | + if service_data is None: |
129 | + # The service does not exist in the environment. |
130 | + print('requesting {} deployment'.format(service_name)) |
131 | + if charm_url is None: |
132 | + try: |
133 | + charm_url = utils.get_charm_url() |
134 | + except (IOError, ValueError) as err: |
135 | + msg = 'unable to retrieve the {} charm URL from the API: {}' |
136 | + logging.warn(msg.format(service_name, err)) |
137 | + charm_url = settings.DEFAULT_CHARM_URL |
138 | + print('charm URL: {}'.format(charm_url)) |
139 | + # Deploy the service without units. |
140 | + try: |
141 | + env.deploy(service_name, charm_url, num_units=0) |
142 | + except jujuclient.EnvError as err: |
143 | + raise ProgramExit('bad API response: {}'.format(err.message)) |
144 | + print('{} deployment request accepted'.format(service_name)) |
145 | + service_exposed = False |
146 | + else: |
147 | + # We already have the service in the environment. |
148 | + print('service {} already deployed'.format(service_name)) |
149 | + charm_url = service_data['CharmURL'] |
150 | + print('charm URL: {}'.format(charm_url)) |
151 | + # XXX frankban: warn the user if the actual charm_url != the given |
152 | + # charm_url, and warn louder if the charm URL does not match |
153 | + # /\/juju-gui-\d+$/. |
154 | + service_exposed = service_data.get('Exposed', False) |
155 | + # At this point the service is surely deployed in the environment: expose |
156 | + # it if necessary and add a unit if it is missing. |
157 | + if not service_exposed: |
158 | + print('exposing service {}'.format(service_name)) |
159 | + try: |
160 | + env.expose(service_name) |
161 | + except jujuclient.EnvError as err: |
162 | + raise ProgramExit('bad API response: {}'.format(err.message)) |
163 | + if unit_data is None: |
164 | + # Add a unit to the service. |
165 | + print('requesting new unit deployment') |
166 | + try: |
167 | + response = env.add_unit(service_name, machine_spec=machine) |
168 | + except jujuclient.EnvError as err: |
169 | + raise ProgramExit('bad API response: {}'.format(err.message)) |
170 | + unit_name = response['Units'][0] |
171 | + print('{} deployment request accepted'.format(unit_name)) |
172 | + else: |
173 | + # A service unit is already present in the environment. Go ahead |
174 | + # and try to reuse that unit. |
175 | + unit_name = unit_data['Name'] |
176 | + print('reusing unit {}'.format(unit_name)) |
177 | + return unit_name |
178 | + |
179 | + |
180 | +def watch(env, unit_name): |
181 | + """Start watching the given unit. |
182 | |
183 | Output a human readable messages from each time a relevant change is found. |
184 | |
185 | @@ -153,7 +231,6 @@ |
186 | Raise a ProgramExit if the API server returns an error response, or if |
187 | the service unit is removed or in error. |
188 | """ |
189 | - unit_name = '{}/0'.format(service_name) |
190 | processor = functools.partial(utils.unit_changes, unit_name) |
191 | watcher = env.watch_changes(processor) |
192 | address = current_status = '' |
193 | @@ -176,7 +253,7 @@ |
194 | if not address: |
195 | address = data['PublicAddress'] |
196 | if address: |
197 | - print('installing {} on {}'.format(unit_name, address)) |
198 | + print('setting up {} on {}'.format(unit_name, address)) |
199 | # Notify status changes. |
200 | if status != current_status: |
201 | if status == 'pending': |
202 | @@ -185,7 +262,8 @@ |
203 | print('{} is installed'.format(unit_name)) |
204 | elif address and status == 'started': |
205 | # We can exit this loop. |
206 | - print('{} is ready'.format(unit_name)) |
207 | + print('{} is ready on machine {}'.format( |
208 | + unit_name, data['MachineId'])) |
209 | return address |
210 | current_status = status |
211 | |
212 | |
213 | === modified file 'quickstart/juju.py' |
214 | --- quickstart/juju.py 2013-10-30 11:44:35 +0000 |
215 | +++ quickstart/juju.py 2013-11-18 16:54:22 +0000 |
216 | @@ -41,35 +41,25 @@ |
217 | deployments to specific machines. |
218 | """ |
219 | |
220 | - def deploy( |
221 | - self, service_name, charm_url, num_units=1, config=None, |
222 | - constraints=None, to=None): |
223 | - """Deploy a charm. Local charms are not supported. |
224 | + def add_unit(self, service_name, machine_spec=None): |
225 | + """Add a unit to the given service and machine. |
226 | |
227 | - This method is overridden to add the ability to deploy to a specific |
228 | - machine (i.e. support the ToMachineSpec API parameter). |
229 | + This method is present in newer versions of python-jujuclient and can |
230 | + be safely deleted when upgrading to the new releases. |
231 | """ |
232 | - service_config = {} |
233 | - if config is not None: |
234 | - service_config = self._prepare_strparams(config) |
235 | - service_constraints = {} |
236 | - if constraints is not None: |
237 | - service_constraints = self._prepare_constraints(constraints) |
238 | + # XXX frankban 2013-11-15: remove this method when upgrading to |
239 | + # python-jujuclient >= 0.15. |
240 | params = { |
241 | 'ServiceName': service_name, |
242 | - 'CharmURL': charm_url, |
243 | - 'NumUnits': num_units, |
244 | - 'Config': service_config, |
245 | - 'Constraints': service_constraints, |
246 | + 'NumUnits': 1, |
247 | } |
248 | - if to is not None: |
249 | - params['ToMachineSpec'] = str(to) |
250 | - request = { |
251 | + if machine_spec is not None: |
252 | + params['ToMachineSpec'] = machine_spec |
253 | + return self._rpc({ |
254 | 'Type': 'Client', |
255 | - 'Request': 'ServiceDeploy', |
256 | + 'Request': 'AddServiceUnits', |
257 | 'Params': params, |
258 | - } |
259 | - return self._rpc(request) |
260 | + }) |
261 | |
262 | def deploy_bundle(self, yaml, name=None): |
263 | """Deploy a bundle.""" |
264 | @@ -83,13 +73,28 @@ |
265 | } |
266 | return self._rpc(request) |
267 | |
268 | + def get_status(self): |
269 | + """Return the current status of the environment. |
270 | + |
271 | + The status is represented by a single mega-watcher changeset. |
272 | + Each change in the changeset is a tuple (entity, action, data) where: |
273 | + - entity is a string representing the changed content type |
274 | + (e.g. "service" or "unit"); |
275 | + - action is a string representing the event which generated the |
276 | + change (i.e. "change" or "remove"); |
277 | + - data is a dict containing information about the releated entity. |
278 | + """ |
279 | + with self.get_watch() as watcher: |
280 | + changeset = watcher.next() |
281 | + return changeset |
282 | + |
283 | def watch_changes(self, processor): |
284 | """Start watching the changes occurring in the Juju environment. |
285 | |
286 | For each changeset, call the given processor callable, and yield |
287 | the values returned by the processor. |
288 | """ |
289 | - with jujuclient.Watcher(self.conn) as watcher: |
290 | + with self.get_watch() as watcher: |
291 | # The watcher closes when the context manager exit hook is called. |
292 | for changeset in watcher: |
293 | changes = processor(changeset) |
294 | |
295 | === modified file 'quickstart/manage.py' |
296 | --- quickstart/manage.py 2013-11-13 12:23:36 +0000 |
297 | +++ quickstart/manage.py 2013-11-18 16:54:22 +0000 |
298 | @@ -187,7 +187,8 @@ |
299 | help='The Juju GUI charm URL to deploy in the environment. If not ' |
300 | 'provided, the last release of the GUI will be deployed. The ' |
301 | 'charm URL must include the charm version, e.g. ' |
302 | - 'cs:~juju-gui/precise/juju-gui-116') |
303 | + 'cs:~juju-gui/precise/juju-gui-116. This option is ignored if ' |
304 | + 'Juju GUI is already present in the environment') |
305 | parser.add_argument( |
306 | '--no-browser', action='store_false', dest='open_browser', |
307 | help='Avoid opening the browser to the GUI at the end of the process') |
308 | @@ -218,15 +219,23 @@ |
309 | print('juju quickstart v{}'.format(version)) |
310 | print('bootstrapping the {} environment (type: {})'.format( |
311 | options.env_name, options.env_type)) |
312 | - app.bootstrap(options.env_name, debug=options.debug) |
313 | + already_bootstrapped = app.bootstrap(options.env_name, debug=options.debug) |
314 | + if already_bootstrapped: |
315 | + print('reusing the already bootstrapped {} environment'.format( |
316 | + options.env_name)) |
317 | + |
318 | print('retrieving the Juju API address') |
319 | api_url = app.get_api_url(options.env_name) |
320 | print('connecting to {}'.format(api_url)) |
321 | env = app.connect(api_url, options.admin_secret) |
322 | - print('requesting Juju GUI deployment') |
323 | - app.deploy_gui(env, settings.JUJU_GUI_NAME, charm_url=options.charm_url) |
324 | - print('Juju GUI deployment request accepted') |
325 | - address = app.watch(env, settings.JUJU_GUI_NAME) |
326 | + |
327 | + # It is not possible to deploy on the bootstrap node if we are using the |
328 | + # local provider. |
329 | + machine = None if options.env_type == 'local' else '0' |
330 | + unit_name = app.deploy_gui( |
331 | + env, settings.JUJU_GUI_NAME, machine, charm_url=options.charm_url, |
332 | + check_preexisting=already_bootstrapped) |
333 | + address = app.watch(env, unit_name) |
334 | url = 'https://{}'.format(address) |
335 | print('url: {}\npassword: {}'.format(url, options.admin_secret)) |
336 | |
337 | |
338 | === modified file 'quickstart/tests/helpers.py' |
339 | --- quickstart/tests/helpers.py 2013-11-06 14:55:30 +0000 |
340 | +++ quickstart/tests/helpers.py 2013-11-18 16:54:22 +0000 |
341 | @@ -150,3 +150,33 @@ |
342 | with self.assertRaises(ValueError) as context_manager: |
343 | yield |
344 | self.assertEqual(error, str(context_manager.exception)) |
345 | + |
346 | + |
347 | +class WatcherDataTestsMixin(object): |
348 | + """Shared methods for testing Juju mega-watcher data.""" |
349 | + |
350 | + def _make_change(self, entity, action, default_data, data): |
351 | + if data is not None: |
352 | + default_data.update(data) |
353 | + return entity, action, default_data |
354 | + |
355 | + def make_service_change(self, action='change', data=None): |
356 | + """Create and return a change on a service. |
357 | + |
358 | + The passed data can be used to override default values. |
359 | + """ |
360 | + default_data = { |
361 | + 'CharmURL': 'cs:precise/juju-gui-47', |
362 | + 'Exposed': True, |
363 | + 'Life': 'alive', |
364 | + 'Name': 'my-gui', |
365 | + } |
366 | + return self._make_change('service', action, default_data, data) |
367 | + |
368 | + def make_unit_change(self, action='change', data=None): |
369 | + """Create and return a change on a unit. |
370 | + |
371 | + The passed data can be used to override default values. |
372 | + """ |
373 | + default_data = {'Name': 'my-gui/47', 'Service': 'my-gui'} |
374 | + return self._make_change('unit', action, default_data, data) |
375 | |
376 | === modified file 'quickstart/tests/test_app.py' |
377 | --- quickstart/tests/test_app.py 2013-11-13 12:23:36 +0000 |
378 | +++ quickstart/tests/test_app.py 2013-11-18 16:54:22 +0000 |
379 | @@ -99,7 +99,8 @@ |
380 | def test_success(self): |
381 | # The environment is successfully bootstrapped. |
382 | with self.patch_multiple_calls(self.make_side_effects()) as mock_call: |
383 | - app.bootstrap(self.env_name) |
384 | + already_bootstrapped = app.bootstrap(self.env_name) |
385 | + self.assertFalse(already_bootstrapped) |
386 | mock_call.assert_has_calls([ |
387 | mock.call('juju', 'bootstrap', '-e', self.env_name), |
388 | ] + self.make_status_calls(1)) |
389 | @@ -107,11 +108,26 @@ |
390 | def test_success_debug(self): |
391 | # The environment is successfully bootstrapped in debug mode. |
392 | with self.patch_multiple_calls(self.make_side_effects()) as mock_call: |
393 | - app.bootstrap(self.env_name, debug=True) |
394 | + already_bootstrapped = app.bootstrap(self.env_name, debug=True) |
395 | + self.assertFalse(already_bootstrapped) |
396 | mock_call.assert_has_calls([ |
397 | mock.call('juju', 'bootstrap', '-e', self.env_name, '--debug'), |
398 | ] + self.make_status_calls(1)) |
399 | |
400 | + def test_already_bootstrapped(self): |
401 | + # The function succeeds and returns True if the environment is already |
402 | + # bootstrapped. |
403 | + side_effects = [ |
404 | + (1, '', '***environment is already bootstrapped**'), |
405 | + (0, self.make_status_output('started'), ''), |
406 | + ] |
407 | + with self.patch_multiple_calls(side_effects) as mock_call: |
408 | + already_bootstrapped = app.bootstrap(self.env_name) |
409 | + self.assertTrue(already_bootstrapped) |
410 | + mock_call.assert_has_calls([ |
411 | + mock.call('juju', 'bootstrap', '-e', self.env_name), |
412 | + ] + self.make_status_calls(1)) |
413 | + |
414 | def test_bootstrap_failure(self): |
415 | # A ProgramExit is raised if an error occurs while bootstrapping. |
416 | with self.patch_call(1, error='bad wolf') as mock_call: |
417 | @@ -251,10 +267,32 @@ |
418 | |
419 | |
420 | @mock_print |
421 | -class TestDeployGui(ProgramExitTestsMixin, unittest.TestCase): |
422 | +class TestDeployGui( |
423 | + ProgramExitTestsMixin, helpers.WatcherDataTestsMixin, |
424 | + unittest.TestCase): |
425 | |
426 | charm_url = 'cs:precise/juju-gui-42' |
427 | |
428 | + def make_env(self, unit_name=None, service_data=None, unit_data=None): |
429 | + """Create and return a mock environment object. |
430 | + |
431 | + Set up the object so that a call to add_unit returns the given |
432 | + unit_name, and a call to status returns a status object containing the |
433 | + service and unit described by the given service_data and unit_data. |
434 | + """ |
435 | + env = mock.Mock() |
436 | + # Set up the add_unit return value. |
437 | + if unit_name is not None: |
438 | + env.add_unit.return_value = {'Units': [unit_name]} |
439 | + #Set up the get_status return value. |
440 | + status = [] |
441 | + if service_data is not None: |
442 | + status.append(self.make_service_change(data=service_data)) |
443 | + if unit_data is not None: |
444 | + status.append(self.make_unit_change(data=unit_data)) |
445 | + env.get_status.return_value = status |
446 | + return env |
447 | + |
448 | def patch_get_charm_url(self, side_effect=None): |
449 | """Patch the get_charm_url helper function.""" |
450 | if side_effect is None: |
451 | @@ -265,65 +303,198 @@ |
452 | def test_deployment(self, mock_print): |
453 | # The function correctly deploys and exposes the service, retrieving |
454 | # the charm URL from the charmworld API. |
455 | - env = mock.Mock() |
456 | - with self.patch_get_charm_url(): |
457 | - app.deploy_gui(env, 'my-gui') |
458 | - env.assert_has_calls([ |
459 | - mock.call.deploy('my-gui', self.charm_url, to=0), |
460 | - mock.call.expose('my-gui') |
461 | - ]) |
462 | - mock_print.assert_called_once_with( |
463 | - 'charm URL: {}'.format(self.charm_url)) |
464 | - |
465 | - def test_deployment_default_charm_url(self, mock_print): |
466 | + env = self.make_env(unit_name='my-gui/42') |
467 | + with self.patch_get_charm_url(): |
468 | + unit_name = app.deploy_gui(env, 'my-gui', '0') |
469 | + self.assertEqual('my-gui/42', unit_name) |
470 | + env.assert_has_calls([ |
471 | + mock.call.deploy('my-gui', self.charm_url, num_units=0), |
472 | + mock.call.expose('my-gui'), |
473 | + mock.call.add_unit('my-gui', machine_spec='0'), |
474 | + ]) |
475 | + # There is no need to call status if the environment was just created. |
476 | + self.assertFalse(env.get_status.called) |
477 | + mock_print.assert_has_calls([ |
478 | + mock.call('requesting my-gui deployment'), |
479 | + mock.call('charm URL: {}'.format(self.charm_url)), |
480 | + mock.call('my-gui deployment request accepted'), |
481 | + mock.call('exposing service my-gui'), |
482 | + mock.call('requesting new unit deployment'), |
483 | + mock.call('my-gui/42 deployment request accepted'), |
484 | + ]) |
485 | + |
486 | + def test_existing_environment_without_entities(self, mock_print): |
487 | + # The deployment is processed in an already bootstrapped environment |
488 | + # with no relevant entities in it. |
489 | + env = self.make_env(unit_name='my-gui/42') |
490 | + with self.patch_get_charm_url(): |
491 | + unit_name = app.deploy_gui( |
492 | + env, 'my-gui', '0', check_preexisting=True) |
493 | + self.assertEqual('my-gui/42', unit_name) |
494 | + env.assert_has_calls([ |
495 | + mock.call.get_status(), |
496 | + mock.call.deploy('my-gui', self.charm_url, num_units=0), |
497 | + mock.call.expose('my-gui'), |
498 | + mock.call.add_unit('my-gui', machine_spec='0'), |
499 | + ]) |
500 | + |
501 | + def test_default_charm_url(self, mock_print): |
502 | # The function correctly deploys and exposes the service, even if it is |
503 | # not able to retrieve the charm URL from the charmworld API. |
504 | - env = mock.Mock() |
505 | - log = 'unable to retrieve the Juju GUI charm URL from the API: boo!' |
506 | + env = self.make_env(unit_name='my-gui/42') |
507 | + log = 'unable to retrieve the my-gui charm URL from the API: boo!' |
508 | with self.patch_get_charm_url(side_effect=IOError('boo!')): |
509 | # A warning is logged which notifies we are using the default URL. |
510 | with helpers.assert_logs([log], level='warn'): |
511 | - app.deploy_gui(env, 'my-gui') |
512 | + app.deploy_gui(env, 'my-gui', '0') |
513 | env.assert_has_calls([ |
514 | - mock.call.deploy('my-gui', settings.DEFAULT_CHARM_URL, to=0), |
515 | - mock.call.expose('my-gui') |
516 | - ]) |
517 | - mock_print.assert_called_once_with( |
518 | - 'charm URL: {}'.format(settings.DEFAULT_CHARM_URL)) |
519 | + mock.call.deploy( |
520 | + 'my-gui', settings.DEFAULT_CHARM_URL, num_units=0), |
521 | + mock.call.expose('my-gui'), |
522 | + mock.call.add_unit('my-gui', machine_spec='0'), |
523 | + ]) |
524 | + mock_print.assert_has_calls([ |
525 | + mock.call('requesting my-gui deployment'), |
526 | + mock.call('charm URL: {}'.format(settings.DEFAULT_CHARM_URL)), |
527 | + ]) |
528 | |
529 | - def test_deployment_provided_charm_url(self, mock_print): |
530 | + def test_provided_charm_url(self, mock_print): |
531 | # The function correctly deploys and exposes the service using a user |
532 | # provided Juju GUI charm URL. |
533 | - env = mock.Mock() |
534 | + env = self.make_env(unit_name='my-gui/42') |
535 | charm_url = 'cs:~juju-gui/precise/juju-gui-116' |
536 | - app.deploy_gui(env, 'my-gui', charm_url=charm_url) |
537 | - env.assert_has_calls([ |
538 | - mock.call.deploy('my-gui', charm_url, to=0), |
539 | - mock.call.expose('my-gui') |
540 | - ]) |
541 | - mock_print.assert_called_once_with('charm URL: {}'.format(charm_url)) |
542 | - |
543 | - def test_api_error(self, mock_print): |
544 | - # A ProgramExit is raised if an error occurs in one of the API calls. |
545 | - env = mock.Mock() |
546 | - env.deploy.side_effect = self.make_env_error('service already exists') |
547 | - expected = 'bad API server response: service already exists' |
548 | - with self.patch_get_charm_url(): |
549 | - with self.assert_program_exit(expected): |
550 | - app.deploy_gui(env, 'another-gui') |
551 | + app.deploy_gui(env, 'my-gui', '0', charm_url=charm_url) |
552 | + env.assert_has_calls([ |
553 | + mock.call.deploy('my-gui', charm_url, num_units=0), |
554 | + mock.call.expose('my-gui'), |
555 | + mock.call.add_unit('my-gui', machine_spec='0'), |
556 | + ]) |
557 | + mock_print.assert_has_calls([ |
558 | + mock.call('requesting my-gui deployment'), |
559 | + mock.call('charm URL: {}'.format(charm_url)), |
560 | + ]) |
561 | + |
562 | + def test_existing_service(self, mock_print): |
563 | + # The deployment is executed reusing an already deployed service. |
564 | + env = self.make_env(unit_name='my-gui/42', service_data={}) |
565 | + unit_name = app.deploy_gui( |
566 | + env, 'my-gui', '0', check_preexisting=True) |
567 | + self.assertEqual('my-gui/42', unit_name) |
568 | + env.assert_has_calls([ |
569 | + mock.call.get_status(), |
570 | + mock.call.add_unit('my-gui', machine_spec='0'), |
571 | + ]) |
572 | + # The service is not re-deployed. |
573 | + self.assertFalse(env.deploy.called) |
574 | + # The service is not re-exposed. |
575 | + self.assertFalse(env.expose.called) |
576 | + mock_print.assert_has_calls([ |
577 | + mock.call('service my-gui already deployed'), |
578 | + mock.call('charm URL: cs:precise/juju-gui-47'), |
579 | + mock.call('requesting new unit deployment'), |
580 | + mock.call('my-gui/42 deployment request accepted'), |
581 | + ]) |
582 | + |
583 | + def test_existing_service_unexposed(self, mock_print): |
584 | + # The existing service is exposed if required. |
585 | + service_data = {'Exposed': False} |
586 | + env = self.make_env(unit_name='my-gui/42', service_data=service_data) |
587 | + unit_name = app.deploy_gui( |
588 | + env, 'my-gui', '1', check_preexisting=True) |
589 | + self.assertEqual('my-gui/42', unit_name) |
590 | + env.assert_has_calls([ |
591 | + mock.call.get_status(), |
592 | + mock.call.expose('my-gui'), |
593 | + mock.call.add_unit('my-gui', machine_spec='1'), |
594 | + ]) |
595 | + # The service is not re-deployed. |
596 | + self.assertFalse(env.deploy.called) |
597 | + mock_print.assert_has_calls([ |
598 | + mock.call('service my-gui already deployed'), |
599 | + mock.call('charm URL: cs:precise/juju-gui-47'), |
600 | + mock.call('exposing service my-gui'), |
601 | + mock.call('requesting new unit deployment'), |
602 | + mock.call('my-gui/42 deployment request accepted'), |
603 | + ]) |
604 | + |
605 | + def test_existing_service_and_unit(self, mock_print): |
606 | + # A unit is reused if a suitable one is already present. |
607 | + env = self.make_env(service_data={}, unit_data={}) |
608 | + unit_name = app.deploy_gui( |
609 | + env, 'my-gui', '0', check_preexisting=True) |
610 | + self.assertEqual('my-gui/47', unit_name) |
611 | + env.get_status.assert_called_once_with() |
612 | + # The service is not re-deployed. |
613 | + self.assertFalse(env.deploy.called) |
614 | + # The service is not re-exposed. |
615 | + self.assertFalse(env.expose.called) |
616 | + # The unit is not re-added. |
617 | + self.assertFalse(env.add_unit.called) |
618 | + mock_print.assert_has_calls([ |
619 | + mock.call('service my-gui already deployed'), |
620 | + mock.call('charm URL: cs:precise/juju-gui-47'), |
621 | + mock.call('reusing unit my-gui/47'), |
622 | + ]) |
623 | + |
624 | + def test_new_machine(self, mock_print): |
625 | + # The unit is correctly deployed in a new machine. |
626 | + env = self.make_env(unit_name='my-gui/42') |
627 | + with self.patch_get_charm_url(): |
628 | + unit_name = app.deploy_gui(env, 'my-gui', None) |
629 | + self.assertEqual('my-gui/42', unit_name) |
630 | + env.assert_has_calls([ |
631 | + mock.call.deploy('my-gui', self.charm_url, num_units=0), |
632 | + mock.call.expose('my-gui'), |
633 | + mock.call.add_unit('my-gui', machine_spec=None), |
634 | + ]) |
635 | + |
636 | + def test_status_error(self, mock_print): |
637 | + # A ProgramExit is raised if an error occurs in the status API call. |
638 | + env = self.make_env() |
639 | + env.get_status.side_effect = self.make_env_error('bad wolf') |
640 | + with self.assert_program_exit('bad API response: bad wolf'): |
641 | + app.deploy_gui( |
642 | + env, 'another-gui', '0', check_preexisting=True) |
643 | + env.get_status.assert_called_once_with() |
644 | + |
645 | + def test_deploy_error(self, mock_print): |
646 | + # A ProgramExit is raised if an error occurs in the deploy API call. |
647 | + env = self.make_env() |
648 | + env.deploy.side_effect = self.make_env_error('bad wolf') |
649 | + with self.patch_get_charm_url(): |
650 | + with self.assert_program_exit('bad API response: bad wolf'): |
651 | + app.deploy_gui(env, 'another-gui', '0') |
652 | env.deploy.assert_called_once_with( |
653 | - 'another-gui', 'cs:precise/juju-gui-42', to=0) |
654 | + 'another-gui', self.charm_url, num_units=0) |
655 | + |
656 | + def test_expose_error(self, mock_print): |
657 | + # A ProgramExit is raised if an error occurs in the expose API call. |
658 | + env = self.make_env() |
659 | + env.expose.side_effect = self.make_env_error('bad wolf') |
660 | + with self.patch_get_charm_url(): |
661 | + with self.assert_program_exit('bad API response: bad wolf'): |
662 | + app.deploy_gui(env, 'another-gui', '0') |
663 | + env.expose.assert_called_once_with('another-gui') |
664 | + |
665 | + def test_add_unit_error(self, mock_print): |
666 | + # A ProgramExit is raised if an error occurs in the add_unit API call. |
667 | + env = self.make_env() |
668 | + env.add_unit.side_effect = self.make_env_error('bad wolf') |
669 | + with self.patch_get_charm_url(): |
670 | + with self.assert_program_exit('bad API response: bad wolf'): |
671 | + app.deploy_gui(env, 'another-gui', '0') |
672 | + env.add_unit.assert_called_once_with('another-gui', machine_spec='0') |
673 | |
674 | def test_other_errors(self, mock_print): |
675 | # Any other errors occurred during the process are not trapped. |
676 | error = ValueError('explode!') |
677 | - env = mock.Mock() |
678 | + env = self.make_env(unit_name='my-gui/42') |
679 | env.expose.side_effect = error |
680 | with self.patch_get_charm_url(): |
681 | with self.assertRaises(ValueError) as context_manager: |
682 | - app.deploy_gui(env, 'juju-gui') |
683 | + app.deploy_gui(env, 'juju-gui', '0') |
684 | env.deploy.assert_called_once_with( |
685 | - 'juju-gui', 'cs:precise/juju-gui-42', to=0) |
686 | + 'juju-gui', 'cs:precise/juju-gui-42', num_units=0) |
687 | env.expose.assert_called_once_with('juju-gui') |
688 | self.assertIs(error, context_manager.exception) |
689 | |
690 | @@ -334,28 +505,29 @@ |
691 | address = 'unit.example.com' |
692 | change_pending = ('unit', 'change', { |
693 | 'Name': 'django/42', |
694 | - 'Status': 'pending', |
695 | 'PublicAddress': '', |
696 | + 'Status': 'pending', |
697 | }) |
698 | change_reachable = ('unit', 'change', { |
699 | 'Name': 'django/42', |
700 | + 'PublicAddress': address, |
701 | 'Status': 'pending', |
702 | - 'PublicAddress': address, |
703 | }) |
704 | change_installed = ('unit', 'change', { |
705 | 'Name': 'django/42', |
706 | + 'PublicAddress': address, |
707 | 'Status': 'installed', |
708 | - 'PublicAddress': address, |
709 | }) |
710 | change_started = ('unit', 'change', { |
711 | + 'MachineId': '0', |
712 | 'Name': 'django/42', |
713 | + 'PublicAddress': address, |
714 | 'Status': 'started', |
715 | - 'PublicAddress': address, |
716 | }) |
717 | - pending_call = mock.call('django/0 deployment is pending') |
718 | - reachable_call = mock.call('installing django/0 on {}'.format(address)) |
719 | - installed_call = mock.call('django/0 is installed') |
720 | - started_call = mock.call('django/0 is ready') |
721 | + pending_call = mock.call('django/42 deployment is pending') |
722 | + reachable_call = mock.call('setting up django/42 on {}'.format(address)) |
723 | + installed_call = mock.call('django/42 is installed') |
724 | + started_call = mock.call('django/42 is ready on machine 0') |
725 | |
726 | def make_env(self, changes): |
727 | """Create and return a patched Environment instance. |
728 | @@ -375,7 +547,7 @@ |
729 | self.change_installed, |
730 | self.change_started, |
731 | ]) |
732 | - address = app.watch(env, 'django') |
733 | + address = app.watch(env, 'django/42') |
734 | self.assertEqual(self.address, address) |
735 | mock_print.assert_has_calls([ |
736 | self.pending_call, |
737 | @@ -392,7 +564,7 @@ |
738 | self.change_installed, |
739 | self.change_started, |
740 | ]) |
741 | - address = app.watch(env, 'django') |
742 | + address = app.watch(env, 'django/42') |
743 | self.assertEqual(self.address, address) |
744 | mock_print.assert_has_calls([ |
745 | self.reachable_call, |
746 | @@ -404,7 +576,7 @@ |
747 | def test_missing_changes(self, mock_print): |
748 | # Only the started change is strictly required. |
749 | env = self.make_env([self.change_started]) |
750 | - address = app.watch(env, 'django') |
751 | + address = app.watch(env, 'django/42') |
752 | self.assertEqual(self.address, address) |
753 | mock_print.assert_has_calls([ |
754 | self.reachable_call, |
755 | @@ -432,7 +604,7 @@ |
756 | }), |
757 | self.change_started, |
758 | ]) |
759 | - address = app.watch(env, 'django') |
760 | + address = app.watch(env, 'django/42') |
761 | self.assertEqual(self.address, address) |
762 | mock_print.assert_has_calls([ |
763 | self.pending_call, |
764 | @@ -449,7 +621,7 @@ |
765 | ]) |
766 | expected = 'bad API server response: next returned an error' |
767 | with self.assert_program_exit(expected): |
768 | - app.watch(env, 'django') |
769 | + app.watch(env, 'django/42') |
770 | mock_print.assert_has_calls([self.pending_call]) |
771 | |
772 | def test_other_errors(self, mock_print): |
773 | @@ -457,7 +629,7 @@ |
774 | error = ValueError('explode!') |
775 | env = self.make_env([self.change_installed, error]) |
776 | with self.assertRaises(ValueError) as context_manager: |
777 | - app.watch(env, 'django') |
778 | + app.watch(env, 'django/42') |
779 | mock_print.assert_has_calls([self.reachable_call, self.installed_call]) |
780 | self.assertIs(error, context_manager.exception) |
781 | |
782 | @@ -472,8 +644,8 @@ |
783 | }), |
784 | self.change_reachable, |
785 | ]) |
786 | - with self.assert_program_exit('django/0 unexpectedly removed'): |
787 | - app.watch(env, 'django') |
788 | + with self.assert_program_exit('django/42 unexpectedly removed'): |
789 | + app.watch(env, 'django/42') |
790 | mock_print.assert_has_calls([self.pending_call]) |
791 | |
792 | def test_status_error(self, mock_print): |
793 | @@ -483,14 +655,14 @@ |
794 | ('unit', 'change', { |
795 | 'Name': 'django/42', |
796 | 'Status': 'error', |
797 | - 'StatusInfo': 'install hook failure', |
798 | + 'StatusInfo': 'install failure', |
799 | 'PublicAddress': '', |
800 | }), |
801 | self.change_reachable, |
802 | ]) |
803 | - expected = 'django/0 is in an error state: error: install hook failure' |
804 | + expected = 'django/42 is in an error state: error: install failure' |
805 | with self.assert_program_exit(expected): |
806 | - app.watch(env, 'django') |
807 | + app.watch(env, 'django/42') |
808 | mock_print.assert_has_calls([self.pending_call]) |
809 | |
810 | |
811 | |
812 | === modified file 'quickstart/tests/test_juju.py' |
813 | --- quickstart/tests/test_juju.py 2013-10-30 10:16:36 +0000 |
814 | +++ quickstart/tests/test_juju.py 2013-11-18 16:54:22 +0000 |
815 | @@ -25,6 +25,9 @@ |
816 | from quickstart.tests import helpers |
817 | |
818 | |
819 | +patch_rpc = mock.patch('quickstart.juju.Environment._rpc') |
820 | + |
821 | + |
822 | class TestConnect(unittest.TestCase): |
823 | |
824 | api_url = 'wss://api.example.com:17070' |
825 | @@ -45,6 +48,9 @@ |
826 | |
827 | |
828 | class TestEnvironment(unittest.TestCase): |
829 | + # Note that in some of the tests below, rather than exercising quickstart |
830 | + # code, we are actually testing the external jujuclient methods. This is so |
831 | + # by design, and will help us when upgrading the python-jujuclient library. |
832 | |
833 | api_url = 'wss://api.example.com:17070' |
834 | charm_url = 'cs:precise/juju-gui-77' |
835 | @@ -59,8 +65,24 @@ |
836 | # Keep track of watcher changes in the changesets list. |
837 | self.changesets = [] |
838 | |
839 | + def make_add_unit_request(self, **kwargs): |
840 | + """Create and return an "add unit" request. |
841 | + |
842 | + Use kwargs to add or override request parameters. |
843 | + """ |
844 | + params = { |
845 | + 'ServiceName': self.service_name, |
846 | + 'NumUnits': 1, |
847 | + } |
848 | + params.update(kwargs) |
849 | + return { |
850 | + 'Type': 'Client', |
851 | + 'Request': 'AddServiceUnits', |
852 | + 'Params': params, |
853 | + } |
854 | + |
855 | def make_deploy_request(self, **kwargs): |
856 | - """Create and return a deploy request. |
857 | + """Create and return a "deploy" request. |
858 | |
859 | Use kwargs to add or override request parameters. |
860 | """ |
861 | @@ -78,17 +100,43 @@ |
862 | 'Params': params, |
863 | } |
864 | |
865 | + def patch_get_watch(self, return_value): |
866 | + """Patch the Environment.get_watch method. |
867 | + |
868 | + When the resulting mock is used as a context manager, the given return |
869 | + value is returned. |
870 | + """ |
871 | + get_watch_path = 'quickstart.juju.jujuclient.Environment.get_watch' |
872 | + mock_get_watch = mock.MagicMock() |
873 | + mock_get_watch().__enter__.return_value = iter(return_value) |
874 | + mock_get_watch.reset_mock() |
875 | + return mock.patch(get_watch_path, mock_get_watch) |
876 | + |
877 | def processor(self, changeset): |
878 | self.changesets.append(changeset) |
879 | return changeset |
880 | |
881 | - @mock.patch('quickstart.juju.Environment._rpc') |
882 | + @patch_rpc |
883 | + def test_add_unit(self, mock_rpc): |
884 | + # The AddServiceUnits API call is properly generated. |
885 | + self.env.add_unit(self.service_name) |
886 | + mock_rpc.assert_called_once_with(self.make_add_unit_request()) |
887 | + |
888 | + @patch_rpc |
889 | + def test_add_unit_to_machine(self, mock_rpc): |
890 | + # The AddServiceUnits API call is properly generated when deploying a |
891 | + # unit in a specific machine. |
892 | + self.env.add_unit(self.service_name, machine_spec='0') |
893 | + expected = self.make_add_unit_request(ToMachineSpec='0') |
894 | + mock_rpc.assert_called_once_with(expected) |
895 | + |
896 | + @patch_rpc |
897 | def test_deploy(self, mock_rpc): |
898 | # The deploy API call is properly generated. |
899 | self.env.deploy(self.service_name, self.charm_url) |
900 | mock_rpc.assert_called_once_with(self.make_deploy_request()) |
901 | |
902 | - @mock.patch('quickstart.juju.Environment._rpc') |
903 | + @patch_rpc |
904 | def test_deploy_config(self, mock_rpc): |
905 | # The deploy API call is properly generated when passing settings. |
906 | self.env.deploy( |
907 | @@ -98,7 +146,7 @@ |
908 | Config={'key1': 'value1', 'key2': '42'}) |
909 | mock_rpc.assert_called_once_with(expected) |
910 | |
911 | - @mock.patch('quickstart.juju.Environment._rpc') |
912 | + @patch_rpc |
913 | def test_deploy_constraints(self, mock_rpc): |
914 | # The deploy API call is properly generated when passing constraints. |
915 | constraints = {'cpu-cores': 8, 'mem': 16} |
916 | @@ -107,15 +155,14 @@ |
917 | expected = self.make_deploy_request(Constraints=constraints) |
918 | mock_rpc.assert_called_once_with(expected) |
919 | |
920 | - @mock.patch('quickstart.juju.Environment._rpc') |
921 | - def test_deploy_to(self, mock_rpc): |
922 | - # The deploy API call is properly generated when passing a machine |
923 | - # specification. |
924 | - self.env.deploy(self.service_name, self.charm_url, to=0) |
925 | - expected = self.make_deploy_request(ToMachineSpec='0') |
926 | + @patch_rpc |
927 | + def test_deploy_no_units(self, mock_rpc): |
928 | + # The deploy API call is properly generated when passing zero units. |
929 | + self.env.deploy(self.service_name, self.charm_url, num_units=0) |
930 | + expected = self.make_deploy_request(NumUnits=0) |
931 | mock_rpc.assert_called_once_with(expected) |
932 | |
933 | - @mock.patch('quickstart.juju.Environment._rpc') |
934 | + @patch_rpc |
935 | def test_deploy_bundle(self, mock_rpc): |
936 | # The deploy bundle call is properly generated. |
937 | self.env.deploy_bundle('name: contents') |
938 | @@ -126,7 +173,7 @@ |
939 | } |
940 | mock_rpc.assert_called_once_with(expected) |
941 | |
942 | - @mock.patch('quickstart.juju.Environment._rpc') |
943 | + @patch_rpc |
944 | def test_deploy_bundle_with_name(self, mock_rpc): |
945 | # The deploy bundle call is properly generated when passing a name. |
946 | self.env.deploy_bundle('name: contents', name='name') |
947 | @@ -137,58 +184,75 @@ |
948 | } |
949 | mock_rpc.assert_called_once_with(expected) |
950 | |
951 | - @mock.patch('quickstart.juju.jujuclient.Watcher._rpc') |
952 | - def test_watch_changes(self, mock_rpc): |
953 | + @patch_rpc |
954 | + def test_expose(self, mock_rpc): |
955 | + # The expose API call is properly generated. |
956 | + self.env.expose(self.service_name) |
957 | + expected = { |
958 | + 'Type': 'Client', |
959 | + 'Request': 'ServiceExpose', |
960 | + 'Params': {'ServiceName': self.service_name}, |
961 | + } |
962 | + mock_rpc.assert_called_once_with(expected) |
963 | + |
964 | + def test_get_status(self): |
965 | + # The current status of the Juju environment is properly returned. |
966 | + changeset1 = ['change1', 'change2'] |
967 | + changeset2 = ['change3'] |
968 | + with self.patch_get_watch([changeset1, changeset2]) as mock_get_watch: |
969 | + status = self.env.get_status() |
970 | + # The get_status call only waits for the first changeset. |
971 | + self.assertEqual(changeset1, status) |
972 | + # The watcher is correctly closed. |
973 | + self.assertEqual(1, mock_get_watch().__exit__.call_count) |
974 | + |
975 | + def test_watch_changes(self): |
976 | # It is possible to watch for changes using a processor callable. |
977 | changeset1 = ['change1', 'change2'] |
978 | changeset2 = ['change3'] |
979 | - mock_rpc.side_effect = [ |
980 | - # Define the response to the watcher's start call. |
981 | - {'Response': {}, 'AllWatcherId': 42}, |
982 | - # Define two responses to the two subsequent next calls. |
983 | - {'Response': {}, 'Deltas': changeset1}, |
984 | - {'Response': {}, 'Deltas': changeset2}, |
985 | - # Define the response to the watcher's stop call. |
986 | - {'Response': {}}, |
987 | - ] |
988 | - watcher = self.env.watch_changes(self.processor) |
989 | - # The first set of changes is correctly returned. |
990 | - changeset = watcher.next() |
991 | - self.assertEqual(changeset1, changeset) |
992 | - # The second set of changes is correctly returned. |
993 | - changeset = watcher.next() |
994 | - self.assertEqual(changeset2, changeset) |
995 | + with self.patch_get_watch([changeset1, changeset2]) as mock_get_watch: |
996 | + watcher = self.env.watch_changes(self.processor) |
997 | + # The first set of changes is correctly returned. |
998 | + changeset = watcher.next() |
999 | + self.assertEqual(changeset1, changeset) |
1000 | + # The second set of changes is correctly returned. |
1001 | + changeset = watcher.next() |
1002 | + self.assertEqual(changeset2, changeset) |
1003 | # All the changes have been processed. |
1004 | self.assertEqual([changeset1, changeset2], self.changesets) |
1005 | # Ensure the API has been used properly. |
1006 | - mock_rpc.assert_has_calls([ |
1007 | - mock.call({'Type': 'Client', 'Request': 'WatchAll', 'Params': {}}), |
1008 | - mock.call({'Type': 'AllWatcher', 'Request': 'Next', 'Id': 42}), |
1009 | - mock.call({'Type': 'AllWatcher', 'Request': 'Next', 'Id': 42}), |
1010 | - ]) |
1011 | - |
1012 | - @mock.patch('quickstart.juju.jujuclient.Watcher._rpc') |
1013 | - def test_watch_closed(self, mock_rpc): |
1014 | + mock_get_watch().__enter__.assert_called_once_with() |
1015 | + |
1016 | + def test_watch_changes_map(self): |
1017 | + # The processor callable can be used to modify changes. |
1018 | + changeset1 = ['change1', 'change2'] |
1019 | + changeset2 = ['change3'] |
1020 | + with self.patch_get_watch([changeset1, changeset2]): |
1021 | + watcher = self.env.watch_changes(len) |
1022 | + changesets = list(watcher) |
1023 | + self.assertEqual([len(changeset1), len(changeset2)], changesets) |
1024 | + |
1025 | + def test_watch_changes_filter(self): |
1026 | + # The processor callable can be used to filter changes. |
1027 | + changeset1 = ['change1', 'change2'] |
1028 | + changeset2 = ['change3'] |
1029 | + processor = lambda changes: None if len(changes) == 1 else changes |
1030 | + with self.patch_get_watch([changeset1, changeset2]): |
1031 | + watcher = self.env.watch_changes(processor) |
1032 | + changesets = list(watcher) |
1033 | + self.assertEqual([changeset1], changesets) |
1034 | + |
1035 | + def test_watch_closed(self): |
1036 | # A stop API call on the AllWatcher is performed when the watcher is |
1037 | # garbage collected. |
1038 | - mock_rpc.side_effect = [ |
1039 | - # Define the response to the watcher's start call. |
1040 | - {'Response': {}, 'AllWatcherId': 42}, |
1041 | - # Define a response to a next call. |
1042 | - {'Response': {}, 'Deltas': ['change1', 'change2']}, |
1043 | - # Define the response to the watcher's stop call. |
1044 | - {'Response': {}}, |
1045 | - ] |
1046 | - watcher = self.env.watch_changes(self.processor) |
1047 | - # The first set of changes is correctly returned. |
1048 | - watcher.next() |
1049 | - del watcher |
1050 | + changeset = ['change1', 'change2'] |
1051 | + with self.patch_get_watch([changeset]) as mock_get_watch: |
1052 | + watcher = self.env.watch_changes(self.processor) |
1053 | + # The first set of changes is correctly returned. |
1054 | + watcher.next() |
1055 | + del watcher |
1056 | # Ensure the API has been used properly. |
1057 | - mock_rpc.assert_has_calls([ |
1058 | - mock.call({'Type': 'Client', 'Request': 'WatchAll', 'Params': {}}), |
1059 | - mock.call({'Type': 'AllWatcher', 'Request': 'Next', 'Id': 42}), |
1060 | - mock.call({'Type': 'AllWatcher', 'Request': 'Stop', 'Id': 42}), |
1061 | - ]) |
1062 | + self.assertEqual(1, mock_get_watch().__exit__.call_count) |
1063 | |
1064 | |
1065 | class TestWebSocketConnection(unittest.TestCase): |
1066 | |
1067 | === modified file 'quickstart/tests/test_manage.py' |
1068 | --- quickstart/tests/test_manage.py 2013-11-13 12:23:36 +0000 |
1069 | +++ quickstart/tests/test_manage.py 2013-11-18 16:54:22 +0000 |
1070 | @@ -356,10 +356,11 @@ |
1071 | mock_app.connect.assert_called_once_with( |
1072 | mock_app.get_api_url(), options.admin_secret) |
1073 | mock_app.deploy_gui.assert_called_once_with( |
1074 | - mock_app.connect(), settings.JUJU_GUI_NAME, |
1075 | - charm_url=options.charm_url) |
1076 | + mock_app.connect(), settings.JUJU_GUI_NAME, '0', |
1077 | + charm_url=options.charm_url, |
1078 | + check_preexisting=mock_app.bootstrap()) |
1079 | mock_app.watch.assert_called_once_with( |
1080 | - mock_app.connect(), settings.JUJU_GUI_NAME) |
1081 | + mock_app.connect(), mock_app.deploy_gui()) |
1082 | mock_open.assert_called_once_with( |
1083 | 'https://{}'.format(mock_app.watch())) |
1084 | self.assertFalse(mock_app.deploy_bundle.called) |
1085 | |
1086 | === modified file 'quickstart/tests/test_utils.py' |
1087 | --- quickstart/tests/test_utils.py 2013-11-13 12:23:36 +0000 |
1088 | +++ quickstart/tests/test_utils.py 2013-11-18 16:54:22 +0000 |
1089 | @@ -178,6 +178,79 @@ |
1090 | mock_call.assert_called_once_with('juju', 'switch') |
1091 | |
1092 | |
1093 | +class TestGetServiceInfo(helpers.WatcherDataTestsMixin, unittest.TestCase): |
1094 | + |
1095 | + def test_service_and_unit(self): |
1096 | + # The data about the given service and unit is correctly returned. |
1097 | + service_change = self.make_service_change() |
1098 | + unit_change = self.make_unit_change() |
1099 | + status = [service_change, unit_change] |
1100 | + expected = (service_change[2], unit_change[2]) |
1101 | + self.assertEqual(expected, utils.get_service_info(status, 'my-gui')) |
1102 | + |
1103 | + def test_service_only(self): |
1104 | + # The data about the given service without units is correctly returned. |
1105 | + service_change = self.make_service_change() |
1106 | + status = [service_change] |
1107 | + expected = (service_change[2], None) |
1108 | + self.assertEqual(expected, utils.get_service_info(status, 'my-gui')) |
1109 | + |
1110 | + def test_service_removed(self): |
1111 | + # A tuple (None, None) is returned if the service is being removed. |
1112 | + status = [ |
1113 | + self.make_service_change(action='remove'), |
1114 | + self.make_unit_change(), |
1115 | + ] |
1116 | + expected = (None, None) |
1117 | + self.assertEqual(expected, utils.get_service_info(status, 'my-gui')) |
1118 | + |
1119 | + def test_another_service(self): |
1120 | + # A tuple (None, None) is returned if the service is not found. |
1121 | + status = [ |
1122 | + self.make_service_change(data={'Name': 'another-service'}), |
1123 | + self.make_unit_change(), |
1124 | + ] |
1125 | + expected = (None, None) |
1126 | + self.assertEqual(expected, utils.get_service_info(status, 'my-gui')) |
1127 | + |
1128 | + def test_service_not_alive(self): |
1129 | + # A tuple (None, None) is returned if the service is not alive. |
1130 | + status = [ |
1131 | + self.make_service_change(data={'Life': 'dying'}), |
1132 | + self.make_unit_change(), |
1133 | + ] |
1134 | + expected = (None, None) |
1135 | + self.assertEqual(expected, utils.get_service_info(status, 'my-gui')) |
1136 | + |
1137 | + def test_unit_removed(self): |
1138 | + # The unit data is not returned if the unit is being removed. |
1139 | + service_change = self.make_service_change() |
1140 | + status = [service_change, self.make_unit_change(action='remove')] |
1141 | + expected = (service_change[2], None) |
1142 | + self.assertEqual(expected, utils.get_service_info(status, 'my-gui')) |
1143 | + |
1144 | + def test_another_unit(self): |
1145 | + # The unit data is not returned if the unit belongs to another service. |
1146 | + service_change = self.make_service_change() |
1147 | + status = [ |
1148 | + service_change, |
1149 | + self.make_unit_change(data={'Service': 'another-service'}), |
1150 | + ] |
1151 | + expected = (service_change[2], None) |
1152 | + self.assertEqual(expected, utils.get_service_info(status, 'my-gui')) |
1153 | + |
1154 | + def test_no_services(self): |
1155 | + # A tuple (None, None) is returned no services are found. |
1156 | + status = [self.make_unit_change()] |
1157 | + expected = (None, None) |
1158 | + self.assertEqual(expected, utils.get_service_info(status, 'my-gui')) |
1159 | + |
1160 | + def test_no_entities(self): |
1161 | + # A tuple (None, None) is returned no entities are found. |
1162 | + expected = (None, None) |
1163 | + self.assertEqual(expected, utils.get_service_info([], 'my-gui')) |
1164 | + |
1165 | + |
1166 | class TestParseBundle( |
1167 | helpers.BundleFileTestsMixin, helpers.ValueErrorTestsMixin, |
1168 | unittest.TestCase): |
1169 | |
1170 | === modified file 'quickstart/utils.py' |
1171 | --- quickstart/utils.py 2013-11-13 12:23:36 +0000 |
1172 | +++ quickstart/utils.py 2013-11-18 16:54:22 +0000 |
1173 | @@ -111,6 +111,31 @@ |
1174 | return match.groups()[0] |
1175 | |
1176 | |
1177 | +def get_service_info(status, service_name): |
1178 | + """Retrieve information on the given service and on its first alive unit. |
1179 | + |
1180 | + Return a tuple containing two values: (service data, unit data). |
1181 | + Each value can be: |
1182 | + - a dictionary of data about the given entity (service or unit) as |
1183 | + returned by the Juju watcher; |
1184 | + - None, if the entity is not present in the Juju environment. |
1185 | + If the service data is None, the unit data is always None. |
1186 | + """ |
1187 | + services = [ |
1188 | + data for entity, action, data in status if |
1189 | + (entity == 'service') and (action != 'remove') and |
1190 | + (data['Name'] == service_name) and (data['Life'] == 'alive') |
1191 | + ] |
1192 | + if not services: |
1193 | + return None, None |
1194 | + units = [ |
1195 | + data for entity, action, data in status if |
1196 | + entity == 'unit' and action != 'remove' and |
1197 | + data['Service'] == service_name |
1198 | + ] |
1199 | + return services[0], units[0] if units else None |
1200 | + |
1201 | + |
1202 | def parse_bundle(bundle_yaml, bundle_name=None): |
1203 | """Parse the provided bundle YAML encoded contents. |
1204 |
Reviewers: mp+195592_ code.launchpad. net,
Message:
Please take a look.
Description:
Make quickstart idempotent.
- do not bootstrap an environment if already bootstrapped;
- do not deploy the GUI if already there.
Sorry, the diff is long, but there are a lot of tests.
Tests: make check
QA:
- .venv/bin/python juju-quickstart -e ec2
and ensure the GUI is correctly deployed;
- .venv/bin/python juju-quickstart -e ec2
again, to check it recognizes that env is
already bootstrapped and the
GUI unit is already there;
In the following steps, sometimes the browser
can get confused about certs, wss conections
etc. If the GUI is not loading correctly,
try harder, use incognito mode, change the
browser.
- juju destroy-service -e ec2 juju-gui;
- .venv/bin/python juju-quickstart -e ec2
again, to check the service and the unit
are correctly re-deployed;
Incidentally the step above, in the case it
succeeds, also demonstrates that the GUI can
safely be redeployed in the same machine: I
wasn't sure about this and this means we are
cleaning up things correctly in our charm, yay!
- juju unexpose -e ec2 juju-gui;
- .venv/bin/python juju-quickstart -e ec2
again, to check that the service is
properly re-exposed;
- juju destroy-unit -e ec2 juju-gui/0;
- .venv/bin/python juju-quickstart -e ec2
again, to check that the unit is
re-added on the existing service
(this time it should be named juju-gui/1);
- juju destroy-service -e ec2 juju-gui; ~jorge/ mediawiki- simple/ 4/mediawiki- simple;
- juju deploy -e ec2 juju-gui (if juju exits with a
"service already exists" error, retry after a while);
- .venv/bin/python juju-quickstart -e ec2 \
bundle:
The last command, executed right after juju-deploy should
also demonstrates that incidentally quickstart
can also be used to watch an already running
deployment, and that a bundle can still be deployed;
Final check:
- .venv/bin/python juju-quickstart -e ec2;
just to ensure quickstart is not surprised
that the unit is not in the bootstrap node
(i.e. you should see "juju-gui/0 is ready on machine 1").
Thanks a lot for testing all of this.
I added a card to automate the QA above with
a collection of functional tests.
Remember to destroy your ec2 environment.
https:/ /code.launchpad .net/~frankban/ juju-quickstart /idempotent- feature/ +merge/ 195592
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/28250044/
Affected files (+635, -182 lines): __init_ _.py manage. py tests/helpers. py tests/test_ app.py tests/test_ juju.py tests/test_ manage. py tests/test_ utils.py
A [revision details]
M quickstart/
M quickstart/app.py
M quickstart/juju.py
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/utils.py