AlmaLinux / cloud-images

Packer templates and other tools for building AlmaLinux images for various cloud platforms.
MIT License
161 stars 48 forks source link

Vagrantfile for libvirt boxes should not set "connect_via_ssh" and "username" #157

Closed fkrull closed 1 year ago

fkrull commented 1 year ago

The Vagrantfiles for the libvirt boxes (in tpl/vagrant) set the connect_via_ssh and username settings of the vagrant-libvirt plugin. This is a problem because of the way the vagrant-libvirt plugin determines the libvirt URI to use: if any of the connection settings are set (it can't distinguish between the user Vagrantfile and a Vagrantfile supplied by the box), it ignores the LIBVIRT_DEFAULT_URI env var and only uses the settings in the Vagrantfile.

This means that merely by setting connect_via_ssh and username (regardless of value), the Alma Linux baseboxes implicitly disable the use of LIBVIRT_DEFAULT_URI. vagrant-libvirt will always use the system QEMU driver even if that environment variable is set unless the URI settings are overridden again in the user Vagrantfile.

For that reason connect_via_ssh and username should be removed from the Vagrantfile templates. (Note that while driver is also a connection setting, it's specialcased and can stay.)

I'm also skeptical of the storage_pool_name setting. I suspect it might also cause unnecessary and difficult to debug problems for people with nonstandard libvirt setups.

LKHN commented 1 year ago

Thanks a lot for such useful report and suggestions for making the libvirt boxes more portable.

If I build the box without providing custom Vagrantfile, the NFS will be used as the synced folder type. So, we need to use rsync or disable it. What do you think about that? :thinking:

I am testing this template:

# -*- mode: ruby -*-
# vi: set ft=ruby :

Vagrant.configure('2') do |config|
  config.vm.synced_folder '.', '/vagrant', type: 'rsync'
end

And it ends up like this inside the box:

# The contents below were provided by the Packer Vagrant post-processor

Vagrant.configure("2") do |config|
  config.vm.provider :libvirt do |libvirt|
    libvirt.driver = "kvm"
  end
end

# The contents below (if any) are custom contents provided by the
# Packer template during image build.
# -*- mode: ruby -*-
# vi: set ft=ruby :

Vagrant.configure('2') do |config|
  config.vm.synced_folder '.', '/vagrant', type: 'rsync'
end

As you see, packer adds driver setting even we provide our custom Vagrantfile. Hopefully, What I observe on my tests, is the driver settings in the user's Vagrantfile is honored on the vagrant up.

These settings was added with the intention of an out of the box experience when using vagrant init $box and vagrant up. Looks like they are not needed anymore, at least on the 0.12.2 version of vagrant-libvirt plugin.

fkrull commented 1 year ago

The synced_folder setting is fine. It's certainly something a lot of other baseboxes do. I think the libvirt.driver that Packer adds is also ok. I've never seen any issue with it in my own baseboxes.

It could be that the other settings in the libvirt block were necessary in the past, but I'm confident they're not necessary with current versions of libvirt and vagrant-libvirt. Removing them shouldn't change the default behaviour of the box (i.e. vagrant init + vagrant up should still work as before with no customization), but it would make vagrant-libvirt's LIBVIRT_DEFAULT_URI support work with the box.

LKHN commented 1 year ago

I just published the 9.3 box. Please, let me know how it works on your environment: https://app.vagrantup.com/almalinux/boxes/9/versions/9.3.20231118

fkrull commented 1 year ago

I just published the 9.3 box. Please, let me know how it works on your environment: https://app.vagrantup.com/almalinux/boxes/9/versions/9.3.20231118

Can confirm, with the new version LIBVIRT_DEFAULT_URI works. Thanks!