sous-chefs / consul

Development repository for the consul cookbook

https://supermarket.chef.io/cookbooks/consul

Apache License 2.0

192 stars 244 forks source link

Add consul API retries #482

Closed gdavison closed 6 years ago

gdavison commented 6 years ago

When starting up a new machine and creating an ACL, the Consul server is not always ready to serve requests by the time the ACL is ready to be created. This causes intermittent failures in integration tests, and likely in real-world use.

Two retries with an interval of 0.5 seconds seems to work.

Also addressed some cookstyle issues

codecov-io commented 6 years ago

Codecov Report

Merging #482 into master will decrease coverage by 2.17%. The diff coverage is 6.25%.

@@            Coverage Diff             @@
##           master     #482      +/-   ##
==========================================
- Coverage   61.72%   59.54%   -2.18%     
==========================================
  Files           7        8       +1     
  Lines         337      393      +56     
==========================================
+ Hits          208      234      +26     
- Misses        129      159      +30

Impacted Files	Coverage Δ
libraries/consul_acl.rb	`46.42% <6.25%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5fd61fb...0eb5b64. Read the comment docs.

gdavison commented 6 years ago

@legal90 I've removed some bits I should have removed when I copied the code. It was originally much more general, but I've made it specific here.

I haven't tried it for real, because we don't use ACLs yet. My main reason for adding this was to remove the intermittent test failures. I've added an additional attempt, so it will give 1.5 seconds for the consul service to start. It can definitely be tuned if needed.

gdavison commented 6 years ago

Hi @johnbellone, any feedback on the PR?

gdavison commented 6 years ago

Fixes #461

gdavison commented 6 years ago

@johnbellone, yes, it would (still) fail in this case if the local Consul agent were not yet available. Before this change, there would be a race condition which caused intermittent failures, at least is the tests. Now, the behaviour is to try three times before failing.

lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.