-
-
Notifications
You must be signed in to change notification settings - Fork 138
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.
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
fix(vm): fixed startup / shutdown behaviour on HA clusters #508
Conversation
Due to how a Proxmox cluster reacts to a VM shutdown command when running in HA mode, the VM might still be running when the shutdown API calls returns. This commit adds a loop that actively waits for the VM's status to change to "stopped" (while also accounting for the shutdown timeout) after the call's return.
…cluster This commit forces the plugin to wait for a VM to actually run after requesting it to be started. This avoids problems with Proxmox's High Availability mode, where a start request may not be immediately honoured by the cluster.
@all-contributors please add @tseeker for bug, tests |
I've put up a pull request to add @tseeker! 🎉 |
proxmoxtf/resource/vm.go
Outdated
// Shutdown the VM. | ||
// Wait until a VM reaches the desired state. An error is returned if the VM doesn't reach the | ||
// desired state within the given timeout. | ||
func vmWaitState(ctx context.Context, vmAPI *vms.Client, state string, timeout int) diag.Diagnostics { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably reuse this one?
func (c *Client) WaitForVMState(ctx context.Context, state string, timeout int, delay int) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! I hadn't seen it. Thanks for catching that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use the existing function as per my comment above.
LGTM! 🚀
When a Proxmox cluster receives a shutdown or startup command for a VM that is declared as a HA resource, it doesn't honour the request immediately. Instead the API call returns and the state changes later, at the HA manager's discretion. Because of that, the plugin could end up trying to effect inappropriate changes on a VM that wasn't in the expected state.
This PR adds a wait loop after both startup and shutdown commands in order to ensure that the state is what we expect it to be.
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.
/docs
for any user-facing features or additions./examples
for any new or updated resources / data sources.make examples
to verify that the change works as expected.Community Note
Closes #507