Merge lp:~oddbloke/charms/trusty/ubuntu-repository-cache/error-message-fix into lp:charms/trusty/ubuntu-repository-cache

[Dan Watkins]

X-CPC-Summary-Skip: 1

[lp:~oddbloke/charms/trusty/ubuntu-repository-cache/error-message-fix]

213. By Dan Watkins on 2016-04-22

Include "Stale Release file" check for all verification failures

No matter why verify_distribution fails, it would be useful to know that we
have a stale releases file.

[William Grant (wgrant)]

review: Approve

[Nick Moffitt (nick-moffitt)]

This would have helped us track down the yackety Translations mis-publish quicker today!

review: Approve

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/3953/

review: Needs Fixing (automated testing)

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/3954/

review: Needs Fixing (automated testing)

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/3908/

review: Needs Fixing (automated testing)

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/3909/

review: Needs Fixing (automated testing)

[Andrew McLeod (admcleod)]

Hi Dan,

We've noticed that the test has failed the automated testing AFTER it was approved, and when I try to run the tests (bundletester -vFl DEBUG -e AWS) basically the install hook never completes - the debug-log looks like this:

http://pastebin.ubuntu.com/17749455/

and further logs

http://pastebin.ubuntu.com/17751100/

So basically it times out - is there any obvious reason why this might be happening?

review: Needs Fixing

[Andrew McLeod (admcleod)]

Sorry, I jumped the gun there (by about 10 seconds apparently) so I guess the pastebins in my previous comment are irrelevant - but a timeout is the reason this fails:

EBUG:runner:Environment wasn't stood up in time
DEBUG:runner:Exit Code: 100
    100-single_unit.simple FAIL

Is there any way you could perhaps limit the repos synced for the test?

review: Needs Fixing

[Charles Butler (lazypower)]

Greetings Daniel,

Thank you for the contribution to the ubuntu-repository-cache charm. I've encountered some issues with the proposed fix here that goes beyond the timeout listed above.

It appears with revision 212, the branch in lp:~charmers/charms/trusty/ubuntu-repository-cache/trunk was broken. It will require an additional re-sync of charm-helpers to fix the python3 inconsistency of modules the charm expects to be installed.

Additionally, it appears the lp:~charmers/charms/trusty/ubuntu-repository-cache/trunk branch is no longer the primary point of development, and this should instead be directed at:

lp:~cloudware/charms/trusty/ubuntu-repository-cache/trunk

With these concerns outlined, I regret to inform you that I am unable to accept this merge as is, and would advise you to re-submit against the upstream charm, in lp:~cloudware

Thanks again for your contribution, and I look forward to reviewing/landing this contribution in the proper places.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.

review: Needs Resubmitting

[Charles Butler (lazypower)]

In addition to the above review, i've moved this MP's status to Work in Progress while the resubmission is sorted out.

If this MP will require additional actions, please move the status to "Needs Review" so it will show back up in the review queue

review: Needs Resubmitting

Unmerged revisions

213. By Dan Watkins on 2016-04-22

Include "Stale Release file" check for all verification failures

No matter why verify_distribution fails, it would be useful to know that we
have a stale releases file.

212. By Dan Watkins on 2016-04-22

Fix 'Stale Release file.' message

Simple typo means that the default error message would be used even when the
stale message would be more appropriate.

=== modified file 'lib/ubuntu_repository_cache/mirror.py'
--- lib/ubuntu_repository_cache/mirror.py	2015-09-28 20:43:45 +0000
+++ lib/ubuntu_repository_cache/mirror.py	2016-04-22 10:30:34 +0000
@@ -274,11 +274,17 @@
             else:
                 raise
 
+        def raise_content_bad(msg):
+            file_mtime = os.stat(filepath).st_mtime
+            if release['mtime'] < file_mtime:
+                msg = ' '.join((msg, 'Stale Release file.'))
+            raise ContentBad(msg)
+
         size = int(files[filename]['size'])
         if disk_size != size:
             msg = 'Sizes do not match for {} in {}'.format(filename,
                                                            release['name'])
-            raise ContentBad(msg)
+            raise_content_bad(msg)
 
         with open(filepath, 'rb') as infile:
             sha256 = hashlib.sha256()
@@ -286,12 +292,9 @@
                 sha256.update(data)
         digest = sha256.hexdigest()
         if digest != files[filename]['SHA256']:
-            file_mtime = os.stat(filepath).st_mtime
             msg = 'SHA256 mismatch for {} in {}.'.format(filename,
                                                          release['name'])
-            if release['mtime'] < file_mtime:
-                ' '.join((msg, 'Stale Release file.'))
-            raise ContentBad(msg)
+            raise_content_bad(msg)
 
 
 def verify_repository(archive_root):