Juju Charms Collection
cinder package

Merge lp://staging/~jseutter/charms/precise/cinder/ha-support-with-health into lp://staging/~openstack-charmers/charms/precise/cinder/ha-support

Proposed by Jerry Seutter on 2013-03-04

Status:	Merged
Merged at revision:	37
Proposed branch:	lp://staging/~jseutter/charms/precise/cinder/ha-support-with-health
Merge into:	lp://staging/~openstack-charmers/charms/precise/cinder/ha-support
Diff against target:	129 lines (+78/-0) 6 files modified hooks/cinder-hooks (+11/-0) hooks/lib/openstack-common (+37/-0) scripts/add_to_cluster (+2/-0) scripts/health_checks.d/service_ports_live (+13/-0) scripts/health_checks.d/service_running (+13/-0) scripts/remove_from_cluster (+2/-0)
To merge this branch:	bzr merge lp://staging/~jseutter/charms/precise/cinder/ha-support-with-health
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
Adam Gandelman (community)	2013-03-04	Needs Fixing on 2013-03-05
Chad Smith (community)	2013-03-04	Approve on 2013-03-04
Review via email: mp+151565@code.staging.launchpad.net

This proposal supersedes a proposal from 2013-03-02.

Description of the change

This branch adds add_to_cluster, remove_from_cluster and health_scripts.d to the cinder charm.

The health scripts check the following things:
- The crm multicast port is open
- cinder-api service is running
- cinder-scheduler service is running
- cinder-volume service is running

Revision history for this message

Chad Smith (chad.smith) wrote on 2013-03-02: Posted in a previous version of this proposal

please resubmit this merge proposal against lp:~openstack-charmers/charms/precise/cinder/ha-support so it clears up the diff.

review: Needs Fixing

Revision history for this message

Chad Smith (chad.smith) wrote on 2013-03-04:

Thanks for the resubmit Jerry, much easier to read now. I just tested and things look good here.
[1] only change I'd propose is a nit:
you don't need the quotes around the port number definition.

instead of
+ "OPENSTACK_PORT_MCASTPORT=\"$(config-get ha-mcastport)\""

we could do

+ "OPENSTACK_PORT_MCASTPORT=$(config-get ha-mcastport)"

Revision history for this message

Chad Smith (chad.smith) on 2013-03-04:

review: Approve

lp://staging/~jseutter/charms/precise/cinder/ha-support-with-health updated on 2013-03-04

43. By Jerry Seutter on 2013-03-04: Fixed nit - extra quotes.

Revision history for this message

Jerry Seutter (jseutter) wrote on 2013-03-04:

> Thanks for the resubmit Jerry, much easier to read now. I just tested and
> things look good here.
> [1] only change I'd propose is a nit:
> you don't need the quotes around the port number definition.

Thanks, Chad. It should be fixed now.

Revision history for this message

Adam Gandelman (gandelman-a) wrote on 2013-03-05:

Same concerns here wrt the lax use of grep matching.

Also, should you also be checking that the relevant API services are listening on their respective ports as well? Currently the branches configure the actual listening port dynamically based on the existence of haproxy, but there are a branch soon-to-land that will change this so that the service's listening port (as listed in the keystone catalog) will always be the default regardless of the current request pipeline (combination of HTTPS, haproxy, api-server) That is, cinder-api will always be listening on 8776, glance-api always on 9292, etc.

I'm happy to wait on this until those branches have landed, and you guys can continue to improve the health tests as you see fit moving forward, since I don't know what you have going on elsewhere.

https://code.launchpad.net/~jseutter/charms/precise/glance/ha-support-with-health/+merge/151570

review: Needs Fixing

lp://staging/~jseutter/charms/precise/cinder/ha-support-with-health updated on 2013-03-05

44. By Jerry Seutter on 2013-03-05: Better checking of open ports to exclude false positives.

Revision history for this message

Jerry Seutter (jseutter) wrote on 2013-03-06:

> Same concerns here wrt the lax use of grep matching.

I'll fix that.

> Also, should you also be checking that the relevant API services are listening
> on their respective ports as well? Currently the branches configure the
> actual listening port dynamically based on the existence of haproxy, but there
> are a branch soon-to-land that will change this so that the service's
> listening port (as listed in the keystone catalog) will always be the default
> regardless of the current request pipeline (combination of HTTPS, haproxy,
> api-server) That is, cinder-api will always be listening on 8776, glance-api
> always on 9292, etc.

Thanks for pointing this out. I'll probably shelve this for now and wait for that branch to land.

> I'm happy to wait on this until those branches have landed, and you guys can
> continue to improve the health tests as you see fit moving forward, since I
> don't know what you have going on elsewhere.
>
> https://code.launchpad.net/~jseutter/charms/precise/glance/ha-support-with-
> health/+merge/151570

Yep, sounds good.