merge functionality not sufficient on the $cfg hash

[jsmartin]

We cannot override a single default key in the $cfg in appconfig.pp with a hiera hash. For example, if we wanted to override the ring_creation_size, we would have to supply the entire set of values of the riak_core hash or else the merge would result with the riak_core hash only containing the ring_creation_size k/v pair. I believe that we should be able to specify something like this in a hiera hash:

 cfg:
   riak_core:
     ring_creation_size: 128

If we performed a deep_merge install of a regular merge, the value of ring_creation_size would properly be merged with the default $cfg->riak_core hash.

deep_merge is available with gem install deep_merge

More information can be found here: https://github.com/peritor/deep_merge

[haf]

I think we should choose something other than the deep_merge gem, because that gem also merges arrays and is overkill for us. We know that our leafs are all values. I wrote something up, that we can place in the lib folder:

# By Henrik Feldt 2013

def extend kvs1, kvs2
  raise ArgumentError unless is_hashish? kvs1
  raise ArgumentError unless is_hashish? kvs2
  # if kvs2 didn't have value, return kvs1
  return kvs1 if kvs2.nil?
  merged = kvs1.map { |k, v| 
    # recurse on those keys that have hashish values
    is_hashish?(v) && is_hashish?(kvs2.fetch(k)) ?
      # recurse
      [k, extend(v, kvs2.fetch(k))] :
      # get the value from kvs2, defaulting to value from kvs1
      [k, kvs2.fetch(k, v)]
  # union merge all items of kvs2
  } | kvs2.reject { |k, v| kvs1.has_key? k }.to_a
  Hash[ merged ]
end

# the police is coming
def is_hashish? thing
  [:each, :fetch, :"has_key?", :reject].all? { |m| thing.respond_to? m }
end

kvs1 = { :a => 1, :b => { :c => { :x => 2 }, :d => 3, :z => 4 } }
kvs2 = { :b => { :c => { :x => 5 }, :d => 6 }, :e => 7 }
extended = extend kvs1, kvs2
puts "extended: "
p extended
# => {:a=>1, :b=>{:c=>{:x=>5}, :d=>6, :z=>4}, :e=>7}

[andrewjstone]

This is biting me in the ass right now: Duplicate definition: File[undef] is already defined in file /root/puppet/modules/riak/manifests/appconfig.pp at line 148; cannot redefine at /root/puppet/modules/riak/manifests/appconfig.pp:148 on node riak1.stage.onswipe.com

Removing the cfg params entirely fixes it, but of course leaves me with an invalid config. I guess I'll have to overwrite everysingle param or hack params.pp manually for now. This should be looked into again, as it essentially makes hash overiding app.config unusable.

[andrewjstone]

I meant hack appconfig.pp. I suppose I could also use a template instead :(

[haf]

Can you jack in my code above and send a PR?

[andrewjstone]

I worked around the issue last night. I may take a stab at it in the near future though. Thanks Henrik. Hope I didn't sound like a jerk. My puppet fu is not quite expert level so this surprised me.

On Sat, Mar 2, 2013 at 2:48 AM, Henrik Feldt notifications@github.comwrote:

Can you jack in my code above and send a PR?

— Reply to this email directly or view it on GitHubhttps://github.com/basho/puppet-riak/issues/8#issuecomment-14324708 .

[Iristyle]

This is also biting me...think its the same issue.

If I try to configure the pb_backlog value for instance, like

cfg => {
  riak_api => {
    pb_backlog => 64
  }
}

Then what I get in app.config is not what I want... it only has the pb_backlog value and is missing the pb_ip and pb_port values which would normally be automatically filled in. While I can push along the port, I don't know the IP.

[haf]

As I wrote in the commit: could you guys have a look?

[antonlindstrom]

Hi!

Thanks for the fix, quick notes on it:

The merge_hashes is probably misplaced in the lib/puppet/parser/util/ directory, it should be in lib/puppet/parser/functions/ otherwise we'll get hit with "unknown function" error.

The following block seems to be in the wrong place, I'm guessing it should be located before the merge_hashes/2: Wrong number of arguments.. error:

if args[0].is_a? Array
    args = args[0]
end

I'm also getting an error with wrong argument type Hash (expected Module) but haven't had time to check it out further.

Thanks Henrik!

[haf]

Move to #14

[Iristyle]

Sorry for the late response -- this appears to work for me with the pb_backlog setting

[haf]

Woop woop.