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

[Feature] Control xenorchestra_vm power state #263

Closed
ravager-dk opened this issue Sep 29, 2023 · 6 comments
Closed

[Feature] Control xenorchestra_vm power state #263

ravager-dk opened this issue Sep 29, 2023 · 6 comments

Comments

@ravager-dk
Copy link
Contributor

It would be great to have the option to create a vm in a powered off state. The use case would be for changes to any unexposed options prior to starting the VM for first boot.
Likewise an option to set an existing imported vms powerstate to halted or running, would be beneficial for inter-dependencies between services.

@ddelnano
Copy link
Collaborator

ddelnano commented Oct 24, 2023

@ravager-dk thanks for opening the PR for boot after create and suggesting this idea for the VM resource.

I actually have a work in progress change that implements the creation part of this (commit) and includes a test that exercises creation and updates to the power_state argument.

I see adding support for power_state as a more complete solution than boot after create. The former would work for creation and updates, so I see the boot after create option as a temporary solution. In addition to that since it requires adding an argument to the VM resource, that means that a later breaking change would need to be introduced.

I'd prefer to go the full way and provide complete control over the power_state setting. I really appreciate you taking the time for opening #276, but I think it would be best if we pursue that "full fledged support" I described above.

If you are interested in picking up my current work in progress work, I'm happy to assist. Otherwise, I should be able to get that into a reviewable state sometime next week

@ravager-dk
Copy link
Contributor Author

@ddelnano My initial thought was that this would help the use case for xenstoredata #261.
The flow for this would be

  • create VM powered off
  • set xenstoredata
  • power on VM

Exposing the bootAfterCreate param was such an easy one, since it obviously needs handling in any case. Reading your coments I totally get what you are saying. It would be better if a power state argument handled both bootAfterCreate and power on/off.

I have yet to look at your branch, but I would suggest setting bootAfterCreate to false in vmcreate and then after creation if power should be on, call vm.start. This would make it easier to implement xenstoredata or any other setting that might need to be run after creation but before startup.

I am already doing some initial work on xenstoredata #261. I have it working for resource creation, so I just need to get updates supported. I should probably hold off on anything more until power state is implemented.

Writing and running tests in Terraform provider development is very much unknown to me. I will need to look more into how to write and run them and what sort of coverage Terraform Registry requires.

@ddelnano
Copy link
Collaborator

#278 could be merged this week. I wasn't sure if there would be complexity in the VM update path, but it was a small change to the existing code.

This would make it easier to implement xenstoredata or any other setting that might need to be run after creation but before startup.

Sorry for missing your earlier comments, but it does seem like we will need to perform creation across vm.create and vm.set calls.

I will need to look more into how to write and run them and what sort of coverage Terraform Registry requires.

Terraform registry doesn't require any amount of test coverage. Unless it would require a significant investment to write tests for a given feature, I just believe it's a necessary for this project. The VM resource is complex and verifying if a given change breaks any of its functionality broke done very early in the project.

@ddelnano
Copy link
Collaborator

ddelnano commented Dec 4, 2023

@ravager-dk #281 implements separating the VM creation into two steps (create followed by start). It's not possible to do that for all VM create requests because destroyCloudConfigVdiAfterBoot relies on boot after create.

I need to revamp #278 to include that change. I want to see that through before merging #281, but hoping that should happen soon.

@ddelnano
Copy link
Collaborator

@ravager-dk this is complete with #281 and #278 merged. This will be available in the next terraform provider release (v0.26.0), which should be released today.

@ddelnano
Copy link
Collaborator

v0.26.0 is available now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants