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

feat: Add 'import-from' parameter to qemu #606

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

jamesharr
Copy link
Contributor

@jamesharr jamesharr commented Aug 30, 2022

I think this addresses #605. It adds an optional import-disk parameter for VMs.

I have no idea how to test this, or if I'm getting the additions made in the correct place. Please review with scrutiny and/or modify as-needed.

@jamesharr jamesharr changed the title feat: Add 'import-disk' parameter to qemu feat: Add 'import-from' parameter to qemu Aug 30, 2022
@mleone87
Copy link
Collaborator

mleone87 commented Sep 1, 2022

This feature should be used in the next version of the provider for sure!

However, I feel that putting this in the resource properties would cause a lot of problem, this is not a parameter to be passed to the API every time we make a change(just an example).
The proxmox developers thought of it as a one-time pass parameter so it should be used only in resource creation(or recreation)

@jamesharr
Copy link
Contributor Author

Ty. I agree with that assessment. For now, we're cloning from a template VM, but I would like to use this in the future.

I might need some pointers on how to accomplish this... I have limited experience using this TF provider let alone developing a TF provider, so I'm kind of lost with how to only apply that parameter on initial creation. I'll poke around today a bit but I'm not sure how far I'll get.

One other thing I noticed is that this might be an alternative to using iso or clone on the VM. When importing a disk image, neither parameter is needed. At the moment, one of those options is required, so I think this patch needs to add something to allow for a 3rd option instead of import/iso.

@jamesharr
Copy link
Contributor Author

jamesharr commented Sep 1, 2022

I have a local development environment going. I found another issue that we'll run into, specifically that I want to use cloud-init without cloning from an existing template.

proxmox_vm_qemu.control-node: Creating...
╷
│ Error: cloud-init parameters only supported on clones or updates
│ 
│   with proxmox_vm_qemu.control-node,
│   on main.tf line 93, in resource "proxmox_vm_qemu" "control-node":
│   93: resource "proxmox_vm_qemu" "control-node" {
│ 
╵
Command failed 1

I'm going to modify the originally issue to include more details of my proposed use-case since this is turning into a bigger thing than just adding a parameter. That way, I'll have something to work from.

@myplixa
Copy link

myplixa commented Nov 7, 2023

Hi guys, can you tell me when will be the release of this feature?

@mleone87 mleone87 merged commit 88d03ea into Telmate:master Dec 6, 2023
1 check failed
spettinichi pushed a commit to spettinichi/terraform-provider-proxmox that referenced this pull request Jan 26, 2024
* fix(build): Detect KERNEL on MacOS

* feat: Add 'import_disk' parameter to qemu

---------

Co-authored-by: mleone87 <807457+mleone87@users.noreply.github.com>
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.

None yet

3 participants