crowbar-core icon indicating copy to clipboard operation
crowbar-core copied to clipboard

batch build: fix merging of Array attributes

Open aspiers opened this issue 10 years ago • 4 comments

[Port of https://github.com/crowbar/barclamp-crowbar/pull/1337 to this new merged repo]

Switch from Chef::Mixin::DeepMerge.deep_merge! to #easy_merge!

TODO

Background

In easy_diff 0.0.4, #easy_merge! fails to handle merging of Arrays of Hashes, so we used Chef::Mixin::DeepMerge.deep_merge! instead. A solution for this issue has since been provided:

https://github.com/Blargel/easy_diff/pull/5

and subsequently merged and published as 0.0.5, which is now packaged in our repos, so this reason for avoiding #easy_merge! has now gone.

Additionally #deep_merge! didn't correctly merge Arrays at all. One example is with the neutron.ml2_type_drivers attribute, which defaults to ["gre"] whereas mkcloud often sets it to ["gre", "vlan"]. easy_diff's comparison correctly reports no removals and "vlan" as the only addition, but when this addition is fed to #deep_merge!, it resulted in the array being overwritten, so it would end up as ["vlan"], not ["gre", "vlan"].

Since #easy_merge! comes from the same easy_diff gem as `#easy_diff, in theory they should work nicely together (and the above is one concrete example of that), so it's better to stick with that.

aspiers avatar Sep 16 '15 10:09 aspiers

:+1:

tboerger avatar Sep 17 '15 07:09 tboerger

LGTM :+1:

mjura avatar Sep 18 '15 11:09 mjura

This is not ready for merging due to the breakage referenced above.

aspiers avatar Sep 18 '15 12:09 aspiers

Still WIP, but marked by a label now.

aspiers avatar Sep 23 '15 11:09 aspiers