From 2e079ba7a11f6adcd018a360312c36c3f29f698c Mon Sep 17 00:00:00 2001 From: Andrew Jenkins Date: Fri, 20 Sep 2019 13:29:51 -0600 Subject: [PATCH] Add mirror to router interface and implement for istio router The mirror option will be used to tell routers to configure traffic mirroring. Implement mirror for GetRoutes and SetRoutes for Istio. For other routers, GetRoutes always returns mirror == false, and SetRoutes ignores mirror. After this change there is no behavior change because no code sets mirror true (yet). Enhanced TestIstioRouter_SetRoutes and TestIstioRouter_GetRoutes. --- pkg/controller/scheduler.go | 14 +-- pkg/controller/scheduler_test.go | 28 ++++-- pkg/router/appmesh.go | 4 + pkg/router/appmesh_test.go | 8 +- pkg/router/gloo.go | 8 +- pkg/router/gloo_test.go | 11 ++- pkg/router/ingress.go | 9 +- pkg/router/ingress_test.go | 8 +- pkg/router/istio.go | 24 ++++- pkg/router/istio_test.go | 147 ++++++++++++++++++++++++++++--- pkg/router/nop.go | 8 +- pkg/router/router.go | 4 +- pkg/router/router_test.go | 6 ++ pkg/router/smi.go | 4 + pkg/router/supergloo.go | 8 +- pkg/router/supergloo_test.go | 11 ++- 16 files changed, 251 insertions(+), 51 deletions(-) diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go index d3d0294a..035e7aa4 100644 --- a/pkg/controller/scheduler.go +++ b/pkg/controller/scheduler.go @@ -155,7 +155,7 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh // check if virtual service exists // and if it contains weighted destination routes to the primary and canary services - primaryWeight, canaryWeight, err := meshRouter.GetRoutes(cd) + primaryWeight, canaryWeight, mirrored, err := meshRouter.GetRoutes(cd) if err != nil { c.recordEventWarningf(cd, "%v", err) return @@ -176,7 +176,7 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh // route all traffic back to primary primaryWeight = 100 canaryWeight = 0 - if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight); err != nil { + if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight, false); err != nil { c.recordEventWarningf(cd, "%v", err) return } @@ -218,7 +218,7 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh if cd.Status.Phase == flaggerv1.CanaryPhasePromoting { if provider != "kubernetes" { c.recordEventInfof(cd, "Routing all traffic to primary") - if err := meshRouter.SetRoutes(cd, 100, 0); err != nil { + if err := meshRouter.SetRoutes(cd, 100, 0, false); err != nil { c.recordEventWarningf(cd, "%v", err) return } @@ -275,7 +275,7 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh // route all traffic back to primary primaryWeight = 100 canaryWeight = 0 - if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight); err != nil { + if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight, false); err != nil { c.recordEventWarningf(cd, "%v", err) return } @@ -328,7 +328,7 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh if len(cd.Spec.CanaryAnalysis.Match) > 0 && cd.Spec.CanaryAnalysis.Iterations > 0 { // route traffic to canary and increment iterations if cd.Spec.CanaryAnalysis.Iterations > cd.Status.Iterations { - if err := meshRouter.SetRoutes(cd, 0, 100); err != nil { + if err := meshRouter.SetRoutes(cd, 0, 100, false); err != nil { c.recordEventWarningf(cd, "%v", err) return } @@ -390,7 +390,7 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh if cd.Spec.CanaryAnalysis.Iterations == cd.Status.Iterations { if provider != "kubernetes" { c.recordEventInfof(cd, "Routing all traffic to canary") - if err := meshRouter.SetRoutes(cd, 0, 100); err != nil { + if err := meshRouter.SetRoutes(cd, 0, 100, false); err != nil { c.recordEventWarningf(cd, "%v", err) return } @@ -489,7 +489,7 @@ func (c *Controller) shouldSkipAnalysis(cd *flaggerv1.Canary, meshRouter router. // route all traffic to primary primaryWeight = 100 canaryWeight = 0 - if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight); err != nil { + if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight, false); err != nil { c.recordEventWarningf(cd, "%v", err) return false } diff --git a/pkg/controller/scheduler_test.go b/pkg/controller/scheduler_test.go index f48deedd..b40e81ec 100644 --- a/pkg/controller/scheduler_test.go +++ b/pkg/controller/scheduler_test.go @@ -123,7 +123,7 @@ func TestScheduler_NewRevisionReset(t *testing.T) { // advance mocks.ctrl.advanceCanary("podinfo", "default", true) - primaryWeight, canaryWeight, err := mocks.router.GetRoutes(mocks.canary) + primaryWeight, canaryWeight, mirrored, err := mocks.router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } @@ -136,6 +136,10 @@ func TestScheduler_NewRevisionReset(t *testing.T) { t.Errorf("Got canary route %v wanted %v", canaryWeight, 10) } + if mirrored != false { + t.Errorf("Got mirrored %v wanted %v", mirrored, false) + } + // second update dep2.Spec.Template.Spec.ServiceAccountName = "test" _, err = mocks.kubeClient.AppsV1().Deployments("default").Update(dep2) @@ -146,7 +150,7 @@ func TestScheduler_NewRevisionReset(t *testing.T) { // detect changes mocks.ctrl.advanceCanary("podinfo", "default", true) - primaryWeight, canaryWeight, err = mocks.router.GetRoutes(mocks.canary) + primaryWeight, canaryWeight, mirrored, err = mocks.router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } @@ -158,6 +162,10 @@ func TestScheduler_NewRevisionReset(t *testing.T) { if canaryWeight != 0 { t.Errorf("Got canary route %v wanted %v", canaryWeight, 0) } + + if mirrored != false { + t.Errorf("Got mirrored %v wanted %v", mirrored, false) + } } func TestScheduler_Promotion(t *testing.T) { @@ -201,14 +209,14 @@ func TestScheduler_Promotion(t *testing.T) { // detect configs changes mocks.ctrl.advanceCanary("podinfo", "default", true) - primaryWeight, canaryWeight, err := mocks.router.GetRoutes(mocks.canary) + primaryWeight, canaryWeight, mirrored, err := mocks.router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } primaryWeight = 60 canaryWeight = 40 - err = mocks.router.SetRoutes(mocks.canary, primaryWeight, canaryWeight) + err = mocks.router.SetRoutes(mocks.canary, primaryWeight, canaryWeight, mirrored) if err != nil { t.Fatal(err.Error()) } @@ -242,7 +250,7 @@ func TestScheduler_Promotion(t *testing.T) { // finalise mocks.ctrl.advanceCanary("podinfo", "default", true) - primaryWeight, canaryWeight, err = mocks.router.GetRoutes(mocks.canary) + primaryWeight, canaryWeight, mirrored, err = mocks.router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } @@ -255,6 +263,10 @@ func TestScheduler_Promotion(t *testing.T) { t.Errorf("Got canary route %v wanted %v", canaryWeight, 0) } + if mirrored != false { + t.Errorf("Got mirrored %v wanted %v", mirrored, false) + } + primaryDep, err := mocks.kubeClient.AppsV1().Deployments("default").Get("podinfo-primary", metav1.GetOptions{}) if err != nil { t.Fatal(err.Error()) @@ -326,7 +338,7 @@ func TestScheduler_ABTesting(t *testing.T) { mocks.ctrl.advanceCanary("podinfo", "default", true) // check if traffic is routed to canary - primaryWeight, canaryWeight, err := mocks.router.GetRoutes(mocks.canary) + primaryWeight, canaryWeight, mirrored, err := mocks.router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } @@ -339,6 +351,10 @@ func TestScheduler_ABTesting(t *testing.T) { t.Errorf("Got canary route %v wanted %v", canaryWeight, 100) } + if mirrored != false { + t.Errorf("Got mirrored %v wanted %v", mirrored, false) + } + cd, err := mocks.flaggerClient.FlaggerV1alpha3().Canaries("default").Get("podinfo", metav1.GetOptions{}) if err != nil { t.Fatal(err.Error()) diff --git a/pkg/router/appmesh.go b/pkg/router/appmesh.go index c8b36ae0..f25bce9e 100644 --- a/pkg/router/appmesh.go +++ b/pkg/router/appmesh.go @@ -252,6 +252,7 @@ func (ar *AppMeshRouter) reconcileVirtualService(canary *flaggerv1.Canary, name func (ar *AppMeshRouter) GetRoutes(canary *flaggerv1.Canary) ( primaryWeight int, canaryWeight int, + mirrored bool, err error, ) { targetName := canary.Spec.TargetRef.Name @@ -286,6 +287,8 @@ func (ar *AppMeshRouter) GetRoutes(canary *flaggerv1.Canary) ( vsName, targetName, targetName) } + mirrored = false + return } @@ -294,6 +297,7 @@ func (ar *AppMeshRouter) SetRoutes( canary *flaggerv1.Canary, primaryWeight int, canaryWeight int, + mirrored bool, ) error { targetName := canary.Spec.TargetRef.Name vsName := fmt.Sprintf("%s.%s", targetName, canary.Namespace) diff --git a/pkg/router/appmesh_test.go b/pkg/router/appmesh_test.go index 0fbed858..d447a0dd 100644 --- a/pkg/router/appmesh_test.go +++ b/pkg/router/appmesh_test.go @@ -144,12 +144,12 @@ func TestAppmeshRouter_GetSetRoutes(t *testing.T) { t.Fatal(err.Error()) } - err = router.SetRoutes(mocks.appmeshCanary, 60, 40) + err = router.SetRoutes(mocks.appmeshCanary, 60, 40, false) if err != nil { t.Fatal(err.Error()) } - p, c, err := router.GetRoutes(mocks.appmeshCanary) + p, c, m, err := router.GetRoutes(mocks.appmeshCanary) if err != nil { t.Fatal(err.Error()) } @@ -161,4 +161,8 @@ func TestAppmeshRouter_GetSetRoutes(t *testing.T) { if c != 40 { t.Errorf("Got canary weight %v wanted %v", c, 40) } + + if m != false { + t.Errorf("Got mirror %v wanted %v", m, false) + } } diff --git a/pkg/router/gloo.go b/pkg/router/gloo.go index 7cf735c4..6f854fea 100644 --- a/pkg/router/gloo.go +++ b/pkg/router/gloo.go @@ -63,11 +63,11 @@ func NewGlooRouterWithClient(ctx context.Context, routingRuleClient gloov1.Upstr // Reconcile creates or updates the Istio virtual service func (gr *GlooRouter) Reconcile(canary *flaggerv1.Canary) error { // do we have routes already? - if _, _, err := gr.GetRoutes(canary); err == nil { + if _, _, _, err := gr.GetRoutes(canary); err == nil { // we have routes, no need to do anything else return nil } else if solokiterror.IsNotExist(err) { - return gr.SetRoutes(canary, 100, 0) + return gr.SetRoutes(canary, 100, 0, false) } else { return err } @@ -77,6 +77,7 @@ func (gr *GlooRouter) Reconcile(canary *flaggerv1.Canary) error { func (gr *GlooRouter) GetRoutes(canary *flaggerv1.Canary) ( primaryWeight int, canaryWeight int, + mirrored bool, err error, ) { targetName := canary.Spec.TargetRef.Name @@ -101,6 +102,8 @@ func (gr *GlooRouter) GetRoutes(canary *flaggerv1.Canary) ( targetName, canary.Namespace, targetName, targetName) } + mirrored = false + return } @@ -109,6 +112,7 @@ func (gr *GlooRouter) SetRoutes( canary *flaggerv1.Canary, primaryWeight int, canaryWeight int, + mirrored bool, ) error { targetName := canary.Spec.TargetRef.Name diff --git a/pkg/router/gloo_test.go b/pkg/router/gloo_test.go index 408be347..b6ae7d30 100644 --- a/pkg/router/gloo_test.go +++ b/pkg/router/gloo_test.go @@ -68,15 +68,16 @@ func TestGlooRouter_SetRoutes(t *testing.T) { t.Fatal(err.Error()) } - p, c, err := router.GetRoutes(mocks.canary) + p, c, m, err := router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } p = 50 c = 50 + m = false - err = router.SetRoutes(mocks.canary, p, c) + err = router.SetRoutes(mocks.canary, p, c, m) if err != nil { t.Fatal(err.Error()) } @@ -127,7 +128,7 @@ func TestGlooRouter_GetRoutes(t *testing.T) { t.Fatal(err.Error()) } - p, c, err := router.GetRoutes(mocks.canary) + p, c, m, err := router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } @@ -139,4 +140,8 @@ func TestGlooRouter_GetRoutes(t *testing.T) { if c != 0 { t.Errorf("Got canary weight %v wanted %v", c, 0) } + + if m != false { + t.Errorf("Got mirror %v wanted %v", m, false) + } } diff --git a/pkg/router/ingress.go b/pkg/router/ingress.go index eff3ed71..457bc498 100644 --- a/pkg/router/ingress.go +++ b/pkg/router/ingress.go @@ -106,19 +106,20 @@ func (i *IngressRouter) Reconcile(canary *flaggerv1.Canary) error { func (i *IngressRouter) GetRoutes(canary *flaggerv1.Canary) ( primaryWeight int, canaryWeight int, + mirrored bool, err error, ) { canaryIngressName := fmt.Sprintf("%s-canary", canary.Spec.IngressRef.Name) canaryIngress, err := i.kubeClient.ExtensionsV1beta1().Ingresses(canary.Namespace).Get(canaryIngressName, metav1.GetOptions{}) if err != nil { - return 0, 0, err + return 0, 0, false, err } // A/B testing if len(canary.Spec.CanaryAnalysis.Match) > 0 { for k := range canaryIngress.Annotations { if k == i.GetAnnotationWithPrefix("canary-by-cookie") || k == i.GetAnnotationWithPrefix("canary-by-header") { - return 0, 100, nil + return 0, 100, false, nil } } } @@ -128,7 +129,7 @@ func (i *IngressRouter) GetRoutes(canary *flaggerv1.Canary) ( if k == i.GetAnnotationWithPrefix("canary-weight") { val, err := strconv.Atoi(v) if err != nil { - return 0, 0, err + return 0, 0, false, err } canaryWeight = val @@ -137,6 +138,7 @@ func (i *IngressRouter) GetRoutes(canary *flaggerv1.Canary) ( } primaryWeight = 100 - canaryWeight + mirrored = false return } @@ -144,6 +146,7 @@ func (i *IngressRouter) SetRoutes( canary *flaggerv1.Canary, primaryWeight int, canaryWeight int, + mirrored bool, ) error { canaryIngressName := fmt.Sprintf("%s-canary", canary.Spec.IngressRef.Name) canaryIngress, err := i.kubeClient.ExtensionsV1beta1().Ingresses(canary.Namespace).Get(canaryIngressName, metav1.GetOptions{}) diff --git a/pkg/router/ingress_test.go b/pkg/router/ingress_test.go index bbda1771..b37fb27b 100644 --- a/pkg/router/ingress_test.go +++ b/pkg/router/ingress_test.go @@ -56,15 +56,16 @@ func TestIngressRouter_GetSetRoutes(t *testing.T) { t.Fatal(err.Error()) } - p, c, err := router.GetRoutes(mocks.ingressCanary) + p, c, m, err := router.GetRoutes(mocks.ingressCanary) if err != nil { t.Fatal(err.Error()) } p = 50 c = 50 + m = false - err = router.SetRoutes(mocks.ingressCanary, p, c) + err = router.SetRoutes(mocks.ingressCanary, p, c, m) if err != nil { t.Fatal(err.Error()) } @@ -93,8 +94,9 @@ func TestIngressRouter_GetSetRoutes(t *testing.T) { p = 100 c = 0 + m = false - err = router.SetRoutes(mocks.ingressCanary, p, c) + err = router.SetRoutes(mocks.ingressCanary, p, c, m) if err != nil { t.Fatal(err.Error()) } diff --git a/pkg/router/istio.go b/pkg/router/istio.go index cb80a819..d00af277 100644 --- a/pkg/router/istio.go +++ b/pkg/router/istio.go @@ -101,8 +101,6 @@ func (ir *IstioRouter) reconcileDestinationRule(canary *flaggerv1.Canary, name s func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error { targetName := canary.Spec.TargetRef.Name - primaryName := fmt.Sprintf("%s-primary", targetName) - canaryName := fmt.Sprintf("%s-canary", targetName) // set hosts and add the ClusterIP service host if it doesn't exists hosts := canary.Spec.Service.Hosts @@ -133,6 +131,8 @@ func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error { } // create destinations with primary weight 100% and canary weight 0% + primaryName := fmt.Sprintf("%s-primary", targetName) + canaryName := fmt.Sprintf("%s-canary", targetName) canaryRoute := []istiov1alpha3.DestinationWeight{ makeDestination(canary, primaryName, 100), makeDestination(canary, canaryName, 0), @@ -210,9 +210,14 @@ func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error { return fmt.Errorf("VirtualService %s.%s query error %v", targetName, canary.Namespace, err) } - // update service but keep the original destination weights + // update service but keep the original destination weights and mirror if virtualService != nil { - if diff := cmp.Diff(newSpec, virtualService.Spec, cmpopts.IgnoreFields(istiov1alpha3.DestinationWeight{}, "Weight")); diff != "" { + if diff := cmp.Diff( + newSpec, + virtualService.Spec, + cmpopts.IgnoreFields(istiov1alpha3.DestinationWeight{}, "Weight"), + cmpopts.IgnoreFields(istiov1alpha3.HTTPRoute{}, "Mirror"), + ); diff != "" { vtClone := virtualService.DeepCopy() vtClone.Spec = newSpec @@ -232,6 +237,7 @@ func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error { func (ir *IstioRouter) GetRoutes(canary *flaggerv1.Canary) ( primaryWeight int, canaryWeight int, + mirrored bool, err error, ) { targetName := canary.Spec.TargetRef.Name @@ -264,6 +270,9 @@ func (ir *IstioRouter) GetRoutes(canary *flaggerv1.Canary) ( canaryWeight = route.Weight } } + if httpRoute.Mirror != nil && httpRoute.Mirror.Host != "" { + mirrored = true + } if primaryWeight == 0 && canaryWeight == 0 { err = fmt.Errorf("VirtualService %s.%s does not contain routes for %s-primary and %s-canary", @@ -278,6 +287,7 @@ func (ir *IstioRouter) SetRoutes( canary *flaggerv1.Canary, primaryWeight int, canaryWeight int, + mirrored bool, ) error { targetName := canary.Spec.TargetRef.Name primaryName := fmt.Sprintf("%s-primary", targetName) @@ -310,6 +320,12 @@ func (ir *IstioRouter) SetRoutes( }, } + if mirrored { + vsCopy.Spec.Http[0].Mirror = &istiov1alpha3.Destination{ + Host: canaryName, + } + } + // fix routing (A/B testing) if len(canary.Spec.CanaryAnalysis.Match) > 0 { // merge the common routes with the canary ones diff --git a/pkg/router/istio_test.go b/pkg/router/istio_test.go index a013abdb..d80d46bb 100644 --- a/pkg/router/istio_test.go +++ b/pkg/router/istio_test.go @@ -119,15 +119,16 @@ func TestIstioRouter_SetRoutes(t *testing.T) { t.Fatal(err.Error()) } - p, c, err := router.GetRoutes(mocks.canary) + p, c, m, err := router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } - p = 50 - c = 50 + p = 60 + c = 40 + m = false - err = router.SetRoutes(mocks.canary, p, c) + err = router.SetRoutes(mocks.canary, p, c, m) if err != nil { t.Fatal(err.Error()) } @@ -137,16 +138,20 @@ func TestIstioRouter_SetRoutes(t *testing.T) { t.Fatal(err.Error()) } + pHost := fmt.Sprintf("%s-primary", mocks.canary.Spec.TargetRef.Name) + cHost := fmt.Sprintf("%s-canary", mocks.canary.Spec.TargetRef.Name) pRoute := istiov1alpha3.DestinationWeight{} cRoute := istiov1alpha3.DestinationWeight{} + var mirror *istiov1alpha3.Destination for _, http := range vs.Spec.Http { for _, route := range http.Route { - if route.Destination.Host == fmt.Sprintf("%s-primary", mocks.canary.Spec.TargetRef.Name) { + if route.Destination.Host == pHost { pRoute = route } - if route.Destination.Host == fmt.Sprintf("%s-canary", mocks.canary.Spec.TargetRef.Name) { + if route.Destination.Host == cHost { cRoute = route + mirror = http.Mirror } } } @@ -158,6 +163,51 @@ func TestIstioRouter_SetRoutes(t *testing.T) { if cRoute.Weight != c { t.Errorf("Got canary weight %v wanted %v", cRoute.Weight, c) } + + if mirror != nil { + t.Errorf("Got mirror %v wanted nil", mirror) + } + + mirror = nil + p = 100 + c = 0 + m = true + + err = router.SetRoutes(mocks.canary, p, c, m) + if err != nil { + t.Fatal(err.Error()) + } + + vs, err = mocks.meshClient.NetworkingV1alpha3().VirtualServices("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + + for _, http := range vs.Spec.Http { + for _, route := range http.Route { + if route.Destination.Host == pHost { + pRoute = route + } + if route.Destination.Host == cHost { + cRoute = route + mirror = http.Mirror + } + } + } + + if pRoute.Weight != p { + t.Errorf("Got primary weight %v wanted %v", pRoute.Weight, p) + } + + if cRoute.Weight != c { + t.Errorf("Got canary weight %v wanted %v", cRoute.Weight, c) + } + + if mirror == nil { + t.Errorf("Got mirror nil wanted a mirror") + } else if mirror.Host != cHost { + t.Errorf("Got mirror host \"%v\" wanted \"%v\"", mirror.Host, cHost) + } } func TestIstioRouter_GetRoutes(t *testing.T) { @@ -174,7 +224,7 @@ func TestIstioRouter_GetRoutes(t *testing.T) { t.Fatal(err.Error()) } - p, c, err := router.GetRoutes(mocks.canary) + p, c, m, err := router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } @@ -186,6 +236,74 @@ func TestIstioRouter_GetRoutes(t *testing.T) { if c != 0 { t.Errorf("Got canary weight %v wanted %v", c, 0) } + + if m != false { + t.Errorf("Got mirror %v wanted %v", m, false) + } + + mocks.canary = newMockMirror() + + err = router.Reconcile(mocks.canary) + if err != nil { + t.Fatal(err.Error()) + } + + p, c, m, err = router.GetRoutes(mocks.canary) + if err != nil { + t.Fatal(err.Error()) + } + + if p != 100 { + t.Errorf("Got primary weight %v wanted %v", p, 100) + } + + if c != 0 { + t.Errorf("Got canary weight %v wanted %v", c, 0) + } + + // A Canary resource with mirror on does not automatically create mirroring + // in the virtual server (mirroring is activated as a temporary stage). + if m != false { + t.Errorf("Got mirror %v wanted %v", m, false) + } + + // Adjust vs to activate mirroring. + vs, err := mocks.meshClient.NetworkingV1alpha3().VirtualServices("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + + cHost := fmt.Sprintf("%s-canary", mocks.canary.Spec.TargetRef.Name) + for i, http := range vs.Spec.Http { + for _, route := range http.Route { + if route.Destination.Host == cHost { + vs.Spec.Http[i].Mirror = &istiov1alpha3.Destination{ + Host: cHost, + } + } + } + } + _, err = mocks.meshClient.NetworkingV1alpha3().VirtualServices(mocks.canary.Namespace).Update(vs) + if err != nil { + t.Fatal(err.Error()) + } + + p, c, m, err = router.GetRoutes(mocks.canary) + if err != nil { + t.Fatal(err.Error()) + } + + if p != 100 { + t.Errorf("Got primary weight %v wanted %v", p, 100) + } + + if c != 0 { + t.Errorf("Got canary weight %v wanted %v", c, 0) + } + + if m != true { + t.Errorf("Got mirror %v wanted %v", m, true) + } } func TestIstioRouter_HTTPRequestHeaders(t *testing.T) { @@ -276,8 +394,9 @@ func TestIstioRouter_ABTest(t *testing.T) { p := 0 c := 100 + m := false - err = router.SetRoutes(mocks.abtest, p, c) + err = router.SetRoutes(mocks.abtest, p, c, m) if err != nil { t.Fatal(err.Error()) } @@ -287,16 +406,20 @@ func TestIstioRouter_ABTest(t *testing.T) { t.Fatal(err.Error()) } + pHost := fmt.Sprintf("%s-primary", mocks.abtest.Spec.TargetRef.Name) + cHost := fmt.Sprintf("%s-canary", mocks.abtest.Spec.TargetRef.Name) pRoute := istiov1alpha3.DestinationWeight{} cRoute := istiov1alpha3.DestinationWeight{} + var mirror *istiov1alpha3.Destination for _, http := range vs.Spec.Http { for _, route := range http.Route { - if route.Destination.Host == fmt.Sprintf("%s-primary", mocks.abtest.Spec.TargetRef.Name) { + if route.Destination.Host == pHost { pRoute = route } - if route.Destination.Host == fmt.Sprintf("%s-canary", mocks.abtest.Spec.TargetRef.Name) { + if route.Destination.Host == cHost { cRoute = route + mirror = http.Mirror } } } @@ -308,4 +431,8 @@ func TestIstioRouter_ABTest(t *testing.T) { if cRoute.Weight != c { t.Errorf("Got canary weight %v wanted %v", cRoute.Weight, c) } + + if mirror != nil { + t.Errorf("Got mirror %v wanted nil", mirror) + } } diff --git a/pkg/router/nop.go b/pkg/router/nop.go index 66f1d812..d7ec6721 100644 --- a/pkg/router/nop.go +++ b/pkg/router/nop.go @@ -12,13 +12,13 @@ func (*NopRouter) Reconcile(canary *flaggerv1.Canary) error { return nil } -func (*NopRouter) SetRoutes(canary *flaggerv1.Canary, primaryWeight int, canaryWeight int) error { +func (*NopRouter) SetRoutes(canary *flaggerv1.Canary, primaryWeight int, canaryWeight int, mirror bool) error { return nil } -func (*NopRouter) GetRoutes(canary *flaggerv1.Canary) (primaryWeight int, canaryWeight int, err error) { +func (*NopRouter) GetRoutes(canary *flaggerv1.Canary) (primaryWeight int, canaryWeight int, mirror bool, err error) { if canary.Status.Iterations > 0 { - return 0, 100, nil + return 0, 100, false, nil } - return 100, 0, nil + return 100, 0, false, nil } diff --git a/pkg/router/router.go b/pkg/router/router.go index 1729b3a2..f506bd1c 100644 --- a/pkg/router/router.go +++ b/pkg/router/router.go @@ -4,6 +4,6 @@ import flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1alpha3" type Interface interface { Reconcile(canary *flaggerv1.Canary) error - SetRoutes(canary *flaggerv1.Canary, primaryWeight int, canaryWeight int) error - GetRoutes(canary *flaggerv1.Canary) (primaryWeight int, canaryWeight int, err error) + SetRoutes(canary *flaggerv1.Canary, primaryWeight int, canaryWeight int, mirrored bool) error + GetRoutes(canary *flaggerv1.Canary) (primaryWeight int, canaryWeight int, mirrored bool, err error) } diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index 701cae0c..fcc17bf9 100644 --- a/pkg/router/router_test.go +++ b/pkg/router/router_test.go @@ -137,6 +137,12 @@ func newMockCanary() *v1alpha3.Canary { return cd } +func newMockMirror() *v1alpha3.Canary { + cd := newMockCanary() + cd.Spec.CanaryAnalysis.Mirror = true + return cd +} + func newMockABTest() *v1alpha3.Canary { cd := &v1alpha3.Canary{ TypeMeta: metav1.TypeMeta{APIVersion: v1alpha3.SchemeGroupVersion.String()}, diff --git a/pkg/router/smi.go b/pkg/router/smi.go index 4bdb2fbd..2b948684 100644 --- a/pkg/router/smi.go +++ b/pkg/router/smi.go @@ -107,6 +107,7 @@ func (sr *SmiRouter) Reconcile(canary *flaggerv1.Canary) error { func (sr *SmiRouter) GetRoutes(canary *flaggerv1.Canary) ( primaryWeight int, canaryWeight int, + mirrored bool, err error, ) { targetName := canary.Spec.TargetRef.Name @@ -137,6 +138,8 @@ func (sr *SmiRouter) GetRoutes(canary *flaggerv1.Canary) ( targetName, canary.Namespace, primaryName, canaryName) } + mirrored = false + return } @@ -145,6 +148,7 @@ func (sr *SmiRouter) SetRoutes( canary *flaggerv1.Canary, primaryWeight int, canaryWeight int, + mirrored bool, ) error { targetName := canary.Spec.TargetRef.Name canaryName := fmt.Sprintf("%s-canary", targetName) diff --git a/pkg/router/supergloo.go b/pkg/router/supergloo.go index 762e845c..af975ee3 100644 --- a/pkg/router/supergloo.go +++ b/pkg/router/supergloo.go @@ -82,11 +82,11 @@ func (sr *SuperglooRouter) Reconcile(canary *flaggerv1.Canary) error { } // do we have routes already? - if _, _, err := sr.GetRoutes(canary); err == nil { + if _, _, _, err := sr.GetRoutes(canary); err == nil { // we have routes, no need to do anything else return nil } else if solokiterror.IsNotExist(err) { - return sr.SetRoutes(canary, 100, 0) + return sr.SetRoutes(canary, 100, 0, false) } else { return err } @@ -219,6 +219,7 @@ func (sr *SuperglooRouter) createRule(canary *flaggerv1.Canary, namesuffix strin func (sr *SuperglooRouter) GetRoutes(canary *flaggerv1.Canary) ( primaryWeight int, canaryWeight int, + mirrored bool, err error, ) { targetName := canary.Spec.TargetRef.Name @@ -247,6 +248,8 @@ func (sr *SuperglooRouter) GetRoutes(canary *flaggerv1.Canary) ( targetName, canary.Namespace, targetName, targetName) } + mirrored = false + return } @@ -259,6 +262,7 @@ func (sr *SuperglooRouter) SetRoutes( canary *flaggerv1.Canary, primaryWeight int, canaryWeight int, + mirrored bool, ) error { // upstream name is // in gloo-system diff --git a/pkg/router/supergloo_test.go b/pkg/router/supergloo_test.go index aa500b63..9e54e8f1 100644 --- a/pkg/router/supergloo_test.go +++ b/pkg/router/supergloo_test.go @@ -71,15 +71,16 @@ func TestSuperglooRouter_SetRoutes(t *testing.T) { t.Fatal(err.Error()) } - p, c, err := router.GetRoutes(mocks.canary) + p, c, m, err := router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } p = 50 c = 50 + m = false - err = router.SetRoutes(mocks.canary, p, c) + err = router.SetRoutes(mocks.canary, p, c, m) if err != nil { t.Fatal(err.Error()) } @@ -134,7 +135,7 @@ func TestSuperglooRouter_GetRoutes(t *testing.T) { t.Fatal(err.Error()) } - p, c, err := router.GetRoutes(mocks.canary) + p, c, m, err := router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } @@ -146,4 +147,8 @@ func TestSuperglooRouter_GetRoutes(t *testing.T) { if c != 0 { t.Errorf("Got canary weight %v wanted %v", c, 0) } + + if m != false { + t.Errorf("Got mirror %v wanted %v", m, false) + } }