search

istio / api

API definitions for the Istio project
Apache License 2.0
455 stars 550 forks source link

New Status proto for ServiceEntry #3244

Closed ilrudie closed 3 months ago

ilrudie commented 3 months ago

Adds a new proto based on IstioStatus to be used in ServiceEntry. This adds an addresses field to support the auto-allocation v2 controller's persistence requirement.

istio-policy-bot commented 3 months ago

😊 Welcome @ilrudie! This is either your first contribution to the Istio api repo, or it's been a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

ilrudie commented 3 months ago

I'm not sure Istio status - and adding all the baggage and unrelated API - is the right approach.

John had a good point that adding the full K8S Gateway status is too much - but this is a different level of too much.

How about adding just the IP - and a small comment with a point to someone else that uses the same structure. Istio status doesn't have IP, so it's not relevant.

I think we should at least include conditions. I was thinking about this some more last night and currently in v1 we have a situation where everything is ephemeral so if something about an SE changes such that we no longer do auto allocation we quietly ignore that se and possibly reshuffle the state of the world 2-tier hashing... With persistence via status we may not wish to immediately remove the assigned IPs though. Perhaps the IPs stay assigned, at least for some time until we garbage collect, but we note in a condition that the IPs will not be used due to reason X. User maybe corrects that and then the IP go back into effect without needing a new assign.

Maybe this need is not sufficient to duplicate all the IstioStatus fields though.

istio-testing commented 3 months ago

@ilrudie: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
release-notes_api 7c9a9ee5eb59fda807bd59b374ba764f174d611e link false /test release-notes
Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).