Merge lp://staging/~thedac/charms/trusty/rabbitmq-server/le-ignore-min-cluster into lp://staging/~openstack-charmers-archive/charms/trusty/rabbitmq-server/next

Proposed by David Ames
Status: Merged
Merged at revision: 117
Proposed branch: lp://staging/~thedac/charms/trusty/rabbitmq-server/le-ignore-min-cluster
Merge into: lp://staging/~openstack-charmers-archive/charms/trusty/rabbitmq-server/next
Diff against target: 278 lines (+134/-49)
3 files modified
hooks/rabbit_utils.py (+46/-36)
hooks/rabbitmq_server_relations.py (+17/-13)
unit_tests/test_rabbit_utils.py (+71/-0)
To merge this branch: bzr merge lp://staging/~thedac/charms/trusty/rabbitmq-server/le-ignore-min-cluster
Reviewer Review Type Date Requested Status
Billy Olsen Approve
Review via email: mp+273474@code.staging.launchpad.net

Description of the change

Ignore min-cluster-size when juju has leadership election LP:1500204

This MP also addresses two major hurdles to rabbitmq clustering.

1) When more than one node has run stop_app at the same time nodes cannot join the cluster.

The ultimate solution to this is using charmhelpers.coordinator. However, this will require a significant design change. In the meantime, inserting a random wait period before attempting join the cluster is effective in resolving the race.

2) When using juju leadership election, if the elected leader is the third or greater node (i.e. rabbitmq/2 +), the leader would never be joined to the cluster.

Changing the clustering algorithm for each non-leader to join_cluster with the leader resolves this. Also by clustering with the leader rather than non-leader nodes we avoid split-brain clusters.

To post a comment you must log in.
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #10526 rabbitmq-server-next for thedac mp273474
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/10526/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #11332 rabbitmq-server-next for thedac mp273474
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/11332/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_amulet_test #7125 rabbitmq-server-next for thedac mp273474
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/7125/

Revision history for this message
Ryan Beisner (1chb1n) wrote :

Loooookin good....

 :)

Revision history for this message
Chris Glass (tribaal) wrote :

This looks great!

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #11427 rabbitmq-server-next for thedac mp273474
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/11427/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #10618 rabbitmq-server-next for thedac mp273474
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/10618/

122. By David Ames

Return an empty list if a leader has not yet been elected

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #11428 rabbitmq-server-next for thedac mp273474
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/11428/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #10619 rabbitmq-server-next for thedac mp273474
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/10619/

Revision history for this message
Billy Olsen (billy-olsen) :
Revision history for this message
Billy Olsen (billy-olsen) wrote :

Per our discussion on IRC, please add unit tests.

review: Needs Fixing
123. By David Ames

Fix comments

124. By David Ames

Unit tests

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #11429 rabbitmq-server-next for thedac mp273474
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/11429/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #10620 rabbitmq-server-next for thedac mp273474
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/10620/

Revision history for this message
Billy Olsen (billy-olsen) wrote :

LGTM, just waiting on the amulet result

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_amulet_test #7171 rabbitmq-server-next for thedac mp273474
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/7171/

Revision history for this message
Billy Olsen (billy-olsen) wrote :

Approved

review: Approve
Revision history for this message
Chris Glass (tribaal) wrote :

I realize this is already merged, but I'll do my comments anyway.

It would be good to add an amulet test to the branch asserting that a cluster forms with only i.e. 2 units (one less than min-cluster-size) if leader election is available.

Two inline comments.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches