-
Notifications
You must be signed in to change notification settings - Fork 326
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
(QENG-1146) Add vagrant_fusion, vagrant_workstation and vagrant_virtualbox providers #271
Conversation
Can one of the admins verify this patch? |
@@ -41,6 +41,9 @@ def make_vfile hosts, options = {} | |||
v_file << " c.vm.provider :virtualbox do |vb|\n" | |||
v_file << " vb.customize [\"modifyvm\", :id, \"--memory\", \"#{options['vagrant_memsize'] ||= '1024'}\"]\n" | |||
v_file << " end\n" | |||
v_file << " c.vm.provider :vmware_fusion do |v|\n" | |||
v_file << " v.vmx[\"memsize\"] = \"#{options['vagrant_memsize'] ||= '1024'}\"\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that you have split this into two different hypervisors we should handle the differences in vagrant file generation in the appropriate new files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vagrantfiles can be made provider agnostic, and the provider specific settings go inside of the config.vm.provider :whatever
blocks as done here. That way the only difference between using different providers is the argument passed to vagrant up
. But I can refactor it if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I should rephrase that. I think that it does not need to be split out because Vagrantfiles can handle multiple providers and I will leave it as-is unless you would really really like it split out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like vagrant.rb to be provider agnostic. You could have most of the vagrant file generation and then have a call to something like "provider_file_chunk" and get it added in - that should have proper separation while avoiding much code duplication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, will do :).
# By default vmware_fusion creates a .vagrant directory relative to the | ||
# Vagrantfile path. That means beaker tries to scp the VM to itself unless | ||
# we move the VM files elsewhere. | ||
ENV['VAGRANT_VMWARE_CLONE_DIRECTORY'] = '~/.vagrant/vmware_fusion' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that path in a variable anyway? Not a big fan of hard coding it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not. This is the published method of modifying the behavior of the vmware_fusion vagrant plugin as per http://docs.vagrantup.com/v2/vmware/configuration.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The virtualbox vagrant provider already puts its VMs in ~/.vagrant
so I was making the fusion boxes go to the same place.
Waiting on response to review comments + needs a rebase. |
Rebased |
retest this please |
🔴 Test failed. |
Failure unrelated to patch. |
@hunner this will need an update to include the latest patch to vagrant that supports including additional disk to individual virtual box instances. |
k |
Needs to be updated to handle the new support for vagrant virtualbox additional disks. This occurs in the main host creation loop so it's different from the vm size settings. |
Can one of the admins verify this patch? |
Superseded by #383 |
Does this also support the use of VMware Workstation? ...it should if the provider for vagrant can be set to (also, I realise that beaker is currently difficult to install in a Windows environment, but asking this helps sort that out) |
We do not currently support the vmware_workstation provider - please file a github issue to request that feature. |
I started with the bit that broke first #373 |
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
Closes #383 |
Changes since last closed:
|
ok to test |
💚 Test passed. |
<3 |
🔴 Test failed. |
…albox providers Currently the vagrant hypervisor provider just does virtualbox. This allows the vagrant vmware_fusion and vmware_workstation plugins to be used instead.
(QENG-1146) Add vagrant_fusion, vagrant_workstation and vagrant_virtualbox providers
🔴 Test failed. |
Currently the vagrant hypervisor provider just does virtualbox. This allows the vagrant vmware_fusion plugin to be used instead.