Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consul: added startup locking support #22

Conversation

ValFadeev
Copy link

@ValFadeev ValFadeev commented Apr 30, 2017

Further to #20 submitting a working implementation of startup locking for Consul backend.

  • The new property-based tests are missing. I had doubts on how to organise the code given that certain parts are somewhat biased towards etcd at the moment, hence looking for guidance here.
  • The implementation closely follows the Leader Election guide from Consul docs.
  • Added setting initial passing status for the service health check. Since updates begin right after registration, there is hardly a reason this would be harmful. It also helps with testing locally when all nodes run on the same host and would otherwise keep it critical throughout most of the startup process and thus fail to cluster.

The following compose file was used for verification:

version: '2'
services:
  consul:
    image: consul
    container_name: consul
    networks:
      - rabbitmq_network
    ports:
      - "8500:8500"

  rabbit-node0:
    depends_on:
      - consul
    image: rabbitmq-autocluster
    container_name: node0
    networks:
      - rabbitmq_network
    environment:
      AUTOCLUSTER_TYPE: consul
      CONSUL_HOST: consul
      CONSUL_PORT: 8500
      CONSUL_SERVICE_TTL: 60
      AUTOCLUSTER_CLEANUP: 'true'
      CLEANUP_WARN_ONLY: 'false'
      CONSUL_SVC_ADDR_AUTO: 'true'
    ports:
      - "15672:15672"

  rabbit-node1:
    depends_on:
      - consul
    image: rabbitmq-autocluster
    container_name: node1
    networks:
      - rabbitmq_network
    environment:
      AUTOCLUSTER_TYPE: consul
      CONSUL_HOST: consul
      CONSUL_PORT: 8500
      CONSUL_SERVICE_TTL: 60
      AUTOCLUSTER_CLEANUP: 'true'
      CLEANUP_WARN_ONLY: 'false'
      CONSUL_SVC_ADDR_AUTO: 'true'

  rabbit-node2:
    depends_on:
      - consul
    image: rabbitmq-autocluster
    container_name: node2
    networks:
      - rabbitmq_network
    environment:
      AUTOCLUSTER_TYPE: consul
      CONSUL_HOST: consul
      CONSUL_PORT: 8500
      CONSUL_SERVICE_TTL: 60
      AUTOCLUSTER_CLEANUP: 'true'
      CLEANUP_WARN_ONLY: 'false'
      CONSUL_SVC_ADDR_AUTO: 'true'

networks:
  rabbitmq_network:
    driver: bridge

@michaelklishin
Copy link
Member

@ValFadeev #6 is in, note that I renamed augmented_node to candidate_seed_node in the process.

Let me know when you think it's time for our team to QA this.

@ValFadeev ValFadeev force-pushed the rabbitmq-autocluster_startup-locking-consul branch from fc53dfa to d1328fc Compare May 20, 2017 21:36
@ValFadeev ValFadeev changed the base branch from rabbitmq-autocluster_startup-locking-support to stable May 20, 2017 21:37
@ValFadeev ValFadeev changed the title [WIP] Added startup locking support for consul backend Consul: added startup locking support May 20, 2017
@ValFadeev
Copy link
Author

@michaelklishin thank you, I have rebased onto the latest stable and updated the PR base accordingly. Should be ready for review now.

@michaelklishin
Copy link
Member

@ValFadeev perfect, I should have a chance to test it in the next few days. Will be a good segway into the work on https://github.com/rabbitmq/rabbitmq-peer-discovery-consul I had planned next week.

@michaelklishin
Copy link
Member

@ValFadeev thank you!

@ValFadeev
Copy link
Author

@michaelklishin thank you for accepting! I will try to have a look soon how this could potentially integrate with the existing peer discovery.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants