dmacvicar / terraform-provider-libvirt

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

Parametrize cloud-init bus type #1027

Open artur-borys opened 10 months ago

artur-borys commented 10 months ago

Hello, I really like this provider and I appreciate the work you've done.

I've had issues with cloud-init and Q35 machines in the past, gave up on libvirt with Terraform for some time and then I've stumbled upon this issue: https://github.com/dmacvicar/terraform-provider-libvirt/issues/1018

I've decided to make the changes mentioned by issue creator in my fork and it seemed to solve my issue.

I didn't want to blindly make it hard-coded for SATA though, that's why I'm proposing a parameter.

Basically, I think the most backwards compatible approach is to add a new field to the libvirt_domain resource, called cloudinit_bus. By default it will be set to "ide", but can be changed to "sata" for Q35 machines (for example).

I've added some basic acceptance tests, but I'm not proud of them - let's say that they are just to cover the lines. I wanted to add some test that will verify that the VM will be recreated when the bus type is changed, but this project is not using the terraform-plugin-test module and I didn't want to bring it in immediately.

If you think I should make some higher quality tests then let me know and maybe give me some tips - I'm fairly new to Go and especially to testing it, not even mentioning developing Terraform plugins.

thatfunkymunki commented 7 months ago

This was the missing ingredient for me to make a fully working Fedora Cloud (also Alma Linux) system today that had a working cloudinit. Thank you for this PR. I hope it can get merged in soon!!