Merge lp:~barry/ubuntu-system-image/citrain-2.2 into lp:~ubuntu-managed-branches/ubuntu-system-image/system-image

Proposed by Barry Warsaw
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
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 UpdateAvailableStatus signal with the cached
      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-download-manager now supports this by default.
    - LP: #1250817 - Exceptions in the state machine are caught and
      logged, with an appropriate error message added to
      UpdateAvailableStatus signals. These exceptions do not percolate up
      to the GLib main loop.
    - LP: #1279532 - During testing, pass the log dir argument to
      ubuntu-download-manager.
  * d/rules: Add override_dh_python3 rule to set shebang line to
    /usr/bin/python3. (LP: #1283277)
  * d/patches/lp1284217.patch: Removed; applied upstream.

 -- 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 UpdateAvailableStatus signal with the cached
      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-download-manager now supports this by default.
    - LP: #1250817 - Exceptions in the state machine are caught and
      logged, with an appropriate error message added to
      UpdateAvailableStatus signals. These exceptions do not percolate up
      to the GLib main loop.
    - LP: #1279532 - During testing, pass the log dir argument to
      ubuntu-download-manager.
  * d/rules: Add override_dh_python3 rule to set shebang line to
    /usr/bin/python3. (LP: #1283277)
  * d/patches/lp1284217.patch: Removed; applied upstream.

 -- Barry Warsaw <email address hidden> Wed, 05 Mar 2014 16:29:13 -0500

To post a comment you must log in.
Revision history for this message
Manuel de la Peña (mandel) wrote :

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.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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

Subscribers

People subscribed via source and target branches