support hash based routing for soft session affinity #6742

utako · 2018-06-29T19:17:40Z

this fixes #1703.

zachgersh · 2018-07-02T15:15:21Z

/test istio-presubmit

rshriram · 2018-07-02T15:45:05Z

pilot/pkg/networking/core/v1alpha3/route/route.go

@@ -192,7 +192,7 @@ func buildVirtualHostsForVirtualService(
 	}
 	out := make([]VirtualHostWrapper, 0, len(serviceByPort))
 	for port, portServices := range serviceByPort {
-		routes, err := BuildHTTPRoutesForVirtualService(virtualService, serviceRegistry, port, proxyLabels, gatewayName)
+		routes, err := BuildHTTPRoutesForVirtualService(virtualService, serviceRegistry, port, proxyLabels, gatewayName, nil)


you aren't passing the config store here.

nil here as this specific call is for sidecar - no ring hash available there we assume.

We aren't passing in the config store here because this is only used for building sidecar outbound http route configs. We could add different function signatures to make this clearer.

rshriram · 2018-07-02T23:27:08Z

pilot/pkg/networking/core/v1alpha3/route/route.go

 			weighted = append(weighted, &route.WeightedCluster_ClusterWeight{
 				Name:   n,
 				Weight: weight,
 			})
+
+			hashPolicy := getHashPolicy(configStore, hostname)


pass the entire destination (dst.Destination) here. Hash policies can be specified at subset level.

rshriram · 2018-07-02T23:28:25Z

pilot/pkg/networking/core/v1alpha3/route/route.go

+	}
+
+	rule := destinationRule.Spec.(*networking.DestinationRule)
+	consistentHash := rule.GetTrafficPolicy().GetLoadBalancer().GetConsistentHash()


You need to check if there is a consistent hash/lb setting for the destination subset as well. Look at cluster.go to see how its using the subset to override top level lb settings

rshriram · 2018-07-02T23:29:34Z

pilot/pkg/networking/core/v1alpha3/route/route.go

+		return nil
+	}
+
+	cookie := consistentHash.GetHttpCookie()


please convert this into a switch case and add hash policy for http header or source IP..

rshriram

/lgtm

istio-testing · 2018-07-03T01:11:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rshriram, utako

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rshriram]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gyliu513 · 2018-07-03T03:39:24Z

@rshriram @utako are we going to deliver any document for this feature?

utako · 2018-07-03T16:25:50Z

hey @gyliu513,

are you talking about updating these docs?

gyliu513 · 2018-07-03T23:25:29Z

@utako Yes, not only the doc you mentioned, how about adding a new task in https://istio.io/docs/tasks/traffic-management/

* Introducing mcpc, a command-line mcp client tool. (#6715) * Set default requests cpu resource to all deployments (#6229) * Set default requests cpu resource to all deployments * Description added * Requested CPU decreased to meet test env node capacity * Setting requested cpu to 10m as most k8s platforms are defaulting to 0m * Merge master remains * Apply the global resources to the new gateways * Updated goldens to cpu 10m * Clean up and rename the global default resources * Redundant Redis servers were removed + defer in a loop is not recommended. (#6768) * Fix upgrade. (#6792) * Fix upgrade. * Format * Move back the fail close - not clear if we have time to get mixer client in this build, we can remove in next * Add configuration to allow transformation of durations to integers. (#5416) The fluentd handler currently transforms durations to their string representation for log messages. This commit adds a configuration to override this default behaviour and tranform durations to integers of unit millisecond. * Add workload and service dashboard (#6789) * Add workload and service dashboard * fix typo * Skip service dash e2e test for now * Add rushed, missed workload dash * Touch up * Commenting out test until can replicate locally and fix * simplify fmt script (#6798) * simplify fmt script - goimports is a superset of gofmt - shouldn't need to remove spaces - now utilizing -l flag to tell us if there is a diff * always update gofmt to latest * spacing that is now considered incorrect * support hash based routing for soft session affinity (#6742) * support sticky sessions * fmt :( * update servicediscovery fakes * bump istio/api * use updated api for ring hash * support session affinity for sidecar * update destination rule yaml to reflect new api * ttl is a string * unset sdsUdsPath in configmap (#6799) * fix bug so that destination.service.** attributes are collected (#6801) * remove unnecessary generated attributes finding. (#6785) * modify docker template files for proxyv2 (#6790) * Long-running testing improvements (#6800) * Add values for config map settings, including access log. More docs. * Updates and improvements for the stress-testing configs. * Add values for config map settings, including access log. (#6797) * Add values for config map settings, including access log. More docs. * Updates and improvements for the stress-testing configs. * Address review comments * Merged wrong files * Add the setup helm file - this change now depend on the previous PR. * Sync with remote, remove accidentally added files. * Another accidental file * SNI routing from sidecar to gateway with virtual services (#6402) * quick sni matching 1st pass with no refactoring of existing code * use shriram's api sha * quick pass at using tls block * add some validation * copyright * fix lint + remove deadcode * rename protocol tcp_tls -> tls * update back to istio/api master * remove accidentally added test file * add tls block to gateway logic * add todos * basic sni wildcard implementation * add tcp, fix problems with rbac, matching * better tcp + tls validation * address code review comments * remove out of date comment * update comments * fix compile error * use tcp proxy in tcp routing * add tcp routing e2e test * add forgotten vs config file + update description of test * Comments, bug fixes Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * cleanup gateway tcp test Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * moving networking test yamls Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * tcp/tls tests Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * yaml fixes Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * fix file switcheroo * port matches Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * fix authN plugin overwriting TLS context Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * more tests - route via egress gateway Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * yaml fixes Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * initialize prom variables Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * split tests Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * final test fix hopefully Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * revert gateway tweaks Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * Add myself back to OWNERS, sort the list alphabetically. (#6830) * Upload cloudfoundry test result to GCS (#6829) * Fix typos in command-line output.

istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Jun 29, 2018

istio-testing requested review from costinm and ijsnellf June 29, 2018 19:17

googlebot added the cla: yes label Jun 29, 2018

zachgersh requested a review from rshriram June 29, 2018 19:21

rshriram reviewed Jul 2, 2018

View reviewed changes

utako force-pushed the sticky_sessions branch from 0c50625 to 7982419 Compare July 2, 2018 22:17

zachgersh changed the title ~~[WIP] support sticky sessions for weight based routing~~ support sticky sessions for weight based routing Jul 2, 2018

istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Jul 2, 2018

utako force-pushed the sticky_sessions branch from 87496e2 to b65980e Compare July 2, 2018 22:36

rshriram changed the title ~~support sticky sessions for weight based routing~~ support hash based routing for soft session affinity Jul 2, 2018

rshriram reviewed Jul 2, 2018

View reviewed changes

utako added 8 commits July 2, 2018 17:22

support sticky sessions

5f0c71e

fmt :(

f21896a

update servicediscovery fakes

59947d3

bump istio/api

9d3d444

use updated api for ring hash

c03632c

support session affinity for sidecar

bd53289

update destination rule yaml to reflect new api

3296749

ttl is a string

033c53e

utako force-pushed the sticky_sessions branch from f72d022 to 033c53e Compare July 3, 2018 00:22

istio-testing assigned rshriram Jul 3, 2018

rshriram approved these changes Jul 3, 2018

View reviewed changes

istio-testing added the lgtm label Jul 3, 2018

istio-testing added the approved label Jul 3, 2018

rshriram merged commit 276e1b2 into istio:master Jul 3, 2018

utako deleted the sticky_sessions branch July 3, 2018 16:21

utako mentioned this pull request Jul 3, 2018

do not ignore subsets in soft sticky sessions #6822

Merged

ken-experiment-zz mentioned this pull request Jan 2, 2019

Support sticky sessions for weight based routing #1703

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support hash based routing for soft session affinity #6742

support hash based routing for soft session affinity #6742

utako commented Jun 29, 2018

zachgersh commented Jul 2, 2018

rshriram Jul 2, 2018

zachgersh Jul 2, 2018

utako Jul 2, 2018

rshriram Jul 2, 2018

rshriram Jul 2, 2018

rshriram Jul 2, 2018

rshriram left a comment

istio-testing commented Jul 3, 2018

gyliu513 commented Jul 3, 2018

utako commented Jul 3, 2018

gyliu513 commented Jul 3, 2018

support hash based routing for soft session affinity #6742

support hash based routing for soft session affinity #6742

Conversation

utako commented Jun 29, 2018

zachgersh commented Jul 2, 2018

rshriram Jul 2, 2018

Choose a reason for hiding this comment

zachgersh Jul 2, 2018

Choose a reason for hiding this comment

utako Jul 2, 2018

Choose a reason for hiding this comment

rshriram Jul 2, 2018

Choose a reason for hiding this comment

rshriram Jul 2, 2018

Choose a reason for hiding this comment

rshriram Jul 2, 2018

Choose a reason for hiding this comment

rshriram left a comment

Choose a reason for hiding this comment

istio-testing commented Jul 3, 2018

gyliu513 commented Jul 3, 2018

utako commented Jul 3, 2018

gyliu513 commented Jul 3, 2018