add metadata about virtual services and destination rules#11060
add metadata about virtual services and destination rules#11060rshriram wants to merge 3 commits intoistio:release-1.1from
Conversation
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rshriram The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
cc @douglas-reid / @kyessenov |
|
Thanks for tackling this issue! This approach seems reasonable to me, though my understanding of the Pilot codebase is limited at best. It seems like we should be able to extract the attributes via plugin/mixer client. |
pilot/pkg/networking/util/util.go
Outdated
|
|
||
| // IstioMetadataKey is the key under which metadata is added to a route or cluster | ||
| // regarding the virtual service or destination rule used for each | ||
| IstioMetadataKey = "networking.istio.io" |
There was a problem hiding this comment.
For EDS, this is just "istio". I think we can use "istio" here too (less space).
|
Looks reasonable. |
|
@kyessenov / @douglas-reid could one of you take over this and get the tests, etc. working? |
|
@rshriram: The following test failed, say
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
mandarjog
left a comment
There was a problem hiding this comment.
Any tests or generated config examples?
Otherwise looks good.
|
Same comment as Mandar: would be nice to capture the changes to xDS output in a test. |
|
This is not adding functionality. Its just for mixer filter to extract info about the virtual service or destination rule being used.. Also the generated config will be very ugly [its proto struct] |
|
|
Will add tests in a few days but at this moment, this is low priority for me. This was a half hour hack during some meeting, to show you how it can be done. OTOH, if one of you would like to take this PR code and add tests, other embellishments, etc. please feel free to do so |
|
@rshriram Thanks for the change. Yes, it's' up to telemetry team to update proxy to take advantage of the fields. But it would be good to have tests that cover this feature, as a form of contract on the part of envoy config generation. |
|
@rshriram: PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Signed-off-by: Shriram Rajagopalan shriramr@vmware.com