Merge lp:~barry/ubuntu-system-image/citrain-2.2 into lp:~ubuntu-managed-branches/ubuntu-system-image/system-image
- citrain-2.2
- Merge into system-image
Status: | Merged | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Merged at revision: | 231 | ||||||||||||||||||||||||||||
Proposed branch: | lp:~barry/ubuntu-system-image/citrain-2.2 | ||||||||||||||||||||||||||||
Merge into: | lp:~ubuntu-managed-branches/ubuntu-system-image/system-image | ||||||||||||||||||||||||||||
Diff against target: |
1484 lines (+646/-221) 33 files modified
NEWS.rst (+22/-0) PKG-INFO (+1/-1) debian/changelog (+26/-0) debian/patches/lp1284217.patch (+0/-106) debian/patches/series (+0/-1) debian/rules (+3/-0) ini-manpage.rst (+6/-2) setup.cfg (+1/-1) system_image.egg-info/PKG-INFO (+1/-1) system_image.egg-info/SOURCES.txt (+4/-0) system_image.egg-info/entry_points.txt (+1/-1) systemimage/api.py (+16/-3) systemimage/bag.py (+9/-1) systemimage/config.py (+54/-14) systemimage/dbus.py (+29/-13) systemimage/download.py (+13/-39) systemimage/main.py (+6/-2) systemimage/scores.py (+1/-1) systemimage/state.py (+2/-10) systemimage/testing/controller.py (+2/-1) systemimage/testing/dbus.py (+0/-2) systemimage/testing/helpers.py (+11/-0) systemimage/tests/data/channel_02.ini (+3/-0) systemimage/tests/data/config_05.ini (+36/-0) systemimage/tests/data/config_06.ini (+36/-0) systemimage/tests/data/config_07.ini (+36/-0) systemimage/tests/data/config_08.ini (+36/-0) systemimage/tests/test_api.py (+16/-2) systemimage/tests/test_bag.py (+61/-0) systemimage/tests/test_config.py (+56/-4) systemimage/tests/test_dbus.py (+128/-14) systemimage/tests/test_main.py (+29/-1) systemimage/version.txt (+1/-1) |
||||||||||||||||||||||||||||
To merge this branch: | bzr merge lp:~barry/ubuntu-system-image/citrain-2.2 | ||||||||||||||||||||||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Manuel de la Peña (community) | Approve | ||
Ubuntu CI managed package branches | Pending | ||
Review via email: mp+209560@code.launchpad.net |
Commit message
system-image (2.2-0ubuntu1) UNRELEASED; urgency=medium
* New upstream release.
- LP: #1284217 - When CheckForUpdate() is called a second time, while
an auto-download is in progress, but after the first check is
complete, we send an UpdateAvailable
information.
- LP: #1287919 - Close a race condition when manually downloading and
issuing multiple CheckForUpdate calls.
- LP: #1278589 - Support disabling either HTTP or HTTPS services for
custom system image servers.
- Allow the channel.ini file to override the [service] section.
- LP: #1287287 - Do not do atomic renames of temporary download files;
ubuntu-
- LP: #1250817 - Exceptions in the state machine are caught and
logged, with an appropriate error message added to
UpdateAva
to the GLib main loop.
- LP: #1279532 - During testing, pass the log dir argument to
ubuntu-
* d/rules: Add override_dh_python3 rule to set shebang line to
/usr/
* d/patches/
-- Barry Warsaw <email address hidden> Wed, 05 Mar 2014 16:29:13 -0500
Description of the change
system-image (2.2-0ubuntu1) UNRELEASED; urgency=medium
* New upstream release.
- LP: #1284217 - When CheckForUpdate() is called a second time, while
an auto-download is in progress, but after the first check is
complete, we send an UpdateAvailable
information.
- LP: #1287919 - Close a race condition when manually downloading and
issuing multiple CheckForUpdate calls.
- LP: #1278589 - Support disabling either HTTP or HTTPS services for
custom system image servers.
- Allow the channel.ini file to override the [service] section.
- LP: #1287287 - Do not do atomic renames of temporary download files;
ubuntu-
- LP: #1250817 - Exceptions in the state machine are caught and
logged, with an appropriate error message added to
UpdateAva
to the GLib main loop.
- LP: #1279532 - During testing, pass the log dir argument to
ubuntu-
* d/rules: Add override_dh_python3 rule to set shebang line to
/usr/
* d/patches/
-- Barry Warsaw <email address hidden> Wed, 05 Mar 2014 16:29:13 -0500
Preview Diff
1 | === modified file 'NEWS.rst' |
2 | --- NEWS.rst 2014-02-20 23:03:24 +0000 |
3 | +++ NEWS.rst 2014-03-05 23:45:27 +0000 |
4 | @@ -2,6 +2,28 @@ |
5 | NEWS for system-image updater |
6 | ============================= |
7 | |
8 | +2.2 (2014-03-05) |
9 | +================ |
10 | + * When `CheckForUpdate()` is called a second time, while an auto-download is |
11 | + in progress, but after the first check is complete, we send an |
12 | + `UpdateAvailableStatus` signal with the cached information. (LP: #1284217) |
13 | + * Close a race condition when manually downloading and issuing multiple |
14 | + `CheckForUpdate` calls. (LP: #1287919) |
15 | + * Support disabling either the HTTP or HTTPS services for update (but not |
16 | + both). The ``[service]http_port`` or ``[service]https_port`` may be set to |
17 | + the string ``disabled`` and the disabled protocol will fall back to the |
18 | + enabled protocol. Implementation given by Vojtech Bocek. (LP: #1278589) |
19 | + * Allow the channel.ini file to override the ``[service]`` section. |
20 | + * Now that ubuntu-download-manager performs atomic renames of temporary |
21 | + files, system-image no longer needs to do that. (LP: #1287287) |
22 | + * When an exception in the state machine occurs while checking for updates, |
23 | + the exception is caught and logged. When using the CLI, the result is an |
24 | + exit code of 1. When using the D-Bus API, an `UpdateAvailableStatus` |
25 | + signal is sent with `error_reason` set to the exception string. This |
26 | + exception is *not* propagated back to GLib. (LP: #1250817) |
27 | + * Log directory path is passed to ubuntu-download-manager to assist in |
28 | + debugging. Given by Manuel de la Peña. (LP: #1279532) |
29 | + |
30 | 2.1 (2014-02-20) |
31 | ================ |
32 | * Internal improvements to SignatureError for better debugging. (LP: #1279056) |
33 | |
34 | === modified file 'PKG-INFO' |
35 | --- PKG-INFO 2014-02-20 23:03:09 +0000 |
36 | +++ PKG-INFO 2014-03-05 23:45:27 +0000 |
37 | @@ -1,6 +1,6 @@ |
38 | Metadata-Version: 1.0 |
39 | Name: system-image |
40 | -Version: 2.1 |
41 | +Version: 2.2 |
42 | Summary: Ubuntu System Image Based Upgrades |
43 | Home-page: UNKNOWN |
44 | Author: Barry Warsaw |
45 | |
46 | === modified file 'debian/changelog' |
47 | --- debian/changelog 2014-02-25 17:48:27 +0000 |
48 | +++ debian/changelog 2014-03-05 23:45:27 +0000 |
49 | @@ -1,3 +1,29 @@ |
50 | +system-image (2.2-0ubuntu1) UNRELEASED; urgency=medium |
51 | + |
52 | + * New upstream release. |
53 | + - LP: #1284217 - When CheckForUpdate() is called a second time, while |
54 | + an auto-download is in progress, but after the first check is |
55 | + complete, we send an UpdateAvailableStatus signal with the cached |
56 | + information. |
57 | + - LP: #1287919 - Close a race condition when manually downloading and |
58 | + issuing multiple CheckForUpdate calls. |
59 | + - LP: #1278589 - Support disabling either HTTP or HTTPS services for |
60 | + custom system image servers. |
61 | + - Allow the channel.ini file to override the [service] section. |
62 | + - LP: #1287287 - Do not do atomic renames of temporary download files; |
63 | + ubuntu-download-manager now supports this by default. |
64 | + - LP: #1250817 - Exceptions in the state machine are caught and |
65 | + logged, with an appropriate error message added to |
66 | + UpdateAvailableStatus signals. These exceptions do not percolate up |
67 | + to the GLib main loop. |
68 | + - LP: #1279532 - During testing, pass the log dir argument to |
69 | + ubuntu-download-manager. |
70 | + * d/rules: Add override_dh_python3 rule to set shebang line to |
71 | + /usr/bin/python3. (LP: #1283277) |
72 | + * d/patches/lp1284217.patch: Removed; applied upstream. |
73 | + |
74 | + -- Barry Warsaw <barry@ubuntu.com> Wed, 05 Mar 2014 16:29:13 -0500 |
75 | + |
76 | system-image (2.1-0ubuntu4) trusty; urgency=medium |
77 | |
78 | [ Stéphane Graber ] |
79 | |
80 | === removed file 'debian/patches/lp1284217.patch' |
81 | --- debian/patches/lp1284217.patch 2014-02-25 17:43:01 +0000 |
82 | +++ debian/patches/lp1284217.patch 1970-01-01 00:00:00 +0000 |
83 | @@ -1,106 +0,0 @@ |
84 | -Description: Backport of fix for LP: #1284217. This adds an additional |
85 | - UpdateAvailableStatus signal when a second CheckForUpdate is called while an |
86 | - auto-download is in progress, but only if we have cached status available. |
87 | -Origin: http://bazaar.launchpad.net/~ubuntu-system-image/ubuntu-system-image/client/revision/240?start_revid=240 |
88 | -Bug: http://pad.lv/1284217 |
89 | -Forwarded: not-needed |
90 | - |
91 | -=== modified file 'systemimage/dbus.py' |
92 | ---- old/systemimage/dbus.py 2014-02-18 22:31:55 +0000 |
93 | -+++ new/systemimage/dbus.py 2014-02-25 17:27:14 +0000 |
94 | -@@ -131,7 +131,20 @@ |
95 | - # Check-and-acquire the lock. |
96 | - log.info('test and acquire checking lock') |
97 | - if not self._checking.acquire(blocking=False): |
98 | -- # Check is already in progress, so there's nothing more to do. |
99 | -+ # Check is already in progress, so there's nothing more to do. If |
100 | -+ # there's status available (i.e. we are in the auto-downloading |
101 | -+ # phase of the last CFU), then send the status. |
102 | -+ if self._update is not None: |
103 | -+ self.UpdateAvailableStatus( |
104 | -+ self._update.is_available, |
105 | -+ self._downloading, |
106 | -+ self._update.version, |
107 | -+ self._update.size, |
108 | -+ self._update.last_update_date, |
109 | -+ # XXX 2013-08-22 - the u/i cannot currently currently |
110 | -+ # handle the array of dictionaries data type. LP: |
111 | -+ # #1215586 self._update.descriptions, |
112 | -+ "") |
113 | - log.info('checking lock not acquired') |
114 | - return |
115 | - log.info('checking lock acquired') |
116 | - |
117 | -=== modified file 'systemimage/tests/test_dbus.py' |
118 | ---- old/systemimage/tests/test_dbus.py 2014-02-18 20:02:59 +0000 |
119 | -+++ new/systemimage/tests/test_dbus.py 2014-02-25 17:27:14 +0000 |
120 | -@@ -23,13 +23,13 @@ |
121 | - 'TestDBusGetSet', |
122 | - 'TestDBusInfo', |
123 | - 'TestDBusInfoNoDetails', |
124 | -- 'TestDBusLP1277589', |
125 | - 'TestDBusMockFailApply', |
126 | - 'TestDBusMockFailPause', |
127 | - 'TestDBusMockFailResume', |
128 | - 'TestDBusMockNoUpdate', |
129 | - 'TestDBusMockUpdateAutoSuccess', |
130 | - 'TestDBusMockUpdateManualSuccess', |
131 | -+ 'TestDBusMultipleChecksInFlight', |
132 | - 'TestDBusPauseResume', |
133 | - 'TestDBusProgress', |
134 | - 'TestDBusRegressions', |
135 | -@@ -133,6 +133,7 @@ |
136 | - self.schedule(self.iface.CheckForUpdate) |
137 | - |
138 | - def _do_UpdateAvailableStatus(self, signal, path, *args, **kws): |
139 | -+ # We'll keep doing this until we get the UpdateDownloaded signal. |
140 | - self.uas_signals.append(args) |
141 | - self.schedule(self.iface.CheckForUpdate) |
142 | - |
143 | -@@ -1563,7 +1564,7 @@ |
144 | - """) |
145 | - |
146 | - |
147 | --class TestDBusLP1277589(_LiveTesting): |
148 | -+class TestDBusMultipleChecksInFlight(_LiveTesting): |
149 | - def test_multiple_check_for_updates(self): |
150 | - # Log analysis of LP: #1277589 appears to show the following scenario, |
151 | - # reproduced in this test case: |
152 | -@@ -1588,18 +1589,23 @@ |
153 | - # signal, we'll immediately issue *another* CheckForUpdate, which |
154 | - # should run while the auto-download is working. |
155 | - # |
156 | -- # At the end, we should not get another UpdateAvailableStatus signal, |
157 | -- # but we should get the UpdateDownloaded signal. |
158 | -+ # As per LP: #1284217, we will get a second UpdateAvailableStatus |
159 | -+ # signal, since the status is available even while the original |
160 | -+ # request is being downloaded. |
161 | - reactor = DoubleCheckingReactor(self.iface) |
162 | - reactor.run() |
163 | -- self.assertEqual(len(reactor.uas_signals), 1) |
164 | -- (is_available, downloading, available_version, update_size, |
165 | -- last_update_date, |
166 | -- #descriptions, |
167 | -- error_reason) = reactor.uas_signals[0] |
168 | -- self.assertTrue(is_available) |
169 | -- self.assertTrue(downloading) |
170 | -- self.assertEqual(available_version, '1600') |
171 | -- self.assertEqual(update_size, 314572800) |
172 | -- self.assertEqual(last_update_date, 'Unknown') |
173 | -- self.assertEqual(error_reason, '') |
174 | -+ # We need to have received at least 2 signals, but due to timing |
175 | -+ # issues it could possibly be more. |
176 | -+ self.assertGreater(len(reactor.uas_signals), 1) |
177 | -+ # All received signals should have the same information. |
178 | -+ for signal in reactor.uas_signals: |
179 | -+ (is_available, downloading, available_version, update_size, |
180 | -+ last_update_date, |
181 | -+ #descriptions, |
182 | -+ error_reason) = signal |
183 | -+ self.assertTrue(is_available) |
184 | -+ self.assertTrue(downloading) |
185 | -+ self.assertEqual(available_version, '1600') |
186 | -+ self.assertEqual(update_size, 314572800) |
187 | -+ self.assertEqual(last_update_date, 'Unknown') |
188 | -+ self.assertEqual(error_reason, '') |
189 | - |
190 | |
191 | === modified file 'debian/patches/series' |
192 | --- debian/patches/series 2014-02-25 17:40:13 +0000 |
193 | +++ debian/patches/series 2014-03-05 23:45:27 +0000 |
194 | @@ -1,1 +0,0 @@ |
195 | -lp1284217.patch |
196 | |
197 | === modified file 'debian/rules' |
198 | --- debian/rules 2014-02-25 02:25:39 +0000 |
199 | +++ debian/rules 2014-03-05 23:45:27 +0000 |
200 | @@ -32,6 +32,9 @@ |
201 | python$* setup.py install --root=$(CURDIR)/debian/tmp \ |
202 | --install-layout=deb |
203 | |
204 | +override_dh_python3: |
205 | + dh_python3 --shebang=/usr/bin/python3 |
206 | + |
207 | override_dh_auto_install: $(PYTHON3:%=install-python%) |
208 | |
209 | override_dh_install: |
210 | |
211 | === modified file 'ini-manpage.rst' |
212 | --- ini-manpage.rst 2014-02-20 23:03:24 +0000 |
213 | +++ ini-manpage.rst 2014-03-05 23:45:27 +0000 |
214 | @@ -49,10 +49,14 @@ |
215 | provide both HTTP and HTTPS services. |
216 | |
217 | http_port |
218 | - The port for HTTP connections. |
219 | + The port for HTTP connections. This is an integer, or the string |
220 | + ``disabled`` if you wish to disable all HTTP connections and use only |
221 | + HTTPS. It is an error to disable both the HTTP and HTTPS services. |
222 | |
223 | https_port |
224 | - The port for HTTPS connections. |
225 | + The port for HTTPS connections. This is an integer, or the string |
226 | + ``disabled`` if you wish to disable all HTTPS connections and use only |
227 | + HTTP. It is an error to disable both the HTTP and HTTPS services. |
228 | |
229 | channel |
230 | The upgrade channel. |
231 | |
232 | === modified file 'setup.cfg' |
233 | --- setup.cfg 2014-02-20 23:03:24 +0000 |
234 | +++ setup.cfg 2014-03-05 23:45:27 +0000 |
235 | @@ -4,7 +4,7 @@ |
236 | logging-filter = systemimage |
237 | |
238 | [egg_info] |
239 | +tag_date = 0 |
240 | tag_svn_revision = 0 |
241 | -tag_date = 0 |
242 | tag_build = |
243 | |
244 | |
245 | === modified file 'system_image.egg-info/PKG-INFO' |
246 | --- system_image.egg-info/PKG-INFO 2014-02-20 23:03:09 +0000 |
247 | +++ system_image.egg-info/PKG-INFO 2014-03-05 23:45:27 +0000 |
248 | @@ -1,6 +1,6 @@ |
249 | Metadata-Version: 1.0 |
250 | Name: system-image |
251 | -Version: 2.1 |
252 | +Version: 2.2 |
253 | Summary: Ubuntu System Image Based Upgrades |
254 | Home-page: UNKNOWN |
255 | Author: Barry Warsaw |
256 | |
257 | === modified file 'system_image.egg-info/SOURCES.txt' |
258 | --- system_image.egg-info/SOURCES.txt 2014-02-20 23:03:09 +0000 |
259 | +++ system_image.egg-info/SOURCES.txt 2014-03-05 23:45:27 +0000 |
260 | @@ -96,6 +96,10 @@ |
261 | systemimage/tests/data/config_02.ini |
262 | systemimage/tests/data/config_03.ini |
263 | systemimage/tests/data/config_04.ini |
264 | +systemimage/tests/data/config_05.ini |
265 | +systemimage/tests/data/config_06.ini |
266 | +systemimage/tests/data/config_07.ini |
267 | +systemimage/tests/data/config_08.ini |
268 | systemimage/tests/data/dbus-system.conf.in |
269 | systemimage/tests/data/device-signing.gpg |
270 | systemimage/tests/data/expired_cert.pem |
271 | |
272 | === modified file 'system_image.egg-info/entry_points.txt' |
273 | --- system_image.egg-info/entry_points.txt 2014-02-20 23:03:09 +0000 |
274 | +++ system_image.egg-info/entry_points.txt 2014-03-05 23:45:27 +0000 |
275 | @@ -1,4 +1,4 @@ |
276 | [console_scripts] |
277 | +system-image-dbus = systemimage.service:main |
278 | system-image-cli = systemimage.main:main |
279 | -system-image-dbus = systemimage.service:main |
280 | |
281 | |
282 | === modified file 'systemimage/api.py' |
283 | --- systemimage/api.py 2014-02-20 23:03:24 +0000 |
284 | +++ systemimage/api.py 2014-03-05 23:45:27 +0000 |
285 | @@ -22,15 +22,20 @@ |
286 | ] |
287 | |
288 | |
289 | +import logging |
290 | + |
291 | from systemimage.helpers import last_update_date |
292 | from systemimage.state import State |
293 | |
294 | +log = logging.getLogger('systemimage') |
295 | + |
296 | |
297 | class Update: |
298 | """A representation of the available update.""" |
299 | |
300 | - def __init__(self, winners): |
301 | + def __init__(self, winners=None, error=''): |
302 | self._winners = [] if winners is None else winners |
303 | + self.error = error |
304 | |
305 | @property |
306 | def is_available(self): |
307 | @@ -93,8 +98,16 @@ |
308 | :rtype: bool |
309 | """ |
310 | if self._update is None: |
311 | - self._state.run_until('download_files') |
312 | - self._update = Update(self._state.winner) |
313 | + try: |
314 | + self._state.run_until('download_files') |
315 | + except Exception as error: |
316 | + # Rather than letting this percolate up, eventually reaching |
317 | + # the GLib main loop and thus triggering apport, Let's log the |
318 | + # error and set the relevant information in the class. |
319 | + log.exception('check_for_update failed') |
320 | + self._update = Update(error=str(error)) |
321 | + else: |
322 | + self._update = Update(self._state.winner) |
323 | return self._update |
324 | |
325 | def download(self): |
326 | |
327 | === modified file 'systemimage/bag.py' |
328 | --- systemimage/bag.py 2014-02-20 23:03:24 +0000 |
329 | +++ systemimage/bag.py 2014-03-05 23:45:27 +0000 |
330 | @@ -33,6 +33,7 @@ |
331 | return value |
332 | return identity |
333 | |
334 | + |
335 | def make_converter(original): |
336 | converters = defaultdict(default) |
337 | if original is not None: |
338 | @@ -45,6 +46,14 @@ |
339 | self._converters = make_converter(converters) |
340 | self.__original__ = {} |
341 | self.__untranslated__ = {} |
342 | + self._load_items(kws) |
343 | + |
344 | + def update(self, *, converters=None, **kws): |
345 | + if converters is not None: |
346 | + self._converters.update(converters) |
347 | + self._load_items(kws) |
348 | + |
349 | + def _load_items(self, kws): |
350 | for key, value in kws.items(): |
351 | self.__original__[key] = value |
352 | safe_key, converted_value = self._normalize_key_value(key, value) |
353 | @@ -68,7 +77,6 @@ |
354 | def __setitem__(self, key, value): |
355 | if key in self.__original__: |
356 | raise ValueError('Attributes are immutable: {}'.format(key)) |
357 | - self.__original__[key] = value |
358 | safe_key, converted_value = self._normalize_key_value(key, value) |
359 | self.__dict__[safe_key] = converted_value |
360 | self.__untranslated__[key] = converted_value |
361 | |
362 | === modified file 'systemimage/config.py' |
363 | --- systemimage/config.py 2014-02-20 23:03:24 +0000 |
364 | +++ systemimage/config.py 2014-03-05 23:45:27 +0000 |
365 | @@ -17,6 +17,7 @@ |
366 | |
367 | __all__ = [ |
368 | 'Configuration', |
369 | + 'DISABLED', |
370 | 'config', |
371 | ] |
372 | |
373 | @@ -31,14 +32,28 @@ |
374 | Bag, as_loglevel, as_object, as_timedelta, makedirs, temporary_directory) |
375 | |
376 | |
377 | +DISABLED = object() |
378 | + |
379 | + |
380 | def expand_path(path): |
381 | return os.path.abspath(os.path.expanduser(path)) |
382 | |
383 | |
384 | +def port_value_converter(value): |
385 | + if value.lower() in ('disabled', 'disable'): |
386 | + return DISABLED |
387 | + result = int(value) |
388 | + if result < 0: |
389 | + raise ValueError(value) |
390 | + return result |
391 | + |
392 | + |
393 | class Configuration: |
394 | def __init__(self): |
395 | # Defaults. |
396 | self.config_file = None |
397 | + self.service = Bag() |
398 | + self.system = Bag() |
399 | ini_path = resource_filename('systemimage.data', 'client.ini') |
400 | self.load(ini_path) |
401 | self._override = False |
402 | @@ -64,32 +79,57 @@ |
403 | if files_read != [path]: |
404 | raise FileNotFoundError(path) |
405 | self.config_file = path |
406 | - self.service = Bag(converters=dict(http_port=int, |
407 | - https_port=int, |
408 | - build_number=int), |
409 | + self.service.update(converters=dict(http_port=port_value_converter, |
410 | + https_port=port_value_converter, |
411 | + build_number=int), |
412 | **parser['service']) |
413 | + if (self.service.http_port is DISABLED and |
414 | + self.service.https_port is DISABLED): |
415 | + raise ValueError('Cannot disable both http and https ports') |
416 | # Construct the HTTP and HTTPS base urls, which most applications will |
417 | - # actually use. |
418 | + # actually use. We do this in two steps, in order to support |
419 | + # disabling one or the other (but not both) protocols. |
420 | if self.service.http_port == 80: |
421 | - self.service['http_base'] = 'http://{}'.format(self.service.base) |
422 | + http_base = 'http://{}'.format(self.service.base) |
423 | + elif self.service.http_port is DISABLED: |
424 | + http_base = None |
425 | else: |
426 | - self.service['http_base'] = 'http://{}:{}'.format( |
427 | + http_base = 'http://{}:{}'.format( |
428 | self.service.base, self.service.http_port) |
429 | + # HTTPS. |
430 | if self.service.https_port == 443: |
431 | - self.service['https_base'] = 'https://{}'.format(self.service.base) |
432 | + https_base = 'https://{}'.format(self.service.base) |
433 | + elif self.service.https_port is DISABLED: |
434 | + https_base = None |
435 | else: |
436 | - self.service['https_base'] = 'https://{}:{}'.format( |
437 | + https_base = 'https://{}:{}'.format( |
438 | self.service.base, self.service.https_port) |
439 | + # Sanity check and final settings. |
440 | + if http_base is None: |
441 | + assert https_base is not None |
442 | + http_base = https_base |
443 | + if https_base is None: |
444 | + assert http_base is not None |
445 | + https_base = http_base |
446 | + self.service['http_base'] = http_base |
447 | + self.service['https_base'] = https_base |
448 | + try: |
449 | + self.system.update(converters=dict(timeout=as_timedelta, |
450 | + build_file=expand_path, |
451 | + loglevel=as_loglevel, |
452 | + settings_db=expand_path, |
453 | + tempdir=expand_path), |
454 | + **parser['system']) |
455 | + except KeyError: |
456 | + # If we're overriding via a channel.ini file, it's okay if the |
457 | + # [system] section is missing. However, the main configuration |
458 | + # ini file must include all sections. |
459 | + if not override: |
460 | + raise |
461 | # Short-circuit, since we're loading a channel.ini file. |
462 | self._override = override |
463 | if override: |
464 | return |
465 | - self.system = Bag(converters=dict(timeout=as_timedelta, |
466 | - build_file=expand_path, |
467 | - loglevel=as_loglevel, |
468 | - settings_db=expand_path, |
469 | - tempdir=expand_path), |
470 | - **parser['system']) |
471 | self.gpg = Bag(**parser['gpg']) |
472 | self.updater = Bag(**parser['updater']) |
473 | self.hooks = Bag(converters=dict(device=as_object, |
474 | |
475 | === modified file 'systemimage/dbus.py' |
476 | --- systemimage/dbus.py 2014-02-20 23:03:24 +0000 |
477 | +++ systemimage/dbus.py 2014-03-05 23:45:27 +0000 |
478 | @@ -82,7 +82,7 @@ |
479 | |
480 | def _check_for_update(self): |
481 | # Asynchronous method call. |
482 | - log.info('Checking for update') |
483 | + log.info('Enter _check_for_update()') |
484 | self._update = self._api.check_for_update() |
485 | # Do we have an update and can we auto-download it? |
486 | downloading = False |
487 | @@ -93,11 +93,8 @@ |
488 | if auto in ('1', '2'): |
489 | # XXX When we have access to the download service, we can |
490 | # check if we're on the wifi (auto == '1'). |
491 | - GLib.timeout_add(50, self._download, self._checking.release) |
492 | + GLib.timeout_add(50, self._download) |
493 | downloading = True |
494 | - else: |
495 | - log.info('release checking lock from _check_for_update()') |
496 | - self._checking.release() |
497 | self.UpdateAvailableStatus( |
498 | self._update.is_available, |
499 | downloading, |
500 | @@ -107,7 +104,7 @@ |
501 | # XXX 2013-08-22 - the u/i cannot currently currently handle the |
502 | # array of dictionaries data type. LP: #1215586 |
503 | #self._update.descriptions, |
504 | - "") |
505 | + self._update.error) |
506 | # Stop GLib from calling this method again. |
507 | return False |
508 | |
509 | @@ -131,7 +128,20 @@ |
510 | # Check-and-acquire the lock. |
511 | log.info('test and acquire checking lock') |
512 | if not self._checking.acquire(blocking=False): |
513 | - # Check is already in progress, so there's nothing more to do. |
514 | + # Check is already in progress, so there's nothing more to do. If |
515 | + # there's status available (i.e. we are in the auto-downloading |
516 | + # phase of the last CFU), then send the status. |
517 | + if self._update is not None: |
518 | + self.UpdateAvailableStatus( |
519 | + self._update.is_available, |
520 | + self._downloading, |
521 | + self._update.version, |
522 | + self._update.size, |
523 | + self._update.last_update_date, |
524 | + # XXX 2013-08-22 - the u/i cannot currently currently |
525 | + # handle the array of dictionaries data type. LP: |
526 | + # #1215586 self._update.descriptions, |
527 | + "") |
528 | log.info('checking lock not acquired') |
529 | return |
530 | log.info('checking lock acquired') |
531 | @@ -153,7 +163,7 @@ |
532 | eta = 0 |
533 | self.UpdateProgress(percentage, eta) |
534 | |
535 | - def _download(self, release_checking=None): |
536 | + def _download(self): |
537 | if self._downloading and self._paused: |
538 | self._api.resume() |
539 | self._paused = False |
540 | @@ -168,7 +178,8 @@ |
541 | if self._failure_count > 0: |
542 | self._failure_count += 1 |
543 | self.UpdateFailed(self._failure_count, self._last_error) |
544 | - log.info('Update failure count: {}', self._failure_count) |
545 | + log.info('Update failures: {}; last error: {}', |
546 | + self._failure_count, self._last_error) |
547 | return |
548 | self._downloading = True |
549 | log.info('Update is downloading') |
550 | @@ -193,10 +204,7 @@ |
551 | self._rebootable = True |
552 | self._downloading = False |
553 | log.info('release checking lock from _download()') |
554 | - if release_checking is not None: |
555 | - # We were auto-downloading, so we now have to release the checking |
556 | - # lock. If we were manually downloading, there would be no lock. |
557 | - release_checking() |
558 | + self._checking.release() |
559 | # Stop GLib from calling this method again. |
560 | return False |
561 | |
562 | @@ -228,9 +236,17 @@ |
563 | """Cancel a download.""" |
564 | self._loop.keepalive() |
565 | self._api.cancel() |
566 | + # If we're holding the checking lock, release it. |
567 | + try: |
568 | + self._checking.release() |
569 | + log.info('release checking lock from CancelUpdate()') |
570 | + except RuntimeError: |
571 | + # We're not holding the lock. |
572 | + pass |
573 | # We're now in a failure state until the next CheckForUpdate. |
574 | self._failure_count += 1 |
575 | self._last_error = 'Canceled' |
576 | + log.info('CancelUpdate() called') |
577 | # Only send this signal if we were in the middle of downloading. |
578 | if self._downloading: |
579 | self.UpdateFailed(self._failure_count, self._last_error) |
580 | |
581 | === modified file 'systemimage/download.py' |
582 | --- systemimage/download.py 2014-02-20 23:03:24 +0000 |
583 | +++ systemimage/download.py 2014-03-05 23:45:27 +0000 |
584 | @@ -22,16 +22,12 @@ |
585 | ] |
586 | |
587 | |
588 | -import os |
589 | import dbus |
590 | import logging |
591 | -import tempfile |
592 | |
593 | -from contextlib import ExitStack |
594 | from io import StringIO |
595 | from pprint import pformat |
596 | from systemimage.config import config |
597 | -from systemimage.helpers import safe_remove |
598 | from systemimage.reactor import Reactor |
599 | |
600 | |
601 | @@ -102,7 +98,12 @@ |
602 | def _do_progress(self, signal, path, received, total): |
603 | self._print('PROGRESS:', received, total) |
604 | if self._callback is not None: |
605 | - self._callback(received, total) |
606 | + # Be defensive, so yes, use a bare except. If an exception occurs |
607 | + # in the callback, log it, but continue onward. |
608 | + try: |
609 | + self._callback(received, total) |
610 | + except: |
611 | + log.exception('Exception in progress callback') |
612 | |
613 | def _do_canceled(self, signal, path, canceled): |
614 | # Why would we get this signal if it *wasn't* canceled? Anyway, |
615 | @@ -142,6 +143,9 @@ |
616 | self._queued_cancel = False |
617 | self.callback = callback |
618 | |
619 | + def __repr__(self): |
620 | + return '<DBusDownloadManager at 0x{:x}>'.format(id(self)) |
621 | + |
622 | def get_files(self, downloads, *, pausable=False): |
623 | """Download a bunch of files concurrently. |
624 | |
625 | @@ -191,29 +195,12 @@ |
626 | iface = dbus.Interface(service, MANAGER_INTERFACE) |
627 | # Better logging of the requested downloads. |
628 | fp = StringIO() |
629 | - print('Requesting group download:', file=fp) |
630 | + print('[0x{:x}] Requesting group download:'.format(id(self)), file=fp) |
631 | for url, dst in downloads: |
632 | print('\t{} -> {}'.format(url, dst), file=fp) |
633 | log.info('{}'.format(fp.getvalue())) |
634 | - # As a workaround for LP: #1277589, ask u-d-m to download the files to |
635 | - # .tmp files, and if they succeed, then atomically move them into |
636 | - # their real location. |
637 | - renames = [] |
638 | - requests = [] |
639 | - for url, dst in downloads: |
640 | - head, tail = os.path.split(dst) |
641 | - fd, path = tempfile.mkstemp(suffix='.tmp', prefix='', dir=head) |
642 | - os.close(fd) |
643 | - renames.append((path, dst)) |
644 | - requests.append((url, path, '')) |
645 | - # mkstemp() creates the file system path, but if the files exist when |
646 | - # the group download is requested, ubuntu-download-manager will |
647 | - # complain and return an error. So, delete all temporary files now so |
648 | - # udm has a clear path to download to. |
649 | - for path, dst in renames: |
650 | - os.remove(path) |
651 | object_path = iface.createDownloadGroup( |
652 | - requests, # The temporary requests. |
653 | + [(url, dst, '') for url, dst in downloads], |
654 | '', # No hashes yet. |
655 | False, # Don't allow GSM yet. |
656 | # https://bugs.freedesktop.org/show_bug.cgi?id=55594 |
657 | @@ -223,13 +210,13 @@ |
658 | self._iface = dbus.Interface(download, OBJECT_INTERFACE) |
659 | reactor = DownloadReactor(bus, self.callback, pausable) |
660 | reactor.schedule(self._iface.start) |
661 | - log.info('Running group download reactor') |
662 | + log.info('[0x{:x}] Running group download reactor', id(self)) |
663 | reactor.run() |
664 | # This download is complete so the object path is no longer |
665 | # applicable. Setting this to None will cause subsequent cancels to |
666 | # be queued. |
667 | self._iface = None |
668 | - log.info('Group download reactor done') |
669 | + log.info('[0x{:x}] Group download reactor done', id(self)) |
670 | if reactor.error is not None: |
671 | log.error('Reactor error: {}'.format(reactor.error)) |
672 | if reactor.canceled: |
673 | @@ -241,19 +228,6 @@ |
674 | raise Canceled |
675 | if reactor.timed_out: |
676 | raise TimeoutError |
677 | - # Now that everything succeeded, rename the temporary files. Just to |
678 | - # be extra cautious, set up a context manager to safely remove all |
679 | - # temporary files in case of an error. If there are no errors, then |
680 | - # there will be nothing to remove. |
681 | - with ExitStack() as resources: |
682 | - for tmp, dst in renames: |
683 | - resources.callback(safe_remove, tmp) |
684 | - for tmp, dst in renames: |
685 | - os.rename(tmp, dst) |
686 | - # We only get here if all the renames succeeded, so there will be |
687 | - # no temporary files to remove, so we can throw away the new |
688 | - # ExitStack, which holds all the removals. |
689 | - resources.pop_all() |
690 | |
691 | def cancel(self): |
692 | """Cancel any current downloads.""" |
693 | |
694 | === modified file 'systemimage/main.py' |
695 | --- systemimage/main.py 2014-02-20 23:03:24 +0000 |
696 | +++ systemimage/main.py 2014-03-05 23:45:27 +0000 |
697 | @@ -209,7 +209,11 @@ |
698 | state = State(candidate_filter=candidate_filter) |
699 | state.downloader.callback = callback |
700 | if args.dry_run: |
701 | - state.run_until('download_files') |
702 | + try: |
703 | + state.run_until('download_files') |
704 | + except Exception: |
705 | + log.exception('system-image-cli exception') |
706 | + return 1 |
707 | # Say -c <no-such-channel> was given. This will fail. |
708 | if state.winner is None or len(state.winner) == 0: |
709 | print('Already up-to-date') |
710 | @@ -236,7 +240,7 @@ |
711 | list(state) |
712 | except KeyboardInterrupt: |
713 | return 0 |
714 | - except: |
715 | + except Exception: |
716 | log.exception('system-image-cli exception') |
717 | return 1 |
718 | else: |
719 | |
720 | === modified file 'systemimage/scores.py' |
721 | --- systemimage/scores.py 2014-02-20 23:03:24 +0000 |
722 | +++ systemimage/scores.py 2014-03-05 23:45:27 +0000 |
723 | @@ -138,7 +138,7 @@ |
724 | reboots = sum(1 for image in path |
725 | if getattr(image, 'bootme', False)) |
726 | candidate_data.append((build, size, reboots, path)) |
727 | - max_build = build if build > max_build else max_build |
728 | + max_build = max(build, max_build) |
729 | min_size = (size if (min_size is None or size < min_size) |
730 | else min_size) |
731 | # Score the candidates. Any path that doesn't leave you at the |
732 | |
733 | === modified file 'systemimage/state.py' |
734 | --- systemimage/state.py 2014-02-20 23:03:24 +0000 |
735 | +++ systemimage/state.py 2014-03-05 23:45:27 +0000 |
736 | @@ -143,11 +143,7 @@ |
737 | except (StopIteration, IndexError): |
738 | # We're done. |
739 | break |
740 | - try: |
741 | - step() |
742 | - except: |
743 | - log.exception('uncaught exception in state machine') |
744 | - raise |
745 | + step() |
746 | self._debug_step += 1 |
747 | if name[1:] == stop_after: |
748 | break |
749 | @@ -171,11 +167,7 @@ |
750 | # skip this step. |
751 | self._next.appendleft(step) |
752 | break |
753 | - try: |
754 | - step() |
755 | - except: |
756 | - log.exception('uncaught exception in state machine') |
757 | - raise |
758 | + step() |
759 | self._debug_step += 1 |
760 | |
761 | def _cleanup(self): |
762 | |
763 | === modified file 'systemimage/testing/controller.py' |
764 | --- systemimage/testing/controller.py 2014-02-20 23:03:24 +0000 |
765 | +++ systemimage/testing/controller.py 2014-03-05 23:45:27 +0000 |
766 | @@ -117,7 +117,8 @@ |
767 | stop_system_image, |
768 | ), |
769 | ('com.canonical.applications.Downloader', |
770 | - DLSERVICE + ' {self.certs} -disable-timeout -stoppable', |
771 | + DLSERVICE + |
772 | + ' {self.certs} -disable-timeout -stoppable -log-dir {self.tmpdir}', |
773 | start_downloader, |
774 | stop_downloader, |
775 | ), |
776 | |
777 | === modified file 'systemimage/testing/dbus.py' |
778 | --- systemimage/testing/dbus.py 2014-02-20 23:03:24 +0000 |
779 | +++ systemimage/testing/dbus.py 2014-03-05 23:45:27 +0000 |
780 | @@ -31,8 +31,6 @@ |
781 | from systemimage.helpers import makedirs, safe_remove |
782 | from unittest.mock import patch |
783 | |
784 | -from systemimage.testing.helpers import debug |
785 | - |
786 | |
787 | SPACE = ' ' |
788 | SIGNAL_DELAY_SECS = 5 |
789 | |
790 | === modified file 'systemimage/testing/helpers.py' |
791 | --- systemimage/testing/helpers.py 2014-02-20 23:03:24 +0000 |
792 | +++ systemimage/testing/helpers.py 2014-03-05 23:45:27 +0000 |
793 | @@ -17,6 +17,7 @@ |
794 | |
795 | __all__ = [ |
796 | 'ServerTestBase', |
797 | + 'chmod', |
798 | 'configuration', |
799 | 'copy', |
800 | 'data_path', |
801 | @@ -382,6 +383,16 @@ |
802 | os.environ[name] = old_value |
803 | |
804 | |
805 | +@contextmanager |
806 | +def chmod(path, new_mode): |
807 | + old_mode = os.stat(path).st_mode |
808 | + try: |
809 | + os.chmod(path, new_mode) |
810 | + yield |
811 | + finally: |
812 | + os.chmod(path, old_mode) |
813 | + |
814 | + |
815 | def touch_build(version, timestamp=None): |
816 | # LP: #1220238 - assert that no old-style version numbers are being used. |
817 | assert 0 <= version < (1 << 16), ( |
818 | |
819 | === modified file 'systemimage/tests/data/channel_02.ini' |
820 | --- systemimage/tests/data/channel_02.ini 2014-01-30 15:41:03 +0000 |
821 | +++ systemimage/tests/data/channel_02.ini 2014-03-05 23:45:27 +0000 |
822 | @@ -7,3 +7,6 @@ |
823 | |
824 | [system] |
825 | build_file: /etc/path/to/alternative/build-file |
826 | + |
827 | +[dbus] |
828 | +lifetime: 1h |
829 | |
830 | === added file 'systemimage/tests/data/config_05.ini' |
831 | --- systemimage/tests/data/config_05.ini 1970-01-01 00:00:00 +0000 |
832 | +++ systemimage/tests/data/config_05.ini 2014-03-05 23:45:27 +0000 |
833 | @@ -0,0 +1,36 @@ |
834 | +# Configuration file for specifying relatively static information about the |
835 | +# upgrade resolution process. |
836 | + |
837 | +[service] |
838 | +base: phablet.example.com |
839 | +# Disabled port |
840 | +http_port: disabled |
841 | +https_port: 80443 |
842 | +channel: stable |
843 | +build_number: 0 |
844 | + |
845 | +[system] |
846 | +timeout: 10s |
847 | +build_file: /etc/ubuntu-build |
848 | +tempdir: /tmp |
849 | +logfile: /var/log/system-image/client.log |
850 | +loglevel: error |
851 | +settings_db: /var/lib/phablet/settings.db |
852 | + |
853 | +[gpg] |
854 | +archive_master: /etc/phablet/archive-master.tar.xz |
855 | +image_master: /etc/phablet/image-master.tar.xz |
856 | +image_signing: /var/lib/phablet/image-signing.tar.xz |
857 | +device_signing: /var/lib/phablet/device-signing.tar.xz |
858 | + |
859 | +[updater] |
860 | +cache_partition: /android/cache |
861 | +data_partition: /var/lib/phablet/updater |
862 | + |
863 | +[hooks] |
864 | +device: systemimage.device.SystemProperty |
865 | +scorer: systemimage.scores.WeightedScorer |
866 | +reboot: systemimage.reboot.Reboot |
867 | + |
868 | +[dbus] |
869 | +lifetime: 3s |
870 | |
871 | === added file 'systemimage/tests/data/config_06.ini' |
872 | --- systemimage/tests/data/config_06.ini 1970-01-01 00:00:00 +0000 |
873 | +++ systemimage/tests/data/config_06.ini 2014-03-05 23:45:27 +0000 |
874 | @@ -0,0 +1,36 @@ |
875 | +# Configuration file for specifying relatively static information about the |
876 | +# upgrade resolution process. |
877 | + |
878 | +[service] |
879 | +base: phablet.example.com |
880 | +# Disabled port |
881 | +http_port: 80 |
882 | +https_port: disabled |
883 | +channel: stable |
884 | +build_number: 0 |
885 | + |
886 | +[system] |
887 | +timeout: 10s |
888 | +build_file: /etc/ubuntu-build |
889 | +tempdir: /tmp |
890 | +logfile: /var/log/system-image/client.log |
891 | +loglevel: error |
892 | +settings_db: /var/lib/phablet/settings.db |
893 | + |
894 | +[gpg] |
895 | +archive_master: /etc/phablet/archive-master.tar.xz |
896 | +image_master: /etc/phablet/image-master.tar.xz |
897 | +image_signing: /var/lib/phablet/image-signing.tar.xz |
898 | +device_signing: /var/lib/phablet/device-signing.tar.xz |
899 | + |
900 | +[updater] |
901 | +cache_partition: /android/cache |
902 | +data_partition: /var/lib/phablet/updater |
903 | + |
904 | +[hooks] |
905 | +device: systemimage.device.SystemProperty |
906 | +scorer: systemimage.scores.WeightedScorer |
907 | +reboot: systemimage.reboot.Reboot |
908 | + |
909 | +[dbus] |
910 | +lifetime: 3s |
911 | |
912 | === added file 'systemimage/tests/data/config_07.ini' |
913 | --- systemimage/tests/data/config_07.ini 1970-01-01 00:00:00 +0000 |
914 | +++ systemimage/tests/data/config_07.ini 2014-03-05 23:45:27 +0000 |
915 | @@ -0,0 +1,36 @@ |
916 | +# Configuration file for specifying relatively static information about the |
917 | +# upgrade resolution process. |
918 | + |
919 | +[service] |
920 | +base: phablet.example.com |
921 | +# Both ports disabled |
922 | +http_port: disabled |
923 | +https_port: disabled |
924 | +channel: stable |
925 | +build_number: 0 |
926 | + |
927 | +[system] |
928 | +timeout: 10s |
929 | +build_file: /etc/ubuntu-build |
930 | +tempdir: /tmp |
931 | +logfile: /var/log/system-image/client.log |
932 | +loglevel: error |
933 | +settings_db: /var/lib/phablet/settings.db |
934 | + |
935 | +[gpg] |
936 | +archive_master: /etc/phablet/archive-master.tar.xz |
937 | +image_master: /etc/phablet/image-master.tar.xz |
938 | +image_signing: /var/lib/phablet/image-signing.tar.xz |
939 | +device_signing: /var/lib/phablet/device-signing.tar.xz |
940 | + |
941 | +[updater] |
942 | +cache_partition: /android/cache |
943 | +data_partition: /var/lib/phablet/updater |
944 | + |
945 | +[hooks] |
946 | +device: systemimage.device.SystemProperty |
947 | +scorer: systemimage.scores.WeightedScorer |
948 | +reboot: systemimage.reboot.Reboot |
949 | + |
950 | +[dbus] |
951 | +lifetime: 3s |
952 | |
953 | === added file 'systemimage/tests/data/config_08.ini' |
954 | --- systemimage/tests/data/config_08.ini 1970-01-01 00:00:00 +0000 |
955 | +++ systemimage/tests/data/config_08.ini 2014-03-05 23:45:27 +0000 |
956 | @@ -0,0 +1,36 @@ |
957 | +# Configuration file for specifying relatively static information about the |
958 | +# upgrade resolution process. |
959 | + |
960 | +[service] |
961 | +base: phablet.example.com |
962 | +# Negative ports are not allowed. |
963 | +http_port: 80 |
964 | +https_port: -1 |
965 | +channel: stable |
966 | +build_number: 0 |
967 | + |
968 | +[system] |
969 | +timeout: 10s |
970 | +build_file: /etc/ubuntu-build |
971 | +tempdir: /tmp |
972 | +logfile: /var/log/system-image/client.log |
973 | +loglevel: error |
974 | +settings_db: /var/lib/phablet/settings.db |
975 | + |
976 | +[gpg] |
977 | +archive_master: /etc/phablet/archive-master.tar.xz |
978 | +image_master: /etc/phablet/image-master.tar.xz |
979 | +image_signing: /var/lib/phablet/image-signing.tar.xz |
980 | +device_signing: /var/lib/phablet/device-signing.tar.xz |
981 | + |
982 | +[updater] |
983 | +cache_partition: /android/cache |
984 | +data_partition: /var/lib/phablet/updater |
985 | + |
986 | +[hooks] |
987 | +device: systemimage.device.SystemProperty |
988 | +scorer: systemimage.scores.WeightedScorer |
989 | +reboot: systemimage.reboot.Reboot |
990 | + |
991 | +[dbus] |
992 | +lifetime: 3s |
993 | |
994 | === modified file 'systemimage/tests/test_api.py' |
995 | --- systemimage/tests/test_api.py 2014-02-20 23:03:24 +0000 |
996 | +++ systemimage/tests/test_api.py 2014-03-05 23:45:27 +0000 |
997 | @@ -28,11 +28,12 @@ |
998 | from datetime import datetime, timedelta |
999 | from gi.repository import GLib |
1000 | from systemimage.api import Mediator |
1001 | -from systemimage.config import config |
1002 | +from systemimage.config import Configuration, config |
1003 | from systemimage.download import Canceled |
1004 | from systemimage.gpg import SignatureError |
1005 | from systemimage.testing.helpers import ( |
1006 | - ServerTestBase, configuration, copy, setup_index, sign, touch_build) |
1007 | + ServerTestBase, chmod, configuration, copy, setup_index, sign, |
1008 | + touch_build) |
1009 | from unittest.mock import patch |
1010 | |
1011 | |
1012 | @@ -269,3 +270,16 @@ |
1013 | self.assertEqual(len(pauses), 1) |
1014 | self.assertEqual(len(resumes), 1) |
1015 | self.assertGreaterEqual(resumes[0] - pauses[0], timedelta(seconds=2.5)) |
1016 | + |
1017 | + @configuration |
1018 | + def test_state_machine_exceptions(self, ini_file): |
1019 | + # An exception in the state machine captures the exception and returns |
1020 | + # an error string in the Update instance. |
1021 | + self._setup_server_keyrings() |
1022 | + config = Configuration() |
1023 | + config.load(ini_file) |
1024 | + with chmod(config.updater.cache_partition, 0): |
1025 | + update = Mediator().check_for_update() |
1026 | + # There's no winning path, but there is an error. |
1027 | + self.assertFalse(update.is_available) |
1028 | + self.assertIn('Permission denied', update.error) |
1029 | |
1030 | === modified file 'systemimage/tests/test_bag.py' |
1031 | --- systemimage/tests/test_bag.py 2014-02-20 23:03:24 +0000 |
1032 | +++ systemimage/tests/test_bag.py 2014-03-05 23:45:27 +0000 |
1033 | @@ -29,52 +29,113 @@ |
1034 | |
1035 | class TestBag(unittest.TestCase): |
1036 | def test_simple(self): |
1037 | + # Initialize a bag; its attributes are the keywords of the ctor. |
1038 | bag = Bag(a=1, b=2, c=3) |
1039 | self.assertEqual(bag.a, 1) |
1040 | self.assertEqual(bag.b, 2) |
1041 | self.assertEqual(bag.c, 3) |
1042 | |
1043 | def test_dash_translation(self): |
1044 | + # Dashes in keys get turned into underscore in attributes. |
1045 | bag = Bag(**{'a-b': 1, 'c-d': 2, 'e-f': 3}) |
1046 | self.assertEqual(bag.a_b, 1) |
1047 | self.assertEqual(bag.c_d, 2) |
1048 | self.assertEqual(bag.e_f, 3) |
1049 | |
1050 | def test_dash_literal_access(self): |
1051 | + # For keys with dashes, the original name is preserved in getitem. |
1052 | bag = Bag(**{'a-b': 1, 'c-d': 2, 'e-f': 3}) |
1053 | self.assertEqual(bag['a-b'], 1) |
1054 | self.assertEqual(bag['c-d'], 2) |
1055 | self.assertEqual(bag['e-f'], 3) |
1056 | |
1057 | def test_keyword_translation(self): |
1058 | + # Python keywords get a trailing underscore. |
1059 | bag = Bag(**{'global': 1, 'with': 2, 'import': 3}) |
1060 | self.assertEqual(bag.global_, 1) |
1061 | self.assertEqual(bag.with_, 2) |
1062 | self.assertEqual(bag.import_, 3) |
1063 | |
1064 | def test_repr(self): |
1065 | + # The repr of a bag includes its translated keys. |
1066 | bag = Bag(**{'a-b': 1, 'global': 2, 'foo': 3}) |
1067 | self.assertEqual(repr(bag), '<Bag: a_b, foo, global_>') |
1068 | |
1069 | def test_original(self): |
1070 | + # There's a magical attribute containing the original ctor arguments. |
1071 | source = {'a-b': 1, 'global': 2, 'foo': 3} |
1072 | bag = Bag(**source) |
1073 | self.assertEqual(bag.__original__, source) |
1074 | |
1075 | def test_add_key(self): |
1076 | + # We can add new keys/attributes via setitem. |
1077 | bag = Bag(a=1, b=2, c=3) |
1078 | bag['d'] = bag.b + bag.c |
1079 | self.assertEqual(bag.d, 5) |
1080 | |
1081 | def test_add_existing_key(self): |
1082 | + # A key set in the original ctor cannot be changed. |
1083 | bag = Bag(a=1, b=2, c=3) |
1084 | self.assertRaises(ValueError, setitem, bag, 'b', 5) |
1085 | self.assertEqual(bag.b, 2) |
1086 | |
1087 | + def test_add_new_key(self): |
1088 | + # A key added by setitem can be changed. |
1089 | + bag = Bag(a=1, b=2, c=3) |
1090 | + bag['d'] = 4 |
1091 | + bag['d'] = 5 |
1092 | + self.assertEqual(bag.d, 5) |
1093 | + |
1094 | def test_pickle(self): |
1095 | + # Bags can be pickled and unpickled. |
1096 | bag = Bag(a=1, b=2, c=3) |
1097 | pck = pickle.dumps(bag) |
1098 | new_bag = pickle.loads(pck) |
1099 | self.assertEqual(new_bag.a, 1) |
1100 | self.assertEqual(new_bag.b, 2) |
1101 | self.assertEqual(new_bag.c, 3) |
1102 | + |
1103 | + def test_update(self): |
1104 | + # Bags can be updated, similar to dicts. |
1105 | + bag = Bag(a=1, b=2, c=3) |
1106 | + bag.update(b=7, d=9) |
1107 | + self.assertEqual(bag.a, 1) |
1108 | + self.assertEqual(bag.b, 7) |
1109 | + self.assertEqual(bag.c, 3) |
1110 | + self.assertEqual(bag.d, 9) |
1111 | + |
1112 | + def test_converters(self): |
1113 | + # The Bag ctor accepts a mapping of type converter functions. |
1114 | + bag = Bag(converters=dict(a=int, b=int), |
1115 | + a='1', b='2', c='3') |
1116 | + self.assertEqual(bag.a, 1) |
1117 | + self.assertEqual(bag.b, 2) |
1118 | + self.assertEqual(bag.c, '3') |
1119 | + |
1120 | + def test_converters_error(self): |
1121 | + # Type converter function errors get propagated. |
1122 | + converters = dict(a=int, b=int) |
1123 | + keywords = dict(a='1', b='foo', c=3) |
1124 | + self.assertRaises(ValueError, Bag, converters=converters, **keywords) |
1125 | + |
1126 | + def test_update_converters(self): |
1127 | + # The update method also accepts converters. |
1128 | + bag = Bag(a=1, b=2, c=3) |
1129 | + bag.update(converters=dict(d=int), |
1130 | + d='4', e='5') |
1131 | + self.assertEqual(bag.d, 4) |
1132 | + self.assertEqual(bag.e, '5') |
1133 | + |
1134 | + def test_update_converter_overrides(self): |
1135 | + # Converters in the update method permanently override ctor converters. |
1136 | + converters = dict(a=int, b=int) |
1137 | + bag = Bag(converters=converters, a='1', b='2') |
1138 | + self.assertEqual(bag.a, 1) |
1139 | + self.assertEqual(bag.b, 2) |
1140 | + new_converters = dict(a=str) |
1141 | + bag.update(converters=new_converters, a='3', b='4') |
1142 | + self.assertEqual(bag.a, '3') |
1143 | + self.assertEqual(bag.b, 4) |
1144 | + bag.update(a='5', b='6') |
1145 | + self.assertEqual(bag.a, '5') |
1146 | + self.assertEqual(bag.b, 6) |
1147 | |
1148 | === modified file 'systemimage/tests/test_config.py' |
1149 | --- systemimage/tests/test_config.py 2014-02-20 23:03:24 +0000 |
1150 | +++ systemimage/tests/test_config.py 2014-03-05 23:45:27 +0000 |
1151 | @@ -136,6 +136,45 @@ |
1152 | self.assertEqual(config.service.https_base, |
1153 | 'https://phablet.example.com:80443') |
1154 | |
1155 | + def test_disabled_http_port(self): |
1156 | + # config_05.ini has http port disabled and non-standard https port. |
1157 | + ini_file = data_path('config_05.ini') |
1158 | + config = Configuration() |
1159 | + config.load(ini_file) |
1160 | + self.assertEqual(config.service.base, 'phablet.example.com') |
1161 | + self.assertEqual(config.service.http_base, |
1162 | + 'https://phablet.example.com:80443') |
1163 | + self.assertEqual(config.service.https_base, |
1164 | + 'https://phablet.example.com:80443') |
1165 | + |
1166 | + def test_disabled_https_port(self): |
1167 | + # config_06.ini has https port disabled and standard http port. |
1168 | + ini_file = data_path('config_06.ini') |
1169 | + config = Configuration() |
1170 | + config.load(ini_file) |
1171 | + self.assertEqual(config.service.base, 'phablet.example.com') |
1172 | + self.assertEqual(config.service.http_base, |
1173 | + 'http://phablet.example.com') |
1174 | + self.assertEqual(config.service.https_base, |
1175 | + 'http://phablet.example.com') |
1176 | + |
1177 | + def test_both_ports_disabled(self): |
1178 | + # config_07.ini has both http and https ports disabled. |
1179 | + ini_file = data_path('config_07.ini') |
1180 | + config = Configuration() |
1181 | + with self.assertRaises(ValueError) as cm: |
1182 | + config.load(ini_file) |
1183 | + self.assertEqual(cm.exception.args[0], |
1184 | + 'Cannot disable both http and https ports') |
1185 | + |
1186 | + def test_negative_port_number(self): |
1187 | + # config_08.ini has a negative port number. |
1188 | + ini_file = data_path('config_08.ini') |
1189 | + config = Configuration() |
1190 | + with self.assertRaises(ValueError) as cm: |
1191 | + config.load(ini_file) |
1192 | + self.assertEqual(cm.exception.args[0], '-1') |
1193 | + |
1194 | @configuration |
1195 | def test_get_build_number(self, ini_file): |
1196 | # The current build number is stored in a file specified in the |
1197 | @@ -233,15 +272,28 @@ |
1198 | self.assertEqual(config.service.channel, 'proposed') |
1199 | self.assertEqual(config.service.build_number, 1833) |
1200 | |
1201 | + def test_channel_ini_may_override_system_section(self): |
1202 | + # Overrides may include the [system] section. |
1203 | + default_ini = resource_filename('systemimage.data', 'client.ini') |
1204 | + config = Configuration() |
1205 | + config.load(default_ini) |
1206 | + channel_ini = data_path('channel_02.ini') |
1207 | + config.load(channel_ini, override=True) |
1208 | + self.assertEqual(config.system.build_file, |
1209 | + '/etc/path/to/alternative/build-file') |
1210 | + # But other [system] settings which come from client.ini are unchanged. |
1211 | + self.assertEqual(config.system.settings_db, |
1212 | + '/var/lib/system-image/settings.db') |
1213 | + |
1214 | def test_channel_ini_ignored_sections(self): |
1215 | - # Only the [service] section in channel.ini is used. |
1216 | + # Only the [service] and [system] section in channel.ini is used. |
1217 | default_ini = resource_filename('systemimage.data', 'client.ini') |
1218 | config = Configuration() |
1219 | config.load(default_ini) |
1220 | - channel_ini = resource_filename( |
1221 | - 'systemimage.tests.data', 'channel_02.ini') |
1222 | + channel_ini = data_path('channel_02.ini') |
1223 | config.load(channel_ini, override=True) |
1224 | - self.assertEqual(config.system.build_file, '/etc/ubuntu-build') |
1225 | + # channel_02.ini sets this to 1h, but it's not overridden. |
1226 | + self.assertEqual(config.dbus.lifetime, timedelta(minutes=10)) |
1227 | |
1228 | def test_tempdir(self): |
1229 | # config.tempdir is randomly created. |
1230 | |
1231 | === modified file 'systemimage/tests/test_dbus.py' |
1232 | --- systemimage/tests/test_dbus.py 2014-02-20 23:03:24 +0000 |
1233 | +++ systemimage/tests/test_dbus.py 2014-03-05 23:45:27 +0000 |
1234 | @@ -18,18 +18,19 @@ |
1235 | __all__ = [ |
1236 | 'TestDBusApply', |
1237 | 'TestDBusCheckForUpdate', |
1238 | + 'TestDBusCheckForUpdateException', |
1239 | 'TestDBusClient', |
1240 | 'TestDBusDownload', |
1241 | 'TestDBusGetSet', |
1242 | 'TestDBusInfo', |
1243 | 'TestDBusInfoNoDetails', |
1244 | - 'TestDBusLP1277589', |
1245 | 'TestDBusMockFailApply', |
1246 | 'TestDBusMockFailPause', |
1247 | 'TestDBusMockFailResume', |
1248 | 'TestDBusMockNoUpdate', |
1249 | 'TestDBusMockUpdateAutoSuccess', |
1250 | 'TestDBusMockUpdateManualSuccess', |
1251 | + 'TestDBusMultipleChecksInFlight', |
1252 | 'TestDBusPauseResume', |
1253 | 'TestDBusProgress', |
1254 | 'TestDBusRegressions', |
1255 | @@ -49,7 +50,7 @@ |
1256 | from functools import partial |
1257 | from systemimage.bindings import DBusClient |
1258 | from systemimage.config import Configuration |
1259 | -from systemimage.helpers import safe_remove |
1260 | +from systemimage.helpers import atomic, safe_remove |
1261 | from systemimage.reactor import Reactor |
1262 | from systemimage.testing.helpers import ( |
1263 | copy, find_dbus_process, make_http_server, setup_index, setup_keyring_txz, |
1264 | @@ -133,6 +134,7 @@ |
1265 | self.schedule(self.iface.CheckForUpdate) |
1266 | |
1267 | def _do_UpdateAvailableStatus(self, signal, path, *args, **kws): |
1268 | + # We'll keep doing this until we get the UpdateDownloaded signal. |
1269 | self.uas_signals.append(args) |
1270 | self.schedule(self.iface.CheckForUpdate) |
1271 | |
1272 | @@ -140,6 +142,43 @@ |
1273 | self.quit() |
1274 | |
1275 | |
1276 | +class ManualUpdateReactor(Reactor): |
1277 | + def __init__(self, iface): |
1278 | + super().__init__(dbus.SystemBus()) |
1279 | + self.iface = iface |
1280 | + self.rebooting = False |
1281 | + self.react_to('UpdateAvailableStatus') |
1282 | + self.react_to('UpdateProgress') |
1283 | + self.react_to('UpdateDownloaded') |
1284 | + self.react_to('Rebooting') |
1285 | + self.react_to('UpdateFailed') |
1286 | + self.iface.CheckForUpdate() |
1287 | + |
1288 | + def _do_UpdateAvailableStatus(self, signal, path, *args, **kws): |
1289 | + # When the update is available, start the download. |
1290 | + self.iface.DownloadUpdate() |
1291 | + |
1292 | + def _do_UpdateProgress(self, signal, path, *args, **kws): |
1293 | + # Once the download is in progress, initiate another check. Only do |
1294 | + # this on the first progress signal. |
1295 | + if args == (0, 0): |
1296 | + self.iface.CheckForUpdate() |
1297 | + |
1298 | + def _do_UpdateDownloaded(self, signal, path, *args, **kws): |
1299 | + # The update successfully downloaded, so apply the update now. |
1300 | + self.iface.ApplyUpdate() |
1301 | + |
1302 | + def _do_UpdateFailed(self, signal, path, *args, **kws): |
1303 | + # Before LP: #1287919 was fixed, this signal would have been sent. |
1304 | + self.rebooting = False |
1305 | + self.quit() |
1306 | + |
1307 | + def _do_Rebooting(self, signal, path, *args, **kws): |
1308 | + # The system is now rebooting <wink>. |
1309 | + self.rebooting = True |
1310 | + self.quit() |
1311 | + |
1312 | + |
1313 | class _TestBase(unittest.TestCase): |
1314 | """Base class for all DBus testing.""" |
1315 | |
1316 | @@ -1563,7 +1602,7 @@ |
1317 | """) |
1318 | |
1319 | |
1320 | -class TestDBusLP1277589(_LiveTesting): |
1321 | +class TestDBusMultipleChecksInFlight(_LiveTesting): |
1322 | def test_multiple_check_for_updates(self): |
1323 | # Log analysis of LP: #1277589 appears to show the following scenario, |
1324 | # reproduced in this test case: |
1325 | @@ -1588,18 +1627,93 @@ |
1326 | # signal, we'll immediately issue *another* CheckForUpdate, which |
1327 | # should run while the auto-download is working. |
1328 | # |
1329 | - # At the end, we should not get another UpdateAvailableStatus signal, |
1330 | - # but we should get the UpdateDownloaded signal. |
1331 | + # As per LP: #1284217, we will get a second UpdateAvailableStatus |
1332 | + # signal, since the status is available even while the original |
1333 | + # request is being downloaded. |
1334 | reactor = DoubleCheckingReactor(self.iface) |
1335 | reactor.run() |
1336 | - self.assertEqual(len(reactor.uas_signals), 1) |
1337 | + # We need to have received at least 2 signals, but due to timing |
1338 | + # issues it could possibly be more. |
1339 | + self.assertGreater(len(reactor.uas_signals), 1) |
1340 | + # All received signals should have the same information. |
1341 | + for signal in reactor.uas_signals: |
1342 | + (is_available, downloading, available_version, update_size, |
1343 | + last_update_date, |
1344 | + #descriptions, |
1345 | + error_reason) = signal |
1346 | + self.assertTrue(is_available) |
1347 | + self.assertTrue(downloading) |
1348 | + self.assertEqual(available_version, '1600') |
1349 | + self.assertEqual(update_size, 314572800) |
1350 | + self.assertEqual(last_update_date, 'Unknown') |
1351 | + self.assertEqual(error_reason, '') |
1352 | + |
1353 | + def test_multiple_check_for_updates_with_manual_downloading(self): |
1354 | + # Log analysis of LP: #1287919 (a refinement of LP: #1277589 with |
1355 | + # manual downloading enabled) shows that it's possible to enter the |
1356 | + # checking phase while a download of the data files is still running. |
1357 | + # When manually downloading, this will start another check, and as |
1358 | + # part of that check, the blacklist and other files will be deleted |
1359 | + # (in anticipation of them being re-downloaded). When the data files |
1360 | + # are downloaded, the state machine that just did the data download |
1361 | + # may find its files deleted out from underneath it by the state |
1362 | + # machine doing the checking. |
1363 | + self.download_manually() |
1364 | + # Start by creating some big files which will take a while to |
1365 | + # download. |
1366 | + def write_callback(dst): |
1367 | + # Write a 100 MiB sized file. |
1368 | + with open(dst, 'wb') as fp: |
1369 | + for i in range(25600): |
1370 | + fp.write(b'x' * 4096) |
1371 | + self._prepare_index('index_24.json', write_callback) |
1372 | + self._touch_build(0) |
1373 | + # Create a reactor that implements the following test plan: |
1374 | + # * Set the device to download manually. |
1375 | + # * Flash to an older revision |
1376 | + # * Open System Settings and wait for it to say Updates available |
1377 | + # * Click on About this phone |
1378 | + # * Click on Check for Update and wait for it to say Install 1 update |
1379 | + # * Click on Install 1 update and while the files are downloading, |
1380 | + # swipe up from the bottom and click Back |
1381 | + # * Click on Check for Update again |
1382 | + # * Wait for the Update System overlay to come up, and then install |
1383 | + # the update, and reboot |
1384 | + reactor = ManualUpdateReactor(self.iface) |
1385 | + reactor.run() |
1386 | + self.assertTrue(reactor.rebooting) |
1387 | + |
1388 | + |
1389 | +class TestDBusCheckForUpdateException(_LiveTesting): |
1390 | + @classmethod |
1391 | + def setUpClass(cls): |
1392 | + ini_path = SystemImagePlugin.controller.ini_path |
1393 | + with open(ini_path, 'r', encoding='utf-8') as fp: |
1394 | + lines = fp.readlines() |
1395 | + with atomic(ini_path) as fp: |
1396 | + for line in lines: |
1397 | + key, sep, value = line.partition(': ') |
1398 | + if key == 'cache_partition': |
1399 | + cls.bad_path = os.path.join(value.strip(), 'unwritable') |
1400 | + print('{}: {}'.format(key, cls.bad_path), file=fp) |
1401 | + os.makedirs(cls.bad_path, mode=0) |
1402 | + else: |
1403 | + fp.write(line) |
1404 | + super().setUpClass() |
1405 | + |
1406 | + def tearDown(self): |
1407 | + os.chmod(self.bad_path, 0o777) |
1408 | + super().tearDown() |
1409 | + |
1410 | + def test_check_for_update_error(self): |
1411 | + # CheckForUpdate sees an error, in this case because the destination |
1412 | + # directory for downloads is not writable. We'll get an |
1413 | + # UpdateAvailableStatus with an error string. |
1414 | + reactor = SignalCapturingReactor('UpdateAvailableStatus') |
1415 | + reactor.run(self.iface.CheckForUpdate) |
1416 | + self.assertEqual(len(reactor.signals), 1) |
1417 | (is_available, downloading, available_version, update_size, |
1418 | last_update_date, |
1419 | - #descriptions, |
1420 | - error_reason) = reactor.uas_signals[0] |
1421 | - self.assertTrue(is_available) |
1422 | - self.assertTrue(downloading) |
1423 | - self.assertEqual(available_version, '1600') |
1424 | - self.assertEqual(update_size, 314572800) |
1425 | - self.assertEqual(last_update_date, 'Unknown') |
1426 | - self.assertEqual(error_reason, '') |
1427 | + # descriptions, |
1428 | + error_reason) = reactor.signals[0] |
1429 | + self.assertIn('Permission denied', error_reason) |
1430 | |
1431 | === modified file 'systemimage/tests/test_main.py' |
1432 | --- systemimage/tests/test_main.py 2014-02-20 23:03:24 +0000 |
1433 | +++ systemimage/tests/test_main.py 2014-03-05 23:45:27 +0000 |
1434 | @@ -43,7 +43,7 @@ |
1435 | from systemimage.helpers import safe_remove |
1436 | from systemimage.main import main as cli_main |
1437 | from systemimage.testing.helpers import ( |
1438 | - ServerTestBase, configuration, copy, data_path, find_dbus_process, |
1439 | + ServerTestBase, chmod, configuration, copy, data_path, find_dbus_process, |
1440 | temporary_directory, touch_build) |
1441 | from systemimage.testing.nose import SystemImagePlugin |
1442 | from textwrap import dedent |
1443 | @@ -486,6 +486,34 @@ |
1444 | last update: 2013-08-01 12:11:10 |
1445 | """)) |
1446 | |
1447 | + @configuration |
1448 | + def test_state_machine_exceptions(self, ini_file): |
1449 | + # If an exception happens during the state machine run, the error is |
1450 | + # logged and main exits with code 1. |
1451 | + config = Configuration() |
1452 | + config.load(ini_file) |
1453 | + self._resources.enter_context( |
1454 | + patch('systemimage.main.sys.argv', ['argv0', '-C', ini_file])) |
1455 | + # Making the cache directory unwritable is a good way to trigger a |
1456 | + # crash. Be sure to set it back though! |
1457 | + with chmod(config.updater.cache_partition, 0): |
1458 | + exit_code = cli_main() |
1459 | + self.assertEqual(exit_code, 1) |
1460 | + |
1461 | + @configuration |
1462 | + def test_state_machine_exceptions_dry_run(self, ini_file): |
1463 | + # Like above, but doing only a --dry-run. |
1464 | + config = Configuration() |
1465 | + config.load(ini_file) |
1466 | + # Making the cache directory unwritable is a good way to trigger a |
1467 | + # crash. Be sure to set it back though! |
1468 | + self._resources.enter_context( |
1469 | + patch('systemimage.main.sys.argv', |
1470 | + ['argv0', '-C', ini_file, '--dry-run'])) |
1471 | + with chmod(config.updater.cache_partition, 0): |
1472 | + exit_code = cli_main() |
1473 | + self.assertEqual(exit_code, 1) |
1474 | + |
1475 | |
1476 | class TestCLIMainDryRun(ServerTestBase): |
1477 | INDEX_FILE = 'index_14.json' |
1478 | |
1479 | === modified file 'systemimage/version.txt' |
1480 | --- systemimage/version.txt 2014-02-20 23:03:24 +0000 |
1481 | +++ systemimage/version.txt 2014-03-05 23:45:27 +0000 |
1482 | @@ -1,1 +1,1 @@ |
1483 | -2.1 |
1484 | +2.2 |
In terms of code looks good to me with not obvious errors but I'm not senior enough with the code. Would be good to have a second reviewer.