add session persistence support in destination rule #3502

ramaraochavali · 2025-05-10T08:47:11Z

Signed-off-by: Rama Chavali <[email protected]>

ramaraochavali · 2025-05-10T08:49:45Z

/test release-notes

istio-testing · 2025-05-11T05:50:21Z

PR needs rebase.

Instructions 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-sigs/prow repository.

howardjohn · 2025-05-12T14:02:12Z

networking/v1alpha3/destination_rule.proto

@@ -495,6 +495,35 @@ message LoadBalancerSettings {
    uint64 minimum_ring_size = 4 [deprecated=true];
  };

+  // Session persistence settings for the destination. Use this for hard session affinity.


we need very clear docs on the difference between this and consist hash

I think both 'hard affinity' and 'consistent hash' are well defined - but what we do ( 'by encoding the IP:port of the service in a cookie/header' ) needs to be clearly understood by users.

howardjohn · 2025-05-12T14:02:52Z

networking/v1alpha3/destination_rule.proto

+      // Use a cookie to store the session affinity information.
+      COOKIE  = 0;
+      // Use a header to store the session affinity information.
+      HEADER = 1;


how can this possibly be used? how would a client know the opaque value to put here?

It's not that opaque unfortunately - just an IP:port in a proto. I believe Envoy returns the header on the first request, and user is expected to set it in the next request.

Agree with your comment: it needs to be super clear in the comments, and also the case where we can't connect to the IP:port - don't remember if we return an error or just go to another endpoint and return the new updated header.

Envoy returns the header with ip:port encoded and the client has to repeatedly send the header for every request.

The behaviour of Envoy when the ip:port is not found is dependent on the strict flag.

I will update the docs to make all of this clear.

This is weak feature, mostly we istio should be transparent of applications

costinm

Good in general - but needs more docs.

costinm · 2025-05-13T20:32:58Z

networking/v1alpha3/destination_rule.proto

@@ -495,6 +495,35 @@ message LoadBalancerSettings {
    uint64 minimum_ring_size = 4 [deprecated=true];
  };

+  // Session persistence settings for the destination. Use this for hard session affinity.


I think both 'hard affinity' and 'consistent hash' are well defined - but what we do ( 'by encoding the IP:port of the service in a cookie/header' ) needs to be clearly understood by users.

costinm · 2025-05-13T20:35:35Z

networking/v1alpha3/destination_rule.proto

+      // Use a cookie to store the session affinity information.
+      COOKIE  = 0;
+      // Use a header to store the session affinity information.
+      HEADER = 1;


It's not that opaque unfortunately - just an IP:port in a proto. I believe Envoy returns the header on the first request, and user is expected to set it in the next request.

Agree with your comment: it needs to be super clear in the comments, and also the case where we can't connect to the IP:port - don't remember if we return an error or just go to another endpoint and return the new updated header.

costinm · 2025-05-13T20:37:58Z

networking/v1alpha3/destination_rule.proto

+
+    message Cookie {
+      // Lifetime of the cookie. If specified, a cookie with the TTL will be
+      // generated if the cookie is not present. If the TTL is present and zero, 


I don't remember exactly - but there were some concerns with non-session cookies ( around privacy regulations - I think you are required to show the consent screen and all kind of other things ).

I would very much prefer if we start without this setting and only with session cookies, it's a very complicated problem, best to avoid Istio starting to set persistent cookies and dealing with privacy.

costinm · 2025-05-13T20:42:55Z

Not blocking for this CL, but few concerns I have with using DR:

normally DR can be used both by client (consumer) and producer. Envoy implements this in a strange way where the client can actually set cookies and implement persistence - because the client-sidecar can encode the endpoint IP, and server side can also do this by encoding their own IP, but AFAIK our implementation is client side. This may have changed - but needs to be clearly explained.
ambient with Waypoints may support this - but I think it should be configured with the Gateway API equivalent. Either way - we should document if it is supported/used/allowed in Waypoint.
same for Gateways (of both kinds).

ramaraochavali · 2025-05-14T10:05:37Z

I am planning to support this for Sidecars and Gateways and also using Gateway API - Waypoints should use Gateway API to get this behaviour? Is that fine?

costinm · 2025-05-15T02:41:12Z

I think Gateways and Waypoints should use the Gateway API - and the doc needs to be clear on the behavior (the endpoint IP is generated by the gateway/waypoint, behave like normal / standard persistent sessions ). For Sidecars - that's where things are strange, because the persistent session is actually handled on the client side, by client sidecar encoding the picked endpoint - server has nothing to do with it (unless it's a waypoint or gateway - and then things get very weird because there client will stick to the gateways, which is usually not what user wants - and a second header or cookie is needed for sticking with the final backend ). Multicluster or egress gateways in the path have this problem. My point really is that it's not obvious and the docs should be detailed - and it is better to keep the 'normal' cases separate and handled with Gateway API, and not attempt to pretend it's the same or mix them too much.

…

On Wed, May 14, 2025 at 3:05 AM Rama Chavali ***@***.***> wrote: *ramaraochavali* left a comment (istio/api#3502) <#3502 (comment)> I am planning to support this for Sidecars and Gateways and also using Gateway API - Waypoints should use Gateway API to get this behaviour? Is that fine? — Reply to this email directly, view it on GitHub <#3502 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAUR2V6IZQZY7T62Z226ID26MIQNAVCNFSM6AAAAAB42TXOWGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNZZGYYDGNBYGI> . You are receiving this because your review was requested.Message ID: ***@***.***>

hzxuzhonghu · 2025-05-15T11:34:58Z

networking/v1alpha3/destination_rule.proto

+    // The cookie settings for session persistence.
+    Cookie cookie = 3;
+
+    message Cookie {


no cookie name needed?

hzxuzhonghu · 2025-05-15T11:37:36Z

networking/v1alpha3/destination_rule.proto

+    // The type of session persistence to use.
+    Type type = 1;
+
+    // The name of the header or cookie to use for session persistence.


Ah, see it. It is reusable field. Why donot we make header and cookie oneof

costinm · 2025-05-16T18:10:31Z

As I mentioned in the past, persistent sessions are not something new, invented by Istio or Envoy - and they exist outside of Istio too, DestinationRule is not restricted to only configuring rules for talking to istio workloads. For internet services using persistent sessions - the client can't control the name of the cookie, and is usually required to copy all the cookies provided by the server. Even in the case of Istio - depending on topology the request may traverse multiple E/W, Waypoints, managed gateways, etc - so a set of cookies may accumulate ( for example one set by the E/W so future requests are sent to the same cluster, and one by the final endpoint). As DestinationRule is a client feature - I don't think cookie name is useful and I don't think single name will be sufficient - and shouldn't matter anyways because 30 years of practice have been for client to copy all cookies. The header name(s) are less common and are important to specify on client side because unlike cookies there is no way for client to know, but because the client application needs to do the copying - not clear how DestinationRule can help when headers are set by remote load balancers, it won't do anything. For configuring a server - Gateway or Waypoint - the K8S Gateway API is a better fit than DestinationRule ( which in Istio is only used to configure clients ). We do need this API for the case where client sidecar is generating the cookie/header - i.e. sidecar->sidecar - but will break and not work as expected for sidecar -> E/W gateway -> sidecar or for sidecar -> waypoint/gateway -> anything, because the client-side sidecar will set a cookie/header for the next hop, and a persistent session to a specific gateway is not helpful. IMHO a lot of confusion could be avoided with a simpler API that just had a single boolean setting for the case of sidecar -> sidecar ( and nothing else), plus the K8S Gateway for configuring gateways/waypoints. Since client-side affinity to next hop is highly Istio sidecar specific - we can define an istio-specific header to use - we already have other istio headers that encode the identity and IP of the peer, and what 'client session affinity' really does is allow the client app to use a header to override the selection of the endpoint ( in a most convoluted way ). It's not really session affinity - user can base64 encode any IP port ( if it is in the EDS response ) and send the request there, I think using the name 'session persistence' instead of 'explicit endpoint selection' was a mistake.

…

On Thu, May 15, 2025 at 4:37 AM hzxuzhonghu ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In networking/v1alpha3/destination_rule.proto <#3502 (comment)>: > @@ -495,6 +495,35 @@ message LoadBalancerSettings { uint64 minimum_ring_size = 4 [deprecated=true]; }; + // Session persistence settings for the destination. Use this for hard session affinity. + message SessionPersistence { + // Type defines the type of session persistence such as through + // the use of a header or cookie. Default is COOKIE. + enum Type { + // Use a cookie to store the session affinity information. + COOKIE = 0; + // Use a header to store the session affinity information. + HEADER = 1; + } + + // The type of session persistence to use. + Type type = 1; + + // The name of the header or cookie to use for session persistence. Ah, see it. It is reusable field. Why donot we make header and cookie oneof — Reply to this email directly, view it on GitHub <#3502 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAUR2TY4EY3JEBR34PUHFL26R4BLAVCNFSM6AAAAAB42TXOWGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNBTGM3DMOJXGE> . You are receiving this because your review was requested.Message ID: ***@***.***>

ramaraochavali · 2025-06-05T10:48:49Z

@howardjohn @costinm Sorry for the huge delay. Reading all the comments, it looks like the concern is more about adding more docs and explaining the behaviour in detail and there is no objection to this API. Is that a fair summary? If yes, I will work on adding more docs and address remaining comments, we can take it from there

add session persistence support in destination rule

9d47de6

Signed-off-by: Rama Chavali <[email protected]>

ramaraochavali requested a review from a team as a code owner May 10, 2025 08:47

istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 10, 2025

ramaraochavali added the release-notes-none Indicates a PR that does not require release notes. label May 10, 2025

istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label May 11, 2025

howardjohn reviewed May 12, 2025

View reviewed changes

costinm reviewed May 13, 2025

View reviewed changes

hzxuzhonghu reviewed May 15, 2025

View reviewed changes

add session persistence support in destination rule #3502

Are you sure you want to change the base?

add session persistence support in destination rule #3502

Uh oh!

Conversation

ramaraochavali commented May 10, 2025

Uh oh!

ramaraochavali commented May 10, 2025

Uh oh!

istio-testing commented May 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

costinm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

costinm commented May 13, 2025

Uh oh!

ramaraochavali commented May 14, 2025

Uh oh!

costinm commented May 15, 2025 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

costinm commented May 16, 2025 via email

Uh oh!

ramaraochavali commented Jun 5, 2025

Uh oh!

Uh oh!