Allow to recreate VMs if in DONE state #563
Conversation
|
Will update changelog and docs if this gets approved |
|
|
||
| // waitForVMState waits until the VM with vmId has the desiredState. | ||
| // returns an error if timeout is reached. | ||
| func waitForVMState(vmId int, desiredState vm.State, timeout time.Duration) error { |
There was a problem hiding this comment.
VM already has some VM state checking methods, should we share/reuse some code or should we consider that it's ok to have separated test code ?
There was a problem hiding this comment.
Ah, I probably missed that, if there is something to be reuse, to wait for the state then we definitely should use it, can you point me towards it?
There was a problem hiding this comment.
Maybe a bit complex, but there's some existing code here:
https://github.com/OpenNebula/terraform-provider-opennebula/blob/master/opennebula/helpers_vm_state.go#L138
| d.Set("gname", vmInfo.GName) | ||
| d.Set("state", vmInfo.StateRaw) | ||
| d.Set("lcmstate", vmInfo.LCMStateRaw) | ||
| if vm.State(vmInfo.StateRaw) == vm.Done { |
There was a problem hiding this comment.
Is it the proper way to do ?
There's a -replace option to terraform
And if we consider it's ok, do we want this behavior everytime, for everyone or should it be conditioned by an other attribute with a name ?
There was a problem hiding this comment.
As far as I know, unsetting the "ID" parameter is the way to tell terraform that the previously existing resource no longer exists (see an example on AWS's provider here.
A VM in done state can be considered as non-existing anymore. As it is expected that terraform recreates the resource when they do not exist anymore, this behaviour should be default.
An example for that expectation is drift checks, if you create a VM, someone deletes it from the UI, the expectation is that terraform will tell you there is a drift.
The thing to consider is the non-backward compatible behaviour this brings to the vm resource, this might be worth of a major version release.
Maybe this could be behind a recreate_on_delete boolean on the resource on an initial minor update, with a deprecation notice for the next major update.
This also affects the vrouter resource.
There was a problem hiding this comment.
@frousselet what do you think on this ?
Should we consider this behavior to be protected by a recreate_on_delete attribute ?
|
This pull request is stale because it has been open for 30 days with no activity and it is not in a milestone. Remove 'status: stale' label or comment, or this will be closed in 5 days. |
|
nope, not stale... |
|
@Thiryn your work on this is much appreciated. Since it's a bug fix, we don't anticipate it breaking backwards compatibility. We'll go ahead and proceed with the merge |
Community Note
Description
Detect
donestate for VMs and reconcile them via recreation.Also renames
vmvariable intovmInfoto prevent collision with thegithub.com/OpenNebula/one/src/oca/go/src/goca/schemas/vmpackage import.References
Fixes #562
New or Affected Resource(s)
opennebula_virtual_router_instanceopennebula_virtual_machineChecklist
Referencesgo fmt)