Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug: setting CloudInit.UpgradePackages gives error on older PVE #345

Merged
merged 2 commits into from
Jun 23, 2024
Merged

Bug: setting CloudInit.UpgradePackages gives error on older PVE #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82c6d32
refactor: rename function
Tinyblargon Jun 23, 2024
958dd63
fix: ciupgrade set in api on v7
Tinyblargon Jun 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions proxmox/config_qemu.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func (config *ConfigQemu) defaults() {
}
}

func (config ConfigQemu) mapToApiValues(currentConfig ConfigQemu) (rebootRequired bool, params map[string]interface{}, err error) {
func (config ConfigQemu) mapToAPI(currentConfig ConfigQemu, version Version) (rebootRequired bool, params map[string]interface{}, err error) {
// TODO check if cloudInit settings changed, they require a reboot to take effect.
var itemsToDelete string

Expand Down Expand Up @@ -280,7 +280,7 @@ func (config ConfigQemu) mapToApiValues(currentConfig ConfigQemu) (rebootRequire
}

if config.CloudInit != nil {
itemsToDelete += config.CloudInit.mapToAPI(currentConfig.CloudInit, params)
itemsToDelete += config.CloudInit.mapToAPI(currentConfig.CloudInit, params, version)
}

// Create EFI disk
Expand Down Expand Up @@ -666,18 +666,16 @@ func (config *ConfigQemu) setVmr(vmr *VmRef) (err error) {

// currentConfig will be mutated
func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeeded bool, vmr *VmRef, client *Client) (rebootRequired bool, err error) {
err = newConfig.setVmr(vmr)
if err != nil {
return
}
if err = newConfig.Validate(currentConfig); err != nil {
if err = newConfig.setVmr(vmr); err != nil {
return
}

var version Version
if version, err = client.Version(); err != nil {
return
}
if err = newConfig.Validate(currentConfig, version); err != nil {
return
}

var params map[string]interface{}
var exitStatus string
Expand Down Expand Up @@ -756,7 +754,7 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede
vmr.SetNode(newConfig.Node)
}

rebootRequired, params, err = newConfig.mapToApiValues(*currentConfig)
rebootRequired, params, err = newConfig.mapToAPI(*currentConfig, version)
if err != nil {
return
}
Expand Down Expand Up @@ -801,7 +799,7 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede
}
}
} else { // Create
_, params, err = newConfig.mapToApiValues(ConfigQemu{})
_, params, err = newConfig.mapToAPI(ConfigQemu{}, version)
if err != nil {
return
}
Expand All @@ -826,7 +824,7 @@ func (newConfig ConfigQemu) setAdvanced(currentConfig *ConfigQemu, rebootIfNeede
return
}

func (config ConfigQemu) Validate(current *ConfigQemu) (err error) {
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 config.Agent != nil {
Expand All @@ -835,7 +833,7 @@ func (config ConfigQemu) Validate(current *ConfigQemu) (err error) {
}
}
if config.CloudInit != nil {
if err = config.CloudInit.Validate(); err != nil {
if err = config.CloudInit.Validate(version); err != nil {
return
}
}
Expand Down
11 changes: 8 additions & 3 deletions proxmox/config_qemu_cloudinit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ type CloudInit struct {
Username *string `json:"username,omitempty"` // TODO custom type
}

func (config CloudInit) mapToAPI(current *CloudInit, params map[string]interface{}) (delete string) {
const CloudInit_Error_UpgradePackagesPre8 = "upgradePackages is only available in version 8 and above"

func (config CloudInit) mapToAPI(current *CloudInit, params map[string]interface{}, version Version) (delete string) {
if current != nil { // Update
if config.Custom != nil {
params["cicustom"] = config.Custom.mapToAPI(current.Custom)
Expand Down Expand Up @@ -121,7 +123,7 @@ func (config CloudInit) mapToAPI(current *CloudInit, params map[string]interface
}
}
// Shared
if config.UpgradePackages != nil {
if config.UpgradePackages != nil && !version.Smaller(Version{Major: 8}) {
params["ciupgrade"] = Btoi(*config.UpgradePackages)
}
if config.UserPassword != nil {
Expand Down Expand Up @@ -190,12 +192,15 @@ func (CloudInit) mapToSDK(params map[string]interface{}) *CloudInit {
return nil
}

func (ci CloudInit) Validate() error {
func (ci CloudInit) Validate(version Version) error {
if ci.Custom != nil {
if err := ci.Custom.Validate(); err != nil {
return err
}
}
if ci.UpgradePackages != nil && *ci.UpgradePackages && version.Smaller(Version{Major: 8}) {
return errors.New(CloudInit_Error_UpgradePackagesPre8)
}
return ci.NetworkInterfaces.Validate()
}

Expand Down
19 changes: 15 additions & 4 deletions proxmox/config_qemu_cloudinit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,10 @@ func Test_sshKeyUrlEncode(t *testing.T) {

func Test_CloudInit_Validate(t *testing.T) {
tests := []struct {
name string
input CloudInit
output error
name string
input CloudInit
version Version
output error
}{
{name: `Valid CloudInit CloudInitCustom FilePath`,
input: CloudInit{Custom: &CloudInitCustom{
Expand Down Expand Up @@ -111,6 +112,16 @@ func Test_CloudInit_Validate(t *testing.T) {
IPv6: util.Pointer(CloudInitIPv6Config{
Gateway: util.Pointer(IPv6Address("")),
SLAAC: true})}}}},
{name: `Valid CloudInit UpgradePackages v7`,
version: Version{Major: 7, Minor: 255, Patch: 255},
input: CloudInit{UpgradePackages: util.Pointer(false)}},
{name: `Valid CloudInit UpgradePackages v8`,
version: Version{Major: 8},
input: CloudInit{UpgradePackages: util.Pointer(true)}},
{name: `Invalid errors.New(CloudInit_Error_UpgradePackagesPre8)`,
version: Version{Major: 7, Minor: 255, Patch: 255},
input: CloudInit{UpgradePackages: util.Pointer(true)},
output: errors.New(CloudInit_Error_UpgradePackagesPre8)},
{name: `Invalid errors.New(CloudInitSnippetPath_Error_InvalidCharacters)`,
input: CloudInit{Custom: &CloudInitCustom{User: &CloudInitSnippet{
FilePath: CloudInitSnippetPath(test_data_qemu.CloudInitSnippetPath_Character_Illegal()[0])}}},
Expand Down Expand Up @@ -191,7 +202,7 @@ func Test_CloudInit_Validate(t *testing.T) {
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
require.Equal(t, test.output, test.input.Validate())
require.Equal(t, test.output, test.input.Validate(test.version))
})
}
}
Expand Down
Loading
Loading