-
Notifications
You must be signed in to change notification settings - Fork 76
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 issue when internal disk update would clean VM description #418
Conversation
Merge branch 'master' of github.com:vmware/go-vcloud-director
Merge branch 'master' of github.com:vmware/go-vcloud-director
Merge branch 'master' of github.com:vmware/go-vcloud-director
# Conflicts: # govcd/api.go # govcd/api_vcd_test.go # govcd/openapi_endpoints.go # types/v56/constants.go # types/v56/openapi.go
…clean VM description
Looks like this PR contains other PRs, hence is not reviewable in this state. |
# Conflicts: # .changes/v2.14.0/410-features.md # govcd/openapi.go # govcd/tenant_context.go # govcd/vdc_group.go # govcd/vdc_group_test.go
Ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Are there any other fields that we are not sending during disk update, which could have a similar effect?
I am not aware of any anymore. |
.changes/v2.14.0/418-bug-fixes.md
Outdated
@@ -0,0 +1,2 @@ | |||
* Fix Issue #728: `vm.UpdateInternalDisksAsync()` didn't send VM description and as result would delete it [GH-418] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not clear what "it" refers to ("would delete it").
Also, from the conversation in Issue #728, it doesn't seem that this fix addresses that problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it is not 100% as I had very limited information
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated "it"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests passed. Go ahead.
Ref: #728
This fix solves the issue when in terraform VM and internal disk resources are used and internal disk update happens (like resize). So on the second terrafom apply VM description is missing and an update on VM is initiated. Also which currently requires VM power off and power on.