Closed InputObject2 closed 8 months ago
@InputObject2 it was not intended to be a breaking change. It was an oversight during the testing the change and I apologize for that. I need to research the best way to apply the default value, but my current thinking is that the plan diff should be customized to ignore this condition or the state needs to be explicitly migrated to apply the default value.
It seems it should be possible to write an acceptance test for the type of state migration I explained above (https://github.com/hashicorp/terraform-plugin-sdk/issues/253#issuecomment-1142210081).
I have a code change that I believe should address the problem, but I need to further investigate the acceptance test portion of it.
@InputObject2 can you provide detailed steps for reproducing the issue (terraform version, provider version before, terraform code for the VM)?
I tried to reproduce it through this acceptance test, which simulates creating a VM with v0.24.2 of the provider followed by a terraform plan
with v0.25.0. I was expected the second step to have a non empty plan with the diff you provided, however, the plan is empty. I thought that this issue might be terraform version dependent, so I ran the test with v0.14.11 and v1.6.0 but I didn't see any difference in behavior.
I also performed the same test manually and could not reproduce the issue.
Hi! Thanks for diving into this so quick!
This is a bit of a long post but here we go. Here are my versions:
current: terraform 1.5.7, provider v.0.25.0 previous: terraform 1.5.7, provider v0.24.2
Here's some terraform code for it:
Then upgrade the provider
It seems to be hit or miss as to why Terraform decides to see the added property, but changing litteraly anything (in my case I set name_description = "test") and terraform will decide it now sees the parameter and destroys the vm.
resource "xenorchestra_vm" "instance" {
[...]
name_description = "test"
Then if we plan again, now it's detected, and even though name_description is not something that usually needs instance destroyed...
@InputObject2 thanks for the detailed information. I'm able to reproduce it in my acceptance test now :+1:
Unfortunately the state migration testing capability requires terraform-plugin-sdk v2.23.0 or later and a newer go version. Since it's been a while since the sdk has been upgraded, I'm going to upgrade it to the latest version (v2.29.0) as a prerequisite.
I don't believe there will be any complications with upgrading, but our nightly CI that was recently put into place hard codes the given go version. So in addition to the sdk and go upgrade, I need to enhance CI to support parameterizing the Go version before making that change. This should pave the way for testing the provider against different terraform versions and opentofu as well.
I decided to hold off on the build work that I mentioned above. I was able to get the terraform-provider-sdk and the go upgrade tested without too much trouble. I'll try to get the fix for this merged in this week and make a release around the same time frame.
This will be fixed in v0.25.1, which will be released shortly.
The addition of
destroy_cloud_config_vdi_after_boot
in #255 adds a new field to all existing VM instances.Since it forces recreation on change (going from null to true/false in the case of existing VM's), terraform will destroy any existing virtual machines that doesn't specifically ignore_changes on it.
For example, when nothing is specified:
Anyway the quick and easy solution is to add a lifecycle ignore_changes to the VM instances, but this seems like something that could impact some users.
So I guess my question is: was this an intentional effect in 0.25.0? If yes, that seems like something that should be in the release notes.