Merge pull request #345 from Tinyblargon/#344

Bug: setting `CloudInit.UpgradePackages` gives error on older PVE

proxmox/config_qemu.go

@@ -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
 
@@ -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
@@ -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
@@ -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
 		}
@@ -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
 		}
@@ -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 {
@@ -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
 		}
 	}

proxmox/config_qemu_cloudinit.go

@@ -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)
@@ -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 {
@@ -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()
 }
 

proxmox/config_qemu_cloudinit_test.go

@@ -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{
@@ -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])}}},
@@ -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))
 		})
 	}
 }