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

vcsim: add PropertyCollector index support #3451

Merged
merged 1 commit into from
Jun 4, 2024
Merged

Conversation

dougm
Copy link
Member

@dougm dougm commented May 28, 2024

PropertyCollector supports use of index (unique key) to collect a single array element. Further, a property path may also be applied to the given element.

@dougm dougm force-pushed the property-index branch 6 times, most recently from ca415fe to dd81bd7 Compare May 30, 2024 18:12
@dougm dougm requested review from akutz and brakthehack May 30, 2024 18:41
@dougm dougm force-pushed the property-index branch from dd81bd7 to 3a4803b Compare May 30, 2024 19:58
PropertyCollector supports use of index (unique key) to collect a single array element.
Further, a property path may also be applied to the given element.
@dougm dougm force-pushed the property-index branch from 3a4803b to 3b7ff25 Compare May 30, 2024 20:48
run govc vm.change -e "guestinfo.a=1" -e "guestinfo.b=2"
assert_success

run govc object.collect -json $GOVC_VM 'config.extraConfig["guestinfo.b"]'
Copy link
Member

Choose a reason for hiding this comment

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

If you have guestinfo.abc=3 does that match (ie. substring match) or is it doing an exact token match between the separators?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's exact afaik, equal to the Key field of the array type.

return reflect.Value{}, errInvalidField
}

field := item.FieldByName("Key")
Copy link
Member

Choose a reason for hiding this comment

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

I am having trouble tracing how we get here, so maybe this is a non-issue, but what if the property path is for an array of non-struct types, such as an array of integers or strings? I believe a numerical index should work in that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

afaik, property path array index can only be used when struct has a key field, see for example:

% govc object.collect $GOVC_VM disabledMethod
disabledMethod  []string  MakePrimaryVM_Task,TerminateFaultTolerantVM_Task,...

% govc object.collect $GOVC_VM 'disabledMethod[1]'
govc: ServerFaultCode: InvalidProperty

Another case, AuthorizationRole has no key field. And while it does have a unique roleId field, same InvalidProperty result when using a valid roleId

% id=$(govc object.collect -o -json AuthorizationManager:AuthorizationManager roleList | jq .roleList[].roleId | head -1)

% govc object.collect AuthorizationManager:AuthorizationManager "roleList[$id]"
govc: ServerFaultCode: InvalidProperty

This doc also seems to indicate must be key-based:

Nested properties can also refer to entries in a key-based array

@dougm dougm merged commit dab9dd1 into vmware:main Jun 4, 2024
10 checks passed
@dougm dougm deleted the property-index branch June 4, 2024 18:05
dougm added a commit to dougm/govmomi that referenced this pull request Aug 7, 2024
PR vmware#3451 changed mo.ApplyPropertyChange() to panic() on invalid property path.
CancelTask had such a property for TaskInfo, 'canceled'.
Fixed to 'cancelled' and adds test coverage for the CancelTask method.

Fixes vmware#3506
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants