-
Notifications
You must be signed in to change notification settings - Fork 542
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
Duplicate disks created #887
Comments
Cause |
So have you found a solution to this problem? I'm guessing it's something that needs to be fixed with the provider, and not the |
Unfortunately, it isn't very easy for me. ( insufficient knowledge of GoLang ) |
same problem from proxmox 8.1.3 thanks in advance |
@mleone87 Anything new regarding this? |
Having the same problem but with TheGameProfi/proxmox. disk {
size = "16G"
type = "scsi"
storage = "vg_nvme"
ssd = 1
file = "vm-108-disk-0"
volume = "vg_nvme:vm-108-disk-0"
} Not sure how to go around this. |
Having the same issue myself. For now, the provider seems to be unusable. |
I was able to duplicate this issue in Proxmox 7.4-17 as well. This issue doesn't seem to be related to the version of Proxmox. I don't think the issues is where [https://github.com//issues/887#issuecomment-1868043544] pointed to because that line hasn't changed in 3 years. I tried to find the exact commit where this stopped working so we could get a better idea of where the problem was introduced. Unfortunately, there are a lot of commits where the code doesn't compile, which made it difficult to see where the issue was introduced. Here's the best I could do: I spot checked the 24 commits in between and none of them compiled except a8675d3, but that one failed because my VM name was My two guesses for where the problem was introduced are: I. commit 4a602733bbb5b767eeb79e1b27cf98665c904bb4, specifically the block starting on line 1066 which adds an additional disk. That change was merged into the default branch with #732, or Those guesses are listed in the order of where my gut tells me the issue lies. It's also possible that there was some change in some library that the provider uses, of which there are a ton of them and a lot of them were updated between when I could confirm this issue was not present and when I could reproduce this issue. I was actually trying to investigate a issue #704 when I came across this one, so I'm going to go back to trying to identifying the root cause of that issue. Hopefully my notes here will be helpful in someone getting to the bottom of this one before another release is cut. |
Noticed my recent EFI changes were referenced here (#732). Happy to help debug as well. |
This comment was marked as outdated.
This comment was marked as outdated.
I am able to reproduce the issue with the small example below using a provider compiled from commit e51e787 (the latest as of right now):
I am testing against Proxmox 7.4-17. That created |
Thanks! Can reproduce here now. |
Looks like this has broken due to an upstream qemu disks overhaul (Telmate/proxmox-api-go#255) as @everythings-gonna-be-alright suggested (tagging @Tinyblargon), which changed ConfigQemu.QemuDisks behavior. (I suspect the "deprecated" label is in error and it's really just gone/obsolete now.) This may be expected churn for the master branch, so not faulting anyone here. We just need to do the work to bring the terraform provider back in alignment. |
@Tinyblargon Oh nice, I missed that PR. Thanks! |
We are also hitting this problem with proxmox 8.1.3 and provider |
while debugging this issue I have realized that once I create a new qemu VM and I try Weird that during the initial creation of the VM the disk is properly created in the right storage pool and with the right size defined in the terraform code but it's just not added to the terraform state |
I can confirm that the code that @Tinyblargon wrote does fix this problem. I tested using the same small example that I posted earlier this week. I've submitted a merge request with Tinyblargon's changes that can be applied cleanly to the HEAD of the default branch here in this repo. I'm pretty sure I've resolved all the merge conflicts correctly, and I have tested it to make sure it fixes this issue, but if anyone else would be willing and able to give it a review, I'd appreciate having a second pair of eyes on this. And if anyone wants to just check out the code, compile it and verify that it fixes the issue, the code can be found here: https://github.com/hestiahacker/terraform-provider-proxmox/tree/overhaul-qemu-disks Having someone else reproduce my results would be good for avoiding that "works on my box" problem. 🙂 |
I've also tested an updated terraform file which uses the new syntax for configuring multiple disks. This avoids a deprecation warning from being printed, which makes me happy. My updated terraform file is below.
|
@hestiahacker what should we do? are we have to wait until new version release? |
I compiled the latest code and have been using that and it seems to have fixed this issue and #704. If you need a solution now, I'd suggest you take this route. There are instructions for compiling from source but if you are compiling it on a Debian based machine, it'd look something like this: git clone https://github.com/hestiahacker/terraform-provider-proxmox
cd terraform-provider-proxmox
git checkout overhaul-qemu-disks
sudo apt install -y ansible make
ansible-galaxy install gantsign.ansible-role-golang
ansible-playbook go.yml
. /etc/profile.d/golang.sh
make At that point the new provider should be in the PLUGIN_ARCH=linux_amd64
mkdir -p ~/.terraform.d/plugins/registry.example.com/telmate/proxmox/1.0.0/${PLUGIN_ARCH}
cp bin/terraform-provider-proxmox ~/.terraform.d/plugins/registry.example.com/telmate/proxmox/1.0.0/${PLUGIN_ARCH}/ That source path of the
That should work, but if you ever need to recompile it, terraform will complain about the checksum having changed. To deal with that you can either manually remove the offending checksum from the rm .terraform.lock.hcl && terraform get -update && terraform init -upgrade && terraform version Also, do realize that this is the latest code that hasn't even been merged into this repo yet, and a fair amount has changed. So there's some risk of bugs causing you problems. I'd suggest you test the code in your environment and with your configuration even more than you would a new, official release. I've tested it in my environment, but if you're using different features than me, you could hit some code path that has a bug that I didn't run into. If you are in an environment that has a low risk tolerance and you can't test this out, I have two suggestions:
|
@hestiahacker thanks for much for the instructions, I was able to follow them (which is saying something). previously, terraform would create a duplicate scsi disk as well as the virtio, further more each time you "apply" changes it would create another duplicate. Anyway moving on.
I've not changed any of the .tf files so either I have a miss-configuration or as you mentioned somewhere, there is potential for errors. |
Okay Ignore the above, I made a couple of mistakes, I'll post here incase others do the same thing. I forgot to update my providers file
I also needed to update my terraform file, so I changed
I can confirm working now. I have a LCX container, Rocky8 and 9 and Ubuntu VM's all working within the same plan For completeness I did the following I had to run the compile as root, I kept getting ansible/golang errors (built using a LCX container so might have something to do with it) Change the provider file and then follow instructions to fix the lock However when I changed the plan (made a modification to the description)
The disk is not duplicated but has a replication flag ? |
Hey! What is wrong here:
|
@Tchoupinax I've ran a few tests on my end could you try the version in #892 as this has the latest patches. |
Hey @Tinyblargon, Thank you a lot for your work! |
I will try to look into it today or tomorrow. |
Sorry, only got now time to check it out. Tested the newest changes and it worked, and didn't see any Errors. Thanks Hestia & mleone87 & Tinyblargon for the fix :) |
thanks @TheGameProfi for publishing a release to terraform registry including this fix. Much appreciated! |
* feat: re-implement the way qemu disks are handled These changes were pulled from Tinyblargon's branch which was out of sync with the Telmate master branch. I merely dealt with the merge conflicts so we could re-submit a new merge request that can be applied cleanly. Ref: #794 * fix: no functional change, just making github CI happy * Update proxmox-api-go dependency * Apply patches * fix: typos * fix: panic when `disks` is empty * docs: change disks property names * chore: update dependencies * Add debug logging --------- Co-authored-by: hestia <[email protected]> Co-authored-by: mleone87 <[email protected]>
But there is no new version on the hashicorp registry? Or is it published under some other name than Telmate? |
It is published as a fork under my own name, thegameprofi/proxmox |
Hi! |
I've also verified that v3.0.1-rc1 is able to deploy the small example without any problems. Thank you all for the testing, fixing, and releasing. 🙂 |
i still have this problem with terraform v1.7.3 plugin 3.0.1-rc1 and pve 8.1.4, on clone from a template i still get 2 disks and the vm won't boot, even with the new disks syntax. what can i do to avoid that 2 disks being created ? i'm new to terraform and it's totally frustrating. |
@devZer0, could you create a new issue with a screenshot of the hardware of your template, your terraform config, and a screenshot of the hardware of the cloned vm? |
thank you. i have tried further and by chance i found, that when setting the disk size in terraform file to exactly match the size of the disk in vm template, then it won't happen and the disk isn't getting duplicated. weird. |
@ihatethecloud I wanted to try this workound but its failing at parameter verification. I'm using zfs though, not sure is it related or not. latest version of TheGameProfi/proxmox file = "vm-137-disk-0" am i doing something wrong ? |
Don’t use TheGameProfi/proxmox. Build this repo if it has not been pushed to the registry yet. |
version = "3.0.1-rc1" is out from telmate provider. new version comes with different disk schema, I had to update my disk schema in tf file, now it works. its still has some glitches regarding disk type/controller though. anway thx to everyone ! ps:if someone needs a running example with clone_ _from_template+cloudinit+lvm/zfs can ping me. |
Could you give me working example? I can't get mine to work as I get an unbootable disk error and the hardware settings are incorrect. |
https://pastebin.com/8RJnNYUK >> you can use this one for proxmox |
This issue is stale because it has been open for 60 days with no activity. Please update the provider to the latest version and, in the issue persist, provide full configuration and debug logs |
not stale |
This issue is stale because it has been open for 60 days with no activity. Please update the provider to the latest version and, in the issue persist, provide full configuration and debug logs |
/keep open |
I've had the same issues with Proxmox 8 as a lot of people (like #882 and #863), but when I build this project locally from
master
and use that, then both these problems are solved. However, a new problem arises. The provider is supposed to increase the size of the 2G base image disk that I have, but instead, it seems like it just detaches that disk and creates a new empty one (with the correct size). This new disk is of course not bootable, and theterraform apply
command stalls.Any idea if this is a known problem? It didn't seem to me like a duplicate of any other open issues right now.
Proxmox: 8.1.3
Provider: local build of latest commit, a8675d3
The text was updated successfully, but these errors were encountered: