Freeze or dup global state data structures

[c10l]

For some reason, two plugins are interfering with each other in a very odd manner.

Vagrant-Butcher has a method to check whether a chef-client provisioner is running as it's only relevant if so.

Vagrant-Cachier uses a similar method to learn if it needs to cache Chef cookbooks.

Vagrant has recently (I reckon on 1.7.0) made changes to that object and a fix was put in place on Butcher, but Cachier hasn't been fixed yet.

Now, for some reason, if you have both plugins installed, Butcher never does anything. Uninstalling Cachier "fixes" the problem.

More information on this bug report: c10l/vagrant-butcher#57

I reckon the internals of one plugin should not interfere on the internals of another, and I have no idea why this is happening.

Thanks!

[sethvargo]

@cassianoleal are you able to provide debug output or any additional information? I'm not familiar with the inner workings of either of those plugins, so the possibilities for collision are endless...

[c10l]

Sure thing -- https://gist.github.com/Yserz/96ce0cd7e478a5b57dba

I couldn't find any useful information there though, and @Yserz was able to pinpoint the issue and monkey patch a fix through trial and error.

As far as I can tell, both my fix for Vagrant-Butcher and the monkey patch he applied on Vagrant-Cachier deal only with locally scoped variables and methods and should not have interfered with each other.

It's easy to reproduce the issue as well:

• Install vagrant-butcher on a clean Vagrant;
• vagrant up, then vagrant destroy a VM that uses the chef-client provisioner. You should see Butcher deleting the node and client from the Chef Server with these messages:

==> default: Chef node 'name' successfully butchered from the server...
==> default: Chef client 'name' successfully butchered from the server...

• Install vagrant-cachier and enable it by adding config.cache.scope = :box to the Vagrantfile;
• vagrant up and vagrant destroy again, and you won't see Butcher doing its thing.
• Monkey patch https://github.com/fgrehm/vagrant-cachier/blob/master/lib/vagrant-cachier/cap/linux/chef_file_cache_path.rb#L9 to test for both p.name and p.type
• up and destroy again, and you'll see butcher kicking in.

[sethvargo]

Are you potentially hitting this change? Example fix: https://github.com/berkshelf/vagrant-berkshelf/blob/cd5ee78f28ee9f6bec1b72fcdbcb042b0155aa04/lib/vagrant-berkshelf/helpers.rb#L77-L86

[c10l]

Yes, hence the fix mentioned in the OP. That doesn't explain why cachier's lack of a fix interferes with butcher.

[sethvargo]

@cassianoleal I believe the problem here is the use of keep_if:

[1] pry(main)> list = %w(a b c d)
=> ["a", "b", "c", "d"]
[2] pry(main)> result = list.keep_if { |i| i == "a" }
=> ["a"]
[3] pry(main)> result
=> ["a"]
[4] pry(main)> list
=> ["a"]

As you can see, keep_if is actually modifying the given object - it's an alias for select!.

Vagrant cachier should use select instead and then it won't be modifying the list of provisioners. The issue is that it's actually modifying global state in Vagrant.

@mitchellh do you think we should dup or freeze machine.config.vm.provisioners? That shouldn't be a thing that plugins should modify.

/cc @fgrehm

[c10l]

I guess you're right. I've opened a pull request on vagrant-cachier to replace keep_if with select: fgrehm/vagrant-cachier#135

Thanks for your help, I'll close this issue now. :)

[sethvargo]

@cassianoleal I'm going to re-open this. There's still a "bug" here in the sense that we need to freeze or dup these mutable data structures.

[c10l]

Fair enough! :)

[mitchellh]

I'd say we'll keep this a plugin issue. It is quite rare this happens and it would be hard to track down every mutable data structure. Let's just solve it on per-issue basis.