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.

Sign up for GitHub

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

Jump to bottom

Fix read of vcd_vapp_vm resource when VM it isn't found. Allows terraform to recreate resource and not fail #783

Merged
merged 8 commits into from
Feb 22, 2022
Merged

Fix read of vcd_vapp_vm resource when VM it isn't found. Allows terraform to recreate resource and not fail #783

merged 8 commits into from
Feb 22, 2022

Conversation

vbauzys
Copy link
Contributor

@vbauzys vbauzys commented Feb 8, 2022

Ref: #611

@vbauzys
fix read of resource when it isn't found. Allows terraform to recreat…
55b94ff 
…e resource when it isn't found

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
@vbauzys vbauzys changed the title Fix read of VM resource when it isn't found. Allows terraform to recreate resource and not fail Fix read of VM resource when VM it isn't found. Allows terraform to recreate resource and not fail Feb 8, 2022
@vbauzys
add changelog
bfbc41e 
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
@vbauzys vbauzys changed the title Fix read of VM resource when VM it isn't found. Allows terraform to recreate resource and not fail Fix read of vcd_vapp_vm resource when VM it isn't found. Allows terraform to recreate resource and not fail Feb 8, 2022
@vbauzys vbauzys marked this pull request as ready for review February 8, 2022 15:26
@vbauzys vbauzys self-assigned this Feb 8, 2022
@vbauzys vbauzys requested review from adambarreiro and mikeletux and removed request for dataclouder February 8, 2022 15:26
adambarreiro
adambarreiro reviewed Feb 9, 2022
View reviewed changes
.changes/v3.6.0/783-bug-fixes.md Outdated
@@ -0,0 +1 @@
* Fix Issue #611 when read of `vcd_vapp_vm` resource failed when vm isn't found. Now allows terraform to recreate resource when it isn't found. [GH-783]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a bit of nitpicking 😄

Suggested change
* Fix Issue #611 when read of `vcd_vapp_vm` resource failed when vm isn't found. Now allows terraform to recreate resource when it isn't found. [GH-783]
* Fix Issue #611 when read of `vcd_vapp_vm` resource failed when VM isn't found. Now allows Terraform to recreate resource when it isn't found. [GH-783]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you, updated

@vbauzys
update changelog
77be88f 
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
lvirbalas
lvirbalas reviewed Feb 14, 2022
View reviewed changes
Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

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

What about adding a test for this use case?

@vbauzys vbauzys closed this Feb 14, 2022
@vbauzys vbauzys reopened this Feb 14, 2022
@vbauzys
Copy link
Contributor Author

vbauzys commented Feb 14, 2022 •
edited
Loading

What about adding a test for this use case?

Not sure it's possible to check the result of this failure in the test, will check.

@vbauzys
add test
1e7aa9e 
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
@vbauzys
Copy link
Contributor Author

vbauzys commented Feb 15, 2022

What about adding a test for this use case?

Not sure it's possible to check the result of this failure in the test, will check.

added

@vbauzys
add comment
d261d97 
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
@vbauzys
improve test
52bde40 
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
@vbauzys
rollback, not needed change
ddf2d5f 
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Didainius
Didainius approved these changes Feb 18, 2022
View reviewed changes
Copy link
Collaborator

@Didainius Didainius left a comment

Choose a reason for hiding this comment

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

Fantastic :) Tried on both vcd_vapp_vm and vcd_vm - works great.

.changes/v3.6.0/783-bug-fixes.md Outdated
@@ -0,0 +1 @@
* Fix Issue #611 when read of `vcd_vapp_vm` resource failed when VM isn't found. Now allows Terraform to recreate resource when it isn't found. [GH-783]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Fix Issue #611 when read of `vcd_vapp_vm` resource failed when VM isn't found. Now allows Terraform to recreate resource when it isn't found. [GH-783]
* Fix Issue #611 when read of `vcd_vapp_vm` and `vcd_vm` resource failed when VM isn't found. Now allows Terraform to recreate resource when it isn't found. [GH-783]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@vbauzys
improve changelog
0a318b6 
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
mikeletux
mikeletux approved these changes Feb 22, 2022
View reviewed changes
Copy link
Contributor

@mikeletux mikeletux left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Run both vcd_vapp_vm and vcd_vm.
When applying it for the second time when previously deleted manually the vm, it recreates it.

Thanks for the effort @vbauzysvmware

@vbauzys vbauzys merged commit 97e10a8 into vmware:main Feb 22, 2022
@lvirbalas lvirbalas mentioned this pull request May 25, 2022
Closed
@Didainius Didainius mentioned this pull request Nov 4, 2022
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Didainius Didainius Didainius approved these changes

@lvirbalas lvirbalas lvirbalas approved these changes

@mikeletux mikeletux mikeletux approved these changes

@adambarreiro adambarreiro adambarreiro approved these changes

Assignees

@vbauzys vbauzys

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vbauzys @adambarreiro @mikeletux @lvirbalas @Didainius