sprotheroe / vagrant-disksize

Vagrant plugin to resize disks in VirtualBox
MIT License
479 stars 52 forks source link

Breaks other providers if installed on system without VBoxManage #45

Open electrofelix opened 3 years ago

electrofelix commented 3 years ago

Where someone has been using VirtualBox in the past along with other providers, a subsequent removal of VirtualBox will result in an exception for other providers about VBoxManage executable not being found.

Resulting in a message that looks like:

$ vagrant up default --no-provision --provider lxd
Bringing machine 'default' up with 'lxd' provider...
Vagrant could not detect VirtualBox! Make sure VirtualBox is properly installed.
Vagrant uses the `VBoxManage` binary that ships with VirtualBox, and requires
this to be available on the PATH. If VirtualBox is installed, please find the
`VBoxManage` binary and add it to the PATH environmental variable.

From https://github.com/vagrant-libvirt/vagrant-libvirt/issues/748#issuecomment-531612534

It can be difficult for people to understand where this error is coming from, as even if understanding the plugin only works with VirtualBox, they would not be expecting that it would cause this.

It appears to be caused by the code: https://github.com/sprotheroe/vagrant-disksize/blob/56b46a9285758ef6d89875755d2a96f0eb801b97/lib/vagrant/disksize/actions.rb#L11-L16

Because this will be run when the class is evaluated rather than only when it is instantiated, it will run at load time https://github.com/sprotheroe/vagrant-disksize/blob/56b46a9285758ef6d89875755d2a96f0eb801b97/lib/vagrant/disksize.rb#L27 instead of when the class is instantiated and run, which should only occur if the virtualbox provider is in use base on https://github.com/sprotheroe/vagrant-disksize/blob/56b46a9285758ef6d89875755d2a96f0eb801b97/lib/vagrant/disksize.rb#L30.

Based on https://github.com/sprotheroe/vagrant-disksize/blob/56b46a9285758ef6d89875755d2a96f0eb801b97/lib/vagrant/disksize/actions.rb#L18-L26 I'd suggest considering switching to a class variable @@medium using the following:

        def initialize(app, env)
          @app = app
          @machine = env[:machine]
          @enabled = true
          if @machine.provider.to_s !~ /VirtualBox/
            @enabled = false
            env[:ui].error "The vagrant-disksize plugin only supports VirtualBox at present. Disk size will not be changed."
          else
            if !defined?(@@medium)
              VB_Meta = VagrantPlugins::ProviderVirtualBox::Driver::Meta.new()
              if VB_Meta.version >= '5.0'
                @@medium = 'medium'
              else
                @@medium = 'hd'
              end
          end
        end

I would have assumed there should be no need for checking the provider in this function based on what I recall about how the hook mechanism should mean that this action is only triggered if the VirtualBox provider is being used to bring up the machine, however I've limited my suggestion above to follow the existing code.

VladasZ commented 1 year ago

Any updates on this? It is impossible to use vagrant-disksize on M1 mac's because VirtualBox doesn't support arm.