Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parametrize cloud-init bus type #1027

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

artur-borys
Copy link

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: #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.

Those are very "dumb" tests, which basically check:
* if a default value is used
* if the value can be changed

I wanted to test if changing the value makes the domain to be recreated,
but it seems like this provider is not using the new(er)
terraform-plugin-testing package which has that feature.
@thatfunkymunki
Copy link

thatfunkymunki commented Dec 3, 2023

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!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants