From 4d7fae39a82df84ef40fdfa37ad0211e8e37d695 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Fri, 19 Apr 2019 14:41:50 -0400 Subject: [PATCH] add retries and cors --- pkg/router/factory.go | 11 +-- pkg/router/supergloo.go | 150 ++++++++++++++++++++++------------------ 2 files changed, 90 insertions(+), 71 deletions(-) diff --git a/pkg/router/factory.go b/pkg/router/factory.go index d11a9183f..9d8f66ecf 100644 --- a/pkg/router/factory.go +++ b/pkg/router/factory.go @@ -2,6 +2,7 @@ package router import ( "context" + "strings" clientset "github.com/weaveworks/flagger/pkg/client/clientset/versioned" "go.uber.org/zap" @@ -42,18 +43,18 @@ func (factory *Factory) KubernetesRouter(label string) *KubernetesRouter { // MeshRouter returns a service mesh router (Istio or AppMesh) func (factory *Factory) MeshRouter(provider string) Interface { - switch provider { - case "appmesh": + switch { + case provider == "appmesh": return &AppMeshRouter{ logger: factory.logger, flaggerClient: factory.flaggerClient, kubeClient: factory.kubeClient, appmeshClient: factory.meshClient, } - case "supergloo": - supergloo, err := NewSuperglooRouter(context.TODO(), factory.flaggerClient, factory.logger, factory.kubeConfig) + case strings.HasPrefix(provider, "supergloo"): + supergloo, err := NewSuperglooRouter(context.TODO(), provider, factory.flaggerClient, factory.logger, factory.kubeConfig) if err != nil { - panic("TODO") + panic("failed creating supergloo client") } return supergloo default: diff --git a/pkg/router/supergloo.go b/pkg/router/supergloo.go index 24fc02b48..259a76377 100644 --- a/pkg/router/supergloo.go +++ b/pkg/router/supergloo.go @@ -3,16 +3,22 @@ package router import ( "context" "fmt" + "strings" + "time" solokitclients "github.com/solo-io/solo-kit/pkg/api/v1/clients" "github.com/solo-io/solo-kit/pkg/api/v1/clients/factory" "github.com/solo-io/solo-kit/pkg/api/v1/clients/kube" + crdv1 "github.com/solo-io/solo-kit/pkg/api/v1/clients/kube/crd/solo.io/v1" solokitcore "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" solokiterror "github.com/solo-io/solo-kit/pkg/errors" + types "github.com/gogo/protobuf/types" gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" + supergloov1alpha3 "github.com/solo-io/supergloo/pkg/api/external/istio/networking/v1alpha3" supergloov1 "github.com/solo-io/supergloo/pkg/api/v1" flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1alpha3" + istiov1alpha3 "github.com/weaveworks/flagger/pkg/apis/istio/v1alpha3" clientset "github.com/weaveworks/flagger/pkg/client/clientset/versioned" "go.uber.org/zap" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -27,7 +33,7 @@ type SuperglooRouter struct { targetMesh solokitcore.ResourceRef } -func NewSuperglooRouter(ctx context.Context, flaggerClient clientset.Interface, logger *zap.SugaredLogger, cfg *rest.Config) (*SuperglooRouter, error) { +func NewSuperglooRouter(ctx context.Context, provider string, flaggerClient clientset.Interface, logger *zap.SugaredLogger, cfg *rest.Config) (*SuperglooRouter, error) { // TODO if cfg is nil use memory client instead? sharedCache := kube.NewKubeCache(ctx) routingRuleClient, err := supergloov1.NewRoutingRuleClient(&factory.KubeResourceClientFactory{ @@ -37,15 +43,23 @@ func NewSuperglooRouter(ctx context.Context, flaggerClient clientset.Interface, SkipCrdCreation: true, }) if err != nil { + // this should never happen. return nil, fmt.Errorf("creating RoutingRule client %v", err) } if err := routingRuleClient.Register(); err != nil { return nil, err } - // TODO(yuval-k): un hard code this + + // remove the supergloo: prefix + provider = strings.TrimPrefix(provider, "supergloo:") + // split namespace.name : + parts := strings.Split(provider, ".") + if len(parts) != 2 { + return nil, fmt.Errorf("invalid format for supergloo provider") + } targetMesh := solokitcore.ResourceRef{ - Namespace: "supergloo-system", - Name: "yuval", + Namespace: parts[0], + Name: parts[1], } return NewSuperglooRouterWithClient(ctx, routingRuleClient, targetMesh, logger), nil } @@ -56,52 +70,78 @@ func NewSuperglooRouterWithClient(ctx context.Context, routingRuleClient supergl // Reconcile creates or updates the Istio virtual service func (sr *SuperglooRouter) Reconcile(canary *flaggerv1.Canary) error { - // TODO: add header rules - // TODO: add retry rules - // TODO: add CORS rules - // TODO: maybe more? rewrite \ timeout? - // set the rules for the headers, retries, and cors. - targetName := canary.Spec.TargetRef.Name if err := sr.setRetries(canary); err != nil { return err } + if err := sr.setCors(canary); err != nil { + return err + } return sr.SetRoutes(canary, 100, 0) } func (sr *SuperglooRouter) setRetries(canary *flaggerv1.Canary) error { - rule := &supergloov1.RoutingRule{ - Metadata: solokitcore.Metadata{ - Name: canary.Spec.TargetRef.Name + "-retries", - Namespace: canary.Namespace, - }, - TargetMesh: &sr.targetMesh, - DestinationSelector: &supergloov1.PodSelector{ - SelectorType: &supergloov1.PodSelector_UpstreamSelector_{ - UpstreamSelector: &supergloov1.PodSelector_UpstreamSelector{ - Upstreams: []solokitcore.ResourceRef{{ - Name: upstreamName(canary.Namespace, fmt.Sprintf("%s", targetName), canary.Spec.Service.Port), - Namespace: "supergloo-system", - }}, - }, - }, + if canary.Spec.Service.Retries == nil { + return nil + } + retries, err := convertRetries(canary.Spec.Service.Retries) + if err != nil { + return err + } + rule := sr.createRule(canary, "retries", &supergloov1.RoutingRuleSpec{ + RuleType: &supergloov1.RoutingRuleSpec_Retries{ + Retries: retries, }, - Spec: &supergloov1.RoutingRuleSpec{ - RuleType: &supergloov1.RoutingRuleSpec_Retries{ - Retries: canary.Spec.Service.Retries, - }, + }) + + return sr.writeRuleForCanary(canary, rule) +} + +func convertRetries(retries *istiov1alpha3.HTTPRetry) (*supergloov1.RetryPolicy, error) { + perTryTimeout, err := time.ParseDuration(retries.PerTryTimeout) + return &supergloov1.RetryPolicy{ + MaxRetries: &supergloov1alpha3.HTTPRetry{ + Attempts: int32(retries.Attempts), + PerTryTimeout: types.DurationProto(perTryTimeout), + RetryOn: retries.RetryOn, }, + }, err +} + +func (sr *SuperglooRouter) setCors(canary *flaggerv1.Canary) error { + corsPolicy := canary.Spec.Service.CorsPolicy + if corsPolicy == nil { + return nil + } + var maxAgeDuration *types.Duration + if maxAge, err := time.ParseDuration(corsPolicy.MaxAge); err == nil { + maxAgeDuration = types.DurationProto(maxAge) } + rule := sr.createRule(canary, "cors", &supergloov1.RoutingRuleSpec{ + RuleType: &supergloov1.RoutingRuleSpec_CorsPolicy{ + CorsPolicy: &supergloov1alpha3.CorsPolicy{ + AllowOrigin: corsPolicy.AllowOrigin, + AllowMethods: corsPolicy.AllowMethods, + AllowHeaders: corsPolicy.AllowHeaders, + ExposeHeaders: corsPolicy.ExposeHeaders, + MaxAge: maxAgeDuration, + AllowCredentials: &types.BoolValue{Value: corsPolicy.AllowCredentials}, + }, + }, + }) return sr.writeRuleForCanary(canary, rule) } -func (sr *SuperglooRouter) setCors(canary *flaggerv1.Canary) error { - rule := &supergloov1.RoutingRule{ +func (sr *SuperglooRouter) createRule(canary *flaggerv1.Canary, namesuffix string, spec *supergloov1.RoutingRuleSpec) *supergloov1.RoutingRule { + if namesuffix != "" { + namesuffix = "-" + namesuffix + } + return &supergloov1.RoutingRule{ Metadata: solokitcore.Metadata{ - Name: canary.Spec.TargetRef.Name + "-cors", + Name: canary.Spec.TargetRef.Name + namesuffix, Namespace: canary.Namespace, }, TargetMesh: &sr.targetMesh, @@ -109,19 +149,14 @@ func (sr *SuperglooRouter) setCors(canary *flaggerv1.Canary) error { SelectorType: &supergloov1.PodSelector_UpstreamSelector_{ UpstreamSelector: &supergloov1.PodSelector_UpstreamSelector{ Upstreams: []solokitcore.ResourceRef{{ - Name: upstreamName(canary.Namespace, fmt.Sprintf("%s", targetName), canary.Spec.Service.Port), - Namespace: "supergloo-system", + Name: upstreamName(canary.Namespace, fmt.Sprintf("%s", canary.Spec.TargetRef.Name), canary.Spec.Service.Port), + Namespace: sr.targetMesh.Namespace, }}, }, }, }, - Spec: &supergloov1.RoutingRuleSpec{ - RuleType: &supergloov1.RoutingRuleSpec_CorsPolicy{ - CorsPolicy: canary.Spec.Service.CorsPolicy, - }, - }, + Spec: spec, } - return sr.writeRuleForCanary(canary, rule) } // GetRoutes returns the destinations weight for primary and canary @@ -180,7 +215,7 @@ func (sr *SuperglooRouter) SetRoutes( Destination: &gloov1.Destination{ Upstream: solokitcore.ResourceRef{ Name: upstreamName(canary.Namespace, fmt.Sprintf("%s-primary", targetName), canary.Spec.Service.Port), - Namespace: "supergloo-system", + Namespace: sr.targetMesh.Namespace, }, }, Weight: uint32(primaryWeight), @@ -192,7 +227,7 @@ func (sr *SuperglooRouter) SetRoutes( Destination: &gloov1.Destination{ Upstream: solokitcore.ResourceRef{ Name: upstreamName(canary.Namespace, fmt.Sprintf("%s-canary", targetName), canary.Spec.Service.Port), - Namespace: "supergloo-system", + Namespace: sr.targetMesh.Namespace, }, }, Weight: uint32(canaryWeight), @@ -203,32 +238,15 @@ func (sr *SuperglooRouter) SetRoutes( return fmt.Errorf("RoutingRule %s.%s update failed: no valid weights", targetName, canary.Namespace) } - rule := &supergloov1.RoutingRule{ - Metadata: solokitcore.Metadata{ - Name: targetName, - Namespace: canary.Namespace, - }, - TargetMesh: &sr.targetMesh, - DestinationSelector: &supergloov1.PodSelector{ - SelectorType: &supergloov1.PodSelector_UpstreamSelector_{ - UpstreamSelector: &supergloov1.PodSelector_UpstreamSelector{ - Upstreams: []solokitcore.ResourceRef{{ - Name: upstreamName(canary.Namespace, fmt.Sprintf("%s", targetName), canary.Spec.Service.Port), - Namespace: "supergloo-system", - }}, + rule := sr.createRule(canary, "", &supergloov1.RoutingRuleSpec{ + RuleType: &supergloov1.RoutingRuleSpec_TrafficShifting{ + TrafficShifting: &supergloov1.TrafficShifting{ + Destinations: &gloov1.MultiDestination{ + Destinations: destinations, }, }, }, - Spec: &supergloov1.RoutingRuleSpec{ - RuleType: &supergloov1.RoutingRuleSpec_TrafficShifting{ - TrafficShifting: &supergloov1.TrafficShifting{ - Destinations: &gloov1.MultiDestination{ - Destinations: destinations, - }, - }, - }, - }, - } + }) return sr.writeRuleForCanary(canary, rule) } @@ -266,5 +284,5 @@ func (sr *SuperglooRouter) writeRuleForCanary(canary *flaggerv1.Canary, rule *su if err != nil { return fmt.Errorf("RoutingRule %s.%s update failed: %v", targetName, canary.Namespace, err) } - + return nil }