Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1654, auto install vagrant plugin #1735

Merged
merged 6 commits into from Feb 27, 2019

Conversation

Projects
None yet
2 participants
@Mte90
Copy link
Contributor

commented Feb 18, 2019

Summary:

#1654

Checks

  • I've tested this PR with Vagrant vXX and VirtualBox vXX on Operating System
  • This PR is for the develop branch not the master branch
  • I've updated the changelog
  • This PR is complete and ready for review

@Mte90 Mte90 added the enhancement label Feb 18, 2019

@Mte90 Mte90 self-assigned this Feb 18, 2019

@Mte90 Mte90 requested a review from tomjn Feb 18, 2019

@tomjn

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

I don't know about adding vbguest, but have we tested how this would work with the contributor day installs? We spent a good hour trying to download a 10kb gem file to peoples machines over the wifi at WCEU last year so it needs to still be 100% offline when we need it

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2019

If the packages are already installed they are not downloaded from vagrant.
If during contributor days people will follow the instructions this problem doesn't happen.
We can remove the vbguest that is not mandatory compared to the hosts in case.

Another option looking at the docs is https://www.vagrantup.com/docs/vagrantfile/vagrant_settings.html#entry_point that let to configure the path where is the plugin file so is not downloaded.

@tomjn

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

hmm does that expect a gem file or a folder with ruby files? The docs on entry point are a little sparse

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2019

screenshot_20190219_154012
With this new code is using a local gem file that I put in the VVV folder.
I think that is interesting for us so we can do our version of the hostsupdater plugin that execute the hook early.

@tomjn

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

Where did the gem file get placed?

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2019

I put that file in the same folder of the VagrantFile but it is possible to configure the path.
We can do also that the CD-USB generator put that file in the VVV folder and avoid a step during contributor days.

@tomjn

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

Ah that sounds awesome :) Can you test with the contrib day output and double check? Should be enough to adjust the vagrant file in the output rather than rebuild the entire thing

@tomjn

tomjn approved these changes Feb 19, 2019

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2019

I am doing few tests and seems that there are problems when vagrant up is done in a subfolder after the automatic installer.
I am doing more analysis on that.

@Mte90 Mte90 changed the title Fix #1654, auto download vagrant plugin Fix #1654, auto install vagrant plugin Feb 19, 2019

@tomjn

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

@Mte90 you can use vagrant_dir = File.expand_path(File.dirname(__FILE__)), look at how the config file is being loaded

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2019

I switched to another version because there are problems.
First problem seems that using the native vagrant system the package is installed also if it is not the root folder but on starting again there is an error about not possible to load the plugin installed. Also this will install the package as local for that vagrant instance and not global.

So my solution is:

  • if the package is not installed
    • if local file available install it globally
    • else install from internet

We can do another package and call it in another way, also because seems that the original is like abandoned and upload in rubygems as second step.

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2019

I am already using vagrant_dir but seems a bug in vagrant because the path printed is right and also absolute.

@tomjn

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

@Mte90 is that why you use a system call instead of config.vagrant.plugins = {"vagrant-hostsupdater" => {"entry_point" => File.join(vagrant_dir, 'vagrant-hostsupdater.gem' )}}?

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Feb 22, 2019

I removed because there is a bug in vagrant.
The plugin is installed rightly locally only for the project but at the next run there is an error that say that cannot find the folder.
If you try you can see it, instead if installed globallythe problem doesn't happen.

@tomjn

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

but then don't we have to run the vagrant file a second time so the plugin is loaded?

@tomjn

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

And is there a vagrant issue we can refer to for the bug?

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Feb 23, 2019

Using this method or the native one is always required to restart vagrant.
The command is closed and alert for it.
About the vagrant bug I didn't had time to do more tests, if someone can check it too in a way to verify if it is me or a vagrant bug.

@tomjn

This comment has been minimized.

Copy link
Member

commented Feb 23, 2019

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Feb 24, 2019

It is the standard behaviour of vagrant up to be executed again if we use the native system.
Anyway there is a big alert to execute it again, the only other option is to do a shell script that before install the plugin and later execute vagrant but I don't think that is the best solution.

@tomjn

tomjn approved these changes Feb 27, 2019

@tomjn tomjn merged commit 64d393b into Varying-Vagrant-Vagrants:develop Feb 27, 2019

@tomjn

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

This makes hostupdater a requirement ( which most already consider it to be ), I've no qualms with that, the only other thing is that it also makes Vagrant 2.2 a minimum requirement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.