Support consul 1.x.x

[akissa]

Add support for Consul 1.x.x

Consul 1.x.x starting 1.0.0 enforces HTTP verbs.

• https://www.consul.io/docs/upgrade-specific.html#http-verbs-are-enforced-in-many-http-apis

Currently some methods in the plugin code use incorrect verbs.

The pull requests updates the methods to use the correct HTTP verbs.

Types of Changes

• [X] Bugfix (non-breaking change which fixes issue #NNNN)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] Documentation (correction or otherwise)
• [X] Cosmetics (whitespace, appearance)

Checklist

• [X] I have read the CONTRIBUTING.md document
• [ ] I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
• [X] All tests pass locally with my changes
• [X] I have added tests that prove my fix is effective or that my feature works
• [ ] I have added necessary documentation (if appropriate)
• [X] Any dependent changes have been merged and published in related repositories

Further Comments

I have tested with consul 0.9.3 and 1.0.0 to ensure that the changes to not break versions < 1.0.0

[michaelklishin]

Thank you! I'm happy to release a 0.9.1 in the next day or two.

[akissa]

@michaelklishin Thanks. It would be nice to move the token passing to headers which is now the recommended way https://www.consul.io/api/index.html#authentication however that requires consul >= 0.6.0. Would that be okay ?

If that's okay then i can submit a PR to go into 0.9.1 as well.

[michaelklishin]

Any backwards-compatible change is fine. If it's not compatible, we can make it a 0.10.0.

It would be greatly appreciate if we could do the same changes to https://github.com/rabbitmq/rabbitmq-peer-discovery-consul (it's a very similar codebase) :)

[akissa]

@michaelklishin will do

[akissa]

@michaelklishin Am looking at applying the changes to https://github.com/rabbitmq/rabbitmq-peer-discovery-consul am abit confused with the dev model.

Which repo should i fork https://github.com/rabbitmq/rabbitmq-peer-discovery-consul or the umbrella and work in the relevant deps directories ?

The contributing documentation is not very clear on that.

[michaelklishin]

Please fork the peer discovery plugin repo. It should not require the umbrella for most development tasks. Our team uses the umbrella so that would work as well but the PR has to be against the plugin repo (no doubt you understand that, clarifying for other potential readers).

Thank you!

On Wed, 18 Oct 2017 at 14:54, Andrew Colin Kissa notifications@github.com wrote:

@michaelklishin https://github.com/michaelklishin Am looking at applying the changes to https://github.com/rabbitmq/rabbitmq-peer-discovery-consul am abit confused with the dev model.

Which repo should i fork https://github.com/rabbitmq/rabbitmq-peer-discovery-consul or the umbrella and work in the relevant deps directories ?

The contributing documentation is not very clear on that.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/rabbitmq/rabbitmq-autocluster/pull/52#issuecomment-337566418, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAEQh88NYn8LaBM1PyMNShBjZZTpmWvks5stebzgaJpZM4P8xbJ .

-- Staff Software Engineer, Pivotal/RabbitMQ

[akissa]

@michaelklishin Changes for 3.7.x submitted. https://github.com/rabbitmq/rabbitmq-peer-discovery-consul/pull/4 https://github.com/rabbitmq/rabbitmq-peer-discovery-common/pull/4