search

dmacvicar / terraform-provider-libvirt

Terraform provider to provision infrastructure with Linux's KVM using libvirt
Apache License 2.0
1.59k stars 458 forks source link

Fixing issue #811 #812

Open bpetit opened 3 years ago

bpetit commented 3 years ago

Fixing https://github.com/dmacvicar/terraform-provider-libvirt/issues/811

For some reason disk.Driver.Type ends up with value 'raw', even when file name of the disk drive has a ".qcow2" suffix. Is seems like checking that the suffix is there and then apply qcow2 as the driver type is not done explicitely, so raw is choosen in this case.

This fix proposal fixed the workflow (described in issue 811) on my environment, tests are passing successfully. I propose you this fix and hope it is useful. Feel free to slap my fingers if I missed something in the contribution process or assumed something wrong or anything :)

Thank you for your great work ! I use the provider a lot and love it.

dmacvicar commented 3 years ago

Thanks for the PR!

Do you think you could reproduce the issue with a testcase ie. enhancing the ones in resource_libvirt_volume_test.go enough to make it fail before applying the fix?

bpetit commented 3 years ago

I'll have a look at that and try !

joe-e-brown commented 3 years ago

The bug doesn't affect the volume definition, but rather the domain definition. Whereas a volume may have qcow2 format, the domain is defined with XML which specifies the RAW driver instead. This results in the domain being unbootable because the volume driver doesn't match the volume format. I propose that the tests in resource_libvirt_domain_test.go should be enhanced to test the volume driver selection after domain creation.

bpetit commented 3 years ago

I didn't take time to look at the tests implementation yet. It will require some digging on my side so if someone has a clear idea on how it should be done properly, please do :)