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 working with orphaned devices #1005

Merged
merged 1 commit into from
Mar 24, 2020
Merged

Fix working with orphaned devices #1005

merged 1 commit into from
Mar 24, 2020

Conversation

Fodoj
Copy link

@Fodoj Fodoj commented Mar 19, 2020

Fixes this bug: #875

Problem is that if VM had orphaned disk saved in state file and same VM gets another orphaned disk after that, Terraform will add new disk with the same orpahed_disk_0 label. This breaks the run because label names must be unique according to virtual_machine_disk_subresource.go

The fix itself simply detects orphaned disks among already existing resources and skips them, so that they are re-added to state file as orphaned disks again, thus making this provider iterate over compelte list of orphaned disks and not only newly discovered ones.

@ghost ghost added the size/xs Relative Sizing: Extra-Small label Mar 19, 2020
@jetersen
Copy link

link to see current commit author: https://github.com/terraform-providers/terraform-provider-vsphere/commit/d943fe089a9ef60af52e532cb0edd0baaf9fd131.patch

@Fodoj
You might want to set up an email alias on your GitHub account 🥇
https://help.github.com/articles/adding-an-email-address-to-your-github-account/

Or look into fixing your git user.email config 😅
https://help.github.com/articles/setting-your-commit-email-address-in-git
then you would need to amend your commits: https://stackoverflow.com/a/3042512
after changing author

to get proper credit for your commits 🏆

Copy link

@jetersen jetersen left a comment

Choose a reason for hiding this comment

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

Looks good to me, many thanks for taking the time to create a fix and finding the root cause.

@Fodoj
Copy link
Author

Fodoj commented Mar 19, 2020

@jetersen let me re-do the commit with proper identity

@Fodoj
Copy link
Author

Fodoj commented Mar 19, 2020

@jetersen should be fine now. :-)

@jorioux
Copy link

jorioux commented Mar 24, 2020

@Fodoj thank you so much for fixing this issue

Can someone merge this PR please ?

Copy link
Contributor

@bill-rich bill-rich left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for taking the time to track this down and write the fix.

@bill-rich bill-rich merged commit 6449d7d into hashicorp:master Mar 24, 2020
@jetersen
Copy link

A release would be much appreciated :)

@aareet aareet linked an issue Mar 27, 2020 that may be closed by this pull request
@pkqsun
Copy link

pkqsun commented Apr 15, 2020

Hi @Fodoj

We had the same issue in #875 with Terraform v0.12.17 + terraform-provider-vsphere_v1.13.0_x4 + K8s v1.16.4, but it is occasionally.

In normal cases, I did "terraform plan -state=tfstate.xxx" cmd to refresh the infrastructure when there are additional dynamic vsphere volumes attached to the node, but the tfstate file do not change anything.

So my confuse is that with "ignore disks" in lifecycle, why and when will these orphaned disk info be saved into tfstate file?

Please check the remain logs:
https://gist.github.com/pkqsun/79e6e55c0ae6605b7762e5f960250d7

@Fodoj
Copy link
Author

Fodoj commented Apr 15, 2020

@pkqsun even though Terraform ignores changes to disks, they are still saved in the statefile, at least in this particular provider

@pkqsun
Copy link

pkqsun commented Apr 16, 2020

@pkqsun even though Terraform ignores changes to disks, they are still saved in the statefile, at least in this particular provider

Thanks for your info.
After testing, terraform plan do not update tfstate file, however, terraform apply saved these dynamic vsphere volumes to tfstate file even though "Apply complete! Resources: 0 added, 0 changed, 0 destroyed. ".

@ghost
Copy link

ghost commented Apr 24, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
size/xs Relative Sizing: Extra-Small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Error: disk: duplicate name orphaned_disk_0", while disks are ignored.
5 participants