berkshelf / vagrant-berkshelf

A Vagrant plugin to add Berkshelf integration to the Chef provisioners
Other
378 stars 100 forks source link

Silently ignores explicit `chef.cookbooks_path` configuration #274

Closed pezra closed 8 years ago

pezra commented 9 years ago

There are currently two pending PRs to resolve this issue: #268 and #266. Could we get one of them merged and released?

reset commented 9 years ago

@pezra in the past, Chef/Vagrant would complain if the "cookbooks" directory didn't exist or was empty. If this hasn't changed we can't append to the list and we still need to overwrite.

pezra commented 9 years ago

Right, so if the Vagrantfile doesn't explicitly specify a cookbooks directory, the berkshelf cookbooks dirctory should be the only one. OTOH, if the Vagrantfile does explicitly specify one or more cookbook directories those should be included also. #268 does exactly that.

reset commented 9 years ago

@pezra unfortunately if the value is unset, Vagrant forces a default value for you: https://github.com/mitchellh/vagrant/blob/master/plugins/provisioners/chef/config/chef_zero.rb#L43-L47. This is why we need to overwrite.

pezra commented 9 years ago

@reset, is your concern is the following scenario: a vagrantfile that enables berkshelf, does not explicitly specify a cookbook dir and uses the chef_solo provisioner?

I tried that scenario with #268 w/ vagrant 1.7.2 and it worked with both the chef solo and chef zero provisioners.

pezra commented 9 years ago

@reset, et al, any thoughts on this issue?

dmerrick commented 9 years ago

I'm interested in this too. It seems in the current state, I can't include cookbooks from both my Berksfile and my chef.cookbooks_path.

dkinzer commented 9 years ago

@pezra @dmerrick why not just add any other cookbook paths to your Berksfile and have berkshelf be the one true src for all your cookbook dependencies?

pezra commented 9 years ago

@dkinzer, i finally found the time to try your suggestion out and it worked! feels like a bit of a kludge but C'est la vie.

i still think it unfortunate that settings in the vagrant file are silently overridden. at the very least there should be an error telling the user that setting the cookbook path is not support.

dkinzer commented 9 years ago

@pezra, agreed. It's very disorienting when things that used to work break for no logical reason. That's life indeed! :)

chris-rock commented 9 years ago

+1

bigplants commented 9 years ago

+1

williambillypaul commented 9 years ago

+1

gyohk commented 9 years ago

+1

davidpando commented 9 years ago

+1

graf0 commented 8 years ago

+1

todd-a-jacobs commented 8 years ago

+1

kzw commented 8 years ago

+1

apanagiotou commented 8 years ago

+1

tas50 commented 8 years ago

This is fixed by #266