feat(vm): add support for migration when the node name is modified #501
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Added a `migrate` VM flag which changes the provider's behaviour
when the VM's `node_name` is updated. If `true`, the VM will be
migrated to the specified node instead of being re-created.
* Added a `timeout_migrate` setting to control the timeout for VM
migration.
* Fixed a bug in the API's migration data structure that prevented
the online migration flag to be set.
|
That lint failure (which also occurred on my workstation, but I wanted to be sure) doesn't have anything to do with the code I added, yet it doesn't happen in the main branch. Weirder yet, it doesn't look like the section of code it happens on has been touched recently. Not sure if I should fix it in this PR, since doing so kind of mixes concerns. On the other hand, well, the linter fails. Edit: applying the patch below would solve the problem: diff --git a/proxmoxtf/resource/vm.go b/proxmoxtf/resource/vm.go
index 6a2a71a..893f666 100644
--- a/proxmoxtf/resource/vm.go
+++ b/proxmoxtf/resource/vm.go
@@ -3908,7 +3908,6 @@ func vmReadCustom(
currentPCIList := d.Get(mkResourceVirtualEnvironmentVMHostPCI).([]interface{})
pciMap := map[string]interface{}{}
- var orderedPCIList []interface{}
pciDevices := getPCIInfo(vmConfig, d)
for pi, pp := range pciDevices {
@@ -3960,8 +3959,9 @@ func vmReadCustom(
}
sort.Strings(keyList)
- for _, k := range keyList {
- orderedPCIList = append(orderedPCIList, pciMap[k])
+ orderedPCIList := make([]interface{}, len(keyList))
+ for i, k := range keyList {
+ orderedPCIList[i] = pciMap[k]
}
if len(clone) > 0 { |
3 tasks
tseeker
added a commit
to tseeker/terraform-provider-proxmox
that referenced
this pull request
Aug 18, 2023
* This commit fixes a linter error that somehow doesn't manifest
unless some other, unrelated changes trigger it (see bpg#501 and
bpg#505).
* In addition it fixes a similar issue that had so far gone undetected
by the linter.
* Refactored the code in question into a function, since it was mostly
duplicated.
* Simplified a pair of conditionals that had the same code in both
branches.
bpg
pushed a commit
that referenced
this pull request
Aug 19, 2023
fix: linter error in ambush
* This commit fixes a linter error that somehow doesn't manifest
unless some other, unrelated changes trigger it (see #501 and
#505).
* In addition it fixes a similar issue that had so far gone undetected
by the linter.
* Refactored the code in question into a function, since it was mostly
duplicated.
* Simplified a pair of conditionals that had the same code in both
branches.
bpg
approved these changes
Aug 19, 2023
proxmoxtf/resource/vm.go
Outdated
| @@ -1367,6 +1376,12 @@ func VM() *schema.Resource { | |||
| Optional: true, | |||
| Default: dvResourceVirtualEnvironmentVMTimeoutMoveDisk, | |||
| }, | |||
| mkResourceVirtualEnvironmentVMTimeoutMigrate: { | |||
| Type: schema.TypeInt, | |||
| Description: "MoveDisk timeout", | |||
There was a problem hiding this comment.
Looks like the descrition was not updated :)
ghost
mentioned this pull request
bpg
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added a
migrateVM flag which changes the provider's behaviour when the VM'snode_nameis updated. Iftrue, the VM will be migrated to the specified node instead of being re-created.Added a
timeout_migratesetting to control the timeout for VM migration.Fixed a bug in the API's migration data structure that prevented the online migration flag to be set.
Contributor's Note
Please mark the following items with an [x] if they apply to your PR.
Leave the [ ] if they are not applicable, or if you have not completed the item.
/docsfor any user-facing features or additions./examplesfor any new or updated resources / data sources.make examplesto verify that the change works as expected.Community Note
Closes #499