From 42b850ca527a9fd60aba11005a4a15336f97ac3b Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Tue, 5 Mar 2019 02:04:55 +0200 Subject: [PATCH] Replace controller routing management with router pkg --- pkg/controller/controller.go | 9 - pkg/controller/controller_test.go | 17 +- pkg/controller/router.go | 338 ------------------------------ pkg/controller/router_test.go | 193 ----------------- pkg/controller/scheduler.go | 79 ++++--- pkg/controller/scheduler_test.go | 38 ++-- 6 files changed, 72 insertions(+), 602 deletions(-) delete mode 100644 pkg/controller/router.go delete mode 100644 pkg/controller/router_test.go diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 2a68db90..6a28016c 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -42,7 +42,6 @@ type Controller struct { canaries *sync.Map jobs map[string]CanaryJob deployer CanaryDeployer - router CanaryRouter observer CanaryObserver recorder CanaryRecorder notifier *notifier.Slack @@ -81,13 +80,6 @@ func NewController( }, } - router := CanaryRouter{ - logger: logger, - kubeClient: kubeClient, - istioClient: istioClient, - flaggerClient: flaggerClient, - } - observer := CanaryObserver{ metricsServer: metricServer, } @@ -107,7 +99,6 @@ func NewController( jobs: map[string]CanaryJob{}, flaggerWindow: flaggerWindow, deployer: deployer, - router: router, observer: observer, recorder: recorder, notifier: notifier, diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index cbb6b197..f0054146 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -8,6 +8,7 @@ import ( fakeFlagger "github.com/stefanprodan/flagger/pkg/client/clientset/versioned/fake" informers "github.com/stefanprodan/flagger/pkg/client/informers/externalversions" "github.com/stefanprodan/flagger/pkg/logging" + "github.com/stefanprodan/flagger/pkg/router" "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" hpav1 "k8s.io/api/autoscaling/v1" @@ -33,10 +34,10 @@ type Mocks struct { istioClient istioclientset.Interface flaggerClient clientset.Interface deployer CanaryDeployer - router CanaryRouter observer CanaryObserver ctrl *Controller logger *zap.SugaredLogger + router router.Interface } func SetupMocks() Mocks { @@ -70,12 +71,6 @@ func SetupMocks() Mocks { flaggerClient: flaggerClient, }, } - router := CanaryRouter{ - flaggerClient: flaggerClient, - kubeClient: kubeClient, - istioClient: istioClient, - logger: logger, - } observer := CanaryObserver{ metricsServer: "fake", } @@ -96,22 +91,26 @@ func SetupMocks() Mocks { canaries: new(sync.Map), flaggerWindow: time.Second, deployer: deployer, - router: router, observer: observer, recorder: NewCanaryRecorder(false), } ctrl.flaggerSynced = alwaysReady + // init router + rf := router.NewFactory(kubeClient, flaggerClient, logger, istioClient) + var meshRouter router.Interface + meshRouter = rf.IstioRouter() + return Mocks{ canary: canary, observer: observer, - router: router, deployer: deployer, logger: logger, flaggerClient: flaggerClient, istioClient: istioClient, kubeClient: kubeClient, ctrl: ctrl, + router: meshRouter, } } diff --git a/pkg/controller/router.go b/pkg/controller/router.go deleted file mode 100644 index 8dd98585..00000000 --- a/pkg/controller/router.go +++ /dev/null @@ -1,338 +0,0 @@ -package controller - -import ( - "fmt" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - istiov1alpha3 "github.com/knative/pkg/apis/istio/v1alpha3" - istioclientset "github.com/knative/pkg/client/clientset/versioned" - flaggerv1 "github.com/stefanprodan/flagger/pkg/apis/flagger/v1alpha3" - clientset "github.com/stefanprodan/flagger/pkg/client/clientset/versioned" - "go.uber.org/zap" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/client-go/kubernetes" -) - -// CanaryRouter is managing the operations for Kubernetes service kind -// and Istio virtual services -type CanaryRouter struct { - kubeClient kubernetes.Interface - istioClient istioclientset.Interface - flaggerClient clientset.Interface - logger *zap.SugaredLogger -} - -// Sync creates or updates the primary and canary ClusterIP services -// and the Istio virtual service. -func (c *CanaryRouter) Sync(cd *flaggerv1.Canary) error { - err := c.createServices(cd) - if err != nil { - return err - } - err = c.syncVirtualService(cd) - if err != nil { - return err - } - return nil -} - -func (c *CanaryRouter) createServices(cd *flaggerv1.Canary) error { - targetName := cd.Spec.TargetRef.Name - primaryName := fmt.Sprintf("%s-primary", targetName) - canaryService, err := c.kubeClient.CoreV1().Services(cd.Namespace).Get(targetName, metav1.GetOptions{}) - if errors.IsNotFound(err) { - canaryService = &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: targetName, - Namespace: cd.Namespace, - OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(cd, schema.GroupVersionKind{ - Group: flaggerv1.SchemeGroupVersion.Group, - Version: flaggerv1.SchemeGroupVersion.Version, - Kind: flaggerv1.CanaryKind, - }), - }, - }, - Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeClusterIP, - Selector: map[string]string{"app": targetName}, - Ports: []corev1.ServicePort{ - { - Name: "http", - Protocol: corev1.ProtocolTCP, - Port: cd.Spec.Service.Port, - TargetPort: intstr.IntOrString{ - Type: intstr.Int, - IntVal: cd.Spec.Service.Port, - }, - }, - }, - }, - } - - _, err = c.kubeClient.CoreV1().Services(cd.Namespace).Create(canaryService) - if err != nil { - return err - } - c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).Infof("Service %s.%s created", canaryService.GetName(), cd.Namespace) - } - - canaryTestServiceName := fmt.Sprintf("%s-canary", cd.Spec.TargetRef.Name) - canaryTestService, err := c.kubeClient.CoreV1().Services(cd.Namespace).Get(canaryTestServiceName, metav1.GetOptions{}) - if errors.IsNotFound(err) { - canaryTestService = &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: canaryTestServiceName, - Namespace: cd.Namespace, - OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(cd, schema.GroupVersionKind{ - Group: flaggerv1.SchemeGroupVersion.Group, - Version: flaggerv1.SchemeGroupVersion.Version, - Kind: flaggerv1.CanaryKind, - }), - }, - }, - Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeClusterIP, - Selector: map[string]string{"app": targetName}, - Ports: []corev1.ServicePort{ - { - Name: "http", - Protocol: corev1.ProtocolTCP, - Port: cd.Spec.Service.Port, - TargetPort: intstr.IntOrString{ - Type: intstr.Int, - IntVal: cd.Spec.Service.Port, - }, - }, - }, - }, - } - - _, err = c.kubeClient.CoreV1().Services(cd.Namespace).Create(canaryTestService) - if err != nil { - return err - } - c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).Infof("Service %s.%s created", canaryTestService.GetName(), cd.Namespace) - } - - primaryService, err := c.kubeClient.CoreV1().Services(cd.Namespace).Get(primaryName, metav1.GetOptions{}) - if errors.IsNotFound(err) { - primaryService = &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: primaryName, - Namespace: cd.Namespace, - OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(cd, schema.GroupVersionKind{ - Group: flaggerv1.SchemeGroupVersion.Group, - Version: flaggerv1.SchemeGroupVersion.Version, - Kind: flaggerv1.CanaryKind, - }), - }, - }, - Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeClusterIP, - Selector: map[string]string{"app": primaryName}, - Ports: []corev1.ServicePort{ - { - Name: "http", - Protocol: corev1.ProtocolTCP, - Port: cd.Spec.Service.Port, - TargetPort: intstr.IntOrString{ - Type: intstr.Int, - IntVal: cd.Spec.Service.Port, - }, - }, - }, - }, - } - - _, err = c.kubeClient.CoreV1().Services(cd.Namespace).Create(primaryService) - if err != nil { - return err - } - - c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).Infof("Service %s.%s created", primaryService.GetName(), cd.Namespace) - } - - return nil -} - -func (c *CanaryRouter) syncVirtualService(cd *flaggerv1.Canary) error { - targetName := cd.Spec.TargetRef.Name - primaryName := fmt.Sprintf("%s-primary", targetName) - hosts := append(cd.Spec.Service.Hosts, targetName) - gateways := cd.Spec.Service.Gateways - var hasMeshGateway bool - for _, g := range gateways { - if g == "mesh" { - hasMeshGateway = true - } - } - if !hasMeshGateway { - gateways = append(gateways, "mesh") - } - - route := []istiov1alpha3.DestinationWeight{ - { - Destination: istiov1alpha3.Destination{ - Host: primaryName, - Port: istiov1alpha3.PortSelector{ - Number: uint32(cd.Spec.Service.Port), - }, - }, - Weight: 100, - }, - { - Destination: istiov1alpha3.Destination{ - Host: fmt.Sprintf("%s-canary", targetName), - Port: istiov1alpha3.PortSelector{ - Number: uint32(cd.Spec.Service.Port), - }, - }, - Weight: 0, - }, - } - newSpec := istiov1alpha3.VirtualServiceSpec{ - Hosts: hosts, - Gateways: gateways, - Http: []istiov1alpha3.HTTPRoute{ - { - Match: cd.Spec.Service.Match, - Rewrite: cd.Spec.Service.Rewrite, - Timeout: cd.Spec.Service.Timeout, - Retries: cd.Spec.Service.Retries, - AppendHeaders: cd.Spec.Service.AppendHeaders, - Route: route, - }, - }, - } - - virtualService, err := c.istioClient.NetworkingV1alpha3().VirtualServices(cd.Namespace).Get(targetName, metav1.GetOptions{}) - // insert - if errors.IsNotFound(err) { - virtualService = &istiov1alpha3.VirtualService{ - ObjectMeta: metav1.ObjectMeta{ - Name: targetName, - Namespace: cd.Namespace, - OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(cd, schema.GroupVersionKind{ - Group: flaggerv1.SchemeGroupVersion.Group, - Version: flaggerv1.SchemeGroupVersion.Version, - Kind: flaggerv1.CanaryKind, - }), - }, - }, - Spec: newSpec, - } - _, err = c.istioClient.NetworkingV1alpha3().VirtualServices(cd.Namespace).Create(virtualService) - if err != nil { - return fmt.Errorf("VirtualService %s.%s create error %v", targetName, cd.Namespace, err) - } - c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)). - Infof("VirtualService %s.%s created", virtualService.GetName(), cd.Namespace) - return nil - } - - if err != nil { - return fmt.Errorf("VirtualService %s.%s query error %v", targetName, cd.Namespace, err) - } - - // update service but keep the original destination weights - if virtualService != nil { - if diff := cmp.Diff(newSpec, virtualService.Spec, cmpopts.IgnoreTypes(istiov1alpha3.DestinationWeight{})); diff != "" { - //fmt.Println(diff) - vtClone := virtualService.DeepCopy() - vtClone.Spec = newSpec - if len(virtualService.Spec.Http) > 0 { - vtClone.Spec.Http[0].Route = virtualService.Spec.Http[0].Route - } - _, err = c.istioClient.NetworkingV1alpha3().VirtualServices(cd.Namespace).Update(vtClone) - if err != nil { - return fmt.Errorf("VirtualService %s.%s update error %v", targetName, cd.Namespace, err) - } - c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)). - Infof("VirtualService %s.%s updated", virtualService.GetName(), cd.Namespace) - } - } - - return nil -} - -// GetRoutes returns the destinations weight for primary and canary -func (c *CanaryRouter) GetRoutes(cd *flaggerv1.Canary) ( - primary istiov1alpha3.DestinationWeight, - canary istiov1alpha3.DestinationWeight, - err error, -) { - targetName := cd.Spec.TargetRef.Name - vs := &istiov1alpha3.VirtualService{} - vs, err = c.istioClient.NetworkingV1alpha3().VirtualServices(cd.Namespace).Get(targetName, v1.GetOptions{}) - if err != nil { - if errors.IsNotFound(err) { - err = fmt.Errorf("VirtualService %s.%s not found", targetName, cd.Namespace) - return - } - err = fmt.Errorf("VirtualService %s.%s query error %v", targetName, cd.Namespace, err) - return - } - - for _, http := range vs.Spec.Http { - for _, route := range http.Route { - if route.Destination.Host == fmt.Sprintf("%s-primary", targetName) { - primary = route - } - if route.Destination.Host == fmt.Sprintf("%s-canary", targetName) { - canary = route - } - } - } - - if primary.Weight == 0 && canary.Weight == 0 { - err = fmt.Errorf("VirtualService %s.%s does not contain routes for %s-primary and %s-canary", - targetName, cd.Namespace, targetName, targetName) - } - - return -} - -// SetRoutes updates the destinations weight for primary and canary -func (c *CanaryRouter) SetRoutes( - cd *flaggerv1.Canary, - primary istiov1alpha3.DestinationWeight, - canary istiov1alpha3.DestinationWeight, -) error { - targetName := cd.Spec.TargetRef.Name - vs, err := c.istioClient.NetworkingV1alpha3().VirtualServices(cd.Namespace).Get(targetName, v1.GetOptions{}) - if err != nil { - if errors.IsNotFound(err) { - return fmt.Errorf("VirtualService %s.%s not found", targetName, cd.Namespace) - - } - return fmt.Errorf("VirtualService %s.%s query error %v", targetName, cd.Namespace, err) - } - - vsCopy := vs.DeepCopy() - vsCopy.Spec.Http = []istiov1alpha3.HTTPRoute{ - { - Match: cd.Spec.Service.Match, - Rewrite: cd.Spec.Service.Rewrite, - Timeout: cd.Spec.Service.Timeout, - Retries: cd.Spec.Service.Retries, - AppendHeaders: cd.Spec.Service.AppendHeaders, - Route: []istiov1alpha3.DestinationWeight{primary, canary}, - }, - } - - vs, err = c.istioClient.NetworkingV1alpha3().VirtualServices(cd.Namespace).Update(vsCopy) - if err != nil { - return fmt.Errorf("VirtualService %s.%s update failed: %v", targetName, cd.Namespace, err) - - } - return nil -} diff --git a/pkg/controller/router_test.go b/pkg/controller/router_test.go deleted file mode 100644 index 64059b94..00000000 --- a/pkg/controller/router_test.go +++ /dev/null @@ -1,193 +0,0 @@ -package controller - -import ( - "fmt" - "testing" - - istiov1alpha3 "github.com/knative/pkg/apis/istio/v1alpha3" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func TestCanaryRouter_SyncClusterIPServices(t *testing.T) { - mocks := SetupMocks() - err := mocks.router.Sync(mocks.canary) - if err != nil { - t.Fatal(err.Error()) - } - - canarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo-canary", metav1.GetOptions{}) - if err != nil { - t.Fatal(err.Error()) - } - - if canarySvc.Spec.Ports[0].Name != "http" { - t.Errorf("Got svc port name %s wanted %s", canarySvc.Spec.Ports[0].Name, "http") - } - - if canarySvc.Spec.Ports[0].Port != 9898 { - t.Errorf("Got svc port %v wanted %v", canarySvc.Spec.Ports[0].Port, 9898) - } - - primarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo-primary", metav1.GetOptions{}) - if err != nil { - t.Fatal(err.Error()) - } - - if primarySvc.Spec.Ports[0].Name != "http" { - t.Errorf("Got primary svc port name %s wanted %s", primarySvc.Spec.Ports[0].Name, "http") - } - - if primarySvc.Spec.Ports[0].Port != 9898 { - t.Errorf("Got primary svc port %v wanted %v", primarySvc.Spec.Ports[0].Port, 9898) - } -} - -func TestCanaryRouter_GetRoutes(t *testing.T) { - mocks := SetupMocks() - err := mocks.router.Sync(mocks.canary) - if err != nil { - t.Fatal(err.Error()) - } - - p, c, err := mocks.router.GetRoutes(mocks.canary) - if err != nil { - t.Fatal(err.Error()) - } - - if p.Weight != 100 { - t.Errorf("Got primary weight %v wanted %v", p.Weight, 100) - } - - if c.Weight != 0 { - t.Errorf("Got canary weight %v wanted %v", c.Weight, 0) - } -} - -func TestCanaryRouter_SyncVirtualService(t *testing.T) { - mocks := SetupMocks() - err := mocks.router.Sync(mocks.canary) - if err != nil { - t.Fatal(err.Error()) - } - - // test insert - vs, err := mocks.istioClient.NetworkingV1alpha3().VirtualServices("default").Get("podinfo", metav1.GetOptions{}) - if err != nil { - t.Fatal(err.Error()) - } - - if len(vs.Spec.Http) != 1 { - t.Errorf("Got Istio VS Http %v wanted %v", len(vs.Spec.Http), 1) - } - - if len(vs.Spec.Http[0].Route) != 2 { - t.Errorf("Got Istio VS routes %v wanted %v", len(vs.Spec.Http[0].Route), 2) - } - - // test update - cd, err := mocks.flaggerClient.FlaggerV1alpha3().Canaries("default").Get("podinfo", metav1.GetOptions{}) - if err != nil { - t.Fatal(err.Error()) - } - - cdClone := cd.DeepCopy() - hosts := cdClone.Spec.Service.Hosts - hosts = append(hosts, "test.example.com") - cdClone.Spec.Service.Hosts = hosts - canary, err := mocks.flaggerClient.FlaggerV1alpha3().Canaries("default").Update(cdClone) - if err != nil { - t.Fatal(err.Error()) - } - - // apply change - err = mocks.router.Sync(canary) - if err != nil { - t.Fatal(err.Error()) - } - - // verify - vs, err = mocks.istioClient.NetworkingV1alpha3().VirtualServices("default").Get("podinfo", metav1.GetOptions{}) - if err != nil { - t.Fatal(err.Error()) - } - if len(vs.Spec.Hosts) != 2 { - t.Errorf("Got Istio VS hosts %v wanted %v", vs.Spec.Hosts, 2) - } - - // test drift - vsClone := vs.DeepCopy() - gateways := vsClone.Spec.Gateways - gateways = append(gateways, "test-gateway.istio-system") - vsClone.Spec.Gateways = gateways - - vsGateways, err := mocks.istioClient.NetworkingV1alpha3().VirtualServices("default").Update(vsClone) - if err != nil { - t.Fatal(err.Error()) - } - if len(vsGateways.Spec.Gateways) != 2 { - t.Errorf("Got Istio VS gateway %v wanted %v", vsGateways.Spec.Gateways, 2) - } - - // undo change - err = mocks.router.Sync(mocks.canary) - if err != nil { - t.Fatal(err.Error()) - } - - // verify - vs, err = mocks.istioClient.NetworkingV1alpha3().VirtualServices("default").Get("podinfo", metav1.GetOptions{}) - if err != nil { - t.Fatal(err.Error()) - } - if len(vs.Spec.Gateways) != 1 { - t.Errorf("Got Istio VS gateways %v wanted %v", vs.Spec.Gateways, 1) - } -} - -func TestCanaryRouter_SetRoutes(t *testing.T) { - mocks := SetupMocks() - err := mocks.router.Sync(mocks.canary) - if err != nil { - t.Fatal(err.Error()) - } - - p, c, err := mocks.router.GetRoutes(mocks.canary) - if err != nil { - t.Fatal(err.Error()) - } - - p.Weight = 50 - c.Weight = 50 - - err = mocks.router.SetRoutes(mocks.canary, p, c) - if err != nil { - t.Fatal(err.Error()) - } - - vs, err := mocks.istioClient.NetworkingV1alpha3().VirtualServices("default").Get("podinfo", metav1.GetOptions{}) - if err != nil { - t.Fatal(err.Error()) - } - - pRoute := istiov1alpha3.DestinationWeight{} - cRoute := istiov1alpha3.DestinationWeight{} - - for _, http := range vs.Spec.Http { - for _, route := range http.Route { - if route.Destination.Host == fmt.Sprintf("%s-primary", mocks.canary.Spec.TargetRef.Name) { - pRoute = route - } - if route.Destination.Host == fmt.Sprintf("%s-canary", mocks.canary.Spec.TargetRef.Name) { - cRoute = route - } - } - } - - if pRoute.Weight != p.Weight { - t.Errorf("Got primary weight %v wanted %v", pRoute.Weight, c.Weight) - } - - if cRoute.Weight != c.Weight { - t.Errorf("Got canary weight %v wanted %v", cRoute.Weight, c.Weight) - } -} diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go index 934b95ab..95dfda24 100644 --- a/pkg/controller/scheduler.go +++ b/pkg/controller/scheduler.go @@ -2,10 +2,10 @@ package controller import ( "fmt" + "github.com/stefanprodan/flagger/pkg/router" "strings" "time" - istiov1alpha3 "github.com/knative/pkg/apis/istio/v1alpha3" flaggerv1 "github.com/stefanprodan/flagger/pkg/apis/flagger/v1alpha3" "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -86,8 +86,19 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh return } + // init routers + rf := router.NewFactory(c.kubeClient, c.flaggerClient, c.logger, c.istioClient) + var meshRouter router.Interface + meshRouter = rf.IstioRouter() + // create ClusterIP services and virtual service if needed - if err := c.router.Sync(cd); err != nil { + if err := rf.ServiceRouter().Sync(cd); err != nil { + c.recordEventWarningf(cd, "%v", err) + return + } + + // create or update virtual service + if err := meshRouter.Sync(cd); err != nil { c.recordEventWarningf(cd, "%v", err) return } @@ -118,13 +129,13 @@ 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 - primaryRoute, canaryRoute, err := c.router.GetRoutes(cd) + primaryWeight, canaryWeight, err := meshRouter.GetRoutes(cd) if err != nil { c.recordEventWarningf(cd, "%v", err) return } - c.recorder.SetWeight(cd, primaryRoute.Weight, canaryRoute.Weight) + c.recorder.SetWeight(cd, primaryWeight, canaryWeight) // check if canary analysis should start (canary revision has changes) or continue if ok := c.checkCanaryStatus(cd, shouldAdvance); !ok { @@ -137,9 +148,9 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh cd.Spec.TargetRef.Name, cd.Namespace) // route all traffic back to primary - primaryRoute.Weight = 100 - canaryRoute.Weight = 0 - if err := c.router.SetRoutes(cd, primaryRoute, canaryRoute); err != nil { + primaryWeight = 100 + canaryWeight = 0 + if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight); err != nil { c.recordEventWarningf(cd, "%v", err) return } @@ -172,7 +183,7 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh } // check if analysis should be skipped - if skip := c.shouldSkipAnalysis(cd, primaryRoute, canaryRoute); skip { + if skip := c.shouldSkipAnalysis(cd, meshRouter, primaryWeight, canaryWeight); skip { return } @@ -195,14 +206,14 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh } // route all traffic back to primary - primaryRoute.Weight = 100 - canaryRoute.Weight = 0 - if err := c.router.SetRoutes(cd, primaryRoute, canaryRoute); err != nil { + primaryWeight = 100 + canaryWeight = 0 + if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight); err != nil { c.recordEventWarningf(cd, "%v", err) return } - c.recorder.SetWeight(cd, primaryRoute.Weight, canaryRoute.Weight) + c.recorder.SetWeight(cd, primaryWeight, canaryWeight) c.recordEventWarningf(cd, "Canary failed! Scaling down %s.%s", cd.Name, cd.Namespace) @@ -224,7 +235,7 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh // check if the canary success rate is above the threshold // skip check if no traffic is routed to canary - if canaryRoute.Weight == 0 { + if canaryWeight == 0 { c.recordEventInfof(cd, "Starting canary analysis for %s.%s", cd.Spec.TargetRef.Name, cd.Namespace) } else { if ok := c.analyseCanary(cd); !ok { @@ -237,33 +248,33 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh } // increase canary traffic percentage - if canaryRoute.Weight < maxWeight { - primaryRoute.Weight -= cd.Spec.CanaryAnalysis.StepWeight - if primaryRoute.Weight < 0 { - primaryRoute.Weight = 0 + if canaryWeight < maxWeight { + primaryWeight -= cd.Spec.CanaryAnalysis.StepWeight + if primaryWeight < 0 { + primaryWeight = 0 } - canaryRoute.Weight += cd.Spec.CanaryAnalysis.StepWeight - if primaryRoute.Weight > 100 { - primaryRoute.Weight = 100 + canaryWeight += cd.Spec.CanaryAnalysis.StepWeight + if primaryWeight > 100 { + primaryWeight = 100 } - if err := c.router.SetRoutes(cd, primaryRoute, canaryRoute); err != nil { + if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight); err != nil { c.recordEventWarningf(cd, "%v", err) return } // update weight status - if err := c.deployer.SetStatusWeight(cd, canaryRoute.Weight); err != nil { + if err := c.deployer.SetStatusWeight(cd, canaryWeight); err != nil { c.recordEventWarningf(cd, "%v", err) return } - c.recorder.SetWeight(cd, primaryRoute.Weight, canaryRoute.Weight) - c.recordEventInfof(cd, "Advance %s.%s canary weight %v", cd.Name, cd.Namespace, canaryRoute.Weight) + c.recorder.SetWeight(cd, primaryWeight, canaryWeight) + c.recordEventInfof(cd, "Advance %s.%s canary weight %v", cd.Name, cd.Namespace, canaryWeight) // promote canary primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name) - if canaryRoute.Weight == maxWeight { + if canaryWeight == maxWeight { c.recordEventInfof(cd, "Copying %s.%s template spec to %s.%s", cd.Spec.TargetRef.Name, cd.Namespace, primaryName, cd.Namespace) if err := c.deployer.Promote(cd); err != nil { @@ -273,14 +284,14 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh } } else { // route all traffic back to primary - primaryRoute.Weight = 100 - canaryRoute.Weight = 0 - if err := c.router.SetRoutes(cd, primaryRoute, canaryRoute); err != nil { + primaryWeight = 100 + canaryWeight = 0 + if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight); err != nil { c.recordEventWarningf(cd, "%v", err) return } - c.recorder.SetWeight(cd, primaryRoute.Weight, canaryRoute.Weight) + c.recorder.SetWeight(cd, primaryWeight, canaryWeight) c.recordEventInfof(cd, "Promotion completed! Scaling down %s.%s", cd.Spec.TargetRef.Name, cd.Namespace) // shutdown canary @@ -300,19 +311,19 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh } } -func (c *Controller) shouldSkipAnalysis(cd *flaggerv1.Canary, primary istiov1alpha3.DestinationWeight, canary istiov1alpha3.DestinationWeight) bool { +func (c *Controller) shouldSkipAnalysis(cd *flaggerv1.Canary, meshRouter router.Interface, primaryWeight int, canaryWeight int) bool { if !cd.Spec.SkipAnalysis { return false } // route all traffic to primary - primary.Weight = 100 - canary.Weight = 0 - if err := c.router.SetRoutes(cd, primary, canary); err != nil { + primaryWeight = 100 + canaryWeight = 0 + if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight); err != nil { c.recordEventWarningf(cd, "%v", err) return false } - c.recorder.SetWeight(cd, primary.Weight, canary.Weight) + c.recorder.SetWeight(cd, primaryWeight, canaryWeight) // copy spec and configs from canary to primary c.recordEventInfof(cd, "Copying %s.%s template spec to %s-primary.%s", diff --git a/pkg/controller/scheduler_test.go b/pkg/controller/scheduler_test.go index 0ab633d0..e774ee2d 100644 --- a/pkg/controller/scheduler_test.go +++ b/pkg/controller/scheduler_test.go @@ -122,17 +122,17 @@ func TestScheduler_NewRevisionReset(t *testing.T) { // advance mocks.ctrl.advanceCanary("podinfo", "default", true) - primaryRoute, canaryRoute, err := mocks.router.GetRoutes(mocks.canary) + primaryWeight, canaryWeight, err := mocks.router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } - if primaryRoute.Weight != 90 { - t.Errorf("Got primary route %v wanted %v", primaryRoute.Weight, 90) + if primaryWeight != 90 { + t.Errorf("Got primary route %v wanted %v", primaryWeight, 90) } - if canaryRoute.Weight != 10 { - t.Errorf("Got canary route %v wanted %v", canaryRoute.Weight, 10) + if canaryWeight != 10 { + t.Errorf("Got canary route %v wanted %v", canaryWeight, 10) } // second update @@ -145,17 +145,17 @@ func TestScheduler_NewRevisionReset(t *testing.T) { // detect changes mocks.ctrl.advanceCanary("podinfo", "default", true) - primaryRoute, canaryRoute, err = mocks.router.GetRoutes(mocks.canary) + primaryWeight, canaryWeight, err = mocks.router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } - if primaryRoute.Weight != 100 { - t.Errorf("Got primary route %v wanted %v", primaryRoute.Weight, 100) + if primaryWeight != 100 { + t.Errorf("Got primary route %v wanted %v", primaryWeight, 100) } - if canaryRoute.Weight != 0 { - t.Errorf("Got canary route %v wanted %v", canaryRoute.Weight, 0) + if canaryWeight != 0 { + t.Errorf("Got canary route %v wanted %v", canaryWeight, 0) } } @@ -189,14 +189,14 @@ func TestScheduler_Promotion(t *testing.T) { // detect configs changes mocks.ctrl.advanceCanary("podinfo", "default", true) - primaryRoute, canaryRoute, err := mocks.router.GetRoutes(mocks.canary) + primaryWeight, canaryWeight, err := mocks.router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } - primaryRoute.Weight = 60 - canaryRoute.Weight = 40 - err = mocks.ctrl.router.SetRoutes(mocks.canary, primaryRoute, canaryRoute) + primaryWeight = 60 + canaryWeight = 40 + err = mocks.router.SetRoutes(mocks.canary, primaryWeight, canaryWeight) if err != nil { t.Fatal(err.Error()) } @@ -207,17 +207,17 @@ func TestScheduler_Promotion(t *testing.T) { // promote mocks.ctrl.advanceCanary("podinfo", "default", true) - primaryRoute, canaryRoute, err = mocks.router.GetRoutes(mocks.canary) + primaryWeight, canaryWeight, err = mocks.router.GetRoutes(mocks.canary) if err != nil { t.Fatal(err.Error()) } - if primaryRoute.Weight != 100 { - t.Errorf("Got primary route %v wanted %v", primaryRoute.Weight, 100) + if primaryWeight != 100 { + t.Errorf("Got primary route %v wanted %v", primaryWeight, 100) } - if canaryRoute.Weight != 0 { - t.Errorf("Got canary route %v wanted %v", canaryRoute.Weight, 0) + if canaryWeight != 0 { + t.Errorf("Got canary route %v wanted %v", canaryWeight, 0) } primaryDep, err := mocks.kubeClient.AppsV1().Deployments("default").Get("podinfo-primary", metav1.GetOptions{})