feat(vm,lxc): Improved support for different disk size units (#326)

proxmox/types/common_types.go

@@ -1,6 +1,8 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
 
 package types
 
@@ -151,7 +153,7 @@ func (r *CustomPrivileges) UnmarshalJSON(b []byte) error {
 	return nil
 }
 
-// MarshalJSON converts a boolean to a JSON value.
+// MarshalJSON converts a timestamp to a JSON value.
 func (r CustomTimestamp) MarshalJSON() ([]byte, error) {
 	timestamp := time.Time(r)
 	buffer := bytes.NewBufferString(strconv.FormatInt(timestamp.Unix(), 10))
@@ -159,7 +161,7 @@ func (r CustomTimestamp) MarshalJSON() ([]byte, error) {
 	return buffer.Bytes(), nil
 }
 
-// UnmarshalJSON converts a JSON value to a boolean.
+// UnmarshalJSON converts a JSON value to a timestamp.
 func (r *CustomTimestamp) UnmarshalJSON(b []byte) error {
 	s := string(b)
 	i, err := strconv.ParseInt(s, 10, 64)

proxmox/types/disk_size.go

@@ -0,0 +1,113 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+package types
+
+import (
+	"encoding/json"
+	"fmt"
+	"math"
+	"regexp"
+	"strconv"
+	"strings"
+)
+
+// Regex used to identify size strings. Case-insensitive. Covers megabytes, gigabytes and terabytes.
+var sizeRegex = regexp.MustCompile(`(?i)^(\d+(\.\d+)?)(k|kb|kib|m|mb|mib|g|gb|gib|t|tb|tib)?$`)
+
+// DiskSize allows a JSON integer value to also be a string. This is mapped to `<DiskSize>` data type in Proxmox API.
+// Represents a disk size in bytes.
+type DiskSize int64
+
+// String returns the string representation of the disk size.
+func (r DiskSize) String() string {
+	return formatDiskSize(int64(r))
+}
+
+// InGigabytes returns the disk size in gigabytes.
+func (r DiskSize) InGigabytes() int {
+	return int(int64(r) / 1024 / 1024 / 1024)
+}
+
+// DiskSizeFromGigabytes creates a DiskSize from gigabytes.
+func DiskSizeFromGigabytes(size int) DiskSize {
+	return DiskSize(size * 1024 * 1024 * 1024)
+}
+
+// MarshalJSON marshals a disk size into a Proxmox API `<DiskSize>` string.
+func (r DiskSize) MarshalJSON() ([]byte, error) {
+	bytes, err := json.Marshal(formatDiskSize(int64(r)))
+	if err != nil {
+		return nil, fmt.Errorf("cannot marshal disk size: %w", err)
+	}
+	return bytes, nil
+}
+
+// UnmarshalJSON unmarshals a disk size from a Proxmox API `<DiskSize>` string.
+func (r *DiskSize) UnmarshalJSON(b []byte) error {
+	s := string(b)
+
+	size, err := parseDiskSize(&s)
+	if err != nil {
+		return err
+	}
+	*r = DiskSize(size)
+
+	return nil
+}
+
+// parseDiskSize parses a disk size string into a number of bytes
+func parseDiskSize(size *string) (int64, error) {
+	if size == nil {
+		return 0, nil
+	}
+
+	matches := sizeRegex.FindStringSubmatch(*size)
+	if len(matches) > 0 {
+		fsize, err := strconv.ParseFloat(matches[1], 64)
+		if err != nil {
+			return -1, fmt.Errorf("cannot parse disk size \"%s\": %w", *size, err)
+		}
+		switch strings.ToLower(matches[3]) {
+		case "k", "kb", "kib":
+			fsize *= 1024
+		case "m", "mb", "mib":
+			fsize = fsize * 1024 * 1024
+		case "g", "gb", "gib":
+			fsize = fsize * 1024 * 1024 * 1024
+		case "t", "tb", "tib":
+			fsize = fsize * 1024 * 1024 * 1024 * 1024
+		}
+
+		return int64(math.Ceil(fsize)), nil
+	}
+
+	return -1, fmt.Errorf("cannot parse disk size \"%s\"", *size)
+}
+
+func formatDiskSize(size int64) string {
+	if size < 0 {
+		return ""
+	}
+
+	if size < 1024 {
+		return fmt.Sprintf("%d", size)
+	}
+
+	if size < 1024*1024 {
+		return fmt.Sprintf("%.2gK", float64(size)/1024)
+	}
+
+	if size < 1024*1024*1024 {
+		return fmt.Sprintf("%.2gM", float64(size)/1024/1024)
+	}
+
+	if size < 1024*1024*1024*1024 {
+		return fmt.Sprintf("%.2gG", float64(size)/1024/1024/1024)
+	}
+
+	return fmt.Sprintf("%.2gT", float64(size)/1024/1024/1024/1024)
+}

proxmox/types/disk_size_test.go

@@ -0,0 +1,77 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+package types
+
+import "testing"
+
+func TestParseDiskSize(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name    string
+		size    *string
+		want    int64
+		wantErr bool
+	}{
+		{"handle null size", nil, 0, false},
+		{"parse TB", StrPtr("2TB"), 2199023255552, false},
+		{"parse T", StrPtr("2T"), 2199023255552, false},
+		{"parse fraction T", StrPtr("2.2T"), 2418925581108, false},
+		{"parse GB", StrPtr("2GB"), 2147483648, false},
+		{"parse G", StrPtr("2G"), 2147483648, false},
+		{"parse M", StrPtr("2048M"), 2147483648, false},
+		{"parse MB", StrPtr("2048MB"), 2147483648, false},
+		{"parse MiB", StrPtr("2048MiB"), 2147483648, false},
+		{"parse K", StrPtr("1K"), 1024, false},
+		{"parse KB", StrPtr("2KB"), 2048, false},
+		{"parse KiB", StrPtr("4KiB"), 4096, false},
+		{"parse no units as bytes", StrPtr("12345"), 12345, false},
+		{"error on bad format string", StrPtr("20l8G"), -1, true},
+		{"error on unknown unit string", StrPtr("2048W"), -1, true},
+		{"error on arbitrary string", StrPtr("something"), -1, true},
+	}
+	for _, test := range tests {
+		tt := test
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got, err := parseDiskSize(tt.size)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("parseDiskSize() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if got != tt.want {
+				t.Errorf("parseDiskSize() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestFormatDiskSize(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		size int64
+		want string
+	}{
+		{"handle 0 size", 0, "0"},
+		{"handle bytes", 1001, "1001"},
+		{"handle kilobytes", 1234, "1.2K"},
+		{"handle megabytes", 2097152, "2M"},
+		{"handle gigabytes", 2147483648, "2G"},
+		{"handle terabytes", 2199023255552, "2T"},
+	}
+	for _, test := range tests {
+		tt := test
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			if got := formatDiskSize(tt.size); got != tt.want {
+				t.Errorf("formatDiskSize() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

proxmox/types/helpers.go

@@ -0,0 +1,18 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+package types
+
+// StrPtr returns a pointer to a string.
+func StrPtr(s string) *string {
+	return &s
+}
+
+// BoolPtr returns a pointer to a bool.
+func BoolPtr(s bool) *CustomBool {
+	customBool := CustomBool(s)
+	return &customBool
+}

proxmox/utils.go

@@ -1,16 +1,14 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
 
 package proxmox
 
 import (
 	"context"
-	"fmt"
 	"io"
-	"math"
-	"strconv"
-	"strings"
 
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 )
@@ -24,35 +22,3 @@ func CloseOrLogError(ctx context.Context) func(io.Closer) {
 		}
 	}
 }
-
-func ParseDiskSize(size *string) (int, error) {
-	if size == nil {
-		return 0, nil
-	}
-
-	if strings.HasSuffix(*size, "T") {
-		diskSize, err := strconv.Atoi(strings.TrimSuffix(*size, "T"))
-		if err != nil {
-			return -1, fmt.Errorf("failed to parse disk size: %w", err)
-		}
-		return int(math.Ceil(float64(diskSize) * 1024)), nil
-	}
-
-	if strings.HasSuffix(*size, "G") {
-		diskSize, err := strconv.Atoi(strings.TrimSuffix(*size, "G"))
-		if err != nil {
-			return -1, fmt.Errorf("failed to parse disk size: %w", err)
-		}
-		return diskSize, nil
-	}
-
-	if strings.HasSuffix(*size, "M") {
-		diskSize, err := strconv.Atoi(strings.TrimSuffix(*size, "M"))
-		if err != nil {
-			return -1, fmt.Errorf("failed to parse disk size: %w", err)
-		}
-		return int(math.Ceil(float64(diskSize) / 1024)), nil
-	}
-
-	return -1, fmt.Errorf("cannot parse disk size \"%s\"", *size)
-}

proxmox/utils_test.go

@@ -1,3 +1,9 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
 package proxmox
 
 import (
@@ -8,38 +14,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestParseDiskSize(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name    string
-		size    *string
-		want    int
-		wantErr bool
-	}{
-		{"handle null size", nil, 0, false},
-		{"parse terabytes", strPtr("2T"), 2048, false},
-		{"parse gigabytes", strPtr("2G"), 2, false},
-		{"parse megabytes", strPtr("2048M"), 2, false},
-		{"error on arbitrary string", strPtr("something"), -1, true},
-		{"error on missing unit", strPtr("12345"), -1, true},
-	}
-	for _, test := range tests {
-		tt := test
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			got, err := ParseDiskSize(tt.size)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("ParseDiskSize() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if got != tt.want {
-				t.Errorf("ParseDiskSize() got = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
 func TestCloseOrLogError(t *testing.T) {
 	t.Parallel()
 	f := CloseOrLogError(context.Background())

proxmox/virtual_environment_container_types.go

@@ -1,6 +1,8 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
 
 package proxmox
 
@@ -120,7 +122,7 @@ type VirtualEnvironmentContainerCustomNetworkInterfaceArray []VirtualEnvironment
 // VirtualEnvironmentContainerCustomRootFS contains the values for the "rootfs" property.
 type VirtualEnvironmentContainerCustomRootFS struct {
 	ACL          *types.CustomBool `json:"acl,omitempty"          url:"acl,omitempty,int"`
-	DiskSize     *string           `json:"size,omitempty"         url:"size,omitempty"`
+	Size         *types.DiskSize   `json:"size,omitempty"         url:"size,omitempty"`
 	MountOptions *[]string         `json:"mountoptions,omitempty" url:"mountoptions,omitempty"`
 	Quota        *types.CustomBool `json:"quota,omitempty"        url:"quota,omitempty,int"`
 	ReadOnly     *types.CustomBool `json:"ro,omitempty"           url:"ro,omitempty,int"`
@@ -449,8 +451,8 @@ func (r VirtualEnvironmentContainerCustomRootFS) EncodeValues(key string, v *url
 		}
 	}
 
-	if r.DiskSize != nil {
-		values = append(values, fmt.Sprintf("size=%s", *r.DiskSize))
+	if r.Size != nil {
+		values = append(values, fmt.Sprintf("size=%s", *r.Size))
 	}
 
 	if r.MountOptions != nil {
@@ -753,7 +755,11 @@ func (r *VirtualEnvironmentContainerCustomRootFS) UnmarshalJSON(b []byte) error
 				bv := types.CustomBool(v[1] == "1")
 				r.Shared = &bv
 			case "size":
-				r.DiskSize = &v[1]
+				r.Size = new(types.DiskSize)
+				err := r.Size.UnmarshalJSON([]byte(v[1]))
+				if err != nil {
+					return fmt.Errorf("failed to unmarshal disk size: %w", err)
+				}
 			}
 		}
 	}

proxmox/virtual_environment_vm_types.go

@@ -1,6 +1,8 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
 
 package proxmox
 
@@ -78,7 +80,7 @@ type CustomCPUEmulation struct {
 
 // CustomEFIDisk handles QEMU EFI disk parameters.
 type CustomEFIDisk struct {
-	DiskSize   *int    `json:"size,omitempty"   url:"size,omitempty"`
+	Size       *types.DiskSize `json:"size,omitempty"   url:"size,omitempty"`
 	FileVolume string          `json:"file"             url:"file"`
 	Format     *string         `json:"format,omitempty" url:"format,omitempty"`
 }
@@ -174,7 +176,7 @@ type CustomStorageDevice struct {
 	MaxReadSpeedMbps        *int              `json:"mbps_rd,omitempty"     url:"mbps_rd,omitempty"`
 	MaxWriteSpeedMbps       *int              `json:"mbps_wr,omitempty"     url:"mbps_wr,omitempty"`
 	Media                   *string           `json:"media,omitempty"       url:"media,omitempty"`
-	Size                    *string           `json:"size,omitempty"        url:"size,omitempty"`
+	Size                    *types.DiskSize   `json:"size,omitempty"        url:"size,omitempty"`
 	Interface               *string
 	ID                      *string
 	FileID                  *string
@@ -560,7 +562,7 @@ type VirtualEnvironmentVMRebootResponseBody struct {
 type VirtualEnvironmentVMResizeDiskRequestBody struct {
 	Digest   *string           `json:"digest,omitempty"   url:"digest,omitempty"`
 	Disk     string            `json:"disk"               url:"disk"`
-	Size     string            `json:"size"               url:"size"`
+	Size     types.DiskSize    `json:"size"               url:"size"`
 	SkipLock *types.CustomBool `json:"skiplock,omitempty" url:"skiplock,omitempty,int"`
 }
 
@@ -778,8 +780,8 @@ func (r CustomEFIDisk) EncodeValues(key string, v *url.Values) error {
 		values = append(values, fmt.Sprintf("format=%s", *r.Format))
 	}
 
-	if r.DiskSize != nil {
-		values = append(values, fmt.Sprintf("size=%d", *r.DiskSize))
+	if r.Size != nil {
+		values = append(values, fmt.Sprintf("size=%s", *r.Size))
 	}
 
 	v.Add(key, strings.Join(values, ","))
@@ -1490,12 +1492,11 @@ func (r *CustomEFIDisk) UnmarshalJSON(b []byte) error {
 			case "file":
 				r.FileVolume = v[1]
 			case "size":
-				iv, err := ParseDiskSize(&v[1])
+				r.Size = new(types.DiskSize)
+				err := r.Size.UnmarshalJSON([]byte(v[1]))
 				if err != nil {
-					return err
+					return fmt.Errorf("failed to unmarshal disk size: %w", err)
 				}
-
-				r.DiskSize = &iv
 			}
 		}
 	}
@@ -1754,7 +1755,11 @@ func (r *CustomStorageDevice) UnmarshalJSON(b []byte) error {
 			case "media":
 				r.Media = &v[1]
 			case "size":
-				r.Size = &v[1]
+				r.Size = new(types.DiskSize)
+				err := r.Size.UnmarshalJSON([]byte(v[1]))
+				if err != nil {
+					return fmt.Errorf("failed to unmarshal disk size: %w", err)
+				}
 			case "format":
 				r.Format = &v[1]
 			case "iothread":

proxmox/virtual_environment_vm_types_test.go

@@ -1,3 +1,9 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
 package proxmox
 
 import (
@@ -9,6 +15,7 @@ import (
 )
 
 func TestCustomStorageDevice_UnmarshalJSON(t *testing.T) {
+	ds8gig := types.DiskSizeFromGigabytes(8)
 	tests := []struct {
 		name    string
 		line    string
@@ -19,25 +26,25 @@ func TestCustomStorageDevice_UnmarshalJSON(t *testing.T) {
 			name: "simple volume",
 			line: `"local-lvm:vm-2041-disk-0,discard=on,ssd=1,iothread=1,size=8G"`,
 			want: &CustomStorageDevice{
-				Discard:    strPtr("on"),
+				Discard:    types.StrPtr("on"),
 				Enabled:    true,
 				FileVolume: "local-lvm:vm-2041-disk-0",
-				IOThread:   boolPtr(true),
-				Size:       strPtr("8G"),
-				SSD:        boolPtr(true),
+				IOThread:   types.BoolPtr(true),
+				Size:       &ds8gig,
+				SSD:        types.BoolPtr(true),
 			},
 		},
 		{
 			name: "raw volume type",
 			line: `"nfs:2041/vm-2041-disk-0.raw,discard=ignore,ssd=1,iothread=1,size=8G"`,
 			want: &CustomStorageDevice{
-				Discard:    strPtr("ignore"),
+				Discard:    types.StrPtr("ignore"),
 				Enabled:    true,
 				FileVolume: "nfs:2041/vm-2041-disk-0.raw",
-				Format:     strPtr("raw"),
-				IOThread:   boolPtr(true),
-				Size:       strPtr("8G"),
-				SSD:        boolPtr(true),
+				Format:     types.StrPtr("raw"),
+				IOThread:   types.BoolPtr(true),
+				Size:       &ds8gig,
+				SSD:        types.BoolPtr(true),
 			},
 		},
 	}
@@ -51,12 +58,3 @@ func TestCustomStorageDevice_UnmarshalJSON(t *testing.T) {
 		})
 	}
 }
-
-func strPtr(s string) *string {
-	return &s
-}
-
-func boolPtr(s bool) *types.CustomBool {
-	customBool := types.CustomBool(s)
-	return &customBool
-}

proxmoxtf/resource/container.go

@@ -1626,12 +1626,7 @@ func containerRead(ctx context.Context, d *schema.ResourceData, m interface{}) d
 	if containerConfig.RootFS != nil {
 		volumeParts := strings.Split(containerConfig.RootFS.Volume, ":")
 		disk[mkResourceVirtualEnvironmentContainerDiskDatastoreID] = volumeParts[0]
-
-		diskSize, err := proxmox.ParseDiskSize(containerConfig.RootFS.DiskSize)
-		if err != nil {
-			return diag.FromErr(err)
-		}
-		disk[mkResourceVirtualEnvironmentContainerDiskSize] = diskSize
+		disk[mkResourceVirtualEnvironmentContainerDiskSize] = containerConfig.RootFS.Size.InGigabytes()
 	} else {
 		// Default value of "storage" is "local" according to the API documentation.
 		disk[mkResourceVirtualEnvironmentContainerDiskDatastoreID] = "local"

proxmoxtf/resource/vm.go

@@ -1720,12 +1720,7 @@ func vmCreateClone(ctx context.Context, d *schema.ResourceData, m interface{}) d
 			continue
 		}
 
-		compareNumber, err := proxmox.ParseDiskSize(currentDiskInfo.Size)
-		if err != nil {
-			return diag.FromErr(err)
-		}
-
-		if diskSize < compareNumber {
+		if diskSize < currentDiskInfo.Size.InGigabytes() {
 			return diag.Errorf(
 				"disk resize fails requests size (%dG) is lower than current size (%s)",
 				diskSize,
@@ -1743,7 +1738,7 @@ func vmCreateClone(ctx context.Context, d *schema.ResourceData, m interface{}) d
 
 		diskResizeBody := &proxmox.VirtualEnvironmentVMResizeDiskRequestBody{
 			Disk: diskInterface,
-			Size: fmt.Sprintf("%dG", diskSize),
+			Size: types.DiskSizeFromGigabytes(diskSize),
 		}
 
 		moveDisk := false
@@ -1765,7 +1760,7 @@ func vmCreateClone(ctx context.Context, d *schema.ResourceData, m interface{}) d
 			}
 		}
 
-		if diskSize > compareNumber {
+		if diskSize > currentDiskInfo.Size.InGigabytes() {
 			err = veClient.ResizeVMDisk(ctx, nodeName, vmID, diskResizeBody)
 			if err != nil {
 				return diag.FromErr(err)
@@ -2505,8 +2500,8 @@ func vmGetDiskDeviceObjects(
 		diskDevice.Interface = &diskInterface
 		diskDevice.Format = &fileFormat
 		diskDevice.FileID = &fileID
-		sizeString := fmt.Sprintf("%dG", size)
-		diskDevice.Size = &sizeString
+		diskSize := types.DiskSizeFromGigabytes(size)
+		diskDevice.Size = &diskSize
 		diskDevice.SizeInt = &size
 		diskDevice.IOThread = &ioThread
 		diskDevice.Discard = &discard
@@ -3060,16 +3055,7 @@ func vmReadCustom(
 		}
 
 		disk[mkResourceVirtualEnvironmentVMDiskInterface] = di
-
-		diskSize := 0
-
-		var err error
-		diskSize, err = proxmox.ParseDiskSize(dd.Size)
-		if err != nil {
-			return diag.FromErr(err)
-		}
-
-		disk[mkResourceVirtualEnvironmentVMDiskSize] = diskSize
+		disk[mkResourceVirtualEnvironmentVMDiskSize] = dd.Size.InGigabytes()
 
 		if dd.BurstableReadSpeedMbps != nil ||
 			dd.BurstableWriteSpeedMbps != nil ||