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.

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

Allow a VM to be in a state of 'migrating' #496

Merged

Conversation

mifrost
Copy link
Contributor

@mifrost mifrost commented Nov 28, 2018

When applying changes, we have regularly received the error Invalid power_state for instance <ID>: MIGRATING , as live migrations occur on instances, preventing any plan / apply operations.

This change adds "migrating" to the list of acceptable states for an instance to be in. See also #428

@ghost ghost added the size/XS label Nov 28, 2018
@jtopjian
Copy link
Contributor

Thanks for submitting this patch.

Can you explain in more detail why someone would want to make a change when an instance is in the middle of migrating? My initial impression is that this kind of error is actually a good thing.

@mifrost
Copy link
Contributor Author

mifrost commented Nov 28, 2018

@jtopjian No problem

In our case it's because terraform will try to refresh the state of all instances before every plan action. This means that even if there are no changes to make to the migrating instance, the whole action will still fail.

Even if we attempt to avoid the problem with a -target, we still hit it due to a shared dependency as we've used a count parameter.

@jtopjian
Copy link
Contributor

@mifrost OK, that makes sense.

So perhaps instead of having migrating as a valid input (line 356) how about we just let it be a valid status (such as how "error" is being used). Thoughts?

@theopenlab-ci
Copy link

theopenlab-ci bot commented Nov 28, 2018

Build succeeded.

@mifrost
Copy link
Contributor Author

mifrost commented Nov 29, 2018

@jtopjian very good point - I've updated the PR

@theopenlab-ci
Copy link

theopenlab-ci bot commented Nov 29, 2018

Build succeeded.

@jtopjian
Copy link
Contributor

@mifrost This looks good to me. Have you been able to validate this change in your environment? It's a little hard for me to reproduce a case where this would be triggered.

@mifrost
Copy link
Contributor Author

mifrost commented Nov 30, 2018

@jtopjian yes - I've used this branch to work around the issue we were seeing

Copy link
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thank you!

@jtopjian jtopjian merged commit 3832ab4 into terraform-provider-openstack:master Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants