bpg / terraform-provider-proxmox

Terraform Provider for Proxmox
https://registry.terraform.io/providers/bpg/proxmox
Mozilla Public License 2.0
770 stars 125 forks source link

Disable default values? #566

Open miberecz opened 1 year ago

miberecz commented 1 year ago

More of a question then a feature request: Would it be possible to NOT use the default values defined in the Provider?

We are using this Provider with the Pulumi wrapper and there we define our defaults to fit the environment we use, but not all parameters. Those cases are getting the values from the TF provider. We would like to avoid this behavior if possible.

For example: Lets have a VM template we want to clone. The template already has some disks. If I do not define the disks, the default size would apply (8G) which is lower then what's already in the template, thus Proxmox fails that it cannot shrink a disk. This can be done with the GUI: one can just clone a template VM with only providing a name for the new VM.

bpg commented 1 year ago

Hi @miberecz! 👋🏼

Thank you for the report! The use case you've described appears to be more of a bug to me. When cloning a disk, the provider should replicate it from the template as-is if no disk is defined in the target. This issue might be related to #360. I'll be taking a closer look at this over the next few days.

bpg commented 1 year ago

Hm... based on the comment from https://github.com/muhlba91/pulumi-proxmoxve/issues/129 this could be a Pulumi-specific behaviour. But anyway, i'm going to run some tests. The disk management in the provider is somewhat finicky.

otopetrik commented 11 months ago

The disk management in the provider is somewhat finicky.

Noticed that while working on #606. Is there any easy way to log payloads of all PVE API calls ? It would help with cleaning that up.

Disk size should probably be marked Computed, in additional to currently set Optional.

It seems that that any change to any disk in terraform configuration will cause vmUpdate to send the complete defintion of all disks to PVE in the update. Fixing that is the next step, to enable moving data VMs between datastores.

Btw. it seems that almost everything should be Computed and Optional, because it can be set on a template and user should not be required to repeat that in cloned VM configuration.

bpg commented 11 months ago

Noticed that while working on https://github.com/bpg/terraform-provider-proxmox/pull/606. Is there any easy way to log payloads of all PVE API calls ? It would help with cleaning that up.

@otopetrik TF_LOG=DEBUG terraform apply prints out request and response for all HTTP traffic between the provider and PVE. A fair warning, it will also dumps in full any file uploads, etc, so can screw up console pretty badly.

otopetrik commented 11 months ago

TF_LOG=DEBUG terraform apply prints out request and response for all HTTP traffic between the provider and PVE. A fair warning, it will also dumps in full any file uploads, etc, so can screw up console pretty badly.

Thanks @bpg, it works! (lesson learned, just because TF_LOG_PROVIDER=TRACE terraform apply produces a huge amount of text, it does not mean that it includes everything logged the provider)

bpg commented 8 months ago

related: https://github.com/bpg/terraform-provider-proxmox/pull/840#pullrequestreview-1798587205

konstantin-kornienko commented 5 months ago

Have similar issues with other parameters.

Scenario:

The provider applies defaults to cpu.type, removes the cdrom, this is a bit unwanted )

It'd be very helpful to have an option like: "just don't change any parameters in the cloned template, if they are not specified in the terraform resource".

bpg commented 5 months ago

I hear you @konstantin-kornienko & others 😞 I know this is confusing and leads to weird behaviour in cloning, stumbled upon this few times myself. This is a widespread pattern affecting many attributes in vm/lxc, so I guess the workable approach would be to fix them one by one, switching to PVE provided defaults where available.

If you comment to this ticket with the list of attributes that bothers you most, I could probably look at them in the next few weeks.

konstantin-kornienko commented 5 months ago

@bpg, thanks in advance!

for me personally it's cpu.type and cdrom configuration.

I can create a dedicated issue for that, if you like.

bpg commented 5 months ago

I'm making some progress in #1219. The cpu block should come out nicely, cdrom and disks are more messy.

bpg commented 4 months ago

Well, it didn't go well. Can't make all clone and non-clone update cases work properly 😞 I'd rather focus on #1231 implementation.