Telmate / proxmox-api-go

Consume the proxmox API in golang
MIT License
370 stars 234 forks source link

Feature: Disk size in Kibibyte #305

Closed Tinyblargon closed 7 months ago

Tinyblargon commented 8 months ago

Changes the size of all disk to be in Kibibytes instead of Gibibytes. Closes #304

Work done:

Tinyblargon commented 8 months ago

Okay testing it now in the terraform provider, getting the following error lvcreate --help

mleone87 commented 8 months ago

same

the PUT generated from https://github.com/Telmate/terraform-provider-proxmox/blob/b052f99b533c7165a6698102165f188d9418f4c3/proxmox/resource_vm_qemu.go#L1245 seems broken

virtio0: nvmepool:0,format=raw,replicate=0 I left the size without the suffix and it's parsed to 20KiB from provider

Tinyblargon commented 8 months ago

Yeah, the way It's intended to work is:

If size is exactly x amount of gigabytes, create disk with storage_name:disk_size, else we crate it like storage_name:0 and later we rezize the disk to the correct size.

I think i only tested this code with the file backend. Gonna try the volume backed as well.

mleone87 commented 8 months ago

I only adapted the provide to digest the new fields in the struct and it's not working like that, I guess some adjustment is needed

Tinyblargon commented 8 months ago

@mleone87 I've figured it out, when the backend is file the minimum size is 1K but when the backend is LVM the minimum size is 4097K. I'm gonna create a custom type type QemuDiskSize uint and enforce a minimum size of 4097.

Tinyblargon commented 8 months ago

@mleone87 Okay so now when the disk size isn't a nice gibibyte number we will create a disk the size of 0.001 gibibyte, as disk_size in storage_name:disk_size is a float. This initial created drive will be 1048 which is smaller than our minimum of 4097. After disk creation we will resize it to the user provided size.

The most important code is in proxmox/config_qemu.go. For selecting which newly created disk have to be resized the unit tests in proxmox/config_qemu_disk_test.go will have to do.

Worst case scenarios are:

  1. disk is created as 0.001 gibigyte and didn't get resized (no data loss).
  2. disk will be selected for resize and it will already be bigger (will trigger an api error, no data loss).

Kinda happy when everything with the disk stuff is done. Never imagined that implementing every edge case would take this long. Hopefully we can wrap-up #187 soon.

mleone87 commented 7 months ago

@Tinyblargon Not able to adapt the terraform code anymore, my changes are there https://github.com/Telmate/terraform-provider-proxmox/pull/928

provider crashes

panic: interface conversion: interface {} is int, not uint

Tinyblargon commented 7 months ago

@mleone87 Sorry, I forgot to create the PR to fix the Terraform side of things.

mleone87 commented 7 months ago

@Tinyblargon nevermind I got it and it's working, the only thing missing is change the schema definition for the size of disks to accept strings representing the size in kib or a size in KMGBT

I guess this part can be merged?

Tinyblargon commented 7 months ago

@mleone87 The code for the KMGT is ready. I'll make a pull tomorrow.