Skip to content

Commit

Permalink
Merge pull request #349 from Tinyblargon/memory
Browse files Browse the repository at this point in the history
Overhaul: Memory
  • Loading branch information
Tinyblargon authored Jul 27, 2024
2 parents 4c4580d + b6d5422 commit bdb805f
Show file tree
Hide file tree
Showing 9 changed files with 1,578 additions and 886 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,4 @@ jobs:
run: golint ./...

- name: Run proxmox unit tests
run: go test -race -vet=off ./proxmox/...
run: go test -race -vet=off ./proxmox/... ./internal/...
31 changes: 31 additions & 0 deletions internal/parse/parse.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package parse

import (
"errors"
"strconv"
)

func Int(input interface{}) (int, error) {
switch input := input.(type) {
case float64:
return int(input), nil
case string:
mem, err := strconv.ParseFloat(input, 64)
if err != nil {
return 0, err
}
return int(mem), nil
}
return 0, nil
}

func Uint(input interface{}) (uint, error) {
tmpInt, err := Int(input)
if err != nil {
return 0, err
}
if tmpInt < 0 {
return 0, errors.New("negative value")
}
return uint(tmpInt), nil
}
79 changes: 79 additions & 0 deletions internal/parse/parse_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package parse

import (
"testing"

"github.com/stretchr/testify/require"
)

func Test_Int(t *testing.T) {
tests := []struct {
name string
input interface{}
output int
err bool
}{
{name: `float64 negative`,
input: float64(-1),
output: -1},
{name: `float64 positive`,
input: float64(1),
output: 1},
{name: `string invalid`,
input: "a",
err: true},
{name: `string negative`,
input: "-1",
output: -1},
{name: `string positive`,
input: "1",
output: 1},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
tmpOutput, tmpErr := Int(test.input)
if test.err {
require.Error(t, tmpErr)
} else {
require.NoError(t, tmpErr)
require.Equal(t, test.output, tmpOutput)
}
})
}
}

func Test_Uint(t *testing.T) {
tests := []struct {
name string
input interface{}
output uint
err bool
}{
{name: `float64 negative`,
input: float64(-1),
err: true},
{name: `float64 positive`,
input: float64(1),
output: 1},
{name: `string negative`,
input: "-1",
err: true},
{name: `string invalid`,
input: "a",
err: true},
{name: `string positive`,
input: "1",
output: 1},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
tmpOutput, tmpErr := Uint(test.input)
if test.err {
require.Error(t, tmpErr)
} else {
require.NoError(t, tmpErr)
require.Equal(t, test.output, tmpOutput)
}
})
}
}
73 changes: 34 additions & 39 deletions proxmox/config_qemu.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ type (
type ConfigQemu struct {
Agent *QemuGuestAgent `json:"agent,omitempty"`
Args string `json:"args,omitempty"`
Balloon int `json:"balloon,omitempty"` // TODO should probably be a bool
Bios string `json:"bios,omitempty"`
Boot string `json:"boot,omitempty"` // TODO should be an array of custom enums
BootDisk string `json:"bootdisk,omitempty"` // TODO discuss deprecation? Only returned as it's deprecated in the proxmox api
Expand All @@ -48,9 +47,9 @@ type ConfigQemu struct {
Iso *IsoFile `json:"iso,omitempty"` // Same as Disks.Ide.Disk_2.CdRom.Iso
LinkedVmId uint `json:"linked_id,omitempty"` // Only returned setting it has no effect
Machine string `json:"machine,omitempty"` // TODO should be custom type with enum
Memory int `json:"memory,omitempty"` // TODO should be uint
Name string `json:"name,omitempty"` // TODO should be custom type as there are character and length limitations
Node string `json:"node,omitempty"` // Only returned setting it has no effect, set node in the VmRef instead
Memory *QemuMemory `json:"memory,omitempty"`
Name string `json:"name,omitempty"` // TODO should be custom type as there are character and length limitations
Node string `json:"node,omitempty"` // Only returned setting it has no effect, set node in the VmRef instead
Onboot *bool `json:"onboot,omitempty"`
Pool *PoolName `json:"pool,omitempty"`
Protection *bool `json:"protection,omitempty"`
Expand Down Expand Up @@ -82,6 +81,7 @@ type ConfigQemu struct {

const (
ConfigQemu_Error_UnableToUpdateWithoutReboot string = "unable to update vm without rebooting"
ConfigQemu_Error_MemoryRequired string = "memory is required during creation"
)

// Create - Tell Proxmox API to make the VM
Expand Down Expand Up @@ -174,9 +174,6 @@ func (config ConfigQemu) mapToAPI(currentConfig ConfigQemu, version Version) (re
if config.Agent != nil {
params["agent"] = config.Agent.mapToAPI(currentConfig.Agent)
}
if config.Balloon >= 1 {
params["balloon"] = config.Balloon
}
if config.Bios != "" {
params["bios"] = config.Bios
}
Expand Down Expand Up @@ -204,9 +201,6 @@ func (config ConfigQemu) mapToAPI(currentConfig ConfigQemu, version Version) (re
if config.Machine != "" {
params["machine"] = config.Machine
}
if config.Memory != 0 {
params["memory"] = config.Memory
}
if config.Name != "" {
params["name"] = config.Name
}
Expand Down Expand Up @@ -282,6 +276,9 @@ func (config ConfigQemu) mapToAPI(currentConfig ConfigQemu, version Version) (re
if config.CloudInit != nil {
itemsToDelete += config.CloudInit.mapToAPI(currentConfig.CloudInit, params, version)
}
if config.Memory != nil {
itemsToDelete += config.Memory.mapToAPI(currentConfig.Memory, params)
}

// Create EFI disk
config.CreateQemuEfiParams(params)
Expand Down Expand Up @@ -324,6 +321,7 @@ func (ConfigQemu) mapToStruct(vmr *VmRef, params map[string]interface{}) (*Confi

config := ConfigQemu{
CloudInit: CloudInit{}.mapToSDK(params),
Memory: QemuMemory{}.mapToSDK(params),
}

if vmr != nil {
Expand All @@ -339,12 +337,6 @@ func (ConfigQemu) mapToStruct(vmr *VmRef, params map[string]interface{}) (*Confi
if _, isSet := params["args"]; isSet {
config.Args = strings.TrimSpace(params["args"].(string))
}
if _, isSet := params["balloon"]; isSet {
balloon := int(params["balloon"].(float64))
if balloon > 0 {
config.Balloon = balloon
}
}
//boot by default from hard disk (c), CD-ROM (d), network (n).
if _, isSet := params["boot"]; isSet {
config.Boot = params["boot"].(string)
Expand All @@ -369,18 +361,6 @@ func (ConfigQemu) mapToStruct(vmr *VmRef, params map[string]interface{}) (*Confi
if _, isSet := params["machine"]; isSet {
config.Machine = params["machine"].(string)
}
if _, isSet := params["memory"]; isSet {
switch params["memory"].(type) {
case float64:
config.Memory = int(params["memory"].(float64))
case string:
mem, err := strconv.ParseFloat(params["memory"].(string), 64)
if err != nil {
return nil, err
}
config.Memory = int(mem)
}
}
if _, isSet := params["name"]; isSet {
config.Name = params["name"].(string)
}
Expand Down Expand Up @@ -828,6 +808,32 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede
func (config ConfigQemu) Validate(current *ConfigQemu, version Version) (err error) {
// TODO test all other use cases
// TODO has no context about changes caused by updating the vm
if current == nil { // Create
if config.Memory == nil {
return errors.New(ConfigQemu_Error_MemoryRequired)
} else {
if err = config.Memory.Validate(nil); err != nil {
return
}
}
if config.TPM != nil {
if err = config.TPM.Validate(nil); err != nil {
return
}
}
} else { // Update
if config.Memory != nil {
if err = config.Memory.Validate(current.Memory); err != nil {
return
}
}
if config.TPM != nil {
if err = config.TPM.Validate(current.TPM); err != nil {
return
}
}
}
// Shared
if config.Agent != nil {
if err = config.Agent.Validate(); err != nil {
return
Expand All @@ -849,17 +855,6 @@ func (config ConfigQemu) Validate(current *ConfigQemu, version Version) (err err
return
}
}
if config.TPM != nil {
if current == nil {
if err = config.TPM.Validate(nil); err != nil {
return
}
} else {
if err = config.TPM.Validate(current.TPM); err != nil {
return
}
}
}
if config.Tags != nil {
if err := Tag("").validate(*config.Tags); err != nil {
return err
Expand Down
Loading

0 comments on commit bdb805f

Please sign in to comment.