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

Prevent data disk destruction on mirror #821

Open
rjmateus opened this issue Dec 22, 2020 · 3 comments
Open

Prevent data disk destruction on mirror #821

rjmateus opened this issue Dec 22, 2020 · 3 comments
Assignees

Comments

@rjmateus
Copy link
Member

Add prevent destroy to data disk in case we are creating a mirror disk.

lifecycle { prevent_destroy = true }
@rjmateus
Copy link
Member Author

I was trying to implement this change and hit a roadblock (I think was the same problem during the refactoring, but I'm not totally sure).
Since data disks are created in the provider backend-specific host module [1] I was trying to pass a variable to configure if the data disk destruction should be prevented. (draft PR with partial implementation in [2])

This implementation returns an error:

Error: Variables not allowed

  on backend_modules/libvirt/host/main.tf line 57, in resource "libvirt_volume" "data_disk":
  57:   lifecycle { prevent_destroy = var.prevent_additional_disk_destroy }

Variables may not be used here.

Basically, we cannot use variables on lifecycle definition. Looks like this is a known issue in terraform [3][4], and one of the issues is opened since August 2015.

So, our options are:

  1. Do not change code and not protect the mirror data disk
  2. Protected again destruction all data disks. This will also impact Suse Manager deployments that are using a separate disk for the repository. I check one example in our CI and it's not using the additional disk. However, if it starts to use it we need to manually change code in each execution to be able to destroy it.

@moio what are your thought on this one?

[1] https://github.com/uyuni-project/sumaform/blob/master/backend_modules/libvirt/host/main.tf#L51-L57
[2] #822
[3] hashicorp/terraform#22544
[4] hashicorp/terraform#3116

@moio
Copy link
Contributor

moio commented Jan 8, 2021

Would it be an option to define a separate protected data disk resource, or is it a pile of ugly duplication?

@rjmateus rjmateus self-assigned this Jan 8, 2021
@rjmateus
Copy link
Member Author

rjmateus commented Jan 8, 2021

I think It will not be too difficult to implement. As soon as I have some time I will try it and if it works open a PR

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