Add force option to lxd stop request on error
#2693
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2693 +/- ##
=======================================
Coverage 86.51% 86.51%
=======================================
Files 217 217
Lines 10899 10902 +3
=======================================
+ Hits 9429 9432 +3
Misses 1470 1470
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -378,9 +386,9 @@ const QUrl mp::LXDVirtualMachine::network_leases_url() | |||
| return base_url.toString() + "/networks/" + bridge_name + "/leases"; | |||
| } | |||
|
|
|||
| void mp::LXDVirtualMachine::request_state(const QString& new_state) | |||
| void mp::LXDVirtualMachine::request_state(const QString& new_state, const bool force) | |||
There was a problem hiding this comment.
i have mixed feelings about this. on one hand, it's simple and it gets the job done and it doesn't interfere with the other uses of this function. but on the other hand, the other possible new_states do not take a force option, which would make possible future uses of this function a little confusing, and it would also make it more awkward if we ever need to pass different flags to different actions.
one possible alternative would be to pass a const QJsonObject& extra_fields = {} instead of a const bool force and merge the two json objects, then call the method as request_state("stop", {{"force", true}}). this would allow for clearer and more extensive customization, although it may be too much for such simple use cases.
so, this can stay as is if you consider the drawbacks to be negligible. what do you think?
There was a problem hiding this comment.
There aren't a large number of different arguments that could be passed to lxd but using an args parameter would make it look cleaner and enable future changes on our end without having to change this function.
https://github.com/lxc/lxd/blob/407205d4bd5d59e4a85d846988011a6fc09a9e72/doc/rest-api.yaml#L2072
There was a problem hiding this comment.
Now that I look at it a little better, I don't think this approach will work nicely with #1909. The LXDVirtualMachine class inherits its stop function from the abstract VirtualMachine class, from which the other backends inherit as well. The virtual stop function should be able to take a bool force flag so that any backend could do a forced stop. But in this current implementation, the stop request is automatically forced if the normal one fails. I think this should be handled at a higher level, since maybe we don't want to shut down an instance by force no matter what, only in some cases, like when deleting and purging it.
If
lxdfails to stop an instance gracefully, retry the request with theforceoption applied.Fixes #2492