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

Add support for Default dropdowns/empty config fields #1587

Closed
joshmcorreia opened this issue Oct 9, 2024 · 5 comments
Closed

Add support for Default dropdowns/empty config fields #1587

joshmcorreia opened this issue Oct 9, 2024 · 5 comments
Labels
✨ enhancement New feature or request

Comments

@joshmcorreia
Copy link
Contributor

Is your feature request related to a problem? Please describe.
In the Proxmox web UI there is the ability to set many dropdowns to "Default", but that option doesn't seem to be present in this provider.

This change would make it so our terraform plans are more flexible in the future if any of the Proxmox cluster defaults change.

Here is a specific example:
The bios field currently requires that the user either select ovmf or seabios, but this is not the same thing as the default field in the Proxmox UI.

Generated manually from the Proxmox UI by choosing the "Default" BIOS dropdown (notice how there is no bios setting set):

# cat 101.conf | grep "bios:"

Generated via the terraform provider:

# cat 101.conf | grep "bios:"
bios: seabios

Here's a list of the config lines that I've found that are added by this provider but they will work with empty defaults:

bios: seabios
cpuunits: 1024
keyboard: en-us
onboot: 0
protection: 0
tablet: 1
template: 0
vga: memory=16,type=std

This list is most likely not exhaustive, but these are the ones that I noticed are different from what I would expect when manually creating a default VM via the Proxmox UI.

Describe the solution you'd like
I think the cleanest implementation would be to only set fields that are necessary and default to not even having an entry in the .conf file unless they are explicitly set by the user.

Describe alternatives you've considered
Alternatively I think it would be nice if the user could specify "Default" as an option for these fields, but it's not as clean as not adding entries to the .conf file.

Additional context
Proxmox Virtual Environment 8.2.7
Plugin version v0.66.1

@joshmcorreia joshmcorreia added the ✨ enhancement New feature or request label Oct 9, 2024
@bpg
Copy link
Owner

bpg commented Oct 10, 2024

Thanks for reporting @joshmcorreia -- I totally understand your concerns!

Unfortunately, there are technical limitations in how the current provider handles defaults. In a nutshell, the older Terraform SDK provides no good way to distinguish between ‘no value’ and a default value in configuration files. I’ve started issue #1231 to address the root cause and migrate the provider implementation to the new Plugin Framework, which will be quite a long journey. However, the early prototype shows some promising results :)

@joshmcorreia
Copy link
Contributor Author

That makes sense. I can work around it for now 😄 Closing since it's a duplicate of #1304 and you're already working on #1231.

@joshmcorreia
Copy link
Contributor Author

@bpg For what it's worth I just noticed that the machine argument doesn't add a line to the config file by default. Is that one special for some reason?

@bpg
Copy link
Owner

bpg commented Oct 10, 2024

@bpg For what it's worth I just noticed that the machine argument doesn't add a line to the config file by default. Is that one special for some reason?

Yeah, it is handled differently, as PVE does not return the machine type if it's default when provider reads back the clone state.

@joshmcorreia
Copy link
Contributor Author

Got it, thank you 👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants