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

fix: remove default values for ept_rvi_mode and hv_mode #2230

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

spacegospod
Copy link
Collaborator

@spacegospod spacegospod commented Jun 26, 2024

Description

I'm removing the default values for ept_rvi_mode and hv_mode from the virtual machine configuration.
These properties are overwritten during the "read" phase and cause in uplace updates on the VM even if the configuration hasn't changed.

We should allow the vCenter to control the defaults instead of setting them ourselves

This is my second take at attempting to fix this problem.
My first attempt was unsuccessful and was reverted. It caused in-place updates to VMs managed by configurations created with older versions of the provider.

The reason for this was that I only removed the default values for these attributes. This meant that configurations created with previous versions of the provider had actual values and upgrading to v2.8.0 meant that Terraform was attempting to unset those (since we no longer had the defaults).

The proper way to fix this problem is to not only remove the defaults but mark the attributes as Computed.
This way Terraform ignores the previously assigned defaults and we can ensure backwards compatibility.

Acceptance tests

  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch?

Output from acceptance testing:

Ran TestAccResourceVSphereVirtualMachine_basic and confirmed that even though the scenario is successful the test fails because the plan is not empty. This is due to the default values for ept_rvi_mode and hv_mode.

This problem is reproducible on most VM tests. Some even have ignore_changes blocks to work around this.
The problem is visible on the nightly runs https://github.com/hashicorp/terraform-provider-vsphere/actions/runs/8954207241/job/24593567973#step:10:241

I modified the resoruce by removing the defaults and marking the attributes as computed.
Ran TestAccResourceVSphereVirtualMachine_basic again and did not observe the problem.

I copied the exact same configuration from the test, installed v2.7.0, ran terraform apply and then terraform plan.
I observed that the issue is reproducible. I configured a local override for the provider, ran terraform plan on the same configuration but with the binary which contains the fix. I observed that the issue is not reproducible.

I manually set values to the two attributes, different than the defaults, ran terraform apply and verified that the operation is successful.

Release Note

Release note for CHANGELOG:

NONE

References

Closes #1902

@spacegospod spacegospod self-assigned this Jun 26, 2024
@spacegospod spacegospod requested a review from a team as a code owner June 26, 2024 12:01
@github-actions github-actions bot added provider Type: Provider needs-review Status: Pull Request Needs Review labels Jun 26, 2024
@tenthirtyam tenthirtyam added the bug Type: Bug label Jun 26, 2024
@tenthirtyam tenthirtyam self-requested a review June 26, 2024 12:10
@tenthirtyam tenthirtyam added this to the v2.8.2 milestone Jun 26, 2024
@tenthirtyam tenthirtyam changed the title bugfix: Remove default values for ept_rvi_mode and hv_mode fix: remove default values for ept_rvi_mode and hv_mode Jun 26, 2024
Copy link
Collaborator

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome - looks good and will be great to have this resolved!

@tenthirtyam tenthirtyam merged commit 31ab7e5 into main Jun 26, 2024
6 checks passed
@tenthirtyam tenthirtyam deleted the bugfix/vm-props branch June 26, 2024 17:00
Copy link

This functionality has been released in v2.8.2 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@hashicorp hashicorp locked as resolved and limited conversation to collaborators Jun 29, 2024
@tenthirtyam tenthirtyam removed the needs-review Status: Pull Request Needs Review label Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Type: Bug provider Type: Provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ept_rvi_mode and hv_mode always attemp to update on apply
3 participants