-
Notifications
You must be signed in to change notification settings - Fork 75
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
Add guest properties support for vm
and vapp
#235
Conversation
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.
Pretty sweet. Only question is how will the property updates be implemented on the Terraform side? Is the idea that an update of a single property in the list will cause the whole guest property struct to be re-set?
Yes. The idea is that whatever is set in There is no mechanism to set "one by one" and even if there was - such method is way quicker than setting 100 properties in "one by one" fashion. |
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.
LGTM!
CHANGELOG.md
Outdated
@@ -22,6 +22,8 @@ | |||
* Added methods `AdminOrg.GetVDCByName` and related `GetVDCById`, `GetVDCByNameOrId` | |||
* Added methods `AdminOrg.GetAdminVDCByName` and related `GetAdminVDCById`, `GetAdminVDCByNameOrId` | |||
* Added methods `Catalog.Refresh` and `AdminCatalog.Refresh` | |||
* Added methods `vm.SetGuestProperties` and `vm.GetGuestProperties` [#235](https://github.com/vmware/go-vcloud-director/pull/235) | |||
* Added methods `vapp.SetGuestProperties` and `vapp.GetGuestProperties` [#235](https://github.com/vmware/go-vcloud-director/pull/235) |
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.
Structure has been renamed, but change log entry left old.
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.
I knew I had to miss it somewhere while renaming. Fixed.
// SetProductSectionList sets product section for a vApp | ||
// | ||
// The slice of properties "ProductSectionList.ProductSection.Property" is not necessarily ordered or returned as set before | ||
func (vapp *VApp) SetProductSectionList(productSection *types.ProductSectionList) (*types.ProductSectionList, error) { |
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.
I would add a sentence - a hint - that this product section represents guest properties in all comments of the four public functions.
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.
Added
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.
Typo in "maniapulate", sounds funny though :D
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.
fixed
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.
LGTM
Both VMs and vApps allow to set guest properties which are useful for configuration (when guests support it).
This PR adds
SetGuestProperties
SetProductSectionList
andGetGuestProperties
GetProductSectionList
forvm
andvapp
objects which allow to manipulate guest properties