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

Support for Consul Connect #504

Closed pierresouchay closed 4 years ago

pierresouchay commented 6 years ago

This PR allow to use Connect with Consul 1.2+

For Consul 1.x with version < 1.2.0, the setting is ignored

@johnbellone Do you think it could be possible to release a new version of the cookbook in supermarket once merged?

pierresouchay commented 6 years ago

@gdavison Do you think it would be possible to create a new release of cookbook with this PR included?

codecov-io commented 6 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@65ffbf6). Click here to learn what that means. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #504   +/-   ##
=========================================
  Coverage          ?   82.52%           
=========================================
  Files             ?        9           
  Lines             ?      578           
  Branches          ?        0           
=========================================
  Hits              ?      477           
  Misses            ?      101           
  Partials          ?        0
Impacted Files Coverage Δ
libraries/consul_config_v1.rb 100% <100%> (ø)

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 65ffbf6...7ee587c. Read the comment docs.

sc7565 commented 6 years ago

@pierresouchay .. thanks for making the changes, waiting for these changes to get merged

pierresouchay commented 6 years ago

@sc7565 I also would like to get it merged quickly ;)

It is a bit unfortunate that the cookbook needs to be updated for each new feature ;(

Having a way to add arbitrary config would be a nice addition

@gdavison @johnbellone Could we help you in a way to get new version of cookbook released?

gdavison commented 6 years ago

@pierresouchay I don't know if I have access to push a new version to Supermarket. @johnbellone ?

pierresouchay commented 6 years ago

@gdavison That's would be great, thank you so much! But it would probably require a Review of this PR first, do you fee the change is Ok?

gdavison commented 6 years ago

It looks this this PR is failing a number of unit tests, and also on Ubuntu 16.04. Ubuntu 16.04 could be a race condition, but I thought I had resolved those. Otherwise, looks good :)

gdavison commented 6 years ago

Yes, looks like the race conditions are back for ACLs. Dang.

pierresouchay commented 6 years ago

@gdavison The error is quite weird:

NoMethodError
-------------
undefined method `new_resource' for ConsulCookbook::Resource::ConsulConfigV1

All unit tests do pass on my machine. How are those tests executed exactly?

Finished in 16.89 seconds (files took 3.06 seconds to load)
86 examples, 0 failures

Randomized with seed 51397
sc7565 commented 6 years ago

@pierresouchay the branch is out of date, sync might fix above checks.

pierresouchay commented 6 years ago

@johnbellone @gdavison @sc7565 DONE => rebase with master => but still an error (cannot reproduce on my machine)

PierreRambaud commented 6 years ago

@pierresouchay I'm currently able to reproduce all errors. Which version of chef are you using? Maybe the old Resource creation isn't valid, or we maybe need to update the Gemfile.lock. I will try as soon as possible.

PierreRambaud commented 6 years ago

I downgrade chefspec to version 6, and everything works fine with chef 12 & 13. But doesn't working with chefspec 7+. We maybe need to update how we build resources.

sc7565 commented 6 years ago

@PierreRambaud still failing? can you help us in this

pierresouchay commented 6 years ago

@sc7565 For the last error, it does seems like a timeout on the Travis worker instead... trying to push force to re-trigger it

PierreRambaud commented 6 years ago

And it works ;) @gdavison is it ok for you?

pierresouchay commented 6 years ago

@johnbellone @gdavison @sc7565 @PierreRambaud Anyone for a review now since all tests are now passing?

xorima commented 4 years ago

Hey,

Thank you for the PR. We are closing due to inactivity. If you feel this feature should be merged in, please reopen and rebase as your contributions are always welcome.

Sous Chefs