Skip to content
This repository has been archived by the owner on Jan 6, 2023. It is now read-only.

Allow to force vagrant provider in vagrant up #187

Closed
wants to merge 1 commit into from

Conversation

apatard
Copy link
Member

@apatard apatard commented Sep 6, 2022

Currently, vagrant is still choosing the provider to use on its own
liking. While it's fine for cases like our CI or people who are fine
with default VM configurations, it's not always the case.

See:

So, add a new configuration setting to call vagrant up with
--provider, in order to force the provider to use and lead to an
error if it's not usable. The default value is to keep current
behaviour to not break things.

Signed-off-by: Arnaud Patard [email protected]

Currently, vagrant is still choosing the provider to use on its own
liking. While it's fine for cases like our CI or people who are fine
with default VM configurations, it's not always the case.

See:
- ansible-community#174
- https://github.com/ansible-community/molecule-vagrant/issues/181

So, add a new configuration setting to call vagrant up with
--provider, in order to force the provider to use and lead to an
error if it's not usable. The default value is to keep current
behaviour to not break things.

Signed-off-by: Arnaud Patard <[email protected]>
@hswong3i
Copy link
Collaborator

hswong3i commented Sep 7, 2022

I am not a fans for "provider_force" since "provider" should already enough and align with original Vagrant command line and config file option.

The problem we have is we didn't pass that "provider" parameter correctly to where it should associated underlaying command line option and config file, but leave it empty in our previous implementation so Vagrant need to guess with its own default style.

IMHO, original implementation coming with a hidden bug, but not a feature. Adding additional redundant parameter for keeping a buggy implementation is not a fix but overkilling design...

@apatard
Copy link
Member Author

apatard commented Sep 12, 2022

I am not a fans for "provider_force" since "provider" should already enough and align with original Vagrant command line and config file option.

The vagrant command line doesn't force to use --provider. I'm using vagrant (without molecule) to create test VMs and I'm starting them with vagrant up with no extra argument. And it's working as intended. Until this PR, it was not exactly same way of working since it was not possible to use --provider if needed/wanted. I'm merely trying to align things.

If your point is that one must always use --provider, then why not asking vagrant to force --provider usage when using vagrant up ?

The problem we have is we didn't pass that "provider" parameter correctly to where it should associated underlaying command line option and config file, but leave it empty in our previous implementation so Vagrant need to guess with its own default style.

I do understand that but vagrant does the same. I can write my own Vagrantfile with configuration options for virtualbox and run vagrant up and I'll get the problem you're talking of. And some time, it's fine, and some other time, it's not. There's no way to know that.
For instance, look at this part of one of the CI molecule.yml file:

platforms:
  - name: instance
    box: ${TESTBOX:-centos/7}
    instance_raw_config_args:
      - "vm.provision :shell, inline: \"echo #{Dir.pwd} > /tmp/workdir\""

There's nothing specific to a provider in there, so why should I fail when run with vbox instead of libvirt ? The test is still fine, since we only care about /tmp/workdir inside the VM.

So, imho, it's up to the user to ensure that the right provider is used with --provider.

@hswong3i
Copy link
Collaborator

hswong3i commented Sep 12, 2022

What if handle in this way:

  • make our provider parameter become optional
  • If user don’t specify it in their config file, we simply skip in and pass nothing to vagrant
  • If user do specify something, like virtualbox or libvirt or vmware or whatever underlaying vagrant supported, we pass it to vagrant as-is specified

My key point is: let’s align our config file behavior as similar as vagrant as possible, but not adding additional for handling that.

P.S. empty provider NOT ALWAYS equal to VirtualBox, because we could change it from global environment variable VAGRANT_DEFAULT_PROVIDER, see https://www.vagrantup.com/docs/other/environmental-variables

@apatard
Copy link
Member Author

apatard commented Sep 14, 2022

What if handle in this way:

* make our provider parameter become optional

* If user don’t specify it in their config file, we simply skip in and pass nothing to vagrant

* If user do specify something, like virtualbox or libvirt or vmware or whatever underlaying vagrant supported, we pass it to vagrant as-is specified

oh, I kind of like the idea. I've yet to check how to change the CI to cope with that. The CI is doing "clever" things to avoid changing the test scenario molecule.yml file according to the provider used by the CI, and obviously, implementing what you're suggesting will break it.

Also some work to deal with default ram/vcpu configuration as it's provider specific but I guess, not a big problem.

My key point is: let’s align our config file behavior as similar as vagrant as possible, but not adding additional for handling that.

P.S. empty provider NOT ALWAYS equal to VirtualBox, because we could change it from global environment variable VAGRANT_DEFAULT_PROVIDER, see https://www.vagrantup.com/docs/other/environmental-variables

yeah, I know. Kind of makes life harder.

@apatard
Copy link
Member Author

apatard commented Sep 23, 2022

@hswong3i I've implemented the proposed behaviour in #190. It's still in draft form in case something goes wrong in the CI or if you have comments. I'm closing this PR. Please provide feedbacks in #190

@apatard apatard closed this Sep 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants