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.

Sign up for GitHub

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

Jump to bottom

Draft: Ensure traffic weighting still applies with session affinity. #1655

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Draft: Ensure traffic weighting still applies with session affinity. #1655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 80 additions & 25 deletions pkg/router/gateway_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,12 +426,24 @@ func (gwr *GatewayAPIRouter) Finalize(_ *flaggerv1.Canary) error {
return nil
}

func getBackendByServiceName(rule *v1.HTTPRouteRule, svcName string) *v1.HTTPBackendRef {

for i, backendRef := range rule.BackendRefs {
if string(backendRef.BackendObjectReference.Name) == svcName {

return &rule.BackendRefs[i]
}
}
return nil
}

// getSessionAffinityRouteRules returns the HTTPRouteRule objects required to perform
// session affinity based Canary releases.
func (gwr *GatewayAPIRouter) getSessionAffinityRouteRules(canary *flaggerv1.Canary, canaryWeight int,
weightedRouteRule *v1.HTTPRouteRule) ([]v1.HTTPRouteRule, error) {
_, primarySvcName, canarySvcName := canary.GetServiceNames()
stickyRouteRule := *weightedRouteRule
stickyCanaryRouteRule := *weightedRouteRule
stickyPrimaryRouteRule := *weightedRouteRule

// If a canary run is active, we want all responses corresponding to requests hitting the canary deployment
// (due to weighted routing) to include a `Set-Cookie` header. All requests that have the `Cookie` header
Expand All @@ -442,29 +454,41 @@ func (gwr *GatewayAPIRouter) getSessionAffinityRouteRules(canary *flaggerv1.Cana
}

// Add `Set-Cookie` header modifier to the primary backend in the weighted routing rule.
for i, backendRef := range weightedRouteRule.BackendRefs {
if string(backendRef.BackendObjectReference.Name) == canarySvcName {
backendRef.Filters = append(backendRef.Filters, v1.HTTPRouteFilter{
Type: v1.HTTPRouteFilterResponseHeaderModifier,
ResponseHeaderModifier: &v1.HTTPHeaderFilter{
Add: []v1.HTTPHeader{
{
Name: setCookieHeader,
Value: fmt.Sprintf("%s; %s=%d", canary.Status.SessionAffinityCookie, maxAgeAttr,
canary.Spec.Analysis.SessionAffinity.GetMaxAge(),
),
},
},
canaryBackendRef := getBackendByServiceName(weightedRouteRule, canarySvcName)
canaryBackendRef.Filters = append(canaryBackendRef.Filters, v1.HTTPRouteFilter{
Type: v1.HTTPRouteFilterResponseHeaderModifier,
ResponseHeaderModifier: &v1.HTTPHeaderFilter{
Add: []v1.HTTPHeader{
{
Name: setCookieHeader,
Value: fmt.Sprintf("%s; %s=%d", canary.Status.SessionAffinityCookie, maxAgeAttr,
canary.Spec.Analysis.SessionAffinity.GetMaxAge(),
),
},
})
}
weightedRouteRule.BackendRefs[i] = backendRef
}
},
},
})
primaryCookie := fmt.Sprintf("%s=%s", canary.Spec.Analysis.SessionAffinity.CookieName, randSeq())
primaryBackendRef := getBackendByServiceName(weightedRouteRule, primarySvcName)
primaryBackendRef.Filters = append(primaryBackendRef.Filters, v1.HTTPRouteFilter{
Type: v1.HTTPRouteFilterResponseHeaderModifier,
ResponseHeaderModifier: &v1.HTTPHeaderFilter{
Add: []v1.HTTPHeader{
{
Name: setCookieHeader,
Value: fmt.Sprintf("%s; %s=%d", primaryCookie, maxAgeAttr,
canary.Spec.Analysis.SessionAffinity.GetMaxAge(),
),
},
},
},
})

// CANARY COOKIE
// Add `Cookie` header matcher to the sticky routing rule.
cookieKeyAndVal := strings.Split(canary.Status.SessionAffinityCookie, "=")
regexMatchType := v1.HeaderMatchRegularExpression
cookieMatch := v1.HTTPRouteMatch{
canaryCookieMatch := v1.HTTPRouteMatch{
Headers: []v1.HTTPHeaderMatch{
{
Type: &regexMatchType,
Expand All @@ -479,16 +503,47 @@ func (gwr *GatewayAPIRouter) getSessionAffinityRouteRules(canary *flaggerv1.Cana
return nil, err
}

mergedMatches := gwr.mergeMatchConditions([]v1.HTTPRouteMatch{cookieMatch}, svcMatches)
stickyRouteRule.Matches = mergedMatches
stickyRouteRule.BackendRefs = []v1.HTTPBackendRef{
mergedMatches := gwr.mergeMatchConditions([]v1.HTTPRouteMatch{canaryCookieMatch}, svcMatches)
stickyCanaryRouteRule.Matches = mergedMatches
stickyCanaryRouteRule.BackendRefs = []v1.HTTPBackendRef{
{
BackendRef: gwr.makeBackendRef(primarySvcName, 0, canary.Spec.Service.Port),
},
{
BackendRef: gwr.makeBackendRef(canarySvcName, 100, canary.Spec.Service.Port),
},
}

// PRIMARY COOKIE
// Add `Cookie` header matcher to the sticky routing rule.
cookieKeyAndVal = strings.Split(primaryCookie, "=")
regexMatchType = v1.HeaderMatchRegularExpression
primaryCookieMatch := v1.HTTPRouteMatch{
Headers: []v1.HTTPHeaderMatch{
{
Type: &regexMatchType,
Name: cookieHeader,
Value: fmt.Sprintf(".*%s.*%s.*", cookieKeyAndVal[0], cookieKeyAndVal[1]),
},
},
}

svcMatches, err = gwr.mapRouteMatches(canary.Spec.Service.Match)
if err != nil {
return nil, err
}

mergedMatches = gwr.mergeMatchConditions([]v1.HTTPRouteMatch{primaryCookieMatch}, svcMatches)
stickyPrimaryRouteRule.Matches = mergedMatches
stickyPrimaryRouteRule.BackendRefs = []v1.HTTPBackendRef{
{
BackendRef: gwr.makeBackendRef(primarySvcName, 100, canary.Spec.Service.Port),
},
{
BackendRef: gwr.makeBackendRef(canarySvcName, 0, canary.Spec.Service.Port),
},
}

} else {
// If canary weight is 0 and SessionAffinityCookie is non-blank, then it belongs to a previous canary run.
if canary.Status.SessionAffinityCookie != "" {
Expand All @@ -511,9 +566,9 @@ func (gwr *GatewayAPIRouter) getSessionAffinityRouteRules(canary *flaggerv1.Cana
}
svcMatches, _ := gwr.mapRouteMatches(canary.Spec.Service.Match)
mergedMatches := gwr.mergeMatchConditions([]v1.HTTPRouteMatch{cookieMatch}, svcMatches)
stickyRouteRule.Matches = mergedMatches
stickyCanaryRouteRule.Matches = mergedMatches

stickyRouteRule.Filters = append(stickyRouteRule.Filters, v1.HTTPRouteFilter{
stickyCanaryRouteRule.Filters = append(stickyCanaryRouteRule.Filters, v1.HTTPRouteFilter{
Type: v1.HTTPRouteFilterResponseHeaderModifier,
ResponseHeaderModifier: &v1.HTTPHeaderFilter{
Add: []v1.HTTPHeader{
Expand All @@ -529,7 +584,7 @@ func (gwr *GatewayAPIRouter) getSessionAffinityRouteRules(canary *flaggerv1.Cana
canary.Status.SessionAffinityCookie = ""
}

return []v1.HTTPRouteRule{stickyRouteRule, *weightedRouteRule}, nil
return []v1.HTTPRouteRule{stickyCanaryRouteRule, stickyPrimaryRouteRule, *weightedRouteRule}, nil
}

func (gwr *GatewayAPIRouter) mapRouteMatches(requestMatches []istiov1beta1.HTTPMatchRequest) ([]v1.HTTPRouteMatch, error) {
Expand Down
63 changes: 40 additions & 23 deletions pkg/router/gateway_api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,19 +94,20 @@ func TestGatewayAPIRouter_Routes(t *testing.T) {

hr, err := mocks.meshClient.GatewayapiV1().HTTPRoutes("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
require.NoError(t, err)
assert.Len(t, hr.Spec.Rules, 2)
assert.Len(t, hr.Spec.Rules, 3)

stickyRule := hr.Spec.Rules[0]
weightedRule := hr.Spec.Rules[1]
stickyCanaryRule := hr.Spec.Rules[0]
stickyPrimaryRule := hr.Spec.Rules[1]
weightedRule := hr.Spec.Rules[2]

// stickyRoute should match against a cookie and direct all traffic to the canary when a canary run is active.
cookieMatch := stickyRule.Matches[0].Headers[0]
// stickyCanaryRoute should match against a cookie and direct all traffic to the canary when a canary run is active.
cookieMatch := stickyCanaryRule.Matches[0].Headers[0]
assert.Equal(t, *cookieMatch.Type, v1.HeaderMatchRegularExpression)
assert.Equal(t, string(cookieMatch.Name), cookieHeader)
assert.Contains(t, cookieMatch.Value, cookieKey)

assert.Equal(t, len(stickyRule.BackendRefs), 2)
for _, backendRef := range stickyRule.BackendRefs {
assert.Equal(t, len(stickyCanaryRule.BackendRefs), 2)
for _, backendRef := range stickyCanaryRule.BackendRefs {
if string(backendRef.BackendRef.Name) == pSvcName {
assert.Equal(t, *backendRef.BackendRef.Weight, int32(0))
}
Expand All @@ -115,6 +116,21 @@ func TestGatewayAPIRouter_Routes(t *testing.T) {
}
}

cookieMatch = stickyPrimaryRule.Matches[0].Headers[0]
assert.Equal(t, *cookieMatch.Type, v1.HeaderMatchRegularExpression)
assert.Equal(t, string(cookieMatch.Name), cookieHeader)
assert.Contains(t, cookieMatch.Value, cookieKey)

assert.Equal(t, len(stickyPrimaryRule.BackendRefs), 2)
for _, backendRef := range stickyPrimaryRule.BackendRefs {
if string(backendRef.BackendRef.Name) == pSvcName {
assert.Equal(t, *backendRef.BackendRef.Weight, int32(100))
}
if string(backendRef.BackendRef.Name) == cSvcName {
assert.Equal(t, *backendRef.BackendRef.Weight, int32(0))
}
}

// weightedRoute should do regular weight based routing and inject the Set-Cookie header
// for all responses returned from the canary deployment.
var found bool
Expand Down Expand Up @@ -142,27 +158,28 @@ func TestGatewayAPIRouter_Routes(t *testing.T) {
// HTTPRoute should be unchanged
hr, err = mocks.meshClient.GatewayapiV1().HTTPRoutes("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
require.NoError(t, err)
assert.Len(t, hr.Spec.Rules, 2)
assert.Empty(t, cmp.Diff(hr.Spec.Rules[0], stickyRule))
assert.Empty(t, cmp.Diff(hr.Spec.Rules[1], weightedRule))
assert.Len(t, hr.Spec.Rules, 3)
assert.Empty(t, cmp.Diff(hr.Spec.Rules[0], stickyCanaryRule))
assert.Empty(t, cmp.Diff(hr.Spec.Rules[1], stickyPrimaryRule))
assert.Empty(t, cmp.Diff(hr.Spec.Rules[2], weightedRule))

// further continue the canary run
err = router.SetRoutes(canary, 50, 50, false)
require.NoError(t, err)
hr, err = mocks.meshClient.GatewayapiV1().HTTPRoutes("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
require.NoError(t, err)

stickyRule = hr.Spec.Rules[0]
weightedRule = hr.Spec.Rules[1]
stickyCanaryRule = hr.Spec.Rules[0]
weightedRule = hr.Spec.Rules[2]

// stickyRoute should match against a cookie and direct all traffic to the canary when a canary run is active.
cookieMatch = stickyRule.Matches[0].Headers[0]
cookieMatch = stickyCanaryRule.Matches[0].Headers[0]
assert.Equal(t, *cookieMatch.Type, v1.HeaderMatchRegularExpression)
assert.Equal(t, string(cookieMatch.Name), cookieHeader)
assert.Contains(t, cookieMatch.Value, cookieKey)

assert.Equal(t, len(stickyRule.BackendRefs), 2)
for _, backendRef := range stickyRule.BackendRefs {
assert.Equal(t, len(stickyCanaryRule.BackendRefs), 2)
for _, backendRef := range stickyCanaryRule.BackendRefs {
if string(backendRef.BackendRef.Name) == pSvcName {
assert.Equal(t, *backendRef.BackendRef.Weight, int32(0))
}
Expand Down Expand Up @@ -200,22 +217,22 @@ func TestGatewayAPIRouter_Routes(t *testing.T) {
assert.Empty(t, canary.Status.SessionAffinityCookie)
assert.Contains(t, canary.Status.PreviousSessionAffinityCookie, cookieKey)

stickyRule = hr.Spec.Rules[0]
weightedRule = hr.Spec.Rules[1]
stickyCanaryRule = hr.Spec.Rules[0]
weightedRule = hr.Spec.Rules[2]

// Assert that the stucky rule matches against the previous cookie and tells clients to delete it.
cookieMatch = stickyRule.Matches[0].Headers[0]
cookieMatch = stickyCanaryRule.Matches[0].Headers[0]
assert.Equal(t, *cookieMatch.Type, v1.HeaderMatchRegularExpression)
assert.Equal(t, string(cookieMatch.Name), cookieHeader)
assert.Contains(t, cookieMatch.Value, cookieKey)

assert.Equal(t, stickyRule.Filters[0].Type, v1.HTTPRouteFilterResponseHeaderModifier)
headerModifier := stickyRule.Filters[0].ResponseHeaderModifier
assert.Equal(t, stickyCanaryRule.Filters[0].Type, v1.HTTPRouteFilterResponseHeaderModifier)
headerModifier := stickyCanaryRule.Filters[0].ResponseHeaderModifier
assert.NotNil(t, headerModifier)
assert.Equal(t, string(headerModifier.Add[0].Name), setCookieHeader)
assert.Equal(t, headerModifier.Add[0].Value, fmt.Sprintf("%s; %s=%d", canary.Status.PreviousSessionAffinityCookie, maxAgeAttr, -1))

for _, backendRef := range stickyRule.BackendRefs {
for _, backendRef := range stickyCanaryRule.BackendRefs {
if string(backendRef.BackendRef.Name) == pSvcName {
assert.Equal(t, *backendRef.BackendRef.Weight, int32(100))
}
Expand Down Expand Up @@ -303,7 +320,7 @@ func TestGatewayAPIRouter_getSessionAffinityRouteRules(t *testing.T) {
}
rules, err := router.getSessionAffinityRouteRules(canary, 10, weightedRouteRule)
require.NoError(t, err)
assert.Equal(t, len(rules), 2)
assert.Equal(t, len(rules), 3)
assert.True(t, strings.HasPrefix(canary.Status.SessionAffinityCookie, cookieKey))

stickyRule := rules[0]
Expand All @@ -322,7 +339,7 @@ func TestGatewayAPIRouter_getSessionAffinityRouteRules(t *testing.T) {
}
}

weightedRule := rules[1]
weightedRule := rules[2]
var found bool
for _, backendRef := range weightedRule.BackendRefs {
if string(backendRef.Name) == cSvcName {
Expand Down