Juju Charms Collection
keystone package

Code review comment for lp:~ajkavanagh/charms/trusty/keystone/add-service-checks-lp1524388

  1. Trusty Tahr (14.04)
  2. add-service-checks-lp1524388
  3. Merge into next
Revision history for this message
David Ames (thedac) wrote :

> > One, we have some text conflicts that need to be resolved.
>
> I'm guessing this means some more changes in keystone?

We still have text conflicts for this one.

bzr branch lp:~openstack-charmers/charms/trusty/keystone/next
cd keystone
bzr merge lp:~ajkavanagh/charms/trusty/keystone/add-service-checks-lp1524388
bzr st # shows text conflicts

> I'm also guessing we need to get the charm-helpers change(s) merged first,
> then re-sync on keystone (on this one) + any further changes on keystone, and
> get that in as a merge? Also, did the 'determine_ports()' change alleviate
> your concerns around ports and HA?
>
> Thanks again for the review :)

CH changes were merged. They may need to be resynced to fix part of the above text conflicts.

I like the determine_ports() solution. We might want to add the ports the keystone service is listening on as well (not haproxy) for extra thouroughness. 35347 and 4990 while still including the well known ports 35357 and 5000.

After the above is resolved, we can merge this thing.

review: Needs Fixing

« Back to merge proposal