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

Include accidental deletion & shutdown flags in the provider #342

Closed
OTonGitHub opened this issue Feb 18, 2025 · 11 comments
Closed

Include accidental deletion & shutdown flags in the provider #342

OTonGitHub opened this issue Feb 18, 2025 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@OTonGitHub
Copy link

Image

auto power on flag is available in the provider, but not the protection stuff. Afaik this is not XOA-UI specific, but
this setting also applied to the XOA-CLI right?

But upon checking the avaiable commands by sending a xo.getAllObjects command via a json-rpc over websocket
to the XO-Server, I can't find a command to do this either.

This leads me to believe this is a UI only thing? But no way its stored in the browser storage right, or just not covered somehow, I am probably wrong but I am gonna look at the code now and update my issue, but my original issue is still valid.

@olivierlambert
Copy link
Member

Hi,

Good question, let me ask internally.

@OTonGitHub OTonGitHub changed the title Include accidental deletion & power-off flags in the provider Include accidental deletion & shutdown flags in the provider Feb 18, 2025
@julien-f
Copy link
Member

These settings block deletion or shutdown at XAPI level by setting the blocked_operations entries.

It can be set via the JSON-RPC API:

xo-cli vm.set uuid=3cd5777b-1047-190c-930c-d365d45bdf0a blockedOperations=json:'{ "start": true, "start_on": true, "clean_reboot": false }'

List of operations that can be blocked: https://xapi-project.github.io/xen-api/classes/vm.html#enum_vm_operations

@nathanael-h
Copy link
Member

@iButcat told me he's looking on this issue.

@nathanael-h
Copy link
Member

Related #318

@OTonGitHub
Copy link
Author

OTonGitHub commented Feb 20, 2025

Just adding some extra context..

Documentation on the optional blocked_operations parameter in VM resource provider
I'm assuming these are the enums responsible for prompting confirmation before deletion/shutdown etc from XO.

Image

Terraform

Image

Error

Image

Relevant function from the API seems to be
VM.remove_from_blocked_operations","params":["* session id *","OpaqueRef:ef00c3de-f626-4470-b63c-49712ddc4367","destory"]}
Not sure why this is called every time, even when nothing is removed.

@OTonGitHub
Copy link
Author

I was able to provision the VM with blocked_operations = [] as en empty set or without it, and then update the state immediately after

Image

Confirmed the changes in the XO-UI, the UI was indeed updated accordingly, and it did work. Not sure why it's not possible to pass the flags during VM creation, perhaps some kind of race error.

I still haven't gone through the code, will try to run the template on my own using the xapi and check it out tomorrow, I think we can figure out whats happening after a couple of tests.

Sorry if I made this seem overcomplicated, I shared everything related to the issue that was available to me at the time.

@iButcat
Copy link
Member

iButcat commented Feb 20, 2025

@OTonGitHub Thank you, those informations are really valuable to help me resolve it, I'll keep you update on my side as well.

@OTonGitHub
Copy link
Author

OTonGitHub commented Feb 20, 2025

Just one last thing I'll note, this might be irrelevant but.

Image

Some of these options, for example, accidental shutdown, requires more than 1 flag to be set for it to be turned on.
And ofc that makes sense. In fact, accidental shutdown is enabled in the UI, only if all the operations, suspend, clean_reboot, force_shutdown.. etc, are set in the blocked_operations of the VM. And this logically makes sense and must be the way intended, I don't think that's an issue.

But, I feel like, blocked_operations could be elevated to a higher level function as well, just like done in the XO-UI.
And if adding very granular control like adding & removing the flags, the high level option could be disabled.
I think this can be done in the terraform provider because, Using token & credentials at the same time to authenticate with XO results in a Error: Conflicting configuration arguments from terraform, on the credentials. Just an example.

Or instead of adding a function, a simple solution (mutually exclusive from the problem, so this could be added regardless) might be adding a hover tool-tip next to the option in UI to show the enabled flags responsible for toggling the option. This isn't a bug or anything, just more of a design thing I thought might be worth looking into, or not.

@gCyrille gCyrille added the bug Something isn't working label Feb 24, 2025
@iButcat
Copy link
Member

iButcat commented Feb 25, 2025

Hello @OTonGitHub, PR to fix the issue has been merged to master, you should be able to create a VM with the blocked operations now, let me know if you have any issues or questions before I close the issue

@OTonGitHub
Copy link
Author

I was away for the weekend & didn't have VPN access to my Xen Lab (weekends here are different). So, sorry about taking a few days to respond.

I built the plugin from the master branch https://github.com/vatesfr/terraform-provider-xenorchestra and applied the exact same Terraform file as before on a fresh xcp-ng environment, everything worked as expected. I have tested a couple of times in most realistic ways I'd be using the flags. Edit/Update works without a hitch as well.

It's already kind of documented and should be obvious, but.. just in case, someone ends up here, for reference I'll just note, to turn on "Protect from accidental shutdown" you need these flags in blocked operations

  blocked_operations  = ["clean_reboot",
                         "clean_shutdown",
                         "hard_reboot",
                         "hard_shutdown",
                         "pause",
                         "shutdown",
                         "suspend"]

and for "Protect from accidental deletion" option, the destroy flag.

Perhaps this could be added as a hover-over tool-tip in the XO-UI? to display the flags set for each option that's turned on. It's in the XO mono-repo right? Maybe I'll try as a good first PR if it's a good idea.

Thank you so much @iButcat

@iButcat
Copy link
Member

iButcat commented Mar 2, 2025

Great, happy to help, and glad it works! No worries about the delay in responding—it's not a problem at all!

There’s something we’re currently discussing internally with the team regarding the blocked_operations type. As @julien-f mentioned, for the xo-api query, it can be set to {"key": "value"}. I tried modifying the terraform-provider-xenorchestra to use a set type, which also works on the xo-api side. The blocked operations functioned as expected—I wasn’t able to delete my VM, which was the intended behavior. However, even though this works in the provider and the xo-api, the XO GUI doesn’t display the fields as true even though they are correctly set in the API.

Since I wanted to align with the type provided, I made some modifications but had to roll back to using an array due to the unexpected behavior in the XO GUI.

Given that the xo-api can accept either an array or a set, and this could change in the future, we’ve decided to implement a versioned schema in the Terraform provider. This will include a migration mechanism to handle type changes, ensuring compatibility with older versions until a specified deprecation date. This way, we can notify users when they need to update their configurations while still supporting older versions for a transition period.

This is one of the next tasks we’ll be tackling.

@iButcat iButcat closed this as completed Mar 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants