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

GRPC Proxyless support for EnvoyFilters #47927

Closed
anuragagarwal561994 opened this issue Nov 18, 2023 · 15 comments
Closed

GRPC Proxyless support for EnvoyFilters #47927

anuragagarwal561994 opened this issue Nov 18, 2023 · 15 comments
Labels
area/extensions and telemetry area/networking area/user experience kind/enhancement lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while

Comments

@anuragagarwal561994
Copy link

Describe the feature request
Istio grpc-proxyless feature to have support for EnvoyFilter

Currently there has been limited development for grpc-proxyless feature in istio but it is one of the most performant features of istio which we are using in our production setup and empowers our infra.

Istio is helping us empower our control planes while grpc is effectively handling load balancing for our setup. GRPC XDS is developing at a rapid pace and adding new features with every release. These features are supported via the Envoy XDS API, but not necessarily directly related to istio configurations.

Istio provides support for using EnvoyFilters to pass any custom configuration to envoy if something is not readily supported by istio in the current phase.

Keeping both things in mind, it would be great if the EnvoyFilter support is extended to GRPC Xds use-cases as an experimental approach, which will allow both the grpc-proxyless and istio development to be on differnet tracks yet being supportive of each other.

As an example, we wanted to try out outlier detection, but the algorithms supported by grpc-xds have currently an alternative in istio, hence a suggestion on one of the threads was to instead use EnvoyFilter.

Affected product area (please put an X in all that apply)

[ ] Ambient
[ ] Docs
[ ] Dual Stack
[ ] Installation
[X] Networking
[ ] Performance and Scalability
[X] Extensions and Telemetry
[ ] Security
[ ] Test and Release
[X] User Experience
[ ] Developer Infrastructure
[X] GRPC Proxyless

Affected features (please put an X in all that apply)

[ ] Multi Cluster
[ ] Virtual Machine
[ ] Multi Control Plane

Additional context
I made some changes on my local setup in the istio source code to make this working and I would to contribute for this feature.

Mainly the changes involved were exposing the cluster patcher from clutser.go so it can be accessed in cds.go
Since this will be my first contribution to istio, it would be great if the community can help me better design the code to follow the code development guidelines properly.

@hzxuzhonghu
Copy link
Member

As an example, we wanted to try out outlier detection, but the algorithms supported by grpc-xds have currently an alternative in istio, hence a suggestion on one of the threads was to instead use grpc/grpc-java#10656.

Not sure how do you patch as the outlier detection type is not same with envoy's

@anuragagarwal561994
Copy link
Author

The types are same with enovy, if you are confused with https://github.com/grpc/proposal/blob/master/A50-xds-outlier-detection.md#lb-policy-config

This is an internal representation which is created after grpc-xds has parsed the enovy configuration and is not provided by the XDS API and that is okay.

If you see this

https://github.com/grpc/grpc-java/blob/0987dc401ca55885b99e98a857705396ab47828e/xds/src/main/java/io/grpc/xds/EnvoyServerProtoData.java#L299

It is taking Envoy form of config and then translating it to internal object which is then used

https://github.com/grpc/grpc-java/blob/0987dc401ca55885b99e98a857705396ab47828e/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java

https://github.com/grpc/grpc-java/blob/0987dc401ca55885b99e98a857705396ab47828e/xds/src/main/java/io/grpc/xds/XdsClusterResource.java#L219

I think these are enough to be able to navigate the code.

GRPC proxyless uses only what envoy supports, just adds its own sugar over it. So it doesn't have much things different in terms of the API, hence the above patching will work. I have locally tested it as well, or may be let me know if I got something wrong in understanding the question.

@hzxuzhonghu
Copy link
Member

Do you have an example on outlier? I am asking because i am wondering whether it can be make it in tree

@anuragagarwal561994
Copy link
Author

apiVersion: networking.istio.io/v1alpha3
kind: EnvoyFilter
metadata:
  name: outlier-on-percentage
  namespace: istio-poc
spec:
  configPatches:
  - applyTo: CLUSTER
    match:
      cluster:
        service: grpc-server.istio-poc.svc.cluster.local
    patch:
      operation: MERGE
      value:
        outlier_detection:
          failure_percentage_threshold: 100
          enforcing_failure_percentage: 100
          failure_percentage_minimum_hosts: 0
          failure_percentage_request_volume: 2
  workloadSelector:
    labels:
      app: grpc-client

If I understand correctly you need the EnvoyFilter configuration for the same right, this when I tested with the changes in the linked MR, was propogating to the grpc client.

How the client handles these outliers remains to be tested, but my main goal here was first to let the client understand the config and assuming the client will handle the rest.

@hzxuzhonghu
Copy link
Member

Apply envoyfilter to grpc makes sense. unless xds support in grpc is very early alpha. cc @costinm

@anuragagarwal561994
Copy link
Author

No it is in stable state as far as I am aware of, since traffic director is a feature from Google which is built over it

@anuragagarwal561994
Copy link
Author

I have also created an MR for the same, need guidance on how we can properly implement this.

@costinm
Copy link
Contributor

costinm commented Nov 20, 2023

I am not strongly against it - but I would rather avoid it if possible. GRPC has a smaller surface, and I would prefer simpler/cleaner ways to expose them - annotations/labels are not perfect either, but far easier than filters. I also think features with multiple implementations ( envoy and N gRPC languages) deserve promotion to proper API and are more likely to be stable.

The concern is that it's too easy to add an envoy filter that generates something proxyless grpc will reject, and those multi-implementation features will be stuck in alpha. They are pretty strict - and while XDS is used the content is quite different. That's why we have a separate implementation - it is fully on-demand so scale is far better.

BTW, as an alternative to EnvoyFilter I was thinking of an Gateway-attachment-like model where a real Envoy Cluster CRD could attach to a service. For routes it's a bit more complicated but possible too. This is more for the gateway to backend communications - the customized bootstrap file can technically add full listener/clusters already for envoy but not grpc.

@costinm
Copy link
Contributor

costinm commented Nov 20, 2023 via email

@anuragagarwal561994
Copy link
Author

@costinm although I do agree that there should be a precise contract between the two, but grpc as far as I know rejects what it doesn't attend but doesn't break.

Given the different languages available as far as I know they follow grfc and although they may be 1 feature ahead or behind the other they consume things in similar ways as per the gRFC document.

We just wanted to use istio for grpc proxyless just like a control plane that can communicate with kubernetes much like how traffic director behaves.

Although I do agree that over the period of time one can contribute to make the specs more precise like actually implementing the grpc supported algorithms for both envoy and grpc proxyless but meanwhile there can be a way for adhoc items.

I would also personally prefer to keep everything under destination rule instead of half things present as destination rule and other half as envoy filter, but before that I would like to have the feature available on day 1.

I don't see much documentation or development in grpc proxyless since it has released and has also be pointed out in multiple issues.

@costinm
Copy link
Contributor

costinm commented Nov 20, 2023

I don't disagree. Proxyless in istio is - like many other features - stuck in alpha, and for the most part had very few maintainers and not a lot of attention. I get some 20% time for istio - and grpc is rarely on the priority list.

The main reason ( besides time ) I have been reluctant about is the new Gateway API and ambient. Traffic Director support for proxyless is based on HttpRoute and K8S gateway API surface, and the integration with Ambient - based on opting out of interception specific ports - has been de-prioritized or had push back.

Moving proxyless grpc with Istio old API and without ambient integration doesn't seem useful.

Once ambient ships and we may reopen the discussion on selective interception - and the common API surface gets more settled - we can probably move proxyless forward.

@anuragagarwal561994
Copy link
Author

One reason why I feel this feature is not widely used is because it is not documented even on the istio official website and the fact that people will have to move their services to use grpc.

Every article points to traffic director and there is just one very old blog post available for handling grpc proxyless.

But this is one of the key features which if given time and development can be invincible for the users. We ourselves in our organisation has saved 1000s of $$ with just this feature and made our lives simpler.

I can help with few contributions for the grpc proxyless since we have just adopted it and are eagerly using it in our production system.

Nonetheless what about this issue, does it sound okay to implement EnvoyFilters as starters.

@akamac
Copy link

akamac commented Jan 12, 2024

I second the Anurag's words. We are having a hard time implementing the proxyless gRPC due to very poor documentation. We'd likely see the higher rate of feature adoption if it's documented and stated as mature. For now the only source of some docs is TrafficDirector guides.
Outlier detection support is one of the missing features to have a solid proxyless XDS setup.

@anuragagarwal561994
Copy link
Author

@akamac we had actually started implementing the outlier detection in istio, but a lot of work has come up in our organisation becuase of which we have for now put it on hold for sometime.

@istio-policy-bot istio-policy-bot added the lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while label May 19, 2024
@istio-policy-bot
Copy link

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2023-11-20. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.

@istio-policy-bot istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/extensions and telemetry area/networking area/user experience kind/enhancement lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants