ggiamarchi / vagrant-openstack-provider

Use Vagrant to manage OpenStack Cloud instances.
MIT License
247 stars 101 forks source link

Get shell provisioner class from plugin manager #253

Closed Sharpie closed 8 years ago

Sharpie commented 8 years ago

Vagrant core tries very hard to be lazy and only loads plugin classes when necessary. This caused a break in version 0.7.0 when a test was added to the provision action to check if an object is of class VagrantPlugins::Shell::Provisioner. The vagrant up action triggers a lazy load of this class, while the vagrant provsion action does not.

This patch ensures the provisioner class is loaded by retrieving it through the Vagrant plugin manager instead of using a direct reference.

Fixes ggiamarchi/vagrant-openstack-provider#240

Sharpie commented 8 years ago

Whoof. That is some nasty monkeypatching in the spec tests. One sec...

Sharpie commented 8 years ago

Spec tests fixed.

jhoblitt commented 8 years ago

I've run into this as well.

jhoblitt commented 8 years ago

@Sharpie Hey Charlie -- I tested your branch yesterday and it did indeed allow me to run a simple shell provisioner (yay!). However, several other provisioners didn't run at all (eg., vagrant-puppet-install). I'm traveling today, and haven't had a chance to reproduce, so I'm not even sure if it's a directly related problem -- but I suspect there is still something odd going on with this plugin.

Sharpie commented 8 years ago

@jhoblitt I would be surprised if it was directly related --- this change affects one conditional that causes shell arguments to be transformed when triggered. Provisioners run regardless of how the conditional behaves.

Are you seeing error messages after applying the patch?

ggiamarchi commented 8 years ago

Tested. I confirm this patch fix the bug.

@Sharpie Thanks a lot ! @jhoblitt Feel free to submit a new issue for any other error. Also have a look at this one https://github.com/ggiamarchi/vagrant-openstack-provider/issues/248.

ggiamarchi commented 8 years ago

LGTM