diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index e79d9c4b..e138b66d 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -1250,6 +1250,9 @@ spec: sessionAffinityCookie: description: Session affinity cookie of the current canary run type: string + primarySessionAffinityCookie: + description: Primary session affinity cookie of the current canary run + type: string previousSessionAffinityCookie: description: Session affinity cookie of the previous canary run type: string diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml index e79d9c4b..e138b66d 100644 --- a/charts/flagger/crds/crd.yaml +++ b/charts/flagger/crds/crd.yaml @@ -1250,6 +1250,9 @@ spec: sessionAffinityCookie: description: Session affinity cookie of the current canary run type: string + primarySessionAffinityCookie: + description: Primary session affinity cookie of the current canary run + type: string previousSessionAffinityCookie: description: Session affinity cookie of the previous canary run type: string diff --git a/docs/gitbook/tutorials/istio-progressive-delivery.md b/docs/gitbook/tutorials/istio-progressive-delivery.md index 2112a96c..0baadd6a 100644 --- a/docs/gitbook/tutorials/istio-progressive-delivery.md +++ b/docs/gitbook/tutorials/istio-progressive-delivery.md @@ -404,6 +404,9 @@ You can load `app.example.com` in your browser and refresh it until you see the All subsequent requests after that will be served by `podinfo:6.0.1` and not `podinfo:6.0.0` because of the session affinity configured by Flagger with Istio. +To configure stickiness for the Primary deployment to ensure fair weighted traffic routing, please +checkout the [deployment strategies docs](../usage/deployment-strategies.md#canary-release-with-session-affinity). + ## Traffic mirroring ![Flagger Canary Traffic Shadowing](https://raw.githubusercontent.com/fluxcd/flagger/main/docs/diagrams/flagger-canary-traffic-mirroring.png) diff --git a/docs/gitbook/usage/deployment-strategies.md b/docs/gitbook/usage/deployment-strategies.md index 7df71f6f..9c47a59a 100644 --- a/docs/gitbook/usage/deployment-strategies.md +++ b/docs/gitbook/usage/deployment-strategies.md @@ -474,7 +474,7 @@ can also configure stickiness for the Primary deployment. You can configure this primaryCookieName: primary-flagger-cookie ``` -> Note: This is only supported for the Gateway API provider for now. +> Note: This is only supported for the Gateway API and Istio providers for now. Let's understand what the above configuration does. All the session affinity stuff in the above section still occurs, but now the response header for requests routed to the primary deployment also include a diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml index e79d9c4b..e138b66d 100644 --- a/kustomize/base/flagger/crd.yaml +++ b/kustomize/base/flagger/crd.yaml @@ -1250,6 +1250,9 @@ spec: sessionAffinityCookie: description: Session affinity cookie of the current canary run type: string + primarySessionAffinityCookie: + description: Primary session affinity cookie of the current canary run + type: string previousSessionAffinityCookie: description: Session affinity cookie of the previous canary run type: string diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go index 82fd00ac..950a6fe0 100644 --- a/pkg/apis/flagger/v1beta1/canary.go +++ b/pkg/apis/flagger/v1beta1/canary.go @@ -706,9 +706,9 @@ func (c *Canary) DeploymentStrategy() string { } // BuildCookie returns the cookie that should be used as the value of a Set-Cookie header -func (s *SessionAffinity) BuildCookie(cookieName string) string { +func (s *SessionAffinity) BuildCookie(cookieName string, maxAge int) string { cookie := fmt.Sprintf("%s; %s=%d", cookieName, "Max-Age", - s.GetMaxAge(), + maxAge, ) if s.Domain != "" { diff --git a/pkg/apis/flagger/v1beta1/status.go b/pkg/apis/flagger/v1beta1/status.go index fd92f08d..9b032716 100644 --- a/pkg/apis/flagger/v1beta1/status.go +++ b/pkg/apis/flagger/v1beta1/status.go @@ -78,6 +78,8 @@ type CanaryStatus struct { // +optional SessionAffinityCookie string `json:"sessionAffinityCookie,omitempty"` // +optional + PrimarySessionAffinityCookie string `json:"primarySessionAffinityCookie,omitempty"` + // +optional TrackedConfigs *map[string]string `json:"trackedConfigs,omitempty"` // +optional LastAppliedSpec string `json:"lastAppliedSpec,omitempty"` diff --git a/pkg/router/gateway_api.go b/pkg/router/gateway_api.go index 810bf291..bbcadc28 100644 --- a/pkg/router/gateway_api.go +++ b/pkg/router/gateway_api.go @@ -492,7 +492,10 @@ func (gwr *GatewayAPIRouter) getSessionAffinityRouteRules(canary *flaggerv1.Cana if canary.Status.SessionAffinityCookie == "" { canary.Status.SessionAffinityCookie = fmt.Sprintf("%s=%s", canary.Spec.Analysis.SessionAffinity.CookieName, randSeq()) } - primaryCookie := fmt.Sprintf("%s=%s", canary.Spec.Analysis.SessionAffinity.PrimaryCookieName, randSeq()) + // if the status doesn't have the primary cookie, then generate a new primary cookie. + if canary.Status.PrimarySessionAffinityCookie == "" { + canary.Status.PrimarySessionAffinityCookie = fmt.Sprintf("%s=%s", canary.Spec.Analysis.SessionAffinity.PrimaryCookieName, randSeq()) + } // add response modifier to the canary backend ref in the rule that does weighted routing // to include the canary cookie. @@ -503,7 +506,7 @@ func (gwr *GatewayAPIRouter) getSessionAffinityRouteRules(canary *flaggerv1.Cana Add: []v1.HTTPHeader{ { Name: setCookieHeader, - Value: canary.Spec.Analysis.SessionAffinity.BuildCookie(canary.Status.SessionAffinityCookie), + Value: canary.Spec.Analysis.SessionAffinity.BuildCookie(canary.Status.SessionAffinityCookie, canary.Spec.Analysis.SessionAffinity.GetMaxAge()), }, }, }, @@ -522,10 +525,8 @@ func (gwr *GatewayAPIRouter) getSessionAffinityRouteRules(canary *flaggerv1.Cana ResponseHeaderModifier: &v1.HTTPHeaderFilter{ Add: []v1.HTTPHeader{ { - Name: setCookieHeader, - Value: fmt.Sprintf("%s; %s=%d", primaryCookie, maxAgeAttr, - int(interval.Seconds()), - ), + Name: setCookieHeader, + Value: canary.Spec.Analysis.SessionAffinity.BuildCookie(canary.Status.PrimarySessionAffinityCookie, int(interval.Seconds())), }, }, }, @@ -566,7 +567,7 @@ func (gwr *GatewayAPIRouter) getSessionAffinityRouteRules(canary *flaggerv1.Cana // primary cookie and send them to the primary backend, only if a primary cookie name has // been specified. if canary.Spec.Analysis.SessionAffinity.PrimaryCookieName != "" { - cookieKeyAndVal = strings.Split(primaryCookie, "=") + cookieKeyAndVal = strings.Split(canary.Status.PrimarySessionAffinityCookie, "=") regexMatchType = v1.HeaderMatchRegularExpression primaryCookieMatch := v1.HTTPRouteMatch{ Headers: []v1.HTTPHeaderMatch{ diff --git a/pkg/router/gateway_api_v1beta1.go b/pkg/router/gateway_api_v1beta1.go index 198fe991..e7bd1cf0 100644 --- a/pkg/router/gateway_api_v1beta1.go +++ b/pkg/router/gateway_api_v1beta1.go @@ -424,7 +424,7 @@ func (gwr *GatewayAPIV1Beta1Router) getSessionAffinityRouteRules(canary *flagger Add: []v1beta1.HTTPHeader{ { Name: setCookieHeader, - Value: canary.Spec.Analysis.SessionAffinity.BuildCookie(canary.Status.SessionAffinityCookie), + Value: canary.Spec.Analysis.SessionAffinity.BuildCookie(canary.Status.SessionAffinityCookie, canary.Spec.Analysis.SessionAffinity.GetMaxAge()), }, }, }, diff --git a/pkg/router/istio.go b/pkg/router/istio.go index c588847f..6a63c2bc 100644 --- a/pkg/router/istio.go +++ b/pkg/router/istio.go @@ -49,7 +49,6 @@ type IstioRouter struct { const cookieHeader = "Cookie" const setCookieHeader = "Set-Cookie" -const stickyRouteName = "sticky-route" const maxAgeAttr = "Max-Age" var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") @@ -182,8 +181,8 @@ func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error { // create destinations with primary weight 100% and canary weight 0% canaryRoute := []istiov1beta1.HTTPRouteDestination{ - makeDestination(canary, primaryName, 100), - makeDestination(canary, canaryName, 0), + ir.makeDestination(canary, primaryName, 100), + ir.makeDestination(canary, canaryName, 0), } if canary.Spec.Service.Delegation { @@ -255,7 +254,7 @@ func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error { CorsPolicy: canary.Spec.Service.CorsPolicy, Headers: canary.Spec.Service.Headers, Route: []istiov1beta1.HTTPRouteDestination{ - makeDestination(canary, primaryName, 100), + ir.makeDestination(canary, primaryName, 100), }, }, } @@ -304,15 +303,21 @@ func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error { cmpopts.IgnoreFields(istiov1beta1.HTTPRoute{}, "Mirror", "MirrorPercentage"), } if canary.Spec.Analysis.SessionAffinity != nil { - // We ignore this route as this does not do weighted routing and is handled exclusively - // by SetRoutes(). - ignoreSlice := cmpopts.IgnoreSliceElements(func(t istiov1beta1.HTTPRoute) bool { - if t.Name == stickyRouteName { - return true + ignoreCookieRouteFunc := func(name string) func(r istiov1beta1.HTTPRoute) bool { + return func(r istiov1beta1.HTTPRoute) bool { + // Ignore the rule that does sticky routing, i.e. matches against the `Cookie` header. + for _, match := range r.Match { + if strings.Contains(match.Headers[cookieHeader].Regex, name) { + return true + } + } + return false } - return false - }) - ignoreCmpOptions = append(ignoreCmpOptions, ignoreSlice) + } + ignoreCanaryRoute := cmpopts.IgnoreSliceElements(ignoreCookieRouteFunc(canary.Spec.Analysis.SessionAffinity.CookieName)) + ignorePrimaryRoute := cmpopts.IgnoreSliceElements(ignoreCookieRouteFunc(canary.Spec.Analysis.SessionAffinity.PrimaryCookieName)) + + ignoreCmpOptions = append(ignoreCmpOptions, ignoreCanaryRoute, ignorePrimaryRoute) ignoreCmpOptions = append(ignoreCmpOptions, cmpopts.IgnoreFields(istiov1beta1.HTTPRouteDestination{}, "Headers")) } if v, ok := virtualService.Annotations[kubectlAnnotation]; ok { @@ -481,8 +486,8 @@ func (ir *IstioRouter) SetRoutes( weightedRoute := istiov1beta1.TCPRoute{ Match: canaryToL4Match(canary), Route: []istiov1beta1.HTTPRouteDestination{ - makeDestination(canary, primaryName, primaryWeight), - makeDestination(canary, canaryName, canaryWeight), + ir.makeDestination(canary, primaryName, primaryWeight), + ir.makeDestination(canary, canaryName, canaryWeight), }, } vsCopy.Spec.Tcp = []istiov1beta1.TCPRoute{ @@ -497,7 +502,7 @@ func (ir *IstioRouter) SetRoutes( } // weighted routing (progressive canary) - weightedRoute := istiov1beta1.HTTPRoute{ + weightedRoute := &istiov1beta1.HTTPRoute{ Match: canary.Spec.Service.Match, Rewrite: canary.Spec.Service.GetIstioRewrite(), Timeout: canary.Spec.Service.Timeout, @@ -505,94 +510,20 @@ func (ir *IstioRouter) SetRoutes( CorsPolicy: canary.Spec.Service.CorsPolicy, Headers: canary.Spec.Service.Headers, Route: []istiov1beta1.HTTPRouteDestination{ - makeDestination(canary, primaryName, primaryWeight), - makeDestination(canary, canaryName, canaryWeight), + ir.makeDestination(canary, primaryName, primaryWeight), + ir.makeDestination(canary, canaryName, canaryWeight), }, } vsCopy.Spec.Http = []istiov1beta1.HTTPRoute{ - weightedRoute, + *weightedRoute, } if canary.Spec.Analysis.SessionAffinity != nil { - // 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 - // and match the value of the `Set-Cookie` header will be routed to the canary deployment. - stickyRoute := weightedRoute - stickyRoute.Name = stickyRouteName - if canaryWeight != 0 { - if canary.Status.SessionAffinityCookie == "" { - canary.Status.SessionAffinityCookie = fmt.Sprintf("%s=%s", canary.Spec.Analysis.SessionAffinity.CookieName, randSeq()) - } - - for i, routeDest := range weightedRoute.Route { - if routeDest.Destination.Host == canaryName { - if routeDest.Headers == nil { - routeDest.Headers = &istiov1beta1.Headers{ - Response: &istiov1beta1.HeaderOperations{}, - } - } - routeDest.Headers.Response.Add = map[string]string{ - setCookieHeader: canary.Spec.Analysis.SessionAffinity.BuildCookie(canary.Status.SessionAffinityCookie), - } - } - weightedRoute.Route[i] = routeDest - } - - cookieKeyAndVal := strings.Split(canary.Status.SessionAffinityCookie, "=") - cookieMatch := istiov1beta1.HTTPMatchRequest{ - Headers: map[string]istiov1alpha1.StringMatch{ - cookieHeader: { - Regex: fmt.Sprintf(".*%s.*%s.*", cookieKeyAndVal[0], cookieKeyAndVal[1]), - }, - }, - } - canaryMatch := mergeMatchConditions([]istiov1beta1.HTTPMatchRequest{cookieMatch}, canary.Spec.Service.Match) - stickyRoute.Match = canaryMatch - stickyRoute.Route = []istiov1beta1.HTTPRouteDestination{ - makeDestination(canary, primaryName, 0), - makeDestination(canary, canaryName, 100), - } - } else { - // If canary weight is 0 and SessionAffinityCookie is non-blank, then it belongs to a previous canary run. - if canary.Status.SessionAffinityCookie != "" { - canary.Status.PreviousSessionAffinityCookie = canary.Status.SessionAffinityCookie - } - previousCookie := canary.Status.PreviousSessionAffinityCookie - - // Match against the previous session cookie and delete that cookie - if previousCookie != "" { - cookieKeyAndVal := strings.Split(previousCookie, "=") - cookieMatch := istiov1beta1.HTTPMatchRequest{ - Headers: map[string]istiov1alpha1.StringMatch{ - cookieHeader: { - Regex: fmt.Sprintf(".*%s.*%s.*", cookieKeyAndVal[0], cookieKeyAndVal[1]), - }, - }, - } - canaryMatch := mergeMatchConditions([]istiov1beta1.HTTPMatchRequest{cookieMatch}, canary.Spec.Service.Match) - stickyRoute.Match = canaryMatch - - if stickyRoute.Headers == nil { - stickyRoute.Headers = &istiov1beta1.Headers{ - Response: &istiov1beta1.HeaderOperations{ - Add: map[string]string{}, - }, - } - } else if stickyRoute.Headers.Response == nil { - stickyRoute.Headers.Response = &istiov1beta1.HeaderOperations{ - Add: map[string]string{}, - } - } else if stickyRoute.Headers.Response.Add == nil { - stickyRoute.Headers.Response.Add = map[string]string{} - } - stickyRoute.Headers.Response.Add[setCookieHeader] = fmt.Sprintf("%s; %s=%d", previousCookie, maxAgeAttr, -1) - } - - canary.Status.SessionAffinityCookie = "" - } - vsCopy.Spec.Http = []istiov1beta1.HTTPRoute{ - stickyRoute, weightedRoute, + rules, err := ir.getSessionAffinityRouteRules(canary, canaryWeight, weightedRoute) + if err != nil { + return err } + vsCopy.Spec.Http = rules } if mirrored { @@ -618,8 +549,8 @@ func (ir *IstioRouter) SetRoutes( CorsPolicy: canary.Spec.Service.CorsPolicy, Headers: canary.Spec.Service.Headers, Route: []istiov1beta1.HTTPRouteDestination{ - makeDestination(canary, primaryName, primaryWeight), - makeDestination(canary, canaryName, canaryWeight), + ir.makeDestination(canary, primaryName, primaryWeight), + ir.makeDestination(canary, canaryName, canaryWeight), }, }, { @@ -630,7 +561,7 @@ func (ir *IstioRouter) SetRoutes( CorsPolicy: canary.Spec.Service.CorsPolicy, Headers: canary.Spec.Service.Headers, Route: []istiov1beta1.HTTPRouteDestination{ - makeDestination(canary, primaryName, primaryWeight), + ir.makeDestination(canary, primaryName, primaryWeight), }, }, } @@ -705,7 +636,7 @@ func mergeMatchConditions(canary, defaults []istiov1beta1.HTTPMatchRequest) []is } // makeDestination returns a an destination weight for the specified host -func makeDestination(canary *flaggerv1.Canary, host string, weight int) istiov1beta1.HTTPRouteDestination { +func (ir *IstioRouter) makeDestination(canary *flaggerv1.Canary, host string, weight int) istiov1beta1.HTTPRouteDestination { dest := istiov1beta1.HTTPRouteDestination{ Destination: istiov1beta1.Destination{ Host: host, @@ -740,3 +671,144 @@ func randSeq() string { } return string(b) } + +func getRouteByServiceName(rule *istiov1beta1.HTTPRoute, svcName string) *istiov1beta1.HTTPRouteDestination { + for i, routeDest := range rule.Route { + if routeDest.Destination.Host == svcName { + return &rule.Route[i] + } + } + return nil +} + +// getSessionAffinityRouteRules returns the HTTPRoute objects required to perform +// session affinity based Canary releases. +func (ir *IstioRouter) getSessionAffinityRouteRules(canary *flaggerv1.Canary, canaryWeight int, + weightedRoute *istiov1beta1.HTTPRoute) ([]istiov1beta1.HTTPRoute, error) { + _, primaryName, canaryName := canary.GetServiceNames() + stickyCanaryRoute := *weightedRoute + stickyPrimaryRoute := *weightedRoute + + // 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 + // and match the value of the `Set-Cookie` header will be routed to the canary deployment. + if canaryWeight != 0 { + // if the status doesn't have the canary cookie, then generate a new canary cookie. + if canary.Status.SessionAffinityCookie == "" { + canary.Status.SessionAffinityCookie = fmt.Sprintf("%s=%s", canary.Spec.Analysis.SessionAffinity.CookieName, randSeq()) + } + // if the status doesn't have the primary cookie, then generate a new primary cookie. + if canary.Status.PrimarySessionAffinityCookie == "" { + canary.Status.PrimarySessionAffinityCookie = fmt.Sprintf("%s=%s", canary.Spec.Analysis.SessionAffinity.PrimaryCookieName, randSeq()) + } + + // add response modifier to the canary backend route in the rule that does weighted routing + // to include the canary cookie. + canaryBackendRoute := getRouteByServiceName(weightedRoute, canaryName) + if canaryBackendRoute.Headers == nil { + canaryBackendRoute.Headers = &istiov1beta1.Headers{ + Response: &istiov1beta1.HeaderOperations{}, + } + } + canaryBackendRoute.Headers.Response.Add = map[string]string{ + setCookieHeader: canary.Spec.Analysis.SessionAffinity.BuildCookie(canary.Status.SessionAffinityCookie, canary.Spec.Analysis.SessionAffinity.GetMaxAge()), + } + + // add response modifier to the primary backend route in the rule that does weighted routing + // to include the primary cookie, only if a primary cookie name has been specified. + if canary.Spec.Analysis.SessionAffinity.PrimaryCookieName != "" { + primaryBackendRoute := getRouteByServiceName(weightedRoute, primaryName) + interval, err := time.ParseDuration(canary.Spec.Analysis.Interval) + if err != nil { + return nil, fmt.Errorf("failed to parse canary interval: %w", err) + } + if primaryBackendRoute.Headers == nil { + primaryBackendRoute.Headers = &istiov1beta1.Headers{ + Response: &istiov1beta1.HeaderOperations{}, + } + } + primaryBackendRoute.Headers.Response.Add = map[string]string{ + setCookieHeader: canary.Spec.Analysis.SessionAffinity.BuildCookie(canary.Status.PrimarySessionAffinityCookie, int(interval.Seconds())), + } + } + + // configure the sticky canary rule to match against requests that match against the + // canary cookie and send them to the canary backend. + cookieKeyAndVal := strings.Split(canary.Status.SessionAffinityCookie, "=") + cookieMatch := istiov1beta1.HTTPMatchRequest{ + Headers: map[string]istiov1alpha1.StringMatch{ + cookieHeader: { + Regex: fmt.Sprintf(".*%s.*%s.*", cookieKeyAndVal[0], cookieKeyAndVal[1]), + }, + }, + } + canaryMatch := mergeMatchConditions([]istiov1beta1.HTTPMatchRequest{cookieMatch}, canary.Spec.Service.Match) + stickyCanaryRoute.Match = canaryMatch + stickyCanaryRoute.Route = []istiov1beta1.HTTPRouteDestination{ + ir.makeDestination(canary, primaryName, 0), + ir.makeDestination(canary, canaryName, 100), + } + + // configure the sticky primary rule to match against requests that match against the + // primary cookie and send them to the primary backend, only if a primary cookie name has + // been specified. + if canary.Spec.Analysis.SessionAffinity.PrimaryCookieName != "" { + cookieKeyAndVal := strings.Split(canary.Status.PrimarySessionAffinityCookie, "=") + primaryCookieMatch := istiov1beta1.HTTPMatchRequest{ + Headers: map[string]istiov1alpha1.StringMatch{ + cookieHeader: { + Regex: fmt.Sprintf(".*%s.*%s.*", cookieKeyAndVal[0], cookieKeyAndVal[1]), + }, + }, + } + primaryMatch := mergeMatchConditions([]istiov1beta1.HTTPMatchRequest{primaryCookieMatch}, canary.Spec.Service.Match) + stickyPrimaryRoute.Match = primaryMatch + stickyPrimaryRoute.Route = []istiov1beta1.HTTPRouteDestination{ + ir.makeDestination(canary, primaryName, 100), + ir.makeDestination(canary, canaryName, 0), + } + + return []istiov1beta1.HTTPRoute{stickyCanaryRoute, stickyPrimaryRoute, *weightedRoute}, nil + } + + return []istiov1beta1.HTTPRoute{stickyCanaryRoute, *weightedRoute}, nil + } else { + // If canary weight is 0 and SessionAffinityCookie is non-blank, then it belongs to a previous canary run. + if canary.Status.SessionAffinityCookie != "" { + canary.Status.PreviousSessionAffinityCookie = canary.Status.SessionAffinityCookie + } + previousCookie := canary.Status.PreviousSessionAffinityCookie + + // Match against the previous session cookie and delete that cookie + if previousCookie != "" { + cookieKeyAndVal := strings.Split(previousCookie, "=") + cookieMatch := istiov1beta1.HTTPMatchRequest{ + Headers: map[string]istiov1alpha1.StringMatch{ + cookieHeader: { + Regex: fmt.Sprintf(".*%s.*%s.*", cookieKeyAndVal[0], cookieKeyAndVal[1]), + }, + }, + } + canaryMatch := mergeMatchConditions([]istiov1beta1.HTTPMatchRequest{cookieMatch}, canary.Spec.Service.Match) + stickyCanaryRoute.Match = canaryMatch + + if stickyCanaryRoute.Headers == nil { + stickyCanaryRoute.Headers = &istiov1beta1.Headers{ + Response: &istiov1beta1.HeaderOperations{ + Add: map[string]string{}, + }, + } + } else if stickyCanaryRoute.Headers.Response == nil { + stickyCanaryRoute.Headers.Response = &istiov1beta1.HeaderOperations{ + Add: map[string]string{}, + } + } else if stickyCanaryRoute.Headers.Response.Add == nil { + stickyCanaryRoute.Headers.Response.Add = map[string]string{} + } + stickyCanaryRoute.Headers.Response.Add[setCookieHeader] = fmt.Sprintf("%s; %s=%d", previousCookie, maxAgeAttr, -1) + } + + canary.Status.SessionAffinityCookie = "" + return []istiov1beta1.HTTPRoute{stickyCanaryRoute, *weightedRoute}, nil + } +} diff --git a/pkg/router/istio_test.go b/pkg/router/istio_test.go index fb2ce003..1c2423bd 100644 --- a/pkg/router/istio_test.go +++ b/pkg/router/istio_test.go @@ -22,6 +22,7 @@ import ( "fmt" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" @@ -404,6 +405,175 @@ func TestIstioRouter_SetRoutes(t *testing.T) { }) } +func TestIstioRouter_getSessionAffinityRoutes(t *testing.T) { + mocks := newFixture(nil) + t.Run("without primary cookie", func(t *testing.T) { + canary := mocks.canary.DeepCopy() + cookieKey := "flagger-cookie" + canary.Spec.Analysis.SessionAffinity = &flaggerv1.SessionAffinity{ + CookieName: cookieKey, + MaxAge: 300, + } + + router := &IstioRouter{ + logger: mocks.logger, + flaggerClient: mocks.flaggerClient, + istioClient: mocks.meshClient, + kubeClient: mocks.kubeClient, + } + + _, pSvcName, cSvcName := canary.GetServiceNames() + weightedRoute := &istiov1beta1.HTTPRoute{ + Route: []istiov1beta1.HTTPRouteDestination{ + router.makeDestination(canary, pSvcName, 100), + router.makeDestination(canary, cSvcName, 0), + }, + } + rules, err := router.getSessionAffinityRouteRules(canary, 10, weightedRoute) + require.NoError(t, err) + assert.Equal(t, len(rules), 2) + assert.True(t, strings.HasPrefix(canary.Status.SessionAffinityCookie, cookieKey)) + + stickyRule := rules[0] + cookieMatch := stickyRule.Match[0].Headers[cookieHeader] + assert.NotNil(t, cookieMatch.Regex) + assert.Contains(t, cookieMatch.Regex, cookieKey) + + assert.Equal(t, len(stickyRule.Route), 2) + for _, route := range stickyRule.Route { + if string(route.Destination.Host) == pSvcName { + assert.Equal(t, route.Weight, int(0)) + } + if string(route.Destination.Host) == cSvcName { + assert.Equal(t, route.Weight, int(100)) + } + } + + weightedRule := rules[1] + var found bool + for _, route := range weightedRule.Route { + if string(route.Destination.Host) == cSvcName { + found = true + + assert.NotNil(t, route.Headers.Response.Add) + assert.Equal(t, route.Headers.Response.Add[setCookieHeader], fmt.Sprintf("%s; %s=%d", canary.Status.SessionAffinityCookie, maxAgeAttr, 300)) + } + } + assert.True(t, found) + + rules, err = router.getSessionAffinityRouteRules(canary, 0, weightedRoute) + require.NoError(t, err) + assert.Empty(t, canary.Status.SessionAffinityCookie) + assert.Contains(t, canary.Status.PreviousSessionAffinityCookie, cookieKey) + + stickyRule = rules[0] + cookieMatch = stickyRule.Match[0].Headers[cookieHeader] + assert.NotNil(t, cookieMatch.Regex) + assert.Contains(t, cookieMatch.Regex, cookieKey) + + assert.NotNil(t, stickyRule.Headers.Response.Add) + assert.Equal(t, stickyRule.Headers.Response.Add[setCookieHeader], fmt.Sprintf("%s; %s=%d", canary.Status.PreviousSessionAffinityCookie, maxAgeAttr, -1)) + }) + + t.Run("with primary cookie", func(t *testing.T) { + canary := mocks.canary.DeepCopy() + mocks := newFixture(canary) + canaryCookieKey := "canary-flagger-cookie" + primaryCookieKey := "primary-flagger-cookie" + canary.Spec.Analysis.Interval = "15s" + canary.Spec.Analysis.SessionAffinity = &flaggerv1.SessionAffinity{ + CookieName: canaryCookieKey, + PrimaryCookieName: primaryCookieKey, + MaxAge: 300, + } + + router := &IstioRouter{ + logger: mocks.logger, + flaggerClient: mocks.flaggerClient, + istioClient: mocks.meshClient, + kubeClient: mocks.kubeClient, + } + + _, pSvcName, cSvcName := canary.GetServiceNames() + weightedRoute := &istiov1beta1.HTTPRoute{ + Route: []istiov1beta1.HTTPRouteDestination{ + router.makeDestination(canary, pSvcName, 100), + router.makeDestination(canary, cSvcName, 0), + }, + } + rules, err := router.getSessionAffinityRouteRules(canary, 10, weightedRoute) + require.NoError(t, err) + assert.Equal(t, len(rules), 3) + assert.True(t, strings.HasPrefix(canary.Status.SessionAffinityCookie, canaryCookieKey)) + + canaryStickyRule := rules[0] + cookieMatch := canaryStickyRule.Match[0].Headers[cookieHeader] + assert.NotNil(t, cookieMatch.Regex) + assert.Contains(t, cookieMatch.Regex, canaryCookieKey) + + assert.Equal(t, len(canaryStickyRule.Route), 2) + for _, route := range canaryStickyRule.Route { + if string(route.Destination.Host) == pSvcName { + assert.Equal(t, route.Weight, int(0)) + } + if string(route.Destination.Host) == cSvcName { + assert.Equal(t, route.Weight, int(100)) + } + } + + primaryStickyRule := rules[1] + cookieMatch = primaryStickyRule.Match[0].Headers[cookieHeader] + assert.NotNil(t, cookieMatch.Regex) + assert.Contains(t, cookieMatch.Regex, primaryCookieKey) + + assert.Equal(t, len(primaryStickyRule.Route), 2) + for _, route := range primaryStickyRule.Route { + if string(route.Destination.Host) == pSvcName { + assert.Equal(t, route.Weight, int(100)) + } + if string(route.Destination.Host) == cSvcName { + assert.Equal(t, route.Weight, int(0)) + } + } + + weightedRule := rules[2] + var c int + for _, route := range weightedRule.Route { + if string(route.Destination.Host) == cSvcName { + c += 1 + + assert.NotNil(t, route.Headers.Response.Add) + assert.Equal(t, route.Headers.Response.Add[setCookieHeader], fmt.Sprintf("%s; %s=%d", canary.Status.SessionAffinityCookie, maxAgeAttr, 300)) + } + + if string(route.Destination.Host) == pSvcName { + c += 1 + + assert.NotNil(t, route.Headers.Response.Add) + assert.Contains(t, route.Headers.Response.Add[setCookieHeader], canary.Status.PrimarySessionAffinityCookie) + + interval, err := time.ParseDuration(canary.Spec.Analysis.Interval) + require.NoError(t, err) + assert.Contains(t, route.Headers.Response.Add[setCookieHeader], fmt.Sprintf("%s=%d", maxAgeAttr, int(interval.Seconds()))) + } + } + assert.Equal(t, 2, c) + + rules, err = router.getSessionAffinityRouteRules(canary, 0, weightedRoute) + require.NoError(t, err) + assert.Empty(t, canary.Status.SessionAffinityCookie) + assert.Contains(t, canary.Status.PreviousSessionAffinityCookie, canaryCookieKey) + + canaryStickyRule = rules[0] + cookieMatch = canaryStickyRule.Match[0].Headers[cookieHeader] + assert.NotNil(t, cookieMatch.Regex) + assert.Contains(t, cookieMatch.Regex, canaryCookieKey) + + assert.NotNil(t, canaryStickyRule.Headers.Response.Add) + assert.Equal(t, canaryStickyRule.Headers.Response.Add[setCookieHeader], fmt.Sprintf("%s; %s=%d", canary.Status.PreviousSessionAffinityCookie, maxAgeAttr, -1)) + }) +} + func TestIstioRouter_GetRoutes(t *testing.T) { mocks := newFixture(nil) router := &IstioRouter{ diff --git a/test/gatewayapi/test-session-affinity.sh b/test/gatewayapi/test-session-affinity.sh index d066014e..88c6493c 100755 --- a/test/gatewayapi/test-session-affinity.sh +++ b/test/gatewayapi/test-session-affinity.sh @@ -107,7 +107,7 @@ done echo '>>> Verifying session affinity' if ! URL=http://localhost:8888 HOST=www.example.com CANARY_VERSION=6.1.0 \ CANARY_COOKIE_NAME=canary-flagger-cookie PRIMARY_VERSION=6.0.4 PRIMARY_COOKIE_NAME=primary-flagger-cookie \ - go run ${REPO_ROOT}/test/gatewayapi/verify_session_affinity.go; then + go run ${REPO_ROOT}/test/verify_session_affinity.go; then echo "failed to verify session affinity" exit $? fi diff --git a/test/istio/run.sh b/test/istio/run.sh index b2a3301a..26234e3e 100755 --- a/test/istio/run.sh +++ b/test/istio/run.sh @@ -15,3 +15,6 @@ DIR="$(cd "$(dirname "$0")" && pwd)" "$REPO_ROOT"/test/workloads/init.sh "$DIR"/test-delegation.sh + +"$REPO_ROOT"/test/workloads/init.sh +"$DIR"/test-session-affinity.sh \ No newline at end of file diff --git a/test/istio/test-session-affinity.sh b/test/istio/test-session-affinity.sh new file mode 100755 index 00000000..377f3821 --- /dev/null +++ b/test/istio/test-session-affinity.sh @@ -0,0 +1,174 @@ +#!/usr/bin/env bash + +# This script runs e2e tests for progressive traffic shifting with session affinity, Canary analysis and promotion +# Prerequisites: Kubernetes Kind and Istio + +set -o errexit + +REPO_ROOT=$(git rev-parse --show-toplevel) + +echo '>>> Initialising Gateway' +cat <>> Initialising canary for session affinity' +cat <>> Waiting for primary to be ready' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test get canary/podinfo | grep 'Initialized' && ok=true || ok=false + sleep 5 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n istio-system logs deployment/flagger + echo "No more retries left" + exit 1 + fi +done + +echo '✔ Canary initialization test passed' + +echo '>>> Port forwarding load balancer' +INGRESS_GATEWAY_POD=$(kubectl get pods -n istio-system -l app=istio-ingressgateway -o name | awk -F'/' '{print $2}') +kubectl port-forward -n istio-system "$INGRESS_GATEWAY_POD" 8080:8080 2>&1 > /dev/null & +pf_pid=$! + +cleanup() { + echo ">> Killing port forward process ${pf_pid}" + kill -9 $pf_pid +} +trap "cleanup" EXIT SIGINT + +echo '>>> Triggering canary deployment' +kubectl -n test set image deployment/podinfo podinfod=ghcr.io/stefanprodan/podinfo:6.0.1 + +echo '>>> Waiting for initial traffic shift' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test get canary podinfo -o=jsonpath='{.status.canaryWeight}' | grep '10' && ok=true || ok=false + sleep 5 + kubectl -n istio-system logs deployment/flagger --tail 1 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n istio-system logs deployment/flagger + echo "No more retries left" + exit 1 + fi +done + +echo '>>> Verifying session affinity' +if ! URL=http://localhost:8080 HOST=localhost CANARY_VERSION=6.0.1 \ + CANARY_COOKIE_NAME=canary-flagger-cookie PRIMARY_VERSION=6.0.0 PRIMARY_COOKIE_NAME=primary-flagger-cookie \ + go run ${REPO_ROOT}/test/verify_session_affinity.go; then + echo "failed to verify session affinity" + exit $? +fi + +echo '>>> Waiting for canary promotion' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test describe deployment/podinfo-primary | grep '6.0.1' && ok=true || ok=false + sleep 10 + kubectl -n istio-system logs deployment/flagger --tail 1 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n istio-system logs deployment/flagger + echo "No more retries left" + exit 1 + fi +done + +echo '>>> Waiting for canary finalization' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test get canary/podinfo | grep 'Succeeded' && ok=true || ok=false + sleep 5 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n istio-system logs deployment/flagger + echo "No more retries left" + exit 1 + fi +done + +echo '>>> Verifying cookie cleanup' +canary_cookie=$(kubectl -n test get canary podinfo -o=jsonpath='{.status.previousSessionAffinityCookie}' | xargs) +response=$(curl -H "Cookie: $canary_cookie" -D - http://localhost:8080) + +if [[ $response == *"$canary_cookie"* ]]; then + echo "✔ Found previous cookie in response" +else + echo "⨯ Previous cookie ${canary_cookie} not found in response" + exit 1 +fi + +if [[ $response == *"Max-Age=-1"* ]]; then + echo "✔ Found Max-Age attribute in cookie" +else + echo "⨯ Max-Age attribute not present in cookie" + exit 1 +fi + +echo '✔ Canary release with session affinity promotion test passed' + +kubectl delete -n test canary podinfo diff --git a/test/gatewayapi/verify_session_affinity.go b/test/verify_session_affinity.go similarity index 100% rename from test/gatewayapi/verify_session_affinity.go rename to test/verify_session_affinity.go