chef-boneyard / chef-provisioning

A library for creating machines and infrastructures idempotently in Chef.
Apache License 2.0
524 stars 164 forks source link

machine() add_machine_options fails to merge bootstrap_options #455

Open keen99 opened 9 years ago

keen99 commented 9 years ago

Originally mentioned this in https://github.com/chef/chef-provisioning/issues/383#issuecomment-134378241

Spent more time digging and confirmed that machine()'s add_machine_options uses a different merge strategy than add_machine_options() does.

https://github.com/chef/chef-provisioning/blob/master/lib/chef/provisioning/chef_run_data.rb#L79

    def add_machine_options(options, &block)
      with_machine_options(Chef::Mixin::DeepMerge.hash_only_merge(current_machine_options, options), &block)
    end

vs

https://github.com/chef/chef-provisioning/blob/master/lib/chef/resource/machine.rb#L100

  def add_machine_options(options)
    @machine_options = Cheffish::MergedConfig.new(options, @machine_options)
  end

The end result is if you try to merge bootstrap_options at the machine level, you get:

TypeError
---------
can't dup NilClass

Simple to test:

machine "test" do
    machine_options :bootstrap_options => { :Boption_four => 'four' }
    add_machine_options :bootstrap_options => { :Boption_five => 'five' }
    action :allocate
end

or

  with_machine_options({
    :Moption_one => "one",
    :Moption_two => "one",
    :bootstrap_options => {
      :Boption_one => "one",
      :Boption_two => "one",
     },
  })
  add_machine_options({
    :Moption_two => "two",
    :bootstrap_options => {
      :Boption_two => "two",
     },
  })
machine "test" do
    add_machine_options :bootstrap_options => { :Boption_five => 'five' }
    action :allocate
end

Seems like the fix would be to either consume add_machine_options() for the merge, or switch to using the same DeepMerge strategy. I might be able to get a chance to submit a PR on it...

keen99 commented 9 years ago

Note: I'm testing against chef-provisioning (1.3.0) at the moment, but this doesn't seem to have changed since 2014 with fd2effc

tyler-ball commented 9 years ago

We just need to delete add_machine_options everywhere. See https://github.com/chef/chef-provisioning/pull/457#issuecomment-146016529