broken deep_merge of arrays between cookbook and role defaults

Issue appears to be in chef/lib/chef/run_list/run_list_expansion.rb in the apply_role_attributes method, the role.default_attributes are always merged with an empty hash in @default_attrs at least once, this eats the !merge exclusion which needs to be preserved for the later deep merge with cookbook defaults.

eg:

@default_attrs: {}
role.default_attributes: {"test1"=>{"t1"=>["!merge:deux"], "t2"=>["!merge:"], "t3"=>["!merge:", "deux", "trois"], "t4"=>["!merge:", "quatre", "cinq", "six"], "t5"=>["quatre", "cinq", "six"]}}
deep_merge result: {"test1"=>{"t1"=>[], "t2"=>[], "t3"=>[], "t4"=>[], "t5"=>["quatre", "cinq", "six"]}}

Scratch that last one, doesn't fix the problem if multiple roles are in the run_list. Better solution I think is to delay the negation part of the merge until node.apply_expansion_attributes is run.

Okay so third try at this: the issue is roles are merged with each other in run_list/run_list_expansion.rb apply_role_attributes() this evaluates the !merge negation and removes it from the array. These fully merged attributes are later merged again with node data in node.rb apply_expansion_attributes - the !merge notation has been lost by this point, meaning you can't exclude any node defaults or overrides.

I can't see an easy way to have a !merge in a role be maintained all the way down the chain until the node data is merged - the only solution I see is to introduce some role, node and probably environment specific exclusion notations like !amerge,!a-m (attributes), !emerge,!e-m (environment) !rmerge,!r-m (roles - same as current !merge).

This a bit esoteric but the !merge knockout functionality has proved useful for some of us.

Implemented the idea in the last comment, please pull from

https://github.com/mdkent/chef/commits/CHEF-2737

This change shouldn't affect existing behavior - users have to use the new knockout prefixes to get the new and correct functionality.

With the updated syntax the original example now returns correctly:

role attributes using !a-merge (don't merge file attributes):

default_attributes "test1" => {
 :t1 => [ "!a-merge:deux" ],
 :t2 => [ "!a-merge" ],
 :t3 => [ "!a-merge", "deux", "trois" ],
 :t4 => [ "!a-merge", "quatre", "cinq", "six" ],
 :t5 => [ "quatre", "cinq", "six" ]
}

produces

[Thu, 01 Dec 2011 11:00:06 -0800] INFO: BLA1: ["un", "trois"]
[Thu, 01 Dec 2011 11:00:06 -0800] INFO: BLA2: []
[Thu, 01 Dec 2011 11:00:06 -0800] INFO: BLA3: ["deux", "trois"]
[Thu, 01 Dec 2011 11:00:06 -0800] INFO: BLA4: ["quatre", "cinq", "six"]
[Thu, 01 Dec 2011 11:00:06 -0800] INFO: BLA5: ["un", "deux", "trois", "quatre", "cinq", "six"]

Still not 100% happy with this:

• in a role "!a-merge" will delete everything for that array, rather than just file attributes
• in a role "!a-merge:x" will delete any instance of x, rather than just one from file attributes

Okay I've reworked this yet again and am happy with it:

https://github.com/mdkent/chef/tree/CHEF-2737

Notes

• allowed for full dup'ing of mashes
• deep merged contents are no longer modified in place (will probably prevent some future ugly bug)
• works using every permutation i can dream up (eg: env excludes a file attrib default, env excludes a role override etc) and behavior is consistent

I can and will update deep merge doc to illustrate how this works if this is merged.

Can anyone provide a specific use case for this?

Dan said that some of the original Hosted Chef features that looked like this were removed because they were too complicated to reason about and nobody was using them. It sounds like those features allowed you to control the merge across attribute types, e.g. default, override, normal.

We're trying to reason if this kind of complex functionality is one that provides a level of flexibility that you can't get otherwise.

I'm going to send a note to the dev list as well.

For me it boils down to maximum flexibility in reducing attribute duplication across file attributes/environments and roles. Say we have an attribute file in a cookbook with

attributes/demo.rb:

default[:test1] = {
  :t1 => [ "standard1", "standard2", "standard3" ]
}

and this cookbook works for 95% of our servers but we have one that can't implement "standard2" for some reason so we override it in a role

roles/demo.rb:

override_attributes :test1 => {
  :t1 => [ "standard1", "standard3" ]
}

we end up duplicating the default values rather than excluding what we don't want. Imagine if these arrays are dozens of items long (we have a couple here), it gets ugly.

Of course we could exclude "standard2" with conditions (say based on fqdn) in a recipe, template, within the attribute file itself, or we could argue these attributes shouldn't be in the cookbook at all, but I think supporting the exclusion right in the role is a useful behaviour:

roles/demo.rb:

default_attributes :test1 => {
  :t1 => [ "!a-merge:standard2" ]
}

I agree the behaviour is a bit awkward to verbalize, but some improved examples on http://wiki.opscode.com/display/chef/Deep+Merge could illustrate it's purpose.

As for usage I know the role !merge stuff is getting some use (we use it a lot in inherited roles here) - and was how llaurent stumbled on this behavior and expecting it to work in the same fashion across all the levels of precedence. Lack of interest might be partly my fault as well, after the deep merged stuff was pulled I didn't add a proper doc on it until Aug 16, 2011 - I doubt anyone not trawling through code noticed it until then.

Another thing to note about these changes is they shouldn't cause regressions for anyone relying to the existing behaviour (at least to my knowledge).

Long discussion about this in

http://lists.opscode.com/sympa/arc/chef/2011-12/msg00079.html

not much appetite for merging this, going to close as won't fix and take some of the code into a new cleanup ticket.

Improvements from this filed in CHEF-2818

Subtractive merge is removed with CHEF-3542