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

Add batch APIs for multiple tags to object #2211

Merged
merged 1 commit into from
Dec 17, 2020
Merged

Add batch APIs for multiple tags to object #2211

merged 1 commit into from
Dec 17, 2020

Conversation

embano1
Copy link
Contributor

@embano1 embano1 commented Dec 15, 2020

This PR adds support for the missing multiple-tags-to-object (attach/detach) APIs which are important for efficiency and performance especially in large environments (many tags/objects) and environments with high churn (Kubernetes controllers, event-driven systems).

The following changes were made:

  • Add attach-multiple-tags-to-object and detach-multiple-tags-from-object APIs
  • Add BatchError(s) types
  • Add minimum simulator APIs (see note below)
  • Add/update comments to describe error behavior in detail
  • Add black-box tests (against simulator)

Example output handled in a main.go with the log package where some tags could not be assigned to a valid reference:

2020/12/15 13:47:25 could not attach tags: [error: 0 type: cis.tagging.attach.invalidArgument.cardinality.error reason: Tag urn:vmomi:InventoryServiceTag:e2415c96-b16b-46e2-8c2e-ef8d19fad1b6:GLOBAL is ineligible to attach to urn:vmomi:VirtualMachine:vm-48:5790209b-3064-4fbe-b030-d0301aec0384 because of a cardinality violation.]

Tested against vCenter 6.7U3 which has the following behavior:

  • Missing/invalid object reference: 403 Forbidden
  • No valid REST session: 401 Unauthorized
  • No tagging permissions: ServerFaultCode: Permission to perform this operation was denied
  • Tag not found: 200 with individual cis.tagging.objectNotFound.error in response
  • Cardinality violation: 200 with cis.tagging.attach.invalidArgument.cardinality.error

Note that the existing simulator APIs currently don't support all these failure codes

Fixes: #2208
Related: #2210

Signed-off-by: Michael Gasch [email protected]

@embano1
Copy link
Contributor Author

embano1 commented Dec 15, 2020

WIP, need to implement DetachMultipleTagsFromObject but waiting for first review on the initial changes.

@embano1 embano1 changed the title WIP: Add batch APIs for multiple tags to object Add batch APIs for multiple tags to object Dec 15, 2020
Copy link
Member

@dougm dougm left a comment

Choose a reason for hiding this comment

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

Awesome, thank you for this and esp. including vcsim support and tests @embano1 !
Just one minor question, otherwise lgtm.

vapi/tags/tag_association.go Outdated Show resolved Hide resolved
@dougm dougm added this to the 0.24 milestone Dec 16, 2020
@embano1
Copy link
Contributor Author

embano1 commented Dec 16, 2020

Please hold Doug, found a minor glitch in one test which passed but in the wrong way :)

vapi/tags/tag_association_test.go Show resolved Hide resolved
vapi/tags/tag_association_test.go Outdated Show resolved Hide resolved
vapi/tags/tag_association_test.go Outdated Show resolved Hide resolved
- Add attach-multiple-tags-to-object and detach-multiple-tags-from-object APIs
- Add BatchError(s) types
- Add minimum simulator APIs (see note below)
- Add/update comments to describe error behavior in detail
- Add black-box tests (against simulator)

Tested against vCenter 6.7U3 which has the following behavior:

- Missing/invalid object reference: 403 Forbidden
- No valid REST session: 401 Unauthorized
- No tagging permissions: ServerFaultCode: Permission to perform this operation was denied
- Tag not found: 200 with individual cis.tagging.objectNotFound.error in response
- Cardinality violation: 200 with cis.tagging.attach.invalidArgument.cardinality.error

Note that the existing simulator APIs currently don't support all these failure codes

Fixes: #2208
Related: #2210
Signed-off-by: Michael Gasch <[email protected]>
@embano1
Copy link
Contributor Author

embano1 commented Dec 17, 2020

Done, ready for merge.

@dougm dougm merged commit 83d74f5 into vmware:master Dec 17, 2020
@embano1 embano1 deleted the issue-2208 branch December 18, 2020 07:11
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.

Add "attach multiple tags to object" batch operation
3 participants