-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
do not ignore subsets in soft sticky sessions #6822
Changes from 6 commits
98d51e2
30aac95
a89d72e
950a06c
91ae96b
5f2676b
b87e946
8f28cbc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -399,7 +399,7 @@ func translateRoute(in *networking.HTTPRoute, | |
Weight: weight, | ||
}) | ||
|
||
hashPolicy := getHashPolicy(configStore, hostname) | ||
hashPolicy := getHashPolicy(configStore, dst) | ||
if hashPolicy != nil { | ||
action.HashPolicy = append(action.HashPolicy, hashPolicy) | ||
} | ||
|
@@ -621,30 +621,88 @@ func translateFault(in *networking.HTTPFaultInjection) *xdshttpfault.HTTPFault { | |
return &out | ||
} | ||
|
||
func getHashPolicy(configStore model.IstioConfigStore, hostname model.Hostname) *route.RouteAction_HashPolicy { | ||
func portLevelSettingsConsistentHash(dst *networking.Destination, | ||
pls []*networking.TrafficPolicy_PortTrafficPolicy) *networking.LoadBalancerSettings_ConsistentHashLB { | ||
if dst.Port != nil { | ||
switch dst.Port.Port.(type) { | ||
case *networking.PortSelector_Name: | ||
log.Warnf("using deprecated name on port selector - ignoring") | ||
case *networking.PortSelector_Number: | ||
portNumber := dst.GetPort().GetNumber() | ||
for _, setting := range pls { | ||
number := setting.GetPort().GetNumber() | ||
if number == portNumber { | ||
return setting.GetLoadBalancer().GetConsistentHash() | ||
} | ||
} | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func getHashPolicy(configStore model.IstioConfigStore, dst *networking.DestinationWeight) *route.RouteAction_HashPolicy { | ||
if configStore == nil { | ||
return nil | ||
} | ||
|
||
destinationRule := configStore.DestinationRule(hostname) | ||
destination := dst.GetDestination() | ||
destinationRule := configStore.DestinationRule(model.Hostname(destination.GetHost())) | ||
if destinationRule == nil { | ||
return nil | ||
} | ||
|
||
rule := destinationRule.Spec.(*networking.DestinationRule) | ||
|
||
consistentHash := rule.GetTrafficPolicy().GetLoadBalancer().GetConsistentHash() | ||
|
||
for _, subset := range rule.GetSubsets() { | ||
if subset.GetName() == destination.GetSubset() { | ||
subsetPortLevelSettings := subset.GetTrafficPolicy().GetPortLevelSettings() | ||
consistentHash = subset.GetTrafficPolicy().GetLoadBalancer().GetConsistentHash() | ||
settingsHash := portLevelSettingsConsistentHash(destination, subsetPortLevelSettings) | ||
if settingsHash != nil { | ||
consistentHash = settingsHash | ||
} | ||
|
||
break | ||
} | ||
} | ||
|
||
portLevelSettings := rule.GetTrafficPolicy().GetPortLevelSettings() | ||
settingsHash := portLevelSettingsConsistentHash(destination, portLevelSettings) | ||
if settingsHash != nil { | ||
consistentHash = settingsHash | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to move to top before matching on subsets. Port level settings in subsets overrides the port level settings in top level. |
||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also this should be above the subset search for loop |
||
if consistentHash == nil { | ||
return nil | ||
} | ||
|
||
cookie := consistentHash.GetHttpCookie() | ||
return &route.RouteAction_HashPolicy{ | ||
PolicySpecifier: &route.RouteAction_HashPolicy_Cookie_{ | ||
hashPolicy := &route.RouteAction_HashPolicy{} | ||
switch consistentHash.GetHashKey().(type) { | ||
case *networking.LoadBalancerSettings_ConsistentHashLB_HttpHeaderName: | ||
hashPolicy.PolicySpecifier = &route.RouteAction_HashPolicy_Header_{ | ||
Header: &route.RouteAction_HashPolicy_Header{ | ||
HeaderName: consistentHash.GetHttpHeaderName(), | ||
}, | ||
} | ||
case *networking.LoadBalancerSettings_ConsistentHashLB_HttpCookie: | ||
cookie := consistentHash.GetHttpCookie() | ||
|
||
hashPolicy.PolicySpecifier = &route.RouteAction_HashPolicy_Cookie_{ | ||
Cookie: &route.RouteAction_HashPolicy_Cookie{ | ||
Name: cookie.GetName(), | ||
Ttl: cookie.GetTtl(), | ||
Path: cookie.GetPath(), | ||
}, | ||
}, | ||
} | ||
case *networking.LoadBalancerSettings_ConsistentHashLB_UseSourceIp: | ||
hashPolicy.PolicySpecifier = &route.RouteAction_HashPolicy_ConnectionProperties_{ | ||
ConnectionProperties: &route.RouteAction_HashPolicy_ConnectionProperties{ | ||
SourceIp: consistentHash.GetUseSourceIp(), | ||
}, | ||
} | ||
} | ||
|
||
return hashPolicy | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No if condition here. Essentially there is no inheritance. If something is missing it goes to default values. So you can assign everything to consistentHash and do a nil check in the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd be able to move the nil checks if this were patterned more after
cluster.go
. We'll see about clarifying the behavior so it's more obvious in the next commit. We were hoping to avoid modifying the hash policy in place for clarity's sake.