chef_server_api does not correctly handle MergedConfig

[echohack]

53c8a24

This change causes a few issues:

1 - chef_server_api is expecting a MergedConfig object. When a MergedConfig object gets passed in, it dies on the merge! method because MergedConfig is not a hash. Like so:

NoMethodError
-------------
machine_batch[default] (/tmp/kitchen/cache/cookbooks/provisioner/providers/cluster.rb line 52) had an error:
NoMethodError: chef_client[QA1DLABTESTUBU01] (basic_chef_client::block line 132) had an error:
NoMethodError: Unexpected method merge! for MergedConfig with arguments [{:api_version=>"0"}]

2 - chef_server[:options] ||= {} is null coalescing to a hash, when it should be null coalescing to an empty MergedConfig.

As a result, since chef-provisioning has a dependency on Cheffish of cheffish ~> 1.1, this breaks a number of resources in the live version of chef-provisioning.

[asciifaceman]

I see you retracted your PR. Have you figured this out? I am running into the same thing with chef-client -z utilizing chef-provisioning-vsphere:

NoMethodError: machine[spacelab](vmware-provisioning::launchtest line 51) had an error: NoMethodError: chef_client[spacelab](basic_chef_client::block line 132) had an error: NoMethodError: Unexpected method merge! for MergedConfig with arguments [{:api_version=>"0"}]

[asciifaceman]

I am wondering, isn't "merge!" a typo for "merge" ?

[tyler-ball]

@asciifaceman merge! is an in-place merge while merge will return a new object which represents the merged objects (at least, thats normal Ruby-ism).

[asciifaceman]

So I am starting to think that something weird is going on here because my colleague has my exact same setup and versions but does not experience this when running his provisioning script...

[asciifaceman]

tested with a coworker, his worked fine on Debian without chefdk (just chef-client).

I have Ubuntu 14 + full chefdk environment and it doesn't work.

[asciifaceman]

We have fixed this for now by downgrading out cheffish from 1.3.1 to 1.2.0:

/opt/chefdk/embedded/bin/gem uninstall cheffish
/opt/chefdk/embedded/bin/gem install cheffish -v 1.2.0

We didn't have time to wait for a patch to fix this mistake.

[randomcamel]

I don't think 53c8a24 is the problem: self.chef_server_api was already passing a straight hash through to Cheffish::ServerAPI.new, and has done since at least v1.0.0. Also at least since 1.0.0, Cheffish::ServerAPI.new defaults its options to a Hash: https://github.com/chef/cheffish/blob/v1.0.0/lib/cheffish/server_api.rb#L34.

@echohack @asciifaceman Can you boil your code down to a repro case and a bundle list so we know what versions of everything you're using?

[asciifaceman]

My gems after reverting cheffish to 1.2.0

user@host:~/sf_development/ghe/cookbooks/--obfuscated--$ /opt/chefdk/embedded/bin/gem list

*** LOCAL GEMS ***

activesupport (4.2.3, 3.2.22)
addressable (2.3.8)
app_conf (0.4.2)
appbundler (0.4.0)
archive (0.0.6)
aruba (0.8.1, 0.7.4)
ast (2.1.0)
astrolabe (1.3.1)
aws-sdk-v1 (1.64.0)
axiom-types (0.1.1)
berkshelf (3.3.0, 3.2.4)
berkshelf-api (2.1.1)
berkshelf-api-client (1.3.0)
bigdecimal (1.2.4)
buff-config (1.0.1)
buff-extensions (1.0.0)
buff-ignore (1.1.1)
buff-ruby_engine (0.1.0)
buff-shell_out (0.2.0)
builder (3.2.2)
bundler (1.10.0)
cane (2.6.2)
celluloid (0.16.0)
celluloid-io (0.16.2)
CFPropertyList (2.3.1)
chef (12.4.1)
chef-config (12.4.1)
chef-dk (0.7.0)
chef-provisioning (1.3.0)
chef-provisioning-aws (1.3.1)
chef-provisioning-azure (0.3.2)
chef-provisioning-fog (0.13.2)
chef-provisioning-vagrant (0.9.0)
chef-provisioning-vsphere (0.8.0)
chef-vault (2.6.1)
chef-zero (4.2.3, 1.5.6)
cheffish (1.2)
chefspec (4.3.0)
childprocess (0.5.6)
cleanroom (1.0.0)
codeclimate-test-reporter (0.4.7)
coderay (1.1.0)
coercible (1.0.0)
contracts (0.11.0)
cookbook-omnifetch (0.2.1)
countloc (0.4.0)
crack (0.4.2)
cucumber (2.0.2, 1.3.20)
cucumber-core (1.2.0)
dep-selector-libgecode (1.0.2)
dep_selector (1.0.3)
descendants_tracker (0.0.4)
diff-lcs (1.2.5)
diffy (3.0.7)
docile (1.1.5)
domain_name (0.5.24)
em-winrm (0.7.0)
equalizer (0.0.11)
erubis (2.7.0)
eventmachine (1.0.8)
excon (0.45.4)
fakefs (0.6.7)
faraday (0.9.1)
fauxhai (2.3.0)
ffi (1.9.10)
ffi-yajl (2.2.2)
finstyle (1.5.0)
fission (0.5.0)
fog (1.33.0)
fog-atmos (0.1.0)
fog-aws (0.7.4)
fog-brightbox (0.8.0)
fog-core (1.32.1)
fog-dynect (0.0.1)
fog-ecloud (0.1.1)
fog-google (0.0.7)
fog-json (1.0.2)
fog-local (0.2.1)
fog-powerdns (0.1.1)
fog-profitbricks (0.0.5)
fog-radosgw (0.0.4)
fog-riakcs (0.1.0)
fog-sakuracloud (1.0.1)
fog-serverlove (0.1.2)
fog-softlayer (0.4.7)
fog-storm_on_demand (0.1.1)
fog-terremark (0.1.0)
fog-vmfusion (0.1.0)
fog-voxel (0.1.0)
fog-xml (0.1.2)
foodcritic (4.0.0)
formatador (0.2.5)
fuubar (1.3.3)
gherkin (2.12.2)
git (1.2.9.1)
grape (0.13.0)
grape-msgpack (0.1.2)
gssapi (1.2.0)
guard (2.13.0)
guard-compat (1.2.1)
guard-rspec (4.6.4)
gyoku (1.3.1)
hashie (2.1.2)
highline (1.7.3)
hitimes (1.2.2)
http (0.9.3, 0.9.0)
http-cookie (1.0.2)
http-form_data (1.0.1)
http_parser.rb (0.6.0)
httpclient (2.6.0.1)
i18n (0.7.0)
ice_nine (0.11.1)
inflecto (0.0.2)
inifile (2.0.2)
io-console (0.4.3)
ipaddress (0.8.0)
json (1.8.3, 1.8.1)
kitchen-docker (2.3.0)
kitchen-vagrant (0.18.0)
knife-spork (1.5.0)
knife-windows (0.8.6)
libyajl2 (1.2.0)
listen (3.0.3)
little-plugger (1.1.3)
logging (1.8.2)
lumberjack (1.0.9)
macaddr (1.7.1)
maruku (0.7.2)
metaclass (0.0.4)
method_source (0.8.2)
mime-types (2.6.1)
mini_portile (0.6.2, 0.6.0)
minitar (0.5.4)
minitest (5.8.0, 4.7.5)
mixlib-authentication (1.3.0)
mixlib-cli (1.5.0)
mixlib-config (2.2.1)
mixlib-install (0.7.0)
mixlib-log (1.6.0)
mixlib-shellout (2.1.0)
mocha (1.1.0)
molinillo (0.2.3)
moneta (0.6.0)
msgpack (0.5.12)
multi_json (1.11.2)
multi_test (0.1.2)
multi_xml (0.5.5)
multipart-post (2.0.0)
nenv (0.2.0)
net-http-persistent (2.9.4)
net-scp (1.2.1)
net-ssh (2.9.2)
net-ssh-gateway (1.2.0)
net-ssh-multi (1.2.1)
net-telnet (0.1.1)
nio4r (1.1.1)
nokogiri (1.6.6.2, 1.6.3.1)
nori (2.6.0)
notiffany (0.0.7)
octokit (3.8.0)
ohai (8.5.1)
paint (1.0.0)
parallel (1.6.1)
parser (2.2.2.6)
plist (3.1.0)
polyglot (0.3.5)
powerpack (0.1.1)
pry (0.10.1)
psych (2.0.5)
puma (1.6.3)
rack (1.6.4, 1.5.5)
rack-accept (0.4.5)
rack-mount (0.8.3)
rainbow (2.0.0)
rake (10.4.2, 10.1.0, 0.9.6)
rb-fsevent (0.9.5)
rb-inotify (0.9.5)
rbvmomi (1.8.2)
rdoc (4.1.0)
reel (0.5.0)
retryable (2.0.2, 2.0.1)
ridley (4.2.0)
rspec (3.3.0, 3.0.0)
rspec-core (3.3.2, 3.0.4)
rspec-expectations (3.3.1, 3.0.4)
rspec-its (1.2.0)
rspec-mocks (3.3.2, 3.0.4)
rspec-support (3.3.0, 3.0.4)
rspec_junit_formatter (0.2.3)
rubocop (0.32.1, 0.31.0)
ruby-progressbar (1.7.5)
ruby-shadow (2.4.1)
ruby_gntp (0.3.4)
rubyntlm (0.4.0)
rubyzip (1.1.7)
rufus-lru (1.0.5)
safe_yaml (1.0.4)
sawyer (0.6.0)
semverse (1.2.1)
serverspec (2.21.1, 2.21.0)
sfl (2.2)
shellany (0.0.1)
simplecov (0.10.0)
simplecov-html (0.10.0)
slop (3.6.0)
solve (2.0.1, 1.2.1)
specinfra (2.41.1, 2.40.2)
spork (0.9.2)
stuartpreston-azure-sdk-for-ruby (0.7.1)
syslog-logger (1.6.8)
systemu (2.6.5)
test-kitchen (1.4.2)
test-unit (2.1.6.0)
thor (0.19.1)
thread_safe (0.3.5)
timers (4.0.1)
treetop (1.4.15)
trollop (2.1.2)
tzinfo (1.2.2)
ubuntu_ami (0.4.1)
unf (0.1.4)
unf_ext (0.0.7.1)
uuid (2.3.8)
uuidtools (2.1.5)
varia_model (0.4.0)
virtus (1.0.5)
webmock (1.21.0)
websocket_parser (1.0.0)
winrm (1.3.4, 1.3.3)
winrm-s (0.3.1)
winrm-transport (1.0.2)
wmi-lite (1.0.0)
yajl-ruby (1.2.1)
yard (0.8.7.6)

I unfortunately have iterated my code too much for it to be relevant right at the moment, however I can say the error occurred on cheffish 1.3 and during the run_list portion after the machine had been built using chef/provisioning/vsphere_driver. I will see if I can spin up another dev environment and build a repro case but it might take a bit of time with my workload.

[echohack]

had been built using chef/provisioning/vsphere_driver

Hmmm, based on what @randomcamel is saying and this, I bet something in the vsphere_driver is passing a MergedConfig incorrectly. Something smells funky here: https://github.com/CenturyLinkCloud/chef-provisioning-vsphere/blob/5c9bbba9999de7e0e58f99cccb3b365d7617f413/lib/chef/provisioning/vsphere_driver/driver.rb#L147

Turns out @mwrock (the creator of the vsphere driver) sits right next to me, so I'll chat with him about it. 👍

[tyler-ball]

Okay, I'm going to close this because we don't think the original issue (chef_server_api expecting a merged_config) is accurate.

If we need to open a new issue I am in favor of updating merged_config to have the same API as a real Hash object (adding support for delete, merge, []=, etc.). I think that would clear up a lot of confusion as to when we can expect a merged_config and when we can expect a hash.

[mwrock]

Just to clarify how this happened in this case for the sake of posterity. I reproduced this and its ocurring where the vsphere driver initializes the ConvergenceStrategy. The MachineOptions are a MergedConfig. From what I can tell from the machine provider, its machine_options will always be a MergedConfig.

In the vshere driver I convert the convergence_options to a hash before passing to the convergence strategy but I do not convert its :options. I'll update the driver to ensure those are a plain Hash.

It does seem like this may be TMI for a driver author to be concerned with. APIs are hard.

[mwrock]

chef-provisioning-vsphere 0.8.1 released to handle this

[asciifaceman]

Thank you so much. tomorrow when I have more of my work day available, I will un-downgrade cheffish and see if 1.3 is working for me now :)

[mwrock]

cool. verified 1.3.1 is working now on our side.