From 03221436f01877eb550f8a854fc4f2d3355203af Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 20 Apr 2024 01:57:14 +0200 Subject: [PATCH 01/22] feat: type `ConfigPool` --- proxmox/config_guest.go | 4 ++-- proxmox/config_lxc.go | 2 +- proxmox/config_pool.go | 9 +++++++++ proxmox/config_qemu.go | 9 ++++++--- proxmox/config_qemu_test.go | 12 ++++++------ 5 files changed, 24 insertions(+), 12 deletions(-) create mode 100644 proxmox/config_pool.go diff --git a/proxmox/config_guest.go b/proxmox/config_guest.go index ac7c4e89..131d9d20 100644 --- a/proxmox/config_guest.go +++ b/proxmox/config_guest.go @@ -22,8 +22,8 @@ type GuestResource struct { Name string `json:"name"` // TODO custom type NetworkIn uint `json:"network_in"` NetworkOut uint `json:"network_out"` - Node string `json:"node"` // TODO custom type - Pool string `json:"pool"` // TODO custom type + Node string `json:"node"` // TODO custom type + Pool PoolName `json:"pool"` Status string `json:"status"` // TODO custom type? Tags []string `json:"tags"` // TODO custom type Template bool `json:"template"` diff --git a/proxmox/config_lxc.go b/proxmox/config_lxc.go index 40f0ccca..6d667d54 100644 --- a/proxmox/config_lxc.go +++ b/proxmox/config_lxc.go @@ -37,7 +37,7 @@ type ConfigLxc struct { OnBoot bool `json:"onboot"` OsType string `json:"ostype,omitempty"` Password string `json:"password,omitempty"` - Pool string `json:"pool,omitempty"` + Pool *PoolName `json:"pool,omitempty"` Protection bool `json:"protection"` Restore bool `json:"restore,omitempty"` RootFs QemuDevice `json:"rootfs,omitempty"` diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go new file mode 100644 index 00000000..84db8c98 --- /dev/null +++ b/proxmox/config_pool.go @@ -0,0 +1,9 @@ +package proxmox + +type ConfigPool struct { + Name PoolName `json:"name"` + Comment *string `json:"comment"` + Guests *[]uint `json:"guests"` // TODO: Change type once we have a type for guestID +} + +type PoolName string diff --git a/proxmox/config_qemu.go b/proxmox/config_qemu.go index a12cfc44..5ae69f31 100644 --- a/proxmox/config_qemu.go +++ b/proxmox/config_qemu.go @@ -64,7 +64,7 @@ type ConfigQemu struct { Nameserver string `json:"nameserver,omitempty"` // TODO should be part of a cloud-init struct (cloud-init option) Node string `json:"node,omitempty"` // Only returned setting it has no effect, set node in the VmRef instead Onboot *bool `json:"onboot,omitempty"` - Pool string `json:"pool,omitempty"` // TODO should be custom type as there are character and length limitations + Pool *PoolName `json:"pool,omitempty"` Protection *bool `json:"protection,omitempty"` QemuCores int `json:"cores,omitempty"` // TODO should be uint QemuCpu string `json:"cpu,omitempty"` // TODO should be custom type with enum @@ -362,7 +362,8 @@ func (ConfigQemu) mapToStruct(vmr *VmRef, params map[string]interface{}) (*Confi if vmr != nil { config.Node = vmr.node - config.Pool = vmr.pool + poolCopy := PoolName(vmr.pool) + config.Pool = &poolCopy config.VmID = vmr.vmId } @@ -881,7 +882,9 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede return } - _, err = client.UpdateVMPool(vmr, newConfig.Pool) + if newConfig.Pool != nil { + _, err = client.UpdateVMPool(vmr, string(*newConfig.Pool)) + } return } diff --git a/proxmox/config_qemu_test.go b/proxmox/config_qemu_test.go index a5e15358..f03ee67c 100644 --- a/proxmox/config_qemu_test.go +++ b/proxmox/config_qemu_test.go @@ -5908,11 +5908,11 @@ func Test_ConfigQemu_mapToStruct(t *testing.T) { }, {name: "Node vmr empty", vmr: &VmRef{node: ""}, - output: &ConfigQemu{}, + output: &ConfigQemu{Pool: util.Pointer(PoolName(""))}, }, {name: "Node vmr populated", vmr: &VmRef{node: "test"}, - output: &ConfigQemu{Node: "test"}, + output: &ConfigQemu{Node: "test", Pool: util.Pointer(PoolName(""))}, }, // Pool {name: "Pool vmr nil", @@ -5920,11 +5920,11 @@ func Test_ConfigQemu_mapToStruct(t *testing.T) { }, {name: "Pool vmr empty", vmr: &VmRef{pool: ""}, - output: &ConfigQemu{}, + output: &ConfigQemu{Pool: util.Pointer(PoolName(""))}, }, {name: "Pool vmr populated", vmr: &VmRef{pool: "test"}, - output: &ConfigQemu{Pool: "test"}, + output: &ConfigQemu{Pool: util.Pointer(PoolName("test"))}, }, // VmID {name: "VmID vmr nil", @@ -5932,11 +5932,11 @@ func Test_ConfigQemu_mapToStruct(t *testing.T) { }, {name: "VmID vmr empty", vmr: &VmRef{vmId: 0}, - output: &ConfigQemu{}, + output: &ConfigQemu{Pool: util.Pointer(PoolName(""))}, }, {name: "VmID vmr populated", vmr: &VmRef{vmId: 100}, - output: &ConfigQemu{VmID: 100}, + output: &ConfigQemu{VmID: 100, Pool: util.Pointer(PoolName(""))}, }, // TPM {name: "TPM", From 822c076fb19be87e696e893efcbb5789ff79ca5a Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 20 Apr 2024 02:49:16 +0200 Subject: [PATCH 02/22] test: `PoolName` validate --- proxmox/config_pool.go | 31 ++++++++++++ proxmox/config_pool_test.go | 60 +++++++++++++++++++++++ proxmox/config_qemu.go | 5 ++ proxmox/config_qemu_test.go | 13 +++++ test/data/test_data_pool/type_PoolName.go | 34 +++++++++++++ 5 files changed, 143 insertions(+) create mode 100644 proxmox/config_pool_test.go create mode 100644 test/data/test_data_pool/type_PoolName.go diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index 84db8c98..f3afc551 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -1,9 +1,40 @@ package proxmox +import ( + "errors" + "regexp" +) + type ConfigPool struct { Name PoolName `json:"name"` Comment *string `json:"comment"` Guests *[]uint `json:"guests"` // TODO: Change type once we have a type for guestID } +func (config ConfigPool) Validate() error { + // TODO: Add validation for Guests and Comment + return config.Name.Validate() +} + type PoolName string + +const ( + PoolName_Error_Empty string = "PoolName cannot be empty" + PoolName_Error_Length string = "PoolName may not be longer than 1024 characters" // proxmox does not seem to have a max length, so we artificially cap it at 1024 + PoolName_Error_Characters string = "PoolName may only contain the following characters: a-z, A-Z, 0-9, hyphen (-), and underscore (_)" +) + +var regex_PoolName = regexp.MustCompile(`^[a-zA-Z0-9-_]+$`) + +func (config PoolName) Validate() error { + if config == "" { + return errors.New(PoolName_Error_Empty) + } + if len(config) > 1024 { + return errors.New(PoolName_Error_Length) + } + if !regex_PoolName.MatchString(string(config)) { + return errors.New(PoolName_Error_Characters) + } + return nil +} diff --git a/proxmox/config_pool_test.go b/proxmox/config_pool_test.go new file mode 100644 index 00000000..fb7073fb --- /dev/null +++ b/proxmox/config_pool_test.go @@ -0,0 +1,60 @@ +package proxmox + +import ( + "errors" + "testing" + + "github.com/Telmate/proxmox-api-go/test/data/test_data_pool" + "github.com/stretchr/testify/require" +) + +func Test_ConfigPool_Validate(t *testing.T) { + tests := []struct { + name string + input ConfigPool + output error + }{ + {name: "Valid PoolName", + input: ConfigPool{Name: PoolName(test_data_pool.PoolName_Legal())}}, + {name: "Invalid PoolName Empty", + input: ConfigPool{Name: ""}, + output: errors.New(PoolName_Error_Empty)}, + {name: "Invalid PoolName Length", + input: ConfigPool{Name: PoolName(test_data_pool.PoolName_Max_Illegal())}, + output: errors.New(PoolName_Error_Length)}, + {name: "Invalid PoolName Characters", + input: ConfigPool{Name: PoolName(test_data_pool.PoolName_Error_Characters()[0])}, + output: errors.New(PoolName_Error_Characters)}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.output, test.input.Validate()) + }) + } +} + +func Test_PoolName_Validate(t *testing.T) { + tests := []struct { + name string + input []string + output error + }{ + {name: `Valid PoolName`, + input: test_data_pool.PoolName_Legals()}, + {name: `Invalid Empty`, + output: errors.New(PoolName_Error_Empty)}, + {name: `Invalid Length`, + input: []string{test_data_pool.PoolName_Max_Illegal()}, + output: errors.New(PoolName_Error_Length)}, + {name: `Invalid Characters`, + input: test_data_pool.PoolName_Error_Characters(), + output: errors.New(PoolName_Error_Characters)}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + for _, input := range test.input { + require.Equal(t, test.output, PoolName(input).Validate()) + } + }) + } +} diff --git a/proxmox/config_qemu.go b/proxmox/config_qemu.go index 5ae69f31..084cd7b6 100644 --- a/proxmox/config_qemu.go +++ b/proxmox/config_qemu.go @@ -902,6 +902,11 @@ func (config ConfigQemu) Validate(current *ConfigQemu) (err error) { return } } + if config.Pool != nil && *config.Pool != "" { + if err = config.Pool.Validate(); err != nil { + return + } + } if config.TPM != nil { if current == nil { if err = config.TPM.Validate(nil); err != nil { diff --git a/proxmox/config_qemu_test.go b/proxmox/config_qemu_test.go index f03ee67c..5c732b08 100644 --- a/proxmox/config_qemu_test.go +++ b/proxmox/config_qemu_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/Telmate/proxmox-api-go/internal/util" + "github.com/Telmate/proxmox-api-go/test/data/test_data_pool" "github.com/Telmate/proxmox-api-go/test/data/test_data_qemu" "github.com/stretchr/testify/require" ) @@ -6164,6 +6165,11 @@ func Test_ConfigQemu_Validate(t *testing.T) { }}}, }}, }, + // Valid Pool + {name: "Valid PoolName", + input: ConfigQemu{Pool: util.Pointer(PoolName(test_data_pool.PoolName_Legal()))}}, + {name: "Valid PoolName Empty", + input: ConfigQemu{Pool: util.Pointer(PoolName(""))}}, // Valid Tpm {name: "Valid TPM Create", input: ConfigQemu{TPM: &TpmState{Storage: "test", Version: util.Pointer(TpmVersion("v2.0"))}}}, @@ -7256,6 +7262,13 @@ func Test_ConfigQemu_Validate(t *testing.T) { input: ConfigQemu{Disks: &QemuStorages{VirtIO: &QemuVirtIODisks{Disk_13: &QemuVirtIOStorage{Passthrough: &QemuVirtIOPassthrough{File: "/dev/disk/by-id/scsi1", WorldWideName: "0x5004A3B2C1D0E0F1#"}}}}}, err: errors.New(Error_QemuWorldWideName_Invalid), }, + // Invalid Pool + {name: "Invalid Pool Length", + input: ConfigQemu{Pool: util.Pointer(PoolName(test_data_pool.PoolName_Max_Illegal()))}, + err: errors.New(PoolName_Error_Length)}, + {name: "Invalid Pool Characters", + input: ConfigQemu{Pool: util.Pointer(PoolName(test_data_pool.PoolName_Error_Characters()[0]))}, + err: errors.New(PoolName_Error_Characters)}, // invalid TMP {name: "Invalid TPM errors.New(storage is required) Create", input: ConfigQemu{TPM: &TpmState{Storage: ""}}, diff --git a/test/data/test_data_pool/type_PoolName.go b/test/data/test_data_pool/type_PoolName.go new file mode 100644 index 00000000..5473a7d2 --- /dev/null +++ b/test/data/test_data_pool/type_PoolName.go @@ -0,0 +1,34 @@ +package test_data_pool + +import "strings" + +func PoolName_Error_Characters() []string { + return strings.Split("`~!@#$%^&*()=+{}[]|\\;:'\"<,>.?/", "") +} + +// 1024 valid charaters +func PoolName_Max_Legal() string { + return "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_" +} + +// 1025 valid charaters +func PoolName_Max_Illegal() string { + return PoolName_Max_Legal() + "A" +} + +func PoolName_Legal() string { + return "pool-1" +} + +// Has all the legal runes for th PoolName type. +func PoolName_Legals() []string { + legalRunes := strings.Split("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_", "") + legalStrings := []string{ + "pool1", + "pOol-1", + "PooL_1", + PoolName_Legal(), + PoolName_Max_Legal(), + } + return append(legalRunes, legalStrings[:]...) +} From 20d137342fb385ecc7461506d7c94a1068bb3c5a Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 20 Apr 2024 03:17:30 +0200 Subject: [PATCH 03/22] feat: `ListPools()` --- proxmox/config_pool.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index f3afc551..a7396c4b 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -5,6 +5,22 @@ import ( "regexp" ) +func ListPools(c *Client) ([]PoolName, error) { + raw, err := listPools(c) + if err != nil { + return nil, err + } + pools := make([]PoolName, len(raw)) + for i, e := range raw { + pools[i] = PoolName(e.(map[string]interface{})["poolid"].(string)) + } + return pools, nil +} + +func listPools(c *Client) ([]interface{}, error) { + return c.GetItemListInterfaceArray("/pools") +} + type ConfigPool struct { Name PoolName `json:"name"` Comment *string `json:"comment"` From 61e99e1d61e79aa436ed636dafa3e5d95734753c Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 20 Apr 2024 03:30:18 +0200 Subject: [PATCH 04/22] feat: check pool existence --- proxmox/config_pool.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index a7396c4b..a5c5d143 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -27,6 +27,11 @@ type ConfigPool struct { Guests *[]uint `json:"guests"` // TODO: Change type once we have a type for guestID } +// Same as PoolName.Exists() +func (config ConfigPool) Exists(c *Client) (bool, error) { + return config.Name.Exists(c) +} + func (config ConfigPool) Validate() error { // TODO: Add validation for Guests and Comment return config.Name.Validate() @@ -42,6 +47,25 @@ const ( var regex_PoolName = regexp.MustCompile(`^[a-zA-Z0-9-_]+$`) +func (config PoolName) Exists(c *Client) (bool, error) { + if c == nil { + return false, errors.New(Client_Error_Nil) + } + if err := config.Validate(); err != nil { + return false, err + } + // TODO: permission check + return config.Exists_Unsafe(c) +} + +func (config PoolName) Exists_Unsafe(c *Client) (bool, error) { + raw, err := listPools(c) + if err != nil { + return false, err + } + return ItemInKeyOfArray(raw, "poolid", string(config)), nil +} + func (config PoolName) Validate() error { if config == "" { return errors.New(PoolName_Error_Empty) From 8efaa233c15c43b2442c2a7fe4ddead82b524f4b Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 20 Apr 2024 03:32:09 +0200 Subject: [PATCH 05/22] feat: delete pool --- proxmox/config_pool.go | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index a5c5d143..8144ae0a 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -27,6 +27,11 @@ type ConfigPool struct { Guests *[]uint `json:"guests"` // TODO: Change type once we have a type for guestID } +// Same as PoolName.Delete() +func (config ConfigPool) Delete(c *Client) error { + return config.Name.Delete(c) +} + // Same as PoolName.Exists() func (config ConfigPool) Exists(c *Client) (bool, error) { return config.Name.Exists(c) @@ -40,13 +45,36 @@ func (config ConfigPool) Validate() error { type PoolName string const ( + PoolName_Error_Characters string = "PoolName may only contain the following characters: a-z, A-Z, 0-9, hyphen (-), and underscore (_)" PoolName_Error_Empty string = "PoolName cannot be empty" PoolName_Error_Length string = "PoolName may not be longer than 1024 characters" // proxmox does not seem to have a max length, so we artificially cap it at 1024 - PoolName_Error_Characters string = "PoolName may only contain the following characters: a-z, A-Z, 0-9, hyphen (-), and underscore (_)" + PoolName_Error_NotExists string = "Pool doesn't exist" ) var regex_PoolName = regexp.MustCompile(`^[a-zA-Z0-9-_]+$`) +func (config PoolName) Delete(c *Client) error { + if c == nil { + return errors.New(Client_Error_Nil) + } + if err := config.Validate(); err != nil { + return err + } + // TODO: permission check + exists, err := config.Exists_Unsafe(c) + if err != nil { + return err + } + if !exists { + return errors.New(PoolName_Error_NotExists) + } + return config.Delete_Unsafe(c) +} + +func (config PoolName) Delete_Unsafe(c *Client) error { + return c.Delete("/pools/" + string(config)) +} + func (config PoolName) Exists(c *Client) (bool, error) { if c == nil { return false, errors.New(Client_Error_Nil) From 121b5d0f6ef73a77edaacf77e79eba6a40008e7f Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sun, 19 May 2024 14:20:45 +0200 Subject: [PATCH 06/22] feat: get `pool` from api --- proxmox/config_pool.go | 43 +++++++++++++++++++++++++++++++++++++ proxmox/config_pool_test.go | 40 ++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index 8144ae0a..8651c3b9 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -27,6 +27,29 @@ type ConfigPool struct { Guests *[]uint `json:"guests"` // TODO: Change type once we have a type for guestID } +func (ConfigPool) mapToSDK(params map[string]interface{}) (config ConfigPool) { + if v, isSet := params["poolid"]; isSet { + config.Name = PoolName(v.(string)) + } + if v, isSet := params["comment"]; isSet { + tmp := v.(string) + config.Comment = &tmp + } + if v, isSet := params["members"]; isSet { + guests := make([]uint, 0) + for _, e := range v.([]interface{}) { + param := e.(map[string]interface{}) + if v, isSet := param["vmid"]; isSet { + guests = append(guests, uint(v.(float64))) + } + } + if len(guests) > 0 { + config.Guests = &guests + } + } + return +} + // Same as PoolName.Delete() func (config ConfigPool) Delete(c *Client) error { return config.Name.Delete(c) @@ -94,6 +117,26 @@ func (config PoolName) Exists_Unsafe(c *Client) (bool, error) { return ItemInKeyOfArray(raw, "poolid", string(config)), nil } +func (pool PoolName) Get(c *Client) (*ConfigPool, error) { + if c == nil { + return nil, errors.New(Client_Error_Nil) + } + if err := pool.Validate(); err != nil { + return nil, err + } + // TODO: permission check + return pool.Get_Unsafe(c) +} + +func (pool PoolName) Get_Unsafe(c *Client) (*ConfigPool, error) { + params, err := c.GetItemConfigMapStringInterface("/pools/"+string(pool), "pool", "CONFIG") + if err != nil { + return nil, err + } + config := ConfigPool{}.mapToSDK(params) + return &config, nil +} + func (config PoolName) Validate() error { if config == "" { return errors.New(PoolName_Error_Empty) diff --git a/proxmox/config_pool_test.go b/proxmox/config_pool_test.go index fb7073fb..fb729e45 100644 --- a/proxmox/config_pool_test.go +++ b/proxmox/config_pool_test.go @@ -4,10 +4,50 @@ import ( "errors" "testing" + "github.com/Telmate/proxmox-api-go/internal/util" "github.com/Telmate/proxmox-api-go/test/data/test_data_pool" "github.com/stretchr/testify/require" ) +func Test_ConfigPool_mapToSDK(t *testing.T) { + tests := []struct { + name string + input map[string]interface{} + output ConfigPool + }{ + {name: "All", + input: map[string]interface{}{ + "poolid": "test", + "comment": "test", + "members": []interface{}{ + map[string]interface{}{"vmid": float64(100)}, + map[string]interface{}{"vmid": float64(300)}, + map[string]interface{}{"vmid": float64(200)}}}, + output: ConfigPool{ + Name: "test", + Comment: util.Pointer("test"), + Guests: &[]uint{100, 300, 200}}}, + {name: "poolid", + input: map[string]interface{}{"poolid": "test"}, + output: ConfigPool{Name: "test"}}, + {name: "comment", + input: map[string]interface{}{"comment": "test"}, + output: ConfigPool{Comment: util.Pointer("test")}}, + {name: "members", + input: map[string]interface{}{ + "members": []interface{}{ + map[string]interface{}{"vmid": float64(100)}, + map[string]interface{}{"vmid": float64(300)}, + map[string]interface{}{"vmid": float64(200)}}}, + output: ConfigPool{Guests: &[]uint{100, 300, 200}}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.output, ConfigPool{}.mapToSDK(test.input)) + }) + } +} + func Test_ConfigPool_Validate(t *testing.T) { tests := []struct { name string From 044ab4c9873e0ad23466f52c779911936466453e Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sun, 19 May 2024 14:40:41 +0200 Subject: [PATCH 07/22] feat: remove pool guests --- proxmox/config_pool.go | 49 ++++++++++++++++++++++++++++++++++--- proxmox/config_pool_test.go | 34 +++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index 8651c3b9..7f73bea1 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -3,6 +3,7 @@ package proxmox import ( "errors" "regexp" + "strconv" ) func ListPools(c *Client) ([]PoolName, error) { @@ -68,10 +69,11 @@ func (config ConfigPool) Validate() error { type PoolName string const ( - PoolName_Error_Characters string = "PoolName may only contain the following characters: a-z, A-Z, 0-9, hyphen (-), and underscore (_)" - PoolName_Error_Empty string = "PoolName cannot be empty" - PoolName_Error_Length string = "PoolName may not be longer than 1024 characters" // proxmox does not seem to have a max length, so we artificially cap it at 1024 - PoolName_Error_NotExists string = "Pool doesn't exist" + PoolName_Error_Characters string = "PoolName may only contain the following characters: a-z, A-Z, 0-9, hyphen (-), and underscore (_)" + PoolName_Error_Empty string = "PoolName cannot be empty" + PoolName_Error_Length string = "PoolName may not be longer than 1024 characters" // proxmox does not seem to have a max length, so we artificially cap it at 1024 + PoolName_Error_NotExists string = "Pool doesn't exist" + PoolName_Error_NoGuestsSpecified string = "no guests specified" ) var regex_PoolName = regexp.MustCompile(`^[a-zA-Z0-9-_]+$`) @@ -137,6 +139,45 @@ func (pool PoolName) Get_Unsafe(c *Client) (*ConfigPool, error) { return &config, nil } +func (config PoolName) mapToApi(guestID []uint) map[string]interface{} { + var vms string + for _, e := range guestID { + vms += "," + strconv.FormatInt(int64(e), 10) + } + if len(vms) > 0 { + vms = vms[1:] + } + return map[string]interface{}{ + "poolid": string(config), + "vms": vms, + } +} + +func (config PoolName) RemoveGuests(c *Client, guestID []uint) error { + if c == nil { + return errors.New(Client_Error_Nil) + } + if err := config.Validate(); err != nil { + return err + } + if len(guestID) == 0 { + return errors.New(PoolName_Error_NoGuestsSpecified) + } + // TODO: permission check + if exists, err := config.Exists_Unsafe(c); err != nil { + return err + } else if !exists { + return errors.New(PoolName_Error_NotExists) + } + return config.RemoveGuests_Unsafe(c, guestID) +} + +func (config PoolName) RemoveGuests_Unsafe(c *Client, guestID []uint) error { + params := config.mapToApi(guestID) + params["delete"] = "1" + return c.Put(params, "/pools") +} + func (config PoolName) Validate() error { if config == "" { return errors.New(PoolName_Error_Empty) diff --git a/proxmox/config_pool_test.go b/proxmox/config_pool_test.go index fb729e45..738f1af3 100644 --- a/proxmox/config_pool_test.go +++ b/proxmox/config_pool_test.go @@ -73,6 +73,40 @@ func Test_ConfigPool_Validate(t *testing.T) { } } +func Test_PoolName_mapToApi(t *testing.T) { + type testInput struct { + pool PoolName + guests []uint + } + tests := []struct { + name string + input testInput + output map[string]interface{} + }{ + {name: `All`, + input: testInput{ + pool: "test", + guests: []uint{100, 300, 200}}, + output: map[string]interface{}{ + "poolid": "test", + "vms": "100,300,200"}}, + {name: `Empty`, + input: testInput{}, + output: map[string]interface{}{"poolid": "", "vms": ""}}, + {name: `pool`, + input: testInput{pool: "test"}, + output: map[string]interface{}{"poolid": "test", "vms": ""}}, + {name: `guests`, + input: testInput{guests: []uint{100, 300, 200}}, + output: map[string]interface{}{"poolid": "", "vms": "100,300,200"}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.output, test.input.pool.mapToApi(test.input.guests)) + }) + } +} + func Test_PoolName_Validate(t *testing.T) { tests := []struct { name string From 581d01c340de2c0c6780636dd6e65e5eb939feb1 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sun, 19 May 2024 16:30:12 +0200 Subject: [PATCH 08/22] feat: add guests to pool --- proxmox/config_pool.go | 84 +++++++++++++++++++++++++++++++++++++ proxmox/config_pool_test.go | 49 ++++++++++++++++++++++ proxmox/util.go | 14 +++++++ proxmox/util_test.go | 29 +++++++++++++ 4 files changed, 176 insertions(+) diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index 7f73bea1..471acd75 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -78,6 +78,68 @@ const ( var regex_PoolName = regexp.MustCompile(`^[a-zA-Z0-9-_]+$`) +func (config PoolName) addGuests_Unsafe(c *Client, guestIDs []uint, currentGuests *[]uint) error { + var guestsToAdd []uint + if currentGuests != nil && len(*currentGuests) > 0 { + guestsToAdd = subtractArray(guestIDs, *currentGuests) + } else { + guestsToAdd = guestIDs + } + if len(guestsToAdd) == 0 { + return nil + } + if c.version.Smaller(Version{8, 0, 0}) { + guests, err := ListGuests(c) + if err != nil { + return err + } + for i, e := range PoolName("").guestsToRemoveFromPools(guests, guestsToAdd) { + if err = i.RemoveGuests_Unsafe(c, e); err != nil { + return err + } + } + return config.addGuests_UnsafeV7(c, guestsToAdd) + } + return config.addGuests_UnsafeV8(c, guestsToAdd) +} + +func (config PoolName) addGuests_UnsafeV7(c *Client, guestIDs []uint) error { + params := config.mapToApi(guestIDs) + return c.Put(params, "/pools") +} + +func (config PoolName) addGuests_UnsafeV8(c *Client, guestIDs []uint) error { + params := config.mapToApi(guestIDs) + params["allow-move"] = "1" + return c.Put(params, "/pools") +} + +func (config PoolName) AddGuests(c *Client, guestIDs []uint) error { + if c == nil { + return errors.New(Client_Error_Nil) + } + if err := config.Validate(); err != nil { + return err + } + // TODO: permission check + exists, err := config.Exists_Unsafe(c) + if err != nil { + return err + } + if !exists { + return errors.New(PoolName_Error_NotExists) + } + return config.AddGuests_Unsafe(c, guestIDs) +} + +func (pool PoolName) AddGuests_Unsafe(c *Client, guestIDs []uint) error { + config, err := pool.Get_Unsafe(c) + if err != nil { + return err + } + return pool.addGuests_Unsafe(c, guestIDs, config.Guests) +} + func (config PoolName) Delete(c *Client) error { if c == nil { return errors.New(Client_Error_Nil) @@ -139,6 +201,28 @@ func (pool PoolName) Get_Unsafe(c *Client) (*ConfigPool, error) { return &config, nil } +func (PoolName) guestsToRemoveFromPools(guests []GuestResource, guestsToAdd []uint) map[PoolName][]uint { + // map[guestID]PoolName + guestsMap := make(map[uint]PoolName) + for _, e := range guests { + if e.Pool != "" { + guestsMap[e.Id] = e.Pool + continue + } + } + poolMap := make(map[PoolName][]uint) + for _, e := range guestsToAdd { + if pool, isSet := guestsMap[e]; isSet { + if _, isSet := poolMap[pool]; !isSet { + poolMap[pool] = []uint{e} + } else { + poolMap[pool] = append(poolMap[pool], e) + } + } + } + return poolMap +} + func (config PoolName) mapToApi(guestID []uint) map[string]interface{} { var vms string for _, e := range guestID { diff --git a/proxmox/config_pool_test.go b/proxmox/config_pool_test.go index 738f1af3..995d943c 100644 --- a/proxmox/config_pool_test.go +++ b/proxmox/config_pool_test.go @@ -73,6 +73,55 @@ func Test_ConfigPool_Validate(t *testing.T) { } } +func Test_PoolName_guestsToRemoveFromPools(t *testing.T) { + type testInput struct { + guests []GuestResource + guestsToAdd []uint + } + tests := []struct { + name string + input testInput + output map[PoolName][]uint + }{ + {name: `'guestsToAdd' Not in 'guests'`, + input: testInput{ + guests: []GuestResource{ + {Id: 100, Pool: "test"}, + {Id: 200, Pool: "poolA"}, + {Id: 300, Pool: "test"}}, + guestsToAdd: []uint{700, 800, 900}}, + output: map[PoolName][]uint{}}, + {name: `Empty`, + output: map[PoolName][]uint{}}, + {name: `Empty 'guests'`, + input: testInput{ + guestsToAdd: []uint{100, 300, 200}}, + output: map[PoolName][]uint{}}, + {name: `Empty 'guestsToAdd'`, + input: testInput{ + guests: []GuestResource{ + {Id: 100, Pool: "test"}, + {Id: 200, Pool: "poolA"}, + {Id: 300, Pool: "test"}}}, + output: map[PoolName][]uint{}}, + {name: `Full`, + input: testInput{ + guests: []GuestResource{ + {Id: 100, Pool: "test"}, + {Id: 200, Pool: "poolA"}, + {Id: 300, Pool: "test"}}, + guestsToAdd: []uint{100, 300, 200}}, + output: map[PoolName][]uint{ + "test": {100, 300}, + "poolA": {200}}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.output, PoolName("").guestsToRemoveFromPools(test.input.guests, test.input.guestsToAdd)) + }) + } +} + func Test_PoolName_mapToApi(t *testing.T) { type testInput struct { pool PoolName diff --git a/proxmox/util.go b/proxmox/util.go index 04beb558..d5925ccc 100644 --- a/proxmox/util.go +++ b/proxmox/util.go @@ -249,3 +249,17 @@ func splitStringOfSettings(settings string) map[string]interface{} { } return settingMap } + +// subtracts array B from array A +func subtractArray[T comparable](A, B []T) (result []T) { + elements := make(map[T]bool) + for _, item := range B { + elements[item] = true + } + for _, item := range A { + if !elements[item] { + result = append(result, item) + } + } + return +} diff --git a/proxmox/util_test.go b/proxmox/util_test.go index 065f77c1..3875a70b 100644 --- a/proxmox/util_test.go +++ b/proxmox/util_test.go @@ -110,3 +110,32 @@ func Test_splitStringOfSettings(t *testing.T) { require.Equal(t, e.Output, splitStringOfSettings(e.Input)) } } + +func Test_subtractArray(t *testing.T) { + type testInput struct { + A []string + B []string + } + tests := []struct { + name string + input testInput + output []string + }{ + {name: "A and B different", + input: testInput{A: []string{"a", "b", "c"}, B: []string{"a", "b"}}, + output: []string{"c"}}, + {name: "A and B empty"}, + {name: "A and B same", + input: testInput{A: []string{"a", "b", "c"}, B: []string{"a", "b", "c"}}}, + {name: "A empty", + input: testInput{B: []string{"a", "b", "c"}}}, + {name: "B empty", + input: testInput{A: []string{"a", "b", "c"}}, + output: []string{"a", "b", "c"}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.output, subtractArray(test.input.A, test.input.B)) + }) + } +} From 2a4d1ca7cdf25a99db001b9242034e0a9f72ca6a Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Tue, 21 May 2024 18:10:35 +0200 Subject: [PATCH 09/22] feat: set pool guests --- proxmox/config_pool.go | 111 ++++++++++++++++++++++++++---------- proxmox/config_pool_test.go | 34 ++++------- 2 files changed, 92 insertions(+), 53 deletions(-) diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index 471acd75..078b3da1 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -73,12 +73,13 @@ const ( PoolName_Error_Empty string = "PoolName cannot be empty" PoolName_Error_Length string = "PoolName may not be longer than 1024 characters" // proxmox does not seem to have a max length, so we artificially cap it at 1024 PoolName_Error_NotExists string = "Pool doesn't exist" + PoolName_Error_Exists string = "Pool already exists" PoolName_Error_NoGuestsSpecified string = "no guests specified" ) var regex_PoolName = regexp.MustCompile(`^[a-zA-Z0-9-_]+$`) -func (config PoolName) addGuests_Unsafe(c *Client, guestIDs []uint, currentGuests *[]uint) error { +func (config PoolName) addGuests_Unsafe(c *Client, guestIDs []uint, currentGuests *[]uint, version Version) error { var guestsToAdd []uint if currentGuests != nil && len(*currentGuests) > 0 { guestsToAdd = subtractArray(guestIDs, *currentGuests) @@ -88,30 +89,29 @@ func (config PoolName) addGuests_Unsafe(c *Client, guestIDs []uint, currentGuest if len(guestsToAdd) == 0 { return nil } - if c.version.Smaller(Version{8, 0, 0}) { - guests, err := ListGuests(c) - if err != nil { + if !version.Smaller(Version{8, 0, 0}) { + return config.addGuests_UnsafeV8(c, guestsToAdd) + } + guests, err := ListGuests(c) + if err != nil { + return err + } + for i, e := range PoolName("").guestsToRemoveFromPools(guests, guestsToAdd) { + if err = i.RemoveGuests_Unsafe(c, e); err != nil { return err } - for i, e := range PoolName("").guestsToRemoveFromPools(guests, guestsToAdd) { - if err = i.RemoveGuests_Unsafe(c, e); err != nil { - return err - } - } - return config.addGuests_UnsafeV7(c, guestsToAdd) } - return config.addGuests_UnsafeV8(c, guestsToAdd) + return config.addGuests_UnsafeV7(c, guestsToAdd) } -func (config PoolName) addGuests_UnsafeV7(c *Client, guestIDs []uint) error { - params := config.mapToApi(guestIDs) - return c.Put(params, "/pools") +func (pool PoolName) addGuests_UnsafeV7(c *Client, guestIDs []uint) error { + return pool.put(c, map[string]interface{}{"vms": PoolName("").mapToString(guestIDs)}) } -func (config PoolName) addGuests_UnsafeV8(c *Client, guestIDs []uint) error { - params := config.mapToApi(guestIDs) - params["allow-move"] = "1" - return c.Put(params, "/pools") +func (pool PoolName) addGuests_UnsafeV8(c *Client, guestIDs []uint) error { + return pool.put(c, map[string]interface{}{ + "vms": PoolName("").mapToString(guestIDs), + "allow-move": "1"}) } func (config PoolName) AddGuests(c *Client, guestIDs []uint) error { @@ -133,11 +133,15 @@ func (config PoolName) AddGuests(c *Client, guestIDs []uint) error { } func (pool PoolName) AddGuests_Unsafe(c *Client, guestIDs []uint) error { + version, err := c.GetVersion() + if err != nil { + return err + } config, err := pool.Get_Unsafe(c) if err != nil { return err } - return pool.addGuests_Unsafe(c, guestIDs, config.Guests) + return pool.addGuests_Unsafe(c, guestIDs, config.Guests, version) } func (config PoolName) Delete(c *Client) error { @@ -223,18 +227,19 @@ func (PoolName) guestsToRemoveFromPools(guests []GuestResource, guestsToAdd []ui return poolMap } -func (config PoolName) mapToApi(guestID []uint) map[string]interface{} { - var vms string +// TODO replace once we have a type for guestID +func (PoolName) mapToString(guestID []uint) (vms string) { for _, e := range guestID { vms += "," + strconv.FormatInt(int64(e), 10) } if len(vms) > 0 { vms = vms[1:] } - return map[string]interface{}{ - "poolid": string(config), - "vms": vms, - } + return +} + +func (pool PoolName) put(c *Client, params map[string]interface{}) error { + return c.Put(params, "/pools?poolid="+string(pool)) } func (config PoolName) RemoveGuests(c *Client, guestID []uint) error { @@ -256,10 +261,58 @@ func (config PoolName) RemoveGuests(c *Client, guestID []uint) error { return config.RemoveGuests_Unsafe(c, guestID) } -func (config PoolName) RemoveGuests_Unsafe(c *Client, guestID []uint) error { - params := config.mapToApi(guestID) - params["delete"] = "1" - return c.Put(params, "/pools") +func (pool PoolName) RemoveGuests_Unsafe(c *Client, guestID []uint) error { + return pool.put(c, map[string]interface{}{ + "vms": PoolName("").mapToString(guestID), + "delete": "1"}) +} + +func (pool PoolName) SetGuests(c *Client, guestID []uint) error { + if c == nil { + return errors.New(Client_Error_Nil) + } + if err := pool.Validate(); err != nil { + return err + } + if exists, err := pool.Exists(c); err != nil { + return err + } else if !exists { + return errors.New(PoolName_Error_NotExists) + } + // TODO: permission check + return pool.SetGuests_Unsafe(c, guestID) +} + +func (pool PoolName) SetGuests_Unsafe(c *Client, guestID []uint) error { + version, err := c.GetVersion() + if err != nil { + return err + } + config, err := pool.Get(c) + if err != nil { + return err + } + return pool.setGuests_Unsafe(c, guestID, config.Guests, version) +} + +func (pool PoolName) setGuests_Unsafe(c *Client, guestIDs []uint, currentGuests *[]uint, version Version) error { + var guestsToRemove []uint + for _, e := range *currentGuests { + removeGuest := true + for _, ee := range guestIDs { + if e == ee { + removeGuest = false + break + } + } + if removeGuest { + guestsToRemove = append(guestsToRemove, e) + } + } + if err := pool.RemoveGuests_Unsafe(c, guestsToRemove); err != nil { + return err + } + return pool.addGuests_Unsafe(c, guestIDs, currentGuests, version) } func (config PoolName) Validate() error { diff --git a/proxmox/config_pool_test.go b/proxmox/config_pool_test.go index 995d943c..a451ed4f 100644 --- a/proxmox/config_pool_test.go +++ b/proxmox/config_pool_test.go @@ -122,36 +122,22 @@ func Test_PoolName_guestsToRemoveFromPools(t *testing.T) { } } -func Test_PoolName_mapToApi(t *testing.T) { - type testInput struct { - pool PoolName - guests []uint - } +func Test_PoolName_mapToString(t *testing.T) { tests := []struct { name string - input testInput - output map[string]interface{} + input []uint + output string }{ - {name: `All`, - input: testInput{ - pool: "test", - guests: []uint{100, 300, 200}}, - output: map[string]interface{}{ - "poolid": "test", - "vms": "100,300,200"}}, - {name: `Empty`, - input: testInput{}, - output: map[string]interface{}{"poolid": "", "vms": ""}}, - {name: `pool`, - input: testInput{pool: "test"}, - output: map[string]interface{}{"poolid": "test", "vms": ""}}, - {name: `guests`, - input: testInput{guests: []uint{100, 300, 200}}, - output: map[string]interface{}{"poolid": "", "vms": "100,300,200"}}, + {name: `empty`, + input: []uint{}}, + {name: `full`, + input: []uint{100, 300, 200}, + output: "100,300,200"}, + {name: `nil`}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - require.Equal(t, test.output, test.input.pool.mapToApi(test.input.guests)) + require.Equal(t, test.output, PoolName("").mapToString(test.input)) }) } } From ea3cc0bc9960877a4cab5aa19dbfaa1ea125a0d9 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Tue, 21 May 2024 18:31:28 +0200 Subject: [PATCH 10/22] feat: pool create and update --- proxmox/config_pool.go | 77 +++++++++++++++++++++++++++++++++++++ proxmox/config_pool_test.go | 69 +++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+) diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index 078b3da1..a5a2226e 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -28,6 +28,22 @@ type ConfigPool struct { Guests *[]uint `json:"guests"` // TODO: Change type once we have a type for guestID } +func (config ConfigPool) mapToApi(currentConfig *ConfigPool) map[string]interface{} { + params := map[string]interface{}{} + if currentConfig == nil { //create + params["poolid"] = string(config.Name) + if config.Comment != nil && *config.Comment != "" { + params["comment"] = string(*config.Comment) + } + return params + } + // update + if config.Comment != nil { + params["comment"] = string(*config.Comment) + } + return params +} + func (ConfigPool) mapToSDK(params map[string]interface{}) (config ConfigPool) { if v, isSet := params["poolid"]; isSet { config.Name = PoolName(v.(string)) @@ -51,6 +67,34 @@ func (ConfigPool) mapToSDK(params map[string]interface{}) (config ConfigPool) { return } +func (config ConfigPool) Create(c *Client) error { + if err := config.Validate(); err != nil { + return err + } + // TODO check permissions + if exists, err := config.Name.Exists_Unsafe(c); err != nil { + return err + } else if exists { + return errors.New(PoolName_Error_Exists) + } + return config.Create_Unsafe(c) +} + +// Create_Unsafe creates a new pool without validating the input +func (config ConfigPool) Create_Unsafe(c *Client) error { + version, err := c.GetVersion() + if err != nil { + return err + } + if err := c.Post(config.mapToApi(nil), "/pools"); err != nil { + return err + } + if config.Guests != nil { + return config.Name.addGuests_Unsafe(c, *config.Guests, nil, version) + } + return nil +} + // Same as PoolName.Delete() func (config ConfigPool) Delete(c *Client) error { return config.Name.Delete(c) @@ -61,6 +105,39 @@ func (config ConfigPool) Exists(c *Client) (bool, error) { return config.Name.Exists(c) } +func (config ConfigPool) Update(c *Client) error { + if c == nil { + return errors.New(Client_Error_Nil) + } + if err := config.Validate(); err != nil { + return err + } + if exists, err := config.Name.Exists(c); err != nil { + return err + } else if !exists { + return errors.New(PoolName_Error_NotExists) + } + // TODO check permissions + current, err := config.Name.Get_Unsafe(c) + if err != nil { + return err + } + return config.Update_Unsafe(c, current) +} + +// Update_Unsafe updates a pool without validating the input +func (config ConfigPool) Update_Unsafe(c *Client, current *ConfigPool) error { + if params := config.mapToApi(current); len(params) > 0 { + if err := config.Name.put(c, params); err != nil { + return err + } + } + if config.Guests != nil { + return config.Name.SetGuests_Unsafe(c, *config.Guests) + } + return nil +} + func (config ConfigPool) Validate() error { // TODO: Add validation for Guests and Comment return config.Name.Validate() diff --git a/proxmox/config_pool_test.go b/proxmox/config_pool_test.go index a451ed4f..ba0e1282 100644 --- a/proxmox/config_pool_test.go +++ b/proxmox/config_pool_test.go @@ -9,6 +9,75 @@ import ( "github.com/stretchr/testify/require" ) +func Test_ConfigPool_mapToApi(t *testing.T) { + type testInput struct { + new ConfigPool + current *ConfigPool + } + tests := []struct { + name string + input testInput + output map[string]interface{} + }{ + {name: `Create Full`, + input: testInput{ + new: ConfigPool{ + Name: "test", + Comment: util.Pointer("test-comment"), + Guests: &[]uint{100, 300, 200}}}, + output: map[string]interface{}{ + "poolid": "test", + "comment": "test-comment"}}, + {name: `Create poolid`, + input: testInput{ + new: ConfigPool{Name: "test"}}, + output: map[string]interface{}{"poolid": "test"}}, + {name: `Create comment`, + input: testInput{ + new: ConfigPool{Comment: util.Pointer("test-comment")}}, + output: map[string]interface{}{ + "poolid": "", + "comment": "test-comment"}}, + {name: `Create members`, + input: testInput{ + new: ConfigPool{Guests: &[]uint{100, 300, 200}}}, + output: map[string]interface{}{"poolid": ""}}, + {name: `Update Full`, + input: testInput{ + new: ConfigPool{ + Name: "test", + Comment: util.Pointer("test-comment"), + Guests: &[]uint{100, 300, 200}}, + current: &ConfigPool{ + Name: "test", + Comment: util.Pointer("old-comment"), + Guests: &[]uint{100, 300}}}, + output: map[string]interface{}{ + "comment": "test-comment"}}, + {name: `Update poolid`, + input: testInput{ + new: ConfigPool{Name: "test"}, + current: &ConfigPool{Name: "old"}}, + output: map[string]interface{}{}}, + {name: `Update comment`, + input: testInput{ + new: ConfigPool{Comment: util.Pointer("test-comment")}, + current: &ConfigPool{Comment: util.Pointer("old-comment")}}, + output: map[string]interface{}{ + "comment": "test-comment"}}, + {name: `Update members`, + input: testInput{ + new: ConfigPool{Guests: &[]uint{100, 300, 200}}, + current: &ConfigPool{Guests: &[]uint{100, 300}}}, + output: map[string]interface{}{}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.output, test.input.new.mapToApi(test.input.current)) + }) + } +} + func Test_ConfigPool_mapToSDK(t *testing.T) { tests := []struct { name string From d5b3fa45e1aa9f3a075f526fbc4f978920fd6575 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Tue, 21 May 2024 18:56:14 +0200 Subject: [PATCH 11/22] feat: set pool config --- proxmox/config_pool.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index a5a2226e..77ee46ac 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -105,6 +105,32 @@ func (config ConfigPool) Exists(c *Client) (bool, error) { return config.Name.Exists(c) } +func (config ConfigPool) Set(c *Client) error { + if c == nil { + return errors.New(Client_Error_Nil) + } + if err := config.Validate(); err != nil { + return err + } + // TODO check permissions + return config.Set_Unsafe(c) +} + +func (config ConfigPool) Set_Unsafe(c *Client) error { + exists, err := config.Name.Exists_Unsafe(c) + if err != nil { + return err + } + if exists { + current, err := config.Name.Get_Unsafe(c) + if err != nil { + return err + } + return config.Update_Unsafe(c, current) + } + return config.Create_Unsafe(c) +} + func (config ConfigPool) Update(c *Client) error { if c == nil { return errors.New(Client_Error_Nil) From a45865c8a2a8f74c2173e99d221659afd3fd590a Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Wed, 22 May 2024 08:28:02 +0200 Subject: [PATCH 12/22] refactor: use new way of adding guests to a pool --- proxmox/config_guest.go | 30 ++++++++++++++++++++++++++++++ proxmox/config_pool.go | 1 + proxmox/config_qemu.go | 22 +++++++++++++++------- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/proxmox/config_guest.go b/proxmox/config_guest.go index 131d9d20..128dc270 100644 --- a/proxmox/config_guest.go +++ b/proxmox/config_guest.go @@ -178,6 +178,36 @@ func GuestReboot(vmr *VmRef, client *Client) (err error) { return } +func guestSetPool_Unsafe(c *Client, guestID uint, newPool PoolName, currentPool *PoolName, version Version) (err error) { + if newPool == "" { + if *currentPool != "" { // leave pool + if err = (*currentPool).RemoveGuests_Unsafe(c, []uint{guestID}); err != nil { + return + } + } + } else { + if *currentPool == "" { // join pool + if err = newPool.addGuests_UnsafeV7(c, []uint{guestID}); err != nil { + return + } + } else if newPool != *currentPool { // change pool + if version.Smaller(Version{8, 0, 0}) { + if err = (*currentPool).RemoveGuests_Unsafe(c, []uint{guestID}); err != nil { + return + } + if err = newPool.addGuests_UnsafeV7(c, []uint{guestID}); err != nil { + return + } + } else { + if err = newPool.addGuests_UnsafeV8(c, []uint{guestID}); err != nil { + return + } + } + } + } + return +} + func GuestShutdown(vmr *VmRef, client *Client, force bool) (err error) { if err = client.CheckVmRef(vmr); err != nil { return diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index 77ee46ac..1e78a9f5 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -211,6 +211,7 @@ func (pool PoolName) addGuests_UnsafeV7(c *Client, guestIDs []uint) error { return pool.put(c, map[string]interface{}{"vms": PoolName("").mapToString(guestIDs)}) } +// from 8.0.0 on proxmox can move the guests to the pool while they are still in another pool func (pool PoolName) addGuests_UnsafeV8(c *Client, guestIDs []uint) error { return pool.put(c, map[string]interface{}{ "vms": PoolName("").mapToString(guestIDs), diff --git a/proxmox/config_qemu.go b/proxmox/config_qemu.go index 084cd7b6..dd6b9be4 100644 --- a/proxmox/config_qemu.go +++ b/proxmox/config_qemu.go @@ -747,6 +747,11 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede var exitStatus string if currentConfig != nil { // Update + var version Version + if version, err = client.Version(); err != nil { + return + } + // TODO implement tmp move and version change url := "/nodes/" + vmr.node + "/" + vmr.vmType + "/" + strconv.Itoa(vmr.vmId) + "/config" var itemsToDeleteBeforeUpdate string // this is for items that should be removed before they can be created again e.g. cloud-init disks. (convert to array when needed) @@ -840,6 +845,10 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede return } + if newConfig.Pool != nil { // update pool membership + guestSetPool_Unsafe(client, uint(vmr.vmId), *newConfig.Pool, currentConfig.Pool, version) + } + if stopped { // start vm if it was stopped if rebootIfNeeded { if err = GuestStart(vmr, client); err != nil { @@ -872,19 +881,18 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede if err = resizeNewDisks(vmr, client, newConfig.Disks, nil); err != nil { return } + if newConfig.Pool != nil && *newConfig.Pool != "" { // add guest to pool + // we can use the v7 implementation here because we know the guest isn't in a pool yet + if err = newConfig.Pool.addGuests_UnsafeV7(client, []uint{uint(vmr.vmId)}); err != nil { + return + } + } if err = client.insertCachedPermission(permissionPath(permissionCategory_GuestPath) + "/" + permissionPath(strconv.Itoa(vmr.vmId))); err != nil { return } } _, err = client.UpdateVMHA(vmr, newConfig.HaState, newConfig.HaGroup) - if err != nil { - return - } - - if newConfig.Pool != nil { - _, err = client.UpdateVMPool(vmr, string(*newConfig.Pool)) - } return } From 38683a5cd80d716af03024df123115dc0ccab263 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Wed, 22 May 2024 17:38:50 +0200 Subject: [PATCH 13/22] Add deprecation warning --- proxmox/client.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/proxmox/client.go b/proxmox/client.go index cda34016..5353ffba 100644 --- a/proxmox/client.go +++ b/proxmox/client.go @@ -1531,6 +1531,7 @@ func getStorageAndVolumeName( return storageName, volumeName } +// Deprecated: use ConfigQemu.Update() instead func (c *Client) UpdateVMPool(vmr *VmRef, pool string) (exitStatus interface{}, err error) { // Same pool if vmr.pool == pool { @@ -1668,14 +1669,17 @@ func (c *Client) UpdateVMHA(vmr *VmRef, haState string, haGroup string) (exitSta return } +// TODO: implement replacement func (c *Client) GetPoolList() (pools map[string]interface{}, err error) { return c.GetItemList("/pools") } +// TODO: implement replacement func (c *Client) GetPoolInfo(poolid string) (poolInfo map[string]interface{}, err error) { return c.GetItemConfigMapStringInterface("/pools/"+poolid, "pool", "CONFIG") } +// Deprecated: use ConfigPool.Create() instead func (c *Client) CreatePool(poolid string, comment string) error { return c.Post(map[string]interface{}{ "poolid": poolid, @@ -1683,6 +1687,7 @@ func (c *Client) CreatePool(poolid string, comment string) error { }, "/pools") } +// Deprecated: use ConfigPool.Update() instead func (c *Client) UpdatePoolComment(poolid string, comment string) error { return c.Put(map[string]interface{}{ "poolid": poolid, @@ -1690,6 +1695,7 @@ func (c *Client) UpdatePoolComment(poolid string, comment string) error { }, "/pools/"+poolid) } +// Deprecated: use PoolName.Delete() instead func (c *Client) DeletePool(poolid string) error { return c.Delete("/pools/" + poolid) } From 9ae6b74d63b288a7f706671e260beb26383250c6 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Wed, 22 May 2024 17:39:30 +0200 Subject: [PATCH 14/22] refactor: use new functions --- cli/command/create/create-pool.go | 10 +++++++--- cli/command/update/update-poolcomment.go | 10 +++++++--- main.go | 15 +++++++++------ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/cli/command/create/create-pool.go b/cli/command/create/create-pool.go index d143237d..d9b1bb81 100644 --- a/cli/command/create/create-pool.go +++ b/cli/command/create/create-pool.go @@ -2,6 +2,7 @@ package create import ( "github.com/Telmate/proxmox-api-go/cli" + "github.com/Telmate/proxmox-api-go/proxmox" "github.com/spf13/cobra" ) @@ -11,12 +12,15 @@ var create_poolCmd = &cobra.Command{ Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) (err error) { id := cli.RequiredIDset(args, 0, "PoolID") - var comment string + var comment *string if len(args) > 1 { - comment = args[1] + comment = &args[1] } c := cli.NewClient() - err = c.CreatePool(id, comment) + err = proxmox.ConfigPool{ + Name: proxmox.PoolName(id), + Comment: comment, + }.Create(c) if err != nil { return } diff --git a/cli/command/update/update-poolcomment.go b/cli/command/update/update-poolcomment.go index 1ecf709d..39937829 100644 --- a/cli/command/update/update-poolcomment.go +++ b/cli/command/update/update-poolcomment.go @@ -2,6 +2,7 @@ package update import ( "github.com/Telmate/proxmox-api-go/cli" + "github.com/Telmate/proxmox-api-go/proxmox" "github.com/spf13/cobra" ) @@ -10,13 +11,16 @@ var update_poolCmd = &cobra.Command{ Short: "Updates the comment on the specified pool", Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) (err error) { - var comment string id := cli.RequiredIDset(args, 0, "PoolID") + var comment *string if len(args) > 1 { - comment = args[1] + comment = &args[1] } c := cli.NewClient() - err = c.UpdatePoolComment(id, comment) + err = proxmox.ConfigPool{ + Name: proxmox.PoolName(id), + Comment: comment, + }.Update(c) if err != nil { return } diff --git a/main.go b/main.go index c5f903cc..3fcda0af 100644 --- a/main.go +++ b/main.go @@ -486,8 +486,10 @@ func main() { comment = flag.Args()[2] } - err := c.CreatePool(poolid, comment) - failError(err) + failError(proxmox.ConfigPool{ + Name: proxmox.PoolName(poolid), + Comment: &comment, + }.Create(c)) fmt.Printf("Pool %s created\n", poolid) case "deletePool": @@ -497,8 +499,7 @@ func main() { } poolid := flag.Args()[1] - err := c.DeletePool(poolid) - failError(err) + failError(proxmox.PoolName(poolid).Delete(c)) fmt.Printf("Pool %s removed\n", poolid) case "updatePoolComment": @@ -510,8 +511,10 @@ func main() { poolid := flag.Args()[1] comment := flag.Args()[2] - err := c.UpdatePoolComment(poolid, comment) - failError(err) + failError(proxmox.ConfigPool{ + Name: proxmox.PoolName(poolid), + Comment: &comment, + }.Update(c)) fmt.Printf("Pool %s updated\n", poolid) //Users From 78e8f40d2bd7883ae63e4c440a59d5e7d0f03055 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Wed, 22 May 2024 18:04:26 +0200 Subject: [PATCH 15/22] fix: incorrect logic --- proxmox/config_pool.go | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index 1e78a9f5..f0b64fa7 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -400,22 +400,11 @@ func (pool PoolName) SetGuests_Unsafe(c *Client, guestID []uint) error { } func (pool PoolName) setGuests_Unsafe(c *Client, guestIDs []uint, currentGuests *[]uint, version Version) error { - var guestsToRemove []uint - for _, e := range *currentGuests { - removeGuest := true - for _, ee := range guestIDs { - if e == ee { - removeGuest = false - break - } - } - if removeGuest { - guestsToRemove = append(guestsToRemove, e) + if currentGuests != nil && len(*currentGuests) > 0 { + if err := pool.RemoveGuests_Unsafe(c, subtractArray(*currentGuests, guestIDs)); err != nil { + return err } } - if err := pool.RemoveGuests_Unsafe(c, guestsToRemove); err != nil { - return err - } return pool.addGuests_Unsafe(c, guestIDs, currentGuests, version) } From 80e07bfe7454544125bb3bc84d2f96289a5ac158 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 25 May 2024 12:42:36 +0200 Subject: [PATCH 16/22] fix: v7 and v8 need different api paths --- proxmox/config_guest.go | 12 ++++++++---- proxmox/config_pool.go | 39 ++++++++++++++++++++++++++++----------- proxmox/config_qemu.go | 13 ++++++------- 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/proxmox/config_guest.go b/proxmox/config_guest.go index 128dc270..0ff62c03 100644 --- a/proxmox/config_guest.go +++ b/proxmox/config_guest.go @@ -181,18 +181,22 @@ func GuestReboot(vmr *VmRef, client *Client) (err error) { func guestSetPool_Unsafe(c *Client, guestID uint, newPool PoolName, currentPool *PoolName, version Version) (err error) { if newPool == "" { if *currentPool != "" { // leave pool - if err = (*currentPool).RemoveGuests_Unsafe(c, []uint{guestID}); err != nil { + if err = (*currentPool).RemoveGuests_Unsafe(c, []uint{guestID}, version); err != nil { return } } } else { if *currentPool == "" { // join pool - if err = newPool.addGuests_UnsafeV7(c, []uint{guestID}); err != nil { - return + if version.Smaller(Version{8, 0, 0}) { + if err = newPool.addGuests_UnsafeV7(c, []uint{guestID}); err != nil { + return + } + } else { + newPool.addGuests_UnsafeV8(c, []uint{guestID}) } } else if newPool != *currentPool { // change pool if version.Smaller(Version{8, 0, 0}) { - if err = (*currentPool).RemoveGuests_Unsafe(c, []uint{guestID}); err != nil { + if err = (*currentPool).RemoveGuests_Unsafe(c, []uint{guestID}, version); err != nil { return } if err = newPool.addGuests_UnsafeV7(c, []uint{guestID}); err != nil { diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index f0b64fa7..836f4306 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -153,8 +153,12 @@ func (config ConfigPool) Update(c *Client) error { // Update_Unsafe updates a pool without validating the input func (config ConfigPool) Update_Unsafe(c *Client, current *ConfigPool) error { + version, err := c.GetVersion() + if err != nil { + return err + } if params := config.mapToApi(current); len(params) > 0 { - if err := config.Name.put(c, params); err != nil { + if err = config.Name.put(c, params, version); err != nil { return err } } @@ -200,7 +204,7 @@ func (config PoolName) addGuests_Unsafe(c *Client, guestIDs []uint, currentGuest return err } for i, e := range PoolName("").guestsToRemoveFromPools(guests, guestsToAdd) { - if err = i.RemoveGuests_Unsafe(c, e); err != nil { + if err = i.RemoveGuests_Unsafe(c, e, version); err != nil { return err } } @@ -208,12 +212,12 @@ func (config PoolName) addGuests_Unsafe(c *Client, guestIDs []uint, currentGuest } func (pool PoolName) addGuests_UnsafeV7(c *Client, guestIDs []uint) error { - return pool.put(c, map[string]interface{}{"vms": PoolName("").mapToString(guestIDs)}) + return pool.putV7(c, map[string]interface{}{"vms": PoolName("").mapToString(guestIDs)}) } // from 8.0.0 on proxmox can move the guests to the pool while they are still in another pool func (pool PoolName) addGuests_UnsafeV8(c *Client, guestIDs []uint) error { - return pool.put(c, map[string]interface{}{ + return pool.putV8(c, map[string]interface{}{ "vms": PoolName("").mapToString(guestIDs), "allow-move": "1"}) } @@ -342,13 +346,25 @@ func (PoolName) mapToString(guestID []uint) (vms string) { return } -func (pool PoolName) put(c *Client, params map[string]interface{}) error { +func (pool PoolName) put(c *Client, params map[string]interface{}, version Version) error { + if version.Smaller(Version{8, 0, 0}) { + return pool.putV7(c, params) + } + return pool.putV8(c, params) +} + +func (pool PoolName) putV7(c *Client, params map[string]interface{}) error { + return c.Put(params, "/pools/"+string(pool)) +} + +func (pool PoolName) putV8(c *Client, params map[string]interface{}) error { return c.Put(params, "/pools?poolid="+string(pool)) } func (config PoolName) RemoveGuests(c *Client, guestID []uint) error { - if c == nil { - return errors.New(Client_Error_Nil) + version, err := c.GetVersion() + if err != nil { + return err } if err := config.Validate(); err != nil { return err @@ -362,13 +378,14 @@ func (config PoolName) RemoveGuests(c *Client, guestID []uint) error { } else if !exists { return errors.New(PoolName_Error_NotExists) } - return config.RemoveGuests_Unsafe(c, guestID) + return config.RemoveGuests_Unsafe(c, guestID, version) } -func (pool PoolName) RemoveGuests_Unsafe(c *Client, guestID []uint) error { +func (pool PoolName) RemoveGuests_Unsafe(c *Client, guestID []uint, version Version) error { return pool.put(c, map[string]interface{}{ "vms": PoolName("").mapToString(guestID), - "delete": "1"}) + "delete": "1"}, + version) } func (pool PoolName) SetGuests(c *Client, guestID []uint) error { @@ -401,7 +418,7 @@ func (pool PoolName) SetGuests_Unsafe(c *Client, guestID []uint) error { func (pool PoolName) setGuests_Unsafe(c *Client, guestIDs []uint, currentGuests *[]uint, version Version) error { if currentGuests != nil && len(*currentGuests) > 0 { - if err := pool.RemoveGuests_Unsafe(c, subtractArray(*currentGuests, guestIDs)); err != nil { + if err := pool.RemoveGuests_Unsafe(c, subtractArray(*currentGuests, guestIDs), version); err != nil { return err } } diff --git a/proxmox/config_qemu.go b/proxmox/config_qemu.go index dd6b9be4..6e503fbf 100644 --- a/proxmox/config_qemu.go +++ b/proxmox/config_qemu.go @@ -743,15 +743,15 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede return } + var version Version + if version, err = client.Version(); err != nil { + return + } + var params map[string]interface{} var exitStatus string if currentConfig != nil { // Update - var version Version - if version, err = client.Version(); err != nil { - return - } - // TODO implement tmp move and version change url := "/nodes/" + vmr.node + "/" + vmr.vmType + "/" + strconv.Itoa(vmr.vmId) + "/config" var itemsToDeleteBeforeUpdate string // this is for items that should be removed before they can be created again e.g. cloud-init disks. (convert to array when needed) @@ -882,8 +882,7 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede return } if newConfig.Pool != nil && *newConfig.Pool != "" { // add guest to pool - // we can use the v7 implementation here because we know the guest isn't in a pool yet - if err = newConfig.Pool.addGuests_UnsafeV7(client, []uint{uint(vmr.vmId)}); err != nil { + if err = newConfig.Pool.addGuests_Unsafe(client, []uint{uint(vmr.vmId)}, nil, version); err != nil { return } } From 2fdc8d9d050b927eaee9a9adb75cd8a9087dac13 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 25 May 2024 12:44:04 +0200 Subject: [PATCH 17/22] fix: pool membership not get `GetVmConfig()` does not always get the pool membership --- proxmox/config_qemu.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/proxmox/config_qemu.go b/proxmox/config_qemu.go index 6e503fbf..8ccdb19e 100644 --- a/proxmox/config_qemu.go +++ b/proxmox/config_qemu.go @@ -1006,25 +1006,33 @@ var ( func NewConfigQemuFromApi(vmr *VmRef, client *Client) (config *ConfigQemu, err error) { var vmConfig map[string]interface{} + var vmInfo map[string]interface{} for ii := 0; ii < 3; ii++ { vmConfig, err = client.GetVmConfig(vmr) if err != nil { log.Fatal(err) return nil, err } + // TODO: this is a workaround for the issue that GetVmConfig will not always return the guest info + vmInfo, err = client.GetVmInfo(vmr) + if err != nil { + return nil, err + } // this can happen: // {"data":{"lock":"clone","digest":"eb54fb9d9f120ba0c3bdf694f73b10002c375c38","description":" qmclone temporary file\n"}}) - if vmConfig["lock"] == nil { + if vmInfo["lock"] == nil { break } else { time.Sleep(8 * time.Second) } } - if vmConfig["lock"] != nil { + if vmInfo["lock"] != nil { return nil, fmt.Errorf("vm locked, could not obtain config") } - + if v, isSet := vmInfo["pool"]; isSet { // TODO: this is a workaround for the issue that GetVmConfig will not always return the guest info + vmr.pool = v.(string) + } config, err = ConfigQemu{}.mapToStruct(vmr, vmConfig) if err != nil { return @@ -1033,7 +1041,7 @@ func NewConfigQemuFromApi(vmr *VmRef, client *Client) (config *ConfigQemu, err e config.defaults() // HAstate is return by the api for a vm resource type but not the HAgroup - err = client.ReadVMHA(vmr) + err = client.ReadVMHA(vmr) // TODO: can be optimized, uses same API call as GetVmConfig and GetVmInfo if err == nil { config.HaState = vmr.HaState() config.HaGroup = vmr.HaGroup() From 56e878ac6ac23544d87782499b6481021a446152 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 25 May 2024 13:08:45 +0200 Subject: [PATCH 18/22] feat: ListPoolsWithComments() --- proxmox/client.go | 2 +- proxmox/config_pool.go | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/proxmox/client.go b/proxmox/client.go index 5353ffba..0f51368e 100644 --- a/proxmox/client.go +++ b/proxmox/client.go @@ -1669,7 +1669,7 @@ func (c *Client) UpdateVMHA(vmr *VmRef, haState string, haGroup string) (exitSta return } -// TODO: implement replacement +// Deprecated: use ListPoolsWithComments() instead func (c *Client) GetPoolList() (pools map[string]interface{}, err error) { return c.GetItemList("/pools") } diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index 836f4306..fd405049 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -18,6 +18,23 @@ func ListPools(c *Client) ([]PoolName, error) { return pools, nil } +func ListPoolsWithComments(c *Client) (map[PoolName]string, error) { + raw, err := listPools(c) + if err != nil { + return nil, err + } + pools := make(map[PoolName]string, len(raw)) + for _, e := range raw { + pool := e.(map[string]interface{}) + var comment string + if v, isSet := pool["comment"]; isSet { + comment = v.(string) + } + pools[PoolName(pool["poolid"].(string))] = comment + } + return pools, nil +} + func listPools(c *Client) ([]interface{}, error) { return c.GetItemListInterfaceArray("/pools") } From 8c5e29f9829c5fa4dc0d611d6b771ac4ee44a41e Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 25 May 2024 14:39:15 +0200 Subject: [PATCH 19/22] test: panic tests --- proxmox/config_pool_test.go | 132 ++++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/proxmox/config_pool_test.go b/proxmox/config_pool_test.go index ba0e1282..b556a5f2 100644 --- a/proxmox/config_pool_test.go +++ b/proxmox/config_pool_test.go @@ -9,6 +9,18 @@ import ( "github.com/stretchr/testify/require" ) +func Test_ListPools(t *testing.T) { + var err error + require.NotPanics(t, func() { _, err = ListPools(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_ListPoolsWithComments(t *testing.T) { + var err error + require.NotPanics(t, func() { _, err = ListPoolsWithComments(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + func Test_ConfigPool_mapToApi(t *testing.T) { type testInput struct { new ConfigPool @@ -117,6 +129,54 @@ func Test_ConfigPool_mapToSDK(t *testing.T) { } } +func Test_ConfigPool_Create(t *testing.T) { + var err error + require.NotPanics(t, func() { err = ConfigPool{Name: "test"}.Create(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_ConfigPool_Create_Unsafe(t *testing.T) { + var err error + require.NotPanics(t, func() { err = ConfigPool{Name: "test"}.Create_Unsafe(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_ConfigPool_Delete(t *testing.T) { + var err error + require.NotPanics(t, func() { err = ConfigPool{Name: "test"}.Delete(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_ConfigPool_Exists(t *testing.T) { + var err error + require.NotPanics(t, func() { _, err = ConfigPool{Name: "test"}.Exists(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_ConfigPool_Set(t *testing.T) { + var err error + require.NotPanics(t, func() { err = ConfigPool{Name: "test"}.Set(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_ConfigPool_Set_Unsafe(t *testing.T) { + var err error + require.NotPanics(t, func() { err = ConfigPool{Name: "test"}.Set_Unsafe(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_ConfigPool_Update(t *testing.T) { + var err error + require.NotPanics(t, func() { err = ConfigPool{Name: "test"}.Update(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_ConfigPool_Update_Unsafe(t *testing.T) { + var err error + require.NotPanics(t, func() { err = ConfigPool{Name: "test"}.Update_Unsafe(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + func Test_ConfigPool_Validate(t *testing.T) { tests := []struct { name string @@ -142,6 +202,54 @@ func Test_ConfigPool_Validate(t *testing.T) { } } +func Test_PoolName_AddGuests(t *testing.T) { + var err error + require.NotPanics(t, func() { err = PoolName("test").AddGuests(nil, nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_PoolName_AddGuests_Unsafe(t *testing.T) { + var err error + require.NotPanics(t, func() { err = PoolName("test").AddGuests_Unsafe(nil, nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_PoolName_Delete(t *testing.T) { + var err error + require.NotPanics(t, func() { err = PoolName("test").Delete(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_PoolName_Delete_Unsafe(t *testing.T) { + var err error + require.NotPanics(t, func() { err = PoolName("test").Delete_Unsafe(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_PoolName_Exists(t *testing.T) { + var err error + require.NotPanics(t, func() { _, err = PoolName("test").Exists(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_PoolName_Exists_Unsafe(t *testing.T) { + var err error + require.NotPanics(t, func() { _, err = PoolName("test").Exists_Unsafe(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_PoolName_Get(t *testing.T) { + var err error + require.NotPanics(t, func() { _, err = PoolName("test").Get(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_PoolName_Get_Unsafe(t *testing.T) { + var err error + require.NotPanics(t, func() { _, err = PoolName("test").Get_Unsafe(nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + func Test_PoolName_guestsToRemoveFromPools(t *testing.T) { type testInput struct { guests []GuestResource @@ -191,6 +299,30 @@ func Test_PoolName_guestsToRemoveFromPools(t *testing.T) { } } +func Test_PoolName_RemoveGuests(t *testing.T) { + var err error + require.NotPanics(t, func() { err = PoolName("test").RemoveGuests(nil, nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_PoolName_RemoveGuests_Unsafe(t *testing.T) { + var err error + require.NotPanics(t, func() { err = PoolName("test").RemoveGuests_Unsafe(nil, nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_PoolName_SetGuests(t *testing.T) { + var err error + require.NotPanics(t, func() { err = PoolName("test").SetGuests(nil, nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + +func Test_PoolName_SetGuests_Unsafe(t *testing.T) { + var err error + require.NotPanics(t, func() { err = PoolName("test").SetGuests_Unsafe(nil, nil) }) + require.Equal(t, errors.New(Client_Error_Nil), err) +} + func Test_PoolName_mapToString(t *testing.T) { tests := []struct { name string From 16957a37908d00235211c28849d92df46077c3b7 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 25 May 2024 14:39:58 +0200 Subject: [PATCH 20/22] fix: panics & oddities --- proxmox/client.go | 3 ++ proxmox/config_guest.go | 4 +-- proxmox/config_pool.go | 61 +++++++++++++++++++++-------------------- 3 files changed, 36 insertions(+), 32 deletions(-) diff --git a/proxmox/client.go b/proxmox/client.go index 0f51368e..f210bdca 100644 --- a/proxmox/client.go +++ b/proxmox/client.go @@ -141,6 +141,9 @@ func (c *Client) Login(username string, password string, otp string) (err error) // Updates the client's cached version information and returns it. func (c *Client) GetVersion() (version Version, err error) { + if c == nil { + return Version{}, errors.New(Client_Error_Nil) + } params, err := c.GetItemConfigMapStringInterface("/version", "version", "data") version = version.mapToSDK(params) cachedVersion := Version{ // clones the struct diff --git a/proxmox/config_guest.go b/proxmox/config_guest.go index 0ff62c03..16c39c5c 100644 --- a/proxmox/config_guest.go +++ b/proxmox/config_guest.go @@ -181,7 +181,7 @@ func GuestReboot(vmr *VmRef, client *Client) (err error) { func guestSetPool_Unsafe(c *Client, guestID uint, newPool PoolName, currentPool *PoolName, version Version) (err error) { if newPool == "" { if *currentPool != "" { // leave pool - if err = (*currentPool).RemoveGuests_Unsafe(c, []uint{guestID}, version); err != nil { + if err = (*currentPool).removeGuests_Unsafe(c, []uint{guestID}, version); err != nil { return } } @@ -196,7 +196,7 @@ func guestSetPool_Unsafe(c *Client, guestID uint, newPool PoolName, currentPool } } else if newPool != *currentPool { // change pool if version.Smaller(Version{8, 0, 0}) { - if err = (*currentPool).RemoveGuests_Unsafe(c, []uint{guestID}, version); err != nil { + if err = (*currentPool).removeGuests_Unsafe(c, []uint{guestID}, version); err != nil { return } if err = newPool.addGuests_UnsafeV7(c, []uint{guestID}); err != nil { diff --git a/proxmox/config_pool.go b/proxmox/config_pool.go index fd405049..ae0ff51c 100644 --- a/proxmox/config_pool.go +++ b/proxmox/config_pool.go @@ -36,6 +36,9 @@ func ListPoolsWithComments(c *Client) (map[PoolName]string, error) { } func listPools(c *Client) ([]interface{}, error) { + if c == nil { + return nil, errors.New(Client_Error_Nil) + } return c.GetItemListInterfaceArray("/pools") } @@ -123,9 +126,6 @@ func (config ConfigPool) Exists(c *Client) (bool, error) { } func (config ConfigPool) Set(c *Client) error { - if c == nil { - return errors.New(Client_Error_Nil) - } if err := config.Validate(); err != nil { return err } @@ -139,11 +139,7 @@ func (config ConfigPool) Set_Unsafe(c *Client) error { return err } if exists { - current, err := config.Name.Get_Unsafe(c) - if err != nil { - return err - } - return config.Update_Unsafe(c, current) + return config.Update_Unsafe(c) } return config.Create_Unsafe(c) } @@ -161,19 +157,19 @@ func (config ConfigPool) Update(c *Client) error { return errors.New(PoolName_Error_NotExists) } // TODO check permissions - current, err := config.Name.Get_Unsafe(c) - if err != nil { - return err - } - return config.Update_Unsafe(c, current) + return config.Update_Unsafe(c) } // Update_Unsafe updates a pool without validating the input -func (config ConfigPool) Update_Unsafe(c *Client, current *ConfigPool) error { +func (config ConfigPool) Update_Unsafe(c *Client) error { version, err := c.GetVersion() if err != nil { return err } + current, err := config.Name.Get_Unsafe(c) + if err != nil { + return err + } if params := config.mapToApi(current); len(params) > 0 { if err = config.Name.put(c, params, version); err != nil { return err @@ -221,7 +217,7 @@ func (config PoolName) addGuests_Unsafe(c *Client, guestIDs []uint, currentGuest return err } for i, e := range PoolName("").guestsToRemoveFromPools(guests, guestsToAdd) { - if err = i.RemoveGuests_Unsafe(c, e, version); err != nil { + if err = i.removeGuests_Unsafe(c, e, version); err != nil { return err } } @@ -240,9 +236,6 @@ func (pool PoolName) addGuests_UnsafeV8(c *Client, guestIDs []uint) error { } func (config PoolName) AddGuests(c *Client, guestIDs []uint) error { - if c == nil { - return errors.New(Client_Error_Nil) - } if err := config.Validate(); err != nil { return err } @@ -270,9 +263,6 @@ func (pool PoolName) AddGuests_Unsafe(c *Client, guestIDs []uint) error { } func (config PoolName) Delete(c *Client) error { - if c == nil { - return errors.New(Client_Error_Nil) - } if err := config.Validate(); err != nil { return err } @@ -288,6 +278,9 @@ func (config PoolName) Delete(c *Client) error { } func (config PoolName) Delete_Unsafe(c *Client) error { + if c == nil { + return errors.New(Client_Error_Nil) + } return c.Delete("/pools/" + string(config)) } @@ -311,9 +304,6 @@ func (config PoolName) Exists_Unsafe(c *Client) (bool, error) { } func (pool PoolName) Get(c *Client) (*ConfigPool, error) { - if c == nil { - return nil, errors.New(Client_Error_Nil) - } if err := pool.Validate(); err != nil { return nil, err } @@ -322,6 +312,9 @@ func (pool PoolName) Get(c *Client) (*ConfigPool, error) { } func (pool PoolName) Get_Unsafe(c *Client) (*ConfigPool, error) { + if c == nil { + return nil, errors.New(Client_Error_Nil) + } params, err := c.GetItemConfigMapStringInterface("/pools/"+string(pool), "pool", "CONFIG") if err != nil { return nil, err @@ -378,27 +371,35 @@ func (pool PoolName) putV8(c *Client, params map[string]interface{}) error { return c.Put(params, "/pools?poolid="+string(pool)) } -func (config PoolName) RemoveGuests(c *Client, guestID []uint) error { +func (pool PoolName) RemoveGuests(c *Client, guestID []uint) error { version, err := c.GetVersion() if err != nil { return err } - if err := config.Validate(); err != nil { + if err := pool.Validate(); err != nil { return err } if len(guestID) == 0 { return errors.New(PoolName_Error_NoGuestsSpecified) } // TODO: permission check - if exists, err := config.Exists_Unsafe(c); err != nil { + if exists, err := pool.Exists_Unsafe(c); err != nil { return err } else if !exists { return errors.New(PoolName_Error_NotExists) } - return config.RemoveGuests_Unsafe(c, guestID, version) + return pool.removeGuests_Unsafe(c, guestID, version) +} + +func (pool PoolName) RemoveGuests_Unsafe(c *Client, guestID []uint) error { + version, err := c.GetVersion() + if err != nil { + return err + } + return pool.removeGuests_Unsafe(c, guestID, version) } -func (pool PoolName) RemoveGuests_Unsafe(c *Client, guestID []uint, version Version) error { +func (pool PoolName) removeGuests_Unsafe(c *Client, guestID []uint, version Version) error { return pool.put(c, map[string]interface{}{ "vms": PoolName("").mapToString(guestID), "delete": "1"}, @@ -435,7 +436,7 @@ func (pool PoolName) SetGuests_Unsafe(c *Client, guestID []uint) error { func (pool PoolName) setGuests_Unsafe(c *Client, guestIDs []uint, currentGuests *[]uint, version Version) error { if currentGuests != nil && len(*currentGuests) > 0 { - if err := pool.RemoveGuests_Unsafe(c, subtractArray(*currentGuests, guestIDs), version); err != nil { + if err := pool.removeGuests_Unsafe(c, subtractArray(*currentGuests, guestIDs), version); err != nil { return err } } From 9fb856387d04189c91aa1f2bf2d2e99db009979f Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 25 May 2024 15:08:04 +0200 Subject: [PATCH 21/22] fix: using deprecated functions --- cli/command/delete/delete.go | 2 +- cli/command/list/list-pools.go | 12 ++++++++++-- main.go | 2 +- test/api/Pool/pool_create_destroy_test.go | 5 +++-- test/api/Pool/pool_list_test.go | 3 ++- 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/cli/command/delete/delete.go b/cli/command/delete/delete.go index ec7e8860..b9d75b8a 100644 --- a/cli/command/delete/delete.go +++ b/cli/command/delete/delete.go @@ -29,7 +29,7 @@ func deleteID(args []string, IDtype string) (err error) { case "MetricServer": err = c.DeleteMetricServer(id) case "Pool": - err = c.DeletePool(id) + err = proxmox.PoolName(id).Delete(c) case "Storage": err = c.DeleteStorage(id) case "User": diff --git a/cli/command/list/list-pools.go b/cli/command/list/list-pools.go index b5d07f4b..9f2510e9 100644 --- a/cli/command/list/list-pools.go +++ b/cli/command/list/list-pools.go @@ -1,6 +1,8 @@ package list import ( + "github.com/Telmate/proxmox-api-go/cli" + "github.com/Telmate/proxmox-api-go/proxmox" "github.com/spf13/cobra" ) @@ -8,8 +10,14 @@ var list_poolsCmd = &cobra.Command{ Use: "pools", Short: "Prints a list of Pools in raw json format", Args: cobra.NoArgs, - Run: func(cmd *cobra.Command, args []string) { - listRaw("Pools") + RunE: func(cmd *cobra.Command, args []string) (err error) { + c := cli.NewClient() + pools, err := proxmox.ListPools(c) + if err != nil { + return + } + cli.PrintFormattedJson(listCmd.OutOrStdout(), pools) + return }, } diff --git a/main.go b/main.go index 3fcda0af..c1ac23cd 100644 --- a/main.go +++ b/main.go @@ -450,7 +450,7 @@ func main() { //Pool case "getPoolList": - pools, err := c.GetPoolList() + pools, err := proxmox.ListPoolsWithComments(c) if err != nil { log.Printf("Error listing pools %+v\n", err) os.Exit(1) diff --git a/test/api/Pool/pool_create_destroy_test.go b/test/api/Pool/pool_create_destroy_test.go index 2c90158c..1fcce57f 100644 --- a/test/api/Pool/pool_create_destroy_test.go +++ b/test/api/Pool/pool_create_destroy_test.go @@ -3,6 +3,7 @@ package api_test import ( "testing" + "github.com/Telmate/proxmox-api-go/proxmox" api_test "github.com/Telmate/proxmox-api-go/test/api" "github.com/stretchr/testify/require" ) @@ -10,7 +11,7 @@ import ( func Test_Pool_Create(t *testing.T) { Test := api_test.Test{} _ = Test.CreateTest() - Test.GetClient().CreatePool("test-pool", "Test pool") + proxmox.ConfigPool{Name: "test-pool"}.Create(Test.GetClient()) } func Test_Pool_Is_Created(t *testing.T) { @@ -23,7 +24,7 @@ func Test_Pool_Is_Created(t *testing.T) { func Test_Pool_Delete(t *testing.T) { Test := api_test.Test{} _ = Test.CreateTest() - Test.GetClient().DeletePool("test-pool") + proxmox.ConfigPool{Name: "test-pool"}.Create(Test.GetClient()) } func Test_Pool_Is_Deleted(t *testing.T) { diff --git a/test/api/Pool/pool_list_test.go b/test/api/Pool/pool_list_test.go index d14656e0..8834df50 100644 --- a/test/api/Pool/pool_list_test.go +++ b/test/api/Pool/pool_list_test.go @@ -3,6 +3,7 @@ package api_test import ( "testing" + "github.com/Telmate/proxmox-api-go/proxmox" api_test "github.com/Telmate/proxmox-api-go/test/api" "github.com/stretchr/testify/require" ) @@ -10,7 +11,7 @@ import ( func Test_Pools_List(t *testing.T) { Test := api_test.Test{} _ = Test.CreateTest() - pools, err := Test.GetClient().GetPoolList() + pools, err := proxmox.ListPools(Test.GetClient()) require.NoError(t, err) require.Equal(t, 1, len(pools)) } From 2b79dfcf1e3ed104203a165e2a3b51e4d4c190ab Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 25 May 2024 15:10:21 +0200 Subject: [PATCH 22/22] refactor: remove unused code --- cli/command/list/list.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cli/command/list/list.go b/cli/command/list/list.go index c57ae9f7..b9b00130 100644 --- a/cli/command/list/list.go +++ b/cli/command/list/list.go @@ -27,8 +27,6 @@ func listRaw(IDtype string) { list, err = c.GetMetricsServerList() case "Nodes": list, err = c.GetNodeList() - case "Pools": - list, err = c.GetPoolList() case "Storages": list, err = c.GetStorageList() }