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

Properly sort consul configuration #411

Closed kamaradclimber closed 7 years ago

kamaradclimber commented 7 years ago

Simple sort compare key & values. It fails for hash values.

For instance:

{:data_dir=>"/var/lib/consul",
 :client_addr=>"0.0.0.0",
 :ports=>{"dns"=>-1, "http"=>8500, "rpc"=>8400, "serf_lan"=>8301, "serf_wan"=>8302, "server"=>8300},
 :enable_syslog=>false,
 :retry_join=>["consul01-par.kitchen"],
 :node_name=>"ip-10-0-64-236.us-west-2.compute.internal",
 :server=>false,
 :datacenter=>"par",
 :domain=>"consul",
 :addresses=>{"http"=>"0.0.0.0"},
 :acl_datacenter=>"par",
 :disable_update_check=>true,
 :leave_on_terminate=>false,
 :advertise_addr=>"10.0.64.236",
 :encrypt=>"/AKlGGZMMSUsJLit/+etBw==",
 :verify_incoming=>false,
 :verify_outgoing=>false,
 "node_meta"=>{"rack"=>nil}}

will raise ArgumentError: comparison of Array with Array failed.

Sorting by keys is sufficient

Fix #410

Change-Id: Ieaf05a019c4d636ae8457046ee4e3bf8f697296a

kamaradclimber commented 7 years ago

I've fixed tests (I thought I had actually)

codecov-io commented 7 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@558baf3). Click here to learn what that means.

@@            Coverage Diff            @@
##             master     #411   +/-   ##
=========================================
  Coverage          ?   66.86%           
=========================================
  Files             ?        7           
  Lines             ?      341           
  Branches          ?        0           
=========================================
  Hits              ?      228           
  Misses            ?      113           
  Partials          ?        0
Impacted Files Coverage Δ
libraries/helpers.rb 54.05% <ø> (ø)
libraries/consul_config.rb 97.39% <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 558baf3...b9f7fe5. Read the comment docs.

kamaradclimber commented 7 years ago

tests are really flaky, I'll try to make them pass but they should be corrected.

legal90 commented 7 years ago

@kamaradclimber I've just tried to rebase your branch onto current master state to get everything fixed. Don't you mind I finish it and merge this PR then?

kamaradclimber commented 7 years ago

go ahead @legal90. I was quite suprised my branch has been rebased actually :)

Thanks for your help

legal90 commented 7 years ago

Thank you for the contribution!

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.