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

Revision history for this message
Alex Kavanagh (ajkavanagh) 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.

Sortee the CH changes and also merged latest keystone/next into the branch.

Re the extra ports, would you like them hard-coded into the file, or determined from config (if there is any?)

« Back to merge proposal