Juju Charms Collection
glance-simplestreams-sync package

Merge lp://qastaging/~danilo/charms/trusty/glance-simplestreams-sync/lathiats-endpoints-fix into lp://qastaging/~landscape/charms/trusty/glance-simplestreams-sync/landscape

Proposed by Данило Шеган on 2016-04-14

Status:	Merged
Merged at revision:	63
Proposed branch:	lp://qastaging/~danilo/charms/trusty/glance-simplestreams-sync/lathiats-endpoints-fix
Merge into:	lp://qastaging/~landscape/charms/trusty/glance-simplestreams-sync/landscape
Diff against target:	130 lines (+27/-51) 1 file modified scripts/glance-simplestreams-sync.py (+27/-51)
To merge this branch:	bzr merge lp://qastaging/~danilo/charms/trusty/glance-simplestreams-sync/lathiats-endpoints-fix
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
Adam Collard (community)	2016-04-14	Approve on 2016-04-15
Geoff Teale (community)	2016-04-14	Approve on 2016-04-14
Review via email: mp+291857@code.qastaging.launchpad.net

Description of the change

This is a resubmit of https://code.launchpad.net/~lathiat/charms/trusty/glance-simplestreams-sync/trunk/+merge/290950 against our branch with a few changes on top https://pastebin.canonical.com/154167/ (pep8 cleanups and a fix for an issue I've seen with ClientException).

This *has* been tested in ceph/ceph and swift/iscsi configurations (this requires only 4 nodes, whereas swift/ceph requires 6) and works properly and allows juju to bootstrap with the images.

This also supersedes my original proposal which only worked for ceph/ceph at https://code.launchpad.net/~danilo/charms/trusty/glance-simplestreams-sync/normalize-swift-endpoint/+merge/291612

This has been pushed to cs:~danilo/trusty/glance-simplestreams-sync-17 (see the old MP for testing instructions)

This in-turn is best tested with https://code.launchpad.net/~danilo/landscape/swift-endpoints-fix/+merge/291870 (otherwise, the code that branch removes might override the good value this sets: if that happens, re-run the /etc/cron.daily/glance-simplestreams-sync script on the glance-simplestreams-sync/0 unit).

lp://qastaging/~danilo/charms/trusty/glance-simplestreams-sync/lathiats-endpoints-fix updated on 2016-04-14

66. By Данило Шеган on 2016-04-14: Merge "landscape" branch.

Revision history for this message

Geoff Teale (tealeg) wrote on 2016-04-14:

+1 Awesome.

review: Approve

Revision history for this message

Adam Collard (adam-collard) wrote on 2016-04-15:

Not in this diff but the warning log about finding the wrong number of endpoints is bogus.

    if len(ps_endpoints) != 1:
        log.warning("found {} product-streams endpoints in region {},"
                    " expecting one - not updating"
                    " endpoint".format(ps_endpoints, region,
                                       len(ps_endpoints)))

It has two placeholders and three values to format in there.

It's crying out for tests.

review: Needs Fixing

Revision history for this message

Данило Шеган (danilo) wrote on 2016-04-15:

Yes, it is crying out for tests, no disagreement there. Fixed the warning message about wrong number of endpoints, addressed your other comment inline.

lp://qastaging/~danilo/charms/trusty/glance-simplestreams-sync/lathiats-endpoints-fix updated on 2016-04-15