Old-style provider map is breaking with two cookbooks that both use Chef::Platform.set

[martinb3]

I ran across a problem on Friday where two dependencies, of a cookbook I'm working on, both had a file libraries/z_provider_mapping.rb that contained calls to Chef::Set.platform. One was mysql2_chef_gem and the other was automatic_updates.

While I know the method used by the cookbooks isn't preferred in Chef 12.4.x, there's a lot of cookbooks attempting to maintain compatibility with Chef 11.x by doing things this way. However, when two cookbooks both use this method, I was seeing that automatic_updates was breaking the provider map entries from mysql2_chef_gem. This led to many instances of wrong number of arguments (2 for 0) because Chef would fall through to matching providers by class name, and mysql2_chef_gem has an abstract class at the same name and level of the class hierarchy (so the class selected by name was invalid). I believe this is happening to many other folks using mysql2_chef_gem, e.g. sinfomicien/mysql2_chef_gem#6, sinfomicien/mysql2_chef_gem#5, sinfomicien/mysql2_chef_gem#4.

While I know that the 'correct' thing to do is update cookbooks to use Chef 12-style provides:, I think something is still broken in Chef 12.4.1 when it comes to maintaining compatibility of the platform map behavior. I've created an example cookbook to demonstrate this problem.

In that example, check out the following libraries/z_provider_map.rb (I've pasted the output back into the source, when run on chef 12.4.1):

pp "Chef::Platform.send(:platforms) = #{Chef::Platform.send(:platforms)}"
# Output: "Chef::Platform.send(:platforms) = {:default=>{}}"

pp "Setting platform mapping for :foo"
Chef::Platform.set platform: :centos, version: '< 7.0', resource: :foo, provider: Chef::Provider::Foo::Beta
Chef::Platform.set platform: :centos, version: '>= 7.0', resource: :foo, provider: Chef::Provider::Foo::Alpha
Chef::Platform.set platform: :ubuntu, resource: :foo, provider: Chef::Provider::Foo::Beta
pp "Chef::Platform.send(:platforms) = #{Chef::Platform.send(:platforms)}"
# Output:
# Chef::Platform.send(:platforms) = {
  # :default=>{},
  # :centos=>{\"< 7.0\"=>{:foo=>Chef::Provider::Foo::Beta}, \">= 7.0\"=>{:foo=>Chef::Provider::Foo::Alpha}},
  # :ubuntu=>{:default=>{:foo=>Chef::Provider::Foo::Beta}}
# }

pp "Setting platform mapping for :bar"
Chef::Platform.set platform: :centos, resource: :bar, provider: Chef::Provider::Bar
Chef::Platform.set platform: :ubuntu, resource: :bar, provider: Chef::Provider::Bar
pp "Chef::Platform.send(:platforms) = #{Chef::Platform.send(:platforms)}"
# Output
# Chef::Platform.send(:platforms) = {
  # :default=>{},
  # :centos=>{:default=>{:bar=>Chef::Provider::Bar}},
  # :ubuntu=>{:default=>{:foo=>Chef::Provider::Foo::Beta, :bar=>Chef::Provider::Bar}}
# }

It seems like the second set of Chef::Platform#set calls are wiping out map entries created by the first set of calls. I suspect that either the cookbooks I referenced above are 'doing it wrong' for Chef 11.x compatibility, or there is a subtle bug in the (https://github.com/chef/chef/blob/12.4-stable/lib/chef/platform/provider_mapping.rb#L109)[set method] of platform/provider_mapping.rb.

It's pretty clear that lines like this one will wipe out any existing map entries when called. Is this a mistake in usage where cookbooks from the Chef-11 era shouldn't be using Chef::Platform.set for backwards compatibility in Chef 12? Or is this a case where the z_provider_mapping.rb pattern is okay, but the methods that manipulate the platform map are not supposed to be deleting previous entries?

If you read down this far, thank you!

/CC some of my colleagues, @jarosser06
/CC some interested parties that are relevant for the commits on this code, @lamont-granquist @jkeiser @danielsdeleo @coderanger

[lamont-granquist]

I think there is a bug. Figuring it out definitely isn't very high on my list since the ultimately right thing to do is the "workaround" of correctly using Chef 12 provides syntax (the mysql cookbook is currently the best reference for the most complicated things you can do and be portable across Chef-11/Chef-12.0/Chef-12.4).

[bluejaguar]

As someone new to chef who is trying to install chef (chef-server-core_12.2.0-1) and opscode-manage (opscode-manage_1.21.0-1) and encountering this blocker, hearing that a solution to this 'isn't very high' on the current priority list does not give warm and fuzzies. Can we please get a work-around, patch, and ETA on a solution? Of interest this error does not occur on RHEL 5.9 but is seen on Centos 6.6 and Mint 17.0, 17.2. Currently on Centos 6.6 nginx won't come up (possibly related to sous-chefs/nginx#342) Is chef usually more stable than just now?

[martinb3]

Hi @bluejaguar -- I'm the original bug reporter. This bug affects chef-client versions using a style of resource/provider from Chef < 12.x, so it shouldn't be affecting anyone on the 12.x.x versions (released almost a full 12 months ago). I reported it as I'm having to maintain some cookbooks that require Chef 11.x compatibility.

I'm not sure you're going to be affected based on your versions, but it would be best to address the cookbook author or chef-server project if they are using Chef 11-style provider mappings. The community-contributed nginx cookbook issue you referenced doesn't appear related to me.

Hope the additional detail helps! - @martinb3

[bluejaguar]

Hi Martin, so where should I send in a bug report for this (Linux Mint 17.2)

> opscode-manage reconfigure

[2015-09-17T12:49:39-04:00] INFO: Started chef-zero at chefzero://localhost:8889 with repository at /opt/opscode-manage/embedded
...
[2015-09-17T12:49:39-04:00] INFO: *** Chef 12.4.1 ***
...
[2015-09-17T12:49:46-04:00] WARN: Class Chef::Provider::ComponentRunitSupervisor does not declare 'resource_name :component_runit_supervisor'.
[2015-09-17T12:49:46-04:00] WARN: This will no longer work in Chef 13: you must use 'resource_name' to provide DSL.
�[0m
================================================================================�[0m
�[31mError executing action `create` on resource 'component_runit_supervisor[opscode-manage]'�[0m
================================================================================�[0m

�[0mArgumentError�[0m
-------------�[0m
wrong number of arguments (2 for 0)�[0m

�[0mResource Declaration:�[0m
---------------------�[0m
# In /opt/opscode-manage/embedded/cookbooks/cache/cookbooks/enterprise/recipes/runit.rb
�[0m
�[0m 28: component_runit_supervisor node['enterprise']['name'] do
�[0m 29:   ctl_name node[node['enterprise']['name']]['ctl_name'] ||
�[0m 30:     "#{node['enterprise']['name']}-ctl"
�[0m 31:   sysvinit_id node[node['enterprise']['name']]['sysvinit_id']
�[0m 32:   install_path node[node['enterprise']['name']]['install_path']
�[0m 33: end
�[0m
�[0mCompiled Resource:�[0m
------------------�[0m
# Declared in /opt/opscode-manage/embedded/cookbooks/cache/cookbooks/enterprise/recipes/runit.rb:28:in `from_file'
�[0m
�[0mcomponent_runit_supervisor("opscode-manage") do
�[0m  action [:create]
�[0m  retries 0
�[0m  retry_delay 2
�[0m  default_guard_interpreter :default
�[0m  declared_type :component_runit_supervisor
�[0m  cookbook_name "enterprise"
�[0m  recipe_name "runit"
�[0m  ctl_name "opscode-manage-ctl"
�[0m  sysvinit_id "CM"
�[0m  install_path "/opt/opscode-manage"
�[0mend
�[0m
�[0m[2015-09-17T12:49:46-04:00] INFO: Running queued delayed notifications before re-raising exception
[2015-09-17T12:49:46-04:00] ERROR: Running exception handlers
[2015-09-17T12:49:46-04:00] ERROR: Exception handlers complete
[2015-09-17T12:49:46-04:00] FATAL: Stacktrace dumped to /opt/opscode-manage/embedded/cookbooks/cache/chef-stacktrace.out
[2015-09-17T12:49:46-04:00] ERROR: component_runit_supervisor[opscode-manage] (enterprise::runit line 28) had an error: ArgumentError: wrong number of arguments (2 for 0)
[2015-09-17T12:49:47-04:00] FATAL: Chef::Exceptions::ChildConvergeError: Chef run process exited unsuccessfully (exit code 1)

[coderanger]

@bluejaguar Issues with Chef Server belong over on https://github.com/chef/chef-server. But I can tell you now that Mint is not a supported OS for Chef Server so the answer from the team might be WORKSFORME.

[lamont-granquist]

@martinb3 re:

"because Chef would fall through to matching providers by class name, and mysql2_chef_gem has an abstract class at the same name and level of the class hierarchy ."

That reminds me of yak shaving I did when I cleaned up the mysql cookbook for Chef-11/12.0/12.4 compatibility.

sous-chefs/mysql@2e438b2#diff-3252c3ce1094cb10b371a5b57c233aa1L3

Patches like that where I renamed Chef::Provider::MysqlService::Smf to Chef::Provider::MysqlServiceSmf were done deliberately to avoid Chef 12.4.0 from wiring up that to smf in the DSL automagically (instead it wires up mysql_service_smf which nobody should be using).

I don't think Chef::Platform.set is in any way broken, but it is lowest-priority (by design, since 12.0) and its being overridden by some of the new wiring in 12.4.0 that is at a higher level.

Don't know how I feel about this since cookbook code can always follow the pattern set in the mysql cookbook. At the same time what I ran into there was mildly awful, and I think goes against having magic in the DSL wired up to the class names.

I know that @jkeiser has an opinion that the current behavior is good and saves typing, however, cases like this show that people aren't necessarily going to be thinking about their class names getting wired up automatically to the DSL -- and class names that seem to make sense may stomp on other things in the DSL.

[lamont-granquist]

[ and that might be a bad example, but it was the first one i picked -- i can't recall exactly which class name trolled me hard in the mysql cookbook and made me flatten the namespace and rename all of them -- it may also have been a resource class getting auto-wired-up that was the real issue ]

[martinb3]

I don't think Chef::Platform.set is in any way broken, but it is lowest-priority (by design, since 12.0) and its being overridden by some of the new wiring in 12.4.0 that is at a higher level.

@lamont-granquist I can definitely fix the cookbooks that are affected, so that's not really a problem.

It just seemed like there's probably still a bug in there for folks depending on the fall through logic, though, for anyone using older versions. In my testing, I noticed that the map is being partially wiped out when Chef::Platform.set is called; see my original bug description's code block -- after setting the provider for :bar on :ubuntu and :centos, the provider mapping on :centos for :foo has been lost.

[lamont-granquist]

I don't see where Chef::Platform has been changed that much recently and where it would show up like that. Certainly not an obvious bug. Most of the complicated logic hasn't been touched since Dan split the concerns up.

[lamont-granquist]

Well, Chef::Platform.set is dead, dead, dead now.

Unsure of where this one wound up, but its never getting fixed now, we certainly do not support Chef-11 backcompat in Chef-13/14/15 and Chef-12 is also dead and won't get any new releases.

[lock]

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.