From de03d49f5500ee95e308e47706248d3a6f0f225b Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Sat, 8 Feb 2020 15:04:03 +0200 Subject: [PATCH 1/5] Implement metric threshold range - add CanaryThresholdRange type to Canary API - add optional thresholdRange field to the analysis metric object - implement min/max metric result validation - thresholdRange takes precedence over threshold when both are specified --- pkg/apis/flagger/v1beta1/canary.go | 8 + .../flagger/v1beta1/zz_generated.deepcopy.go | 31 ++++ pkg/controller/scheduler.go | 152 +++++++++++------- 3 files changed, 137 insertions(+), 54 deletions(-) diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go index 9cd4c6194..1e3dcc40c 100644 --- a/pkg/apis/flagger/v1beta1/canary.go +++ b/pkg/apis/flagger/v1beta1/canary.go @@ -133,11 +133,19 @@ type CanaryMetric struct { Interval string `json:"interval,omitempty"` Threshold float64 `json:"threshold"` // +optional + ThresholdRange *CanaryThresholdRange `json:"thresholdRange,omitempty"` + // +optional Query string `json:"query,omitempty"` // +optional TemplateRef *MetricTemplateRef `json:"templateRef,omitempty"` } +// CanaryThresholdRange defines the range used for metrics validation +type CanaryThresholdRange struct { + Min *float64 `json:"min,omitempty"` + Max *float64 `json:"max,omitempty"` +} + type MetricTemplateRef struct { Name string `json:"name"` // +optional diff --git a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go index 92afeef73..c4d6e874e 100644 --- a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go @@ -146,6 +146,11 @@ func (in *CanaryList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CanaryMetric) DeepCopyInto(out *CanaryMetric) { *out = *in + if in.ThresholdRange != nil { + in, out := &in.ThresholdRange, &out.ThresholdRange + *out = new(CanaryThresholdRange) + (*in).DeepCopyInto(*out) + } if in.TemplateRef != nil { in, out := &in.TemplateRef, &out.TemplateRef *out = new(MetricTemplateRef) @@ -297,6 +302,32 @@ func (in *CanaryStatus) DeepCopy() *CanaryStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CanaryThresholdRange) DeepCopyInto(out *CanaryThresholdRange) { + *out = *in + if in.Min != nil { + in, out := &in.Min, &out.Min + *out = new(float64) + **out = **in + } + if in.Max != nil { + in, out := &in.Max, &out.Max + *out = new(float64) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CanaryThresholdRange. +func (in *CanaryThresholdRange) DeepCopy() *CanaryThresholdRange { + if in == nil { + return nil + } + out := new(CanaryThresholdRange) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CanaryWebhook) DeepCopyInto(out *CanaryWebhook) { *out = *in diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go index a499af331..72749a6b6 100644 --- a/pkg/controller/scheduler.go +++ b/pkg/controller/scheduler.go @@ -747,25 +747,25 @@ func (c *Controller) runRollbackHooks(canary *flaggerv1.Canary, phase flaggerv1. return false } -func (c *Controller) runAnalysis(r *flaggerv1.Canary) bool { +func (c *Controller) runAnalysis(canary *flaggerv1.Canary) bool { // run external checks - for _, webhook := range r.Spec.CanaryAnalysis.Webhooks { + for _, webhook := range canary.Spec.CanaryAnalysis.Webhooks { if webhook.Type == "" || webhook.Type == flaggerv1.RolloutHook { - err := CallWebhook(r.Name, r.Namespace, flaggerv1.CanaryPhaseProgressing, webhook) + err := CallWebhook(canary.Name, canary.Namespace, flaggerv1.CanaryPhaseProgressing, webhook) if err != nil { - c.recordEventWarningf(r, "Halt %s.%s advancement external check %s failed %v", - r.Name, r.Namespace, webhook.Name, err) + c.recordEventWarningf(canary, "Halt %s.%s advancement external check %s failed %v", + canary.Name, canary.Namespace, webhook.Name, err) return false } } } - ok := c.runBuiltinMetricChecks(r) + ok := c.runBuiltinMetricChecks(canary) if !ok { return ok } - ok = c.runMetricChecks(r) + ok = c.runMetricChecks(canary) if !ok { return ok } @@ -773,7 +773,7 @@ func (c *Controller) runAnalysis(r *flaggerv1.Canary) bool { return true } -func (c *Controller) runBuiltinMetricChecks(r *flaggerv1.Canary) bool { +func (c *Controller) runBuiltinMetricChecks(canary *flaggerv1.Canary) bool { // override the global provider if one is specified in the canary spec var metricsProvider string // set the metrics provider to Crossover Prometheus when Crossover is the mesh provider @@ -784,8 +784,8 @@ func (c *Controller) runBuiltinMetricChecks(r *flaggerv1.Canary) bool { metricsProvider = c.meshProvider } - if r.Spec.Provider != "" { - metricsProvider = r.Spec.Provider + if canary.Spec.Provider != "" { + metricsProvider = canary.Spec.Provider // set the metrics provider to Linkerd Prometheus when Linkerd is the default mesh provider if strings.Contains(c.meshProvider, "linkerd") { @@ -793,7 +793,7 @@ func (c *Controller) runBuiltinMetricChecks(r *flaggerv1.Canary) bool { } } // set the metrics provider to query Prometheus for the canary Kubernetes service if the canary target is Service - if r.Spec.TargetRef.Kind == "Service" { + if canary.Spec.TargetRef.Kind == "Service" { metricsProvider = metricsProvider + MetricsProviderServiceSuffix } @@ -801,78 +801,110 @@ func (c *Controller) runBuiltinMetricChecks(r *flaggerv1.Canary) bool { observerFactory := c.observerFactory // override the global metrics server if one is specified in the canary spec - if r.Spec.MetricsServer != "" { + if canary.Spec.MetricsServer != "" { var err error - observerFactory, err = observers.NewFactory(r.Spec.MetricsServer) + observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer) if err != nil { - c.recordEventErrorf(r, "Error building Prometheus client for %s %v", r.Spec.MetricsServer, err) + c.recordEventErrorf(canary, "Error building Prometheus client for %s %v", canary.Spec.MetricsServer, err) return false } } observer := observerFactory.Observer(metricsProvider) // run metrics checks - for _, metric := range r.Spec.CanaryAnalysis.Metrics { + for _, metric := range canary.Spec.CanaryAnalysis.Metrics { if metric.Interval == "" { - metric.Interval = r.GetMetricInterval() + metric.Interval = canary.GetMetricInterval() } if metric.Name == "request-success-rate" { - val, err := observer.GetRequestSuccessRate(toMetricModel(r, metric.Interval)) + val, err := observer.GetRequestSuccessRate(toMetricModel(canary, metric.Interval)) if err != nil { if strings.Contains(err.Error(), "no values found") { - c.recordEventWarningf(r, "Halt advancement no values found for %s metric %s probably %s.%s is not receiving traffic", - metricsProvider, metric.Name, r.Spec.TargetRef.Name, r.Namespace) + c.recordEventWarningf(canary, "Halt advancement no values found for %s metric %s probably %s.%s is not receiving traffic", + metricsProvider, metric.Name, canary.Spec.TargetRef.Name, canary.Namespace) } else { - c.recordEventErrorf(r, "Prometheus query failed: %v", err) + c.recordEventErrorf(canary, "Prometheus query failed: %v", err) } return false } - if float64(metric.Threshold) > val { - c.recordEventWarningf(r, "Halt %s.%s advancement success rate %.2f%% < %v%%", - r.Name, r.Namespace, val, metric.Threshold) + + if metric.ThresholdRange != nil { + tr := *metric.ThresholdRange + if tr.Min != nil && val < *tr.Min { + c.recordEventWarningf(canary, "Halt %s.%s advancement success rate %.2f%% < %v%%", + canary.Name, canary.Namespace, val, *tr.Min) + return false + } + if tr.Max != nil && val > *tr.Max { + c.recordEventWarningf(canary, "Halt %s.%s advancement success rate %.2f%% > %v%%", + canary.Name, canary.Namespace, val, *tr.Max) + return false + } + } else if metric.Threshold > val { + c.recordEventWarningf(canary, "Halt %s.%s advancement success rate %.2f%% < %v%%", + canary.Name, canary.Namespace, val, metric.Threshold) return false } - - //c.recordEventInfof(r, "Check %s passed %.2f%% > %v%%", metric.Name, val, metric.Threshold) } if metric.Name == "request-duration" { - val, err := observer.GetRequestDuration(toMetricModel(r, metric.Interval)) + val, err := observer.GetRequestDuration(toMetricModel(canary, metric.Interval)) if err != nil { if strings.Contains(err.Error(), "no values found") { - c.recordEventWarningf(r, "Halt advancement no values found for %s metric %s probably %s.%s is not receiving traffic", - metricsProvider, metric.Name, r.Spec.TargetRef.Name, r.Namespace) + c.recordEventWarningf(canary, "Halt advancement no values found for %s metric %s probably %s.%s is not receiving traffic", + metricsProvider, metric.Name, canary.Spec.TargetRef.Name, canary.Namespace) } else { - c.recordEventErrorf(r, "Prometheus query failed: %v", err) + c.recordEventErrorf(canary, "Prometheus query failed: %v", err) } return false } - t := time.Duration(metric.Threshold) * time.Millisecond - if val > t { - c.recordEventWarningf(r, "Halt %s.%s advancement request duration %v > %v", - r.Name, r.Namespace, val, t) + if metric.ThresholdRange != nil { + tr := *metric.ThresholdRange + if tr.Min != nil && val < time.Duration(*tr.Min)*time.Millisecond { + c.recordEventWarningf(canary, "Halt %s.%s advancement request duration %v < %v", + canary.Name, canary.Namespace, val, time.Duration(*tr.Min)*time.Millisecond) + return false + } + if tr.Max != nil && val > time.Duration(*tr.Max)*time.Millisecond { + c.recordEventWarningf(canary, "Halt %s.%s advancement request duration %v > %v", + canary.Name, canary.Namespace, val, time.Duration(*tr.Max)*time.Millisecond) + return false + } + } else if val > time.Duration(metric.Threshold)*time.Millisecond { + c.recordEventWarningf(canary, "Halt %s.%s advancement request duration %v > %v", + canary.Name, canary.Namespace, val, time.Duration(metric.Threshold)*time.Millisecond) return false } - - //c.recordEventInfof(r, "Check %s passed %v < %v", metric.Name, val, metric.Threshold) } - // custom checks + // in-line PromQL if metric.Query != "" { val, err := observerFactory.Client.RunQuery(metric.Query) if err != nil { if strings.Contains(err.Error(), "no values found") { - c.recordEventWarningf(r, "Halt advancement no values found for custom metric: %s", + c.recordEventWarningf(canary, "Halt advancement no values found for metric: %s", metric.Name) } else { - c.recordEventErrorf(r, "Prometheus query failed for %s: %v", metric.Name, err) + c.recordEventErrorf(canary, "Prometheus query failed for %s: %v", metric.Name, err) } return false } - if val > float64(metric.Threshold) { - c.recordEventWarningf(r, "Halt %s.%s advancement %s %.2f > %v", - r.Name, r.Namespace, metric.Name, val, metric.Threshold) + if metric.ThresholdRange != nil { + tr := *metric.ThresholdRange + if tr.Min != nil && val < *tr.Min { + c.recordEventWarningf(canary, "Halt %s.%s advancement %s %.2f < %v", + canary.Name, canary.Namespace, metric.Name, val, *tr.Min) + return false + } + if tr.Max != nil && val > *tr.Max { + c.recordEventWarningf(canary, "Halt %s.%s advancement %s %.2f > %v", + canary.Name, canary.Namespace, metric.Name, val, *tr.Max) + return false + } + } else if val > metric.Threshold { + c.recordEventWarningf(canary, "Halt %s.%s advancement %s %.2f > %v", + canary.Name, canary.Namespace, metric.Name, val, metric.Threshold) return false } } @@ -881,17 +913,17 @@ func (c *Controller) runBuiltinMetricChecks(r *flaggerv1.Canary) bool { return true } -func (c *Controller) runMetricChecks(r *flaggerv1.Canary) bool { - for _, metric := range r.Spec.CanaryAnalysis.Metrics { +func (c *Controller) runMetricChecks(canary *flaggerv1.Canary) bool { + for _, metric := range canary.Spec.CanaryAnalysis.Metrics { if metric.TemplateRef != nil { - namespace := r.Namespace + namespace := canary.Namespace if metric.TemplateRef.Namespace != "" { namespace = metric.TemplateRef.Namespace } template, err := c.flaggerClient.FlaggerV1beta1().MetricTemplates(namespace).Get(metric.TemplateRef.Name, metav1.GetOptions{}) if err != nil { - c.recordEventErrorf(r, "Metric template %s.%s error: %v", metric.TemplateRef.Name, namespace, err) + c.recordEventErrorf(canary, "Metric template %s.%s error: %v", metric.TemplateRef.Name, namespace, err) return false } @@ -899,7 +931,7 @@ func (c *Controller) runMetricChecks(r *flaggerv1.Canary) bool { if template.Spec.Provider.SecretRef != nil { secret, err := c.kubeClient.CoreV1().Secrets(namespace).Get(template.Spec.Provider.SecretRef.Name, metav1.GetOptions{}) if err != nil { - c.recordEventErrorf(r, "Metric template %s.%s secret %s error: %v", + c.recordEventErrorf(canary, "Metric template %s.%s secret %s error: %v", metric.TemplateRef.Name, namespace, template.Spec.Provider.SecretRef.Name, err) return false } @@ -909,14 +941,14 @@ func (c *Controller) runMetricChecks(r *flaggerv1.Canary) bool { factory := providers.Factory{} provider, err := factory.Provider(template.Spec.Provider, credentials) if err != nil { - c.recordEventErrorf(r, "Metric template %s.%s provider %s error: %v", + c.recordEventErrorf(canary, "Metric template %s.%s provider %s error: %v", metric.TemplateRef.Name, namespace, template.Spec.Provider.Type, err) return false } - query, err := observers.RenderQuery(template.Spec.Query, toMetricModel(r, metric.Interval)) + query, err := observers.RenderQuery(template.Spec.Query, toMetricModel(canary, metric.Interval)) if err != nil { - c.recordEventErrorf(r, "Metric template %s.%s query render error: %v", + c.recordEventErrorf(canary, "Metric template %s.%s query render error: %v", metric.TemplateRef.Name, namespace, err) return false } @@ -924,17 +956,29 @@ func (c *Controller) runMetricChecks(r *flaggerv1.Canary) bool { val, err := provider.RunQuery(query) if err != nil { if strings.Contains(err.Error(), "no values found") { - c.recordEventWarningf(r, "Halt advancement no values found for custom metric: %s", + c.recordEventWarningf(canary, "Halt advancement no values found for custom metric: %s", metric.Name) } else { - c.recordEventErrorf(r, "Metric query failed for %s: %v", metric.Name, err) + c.recordEventErrorf(canary, "Metric query failed for %s: %v", metric.Name, err) } return false } - if val > metric.Threshold { - c.recordEventWarningf(r, "Halt %s.%s advancement %s %.2f > %v", - r.Name, r.Namespace, metric.Name, val, metric.Threshold) + if metric.ThresholdRange != nil { + tr := *metric.ThresholdRange + if tr.Min != nil && val < *tr.Min { + c.recordEventWarningf(canary, "Halt %s.%s advancement %s %.2f < %v", + canary.Name, canary.Namespace, metric.Name, val, *tr.Min) + return false + } + if tr.Max != nil && val > *tr.Max { + c.recordEventWarningf(canary, "Halt %s.%s advancement %s %.2f > %v", + canary.Name, canary.Namespace, metric.Name, val, *tr.Max) + return false + } + } else if val > metric.Threshold { + c.recordEventWarningf(canary, "Halt %s.%s advancement %s %.2f > %v", + canary.Name, canary.Namespace, metric.Name, val, metric.Threshold) return false } } From 228954b5dbc4f424d44ce60cbd0736a7e3d6b424 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Sat, 8 Feb 2020 15:11:11 +0200 Subject: [PATCH 2/5] Improve Canary CRD schema validation - add thresholdRange validation - add Kubernetes Kind validation for target, autoscaler and ingress - add validation for webhook metadata map[string]string - add missing Istio types to schema validation --- artifacts/flagger/crd.yaml | 244 +++++++++++++++++++++++++++--- charts/flagger/templates/crd.yaml | 244 +++++++++++++++++++++++++++--- kustomize/base/flagger/crd.yaml | 244 +++++++++++++++++++++++++++--- 3 files changed, 660 insertions(+), 72 deletions(-) diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index 1189b8307..c97962e4a 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -78,7 +78,7 @@ spec: description: Deployment progress deadline type: number targetRef: - description: Deployment selector + description: Target selector type: object required: ["apiVersion", "kind", "name"] properties: @@ -86,32 +86,35 @@ spec: type: string kind: type: string + enum: + - Deployment + - Service name: type: string autoscalerRef: description: HPA selector - anyOf: - - type: string - - type: object + type: object required: ["apiVersion", "kind", "name"] properties: apiVersion: type: string kind: type: string + enum: + - HorizontalPodAutoscaler name: type: string ingressRef: description: NGINX ingress selector - anyOf: - - type: string - - type: object + type: object required: ["apiVersion", "kind", "name"] properties: apiVersion: type: string kind: type: string + enum: + - Ingress name: type: string service: @@ -136,6 +139,9 @@ spec: portDiscovery: description: Enable port dicovery type: boolean + timeout: + description: HTTP or gRPC request timeout + type: string meshName: description: AppMesh mesh name type: string @@ -144,33 +150,190 @@ spec: type: array items: type: string - timeout: - description: Istio HTTP or gRPC request timeout - type: string + hosts: + description: The list of host names for this service + type: array + items: + type: string trafficPolicy: - description: Istio traffic policy + description: Istio mTLS traffic policy type: object + properties: + tls: + description: TLS related settings for connections to the upstream service + type: object + properties: + caCertificates: + format: string + type: string + clientCertificate: + description: REQUIRED if mode is `MUTUAL`. + format: string + type: string + mode: + enum: + - DISABLE + - SIMPLE + - MUTUAL + - ISTIO_MUTUAL + type: string + privateKey: + description: REQUIRED if mode is `MUTUAL`. + format: string + type: string + sni: + description: SNI string to present to the server + during TLS handshake. + format: string + type: string + subjectAltNames: + items: + format: string + type: string + type: array match: - description: URL match conditions + description: URI match conditions type: array items: type: object + properties: + uri: + type: object + oneOf: + - required: ["exact"] + - required: ["prefix"] + - required: ["suffix"] + - required: ["regex"] + properties: + exact: + format: string + type: string + prefix: + format: string + type: string + suffix: + format: string + type: string + regex: + format: string + type: string + retries: + description: Retry policy for HTTP requests + type: object + properties: + attempts: + description: Number of retries for a given request + format: int32 + type: integer + perTryTimeout: + description: Timeout per retry attempt for a given request + type: string + retryOn: + description: Specifies the conditions under which retry takes place + format: string + type: string rewrite: - description: URL rewrite + description: Rewrite HTTP URIs type: object + properties: + uri: + format: string + type: string headers: description: Headers operations type: object + properties: + request: + properties: + add: + additionalProperties: + format: string + type: string + type: object + remove: + items: + format: string + type: string + type: array + set: + additionalProperties: + format: string + type: string + type: object + type: object + response: + properties: + add: + additionalProperties: + format: string + type: string + type: object + remove: + items: + format: string + type: string + type: array + set: + additionalProperties: + format: string + type: string + type: object + type: object corsPolicy: - description: CORS policy + description: Cross-Origin Resource Sharing policy (CORS) type: object + properties: + allowCredentials: + type: boolean + allowHeaders: + items: + format: string + type: string + type: array + allowMethods: + description: List of HTTP methods allowed to access the resource + items: + format: string + type: string + type: array + allowOrigin: + description: The list of origins that are allowed to perform + CORS requests. + items: + format: string + type: string + type: array + allowOrigins: + description: String patterns that match allowed origins + type: array + items: + type: object + oneOf: + - required: + - exact + - required: + - prefix + - required: + - regex + properties: + exact: + format: string + type: string + prefix: + format: string + type: string + regex: + format: string + type: string + exposeHeaders: + items: + format: string + type: string + type: array + maxAge: + type: string gateways: - description: Gateway list - type: array - items: - type: string - hosts: - description: Host list + description: The list of Istio gateway for this virtual service type: array items: type: string @@ -205,12 +368,35 @@ spec: type: array items: type: object + properties: + headers: + type: object + additionalProperties: + oneOf: + - required: ["exact"] + - required: ["prefix"] + - required: ["suffix"] + - required: ["regex"] + type: object + properties: + exact: + format: string + type: string + prefix: + format: string + type: string + suffix: + format: string + type: string + regex: + format: string + type: string metrics: description: Metric check list for this canary type: array items: type: object - required: ["name", "threshold"] + required: ["name"] properties: name: description: Name of the metric @@ -222,6 +408,16 @@ spec: threshold: description: Max value accepted for this metric type: number + thresholdRange: + description: Range accepted for this metric + type: object + properties: + min: + description: Min value accepted for this metric + type: number + max: + description: Max value accepted for this metric + type: number query: description: Prometheus query type: string @@ -268,9 +464,9 @@ spec: pattern: "^[0-9]+(m|s)" metadata: description: Metadata (key-value pairs) for this webhook - anyOf: - - type: string - - type: object + type: object + additionalProperties: + type: string status: properties: phase: diff --git a/charts/flagger/templates/crd.yaml b/charts/flagger/templates/crd.yaml index bbeec6fc0..360e2d67a 100644 --- a/charts/flagger/templates/crd.yaml +++ b/charts/flagger/templates/crd.yaml @@ -79,7 +79,7 @@ spec: description: Deployment progress deadline type: number targetRef: - description: Deployment selector + description: Target selector type: object required: ["apiVersion", "kind", "name"] properties: @@ -87,32 +87,35 @@ spec: type: string kind: type: string + enum: + - Deployment + - Service name: type: string autoscalerRef: description: HPA selector - anyOf: - - type: string - - type: object + type: object required: ["apiVersion", "kind", "name"] properties: apiVersion: type: string kind: type: string + enum: + - HorizontalPodAutoscaler name: type: string ingressRef: description: NGINX ingress selector - anyOf: - - type: string - - type: object + type: object required: ["apiVersion", "kind", "name"] properties: apiVersion: type: string kind: type: string + enum: + - Ingress name: type: string service: @@ -137,6 +140,9 @@ spec: portDiscovery: description: Enable port dicovery type: boolean + timeout: + description: HTTP or gRPC request timeout + type: string meshName: description: AppMesh mesh name type: string @@ -145,33 +151,190 @@ spec: type: array items: type: string - timeout: - description: Istio HTTP or gRPC request timeout - type: string + hosts: + description: The list of host names for this service + type: array + items: + type: string trafficPolicy: - description: Istio traffic policy + description: Istio mTLS traffic policy type: object + properties: + tls: + description: TLS related settings for connections to the upstream service + type: object + properties: + caCertificates: + format: string + type: string + clientCertificate: + description: REQUIRED if mode is `MUTUAL`. + format: string + type: string + mode: + enum: + - DISABLE + - SIMPLE + - MUTUAL + - ISTIO_MUTUAL + type: string + privateKey: + description: REQUIRED if mode is `MUTUAL`. + format: string + type: string + sni: + description: SNI string to present to the server + during TLS handshake. + format: string + type: string + subjectAltNames: + items: + format: string + type: string + type: array match: - description: URL match conditions + description: URI match conditions type: array items: type: object + properties: + uri: + type: object + oneOf: + - required: ["exact"] + - required: ["prefix"] + - required: ["suffix"] + - required: ["regex"] + properties: + exact: + format: string + type: string + prefix: + format: string + type: string + suffix: + format: string + type: string + regex: + format: string + type: string + retries: + description: Retry policy for HTTP requests + type: object + properties: + attempts: + description: Number of retries for a given request + format: int32 + type: integer + perTryTimeout: + description: Timeout per retry attempt for a given request + type: string + retryOn: + description: Specifies the conditions under which retry takes place + format: string + type: string rewrite: - description: URL rewrite + description: Rewrite HTTP URIs type: object + properties: + uri: + format: string + type: string headers: description: Headers operations type: object + properties: + request: + properties: + add: + additionalProperties: + format: string + type: string + type: object + remove: + items: + format: string + type: string + type: array + set: + additionalProperties: + format: string + type: string + type: object + type: object + response: + properties: + add: + additionalProperties: + format: string + type: string + type: object + remove: + items: + format: string + type: string + type: array + set: + additionalProperties: + format: string + type: string + type: object + type: object corsPolicy: - description: CORS policy + description: Cross-Origin Resource Sharing policy (CORS) type: object + properties: + allowCredentials: + type: boolean + allowHeaders: + items: + format: string + type: string + type: array + allowMethods: + description: List of HTTP methods allowed to access the resource + items: + format: string + type: string + type: array + allowOrigin: + description: The list of origins that are allowed to perform + CORS requests. + items: + format: string + type: string + type: array + allowOrigins: + description: String patterns that match allowed origins + type: array + items: + type: object + oneOf: + - required: + - exact + - required: + - prefix + - required: + - regex + properties: + exact: + format: string + type: string + prefix: + format: string + type: string + regex: + format: string + type: string + exposeHeaders: + items: + format: string + type: string + type: array + maxAge: + type: string gateways: - description: Gateway list - type: array - items: - type: string - hosts: - description: Host list + description: The list of Istio gateway for this virtual service type: array items: type: string @@ -206,12 +369,35 @@ spec: type: array items: type: object + properties: + headers: + type: object + additionalProperties: + oneOf: + - required: ["exact"] + - required: ["prefix"] + - required: ["suffix"] + - required: ["regex"] + type: object + properties: + exact: + format: string + type: string + prefix: + format: string + type: string + suffix: + format: string + type: string + regex: + format: string + type: string metrics: description: Metric check list for this canary type: array items: type: object - required: ["name", "threshold"] + required: ["name"] properties: name: description: Name of the metric @@ -223,6 +409,16 @@ spec: threshold: description: Max value accepted for this metric type: number + thresholdRange: + description: Range accepted for this metric + type: object + properties: + min: + description: Min value accepted for this metric + type: number + max: + description: Max value accepted for this metric + type: number query: description: Prometheus query type: string @@ -269,9 +465,9 @@ spec: pattern: "^[0-9]+(m|s)" metadata: description: Metadata (key-value pairs) for this webhook - anyOf: - - type: string - - type: object + type: object + additionalProperties: + type: string status: properties: phase: diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml index 1189b8307..c97962e4a 100644 --- a/kustomize/base/flagger/crd.yaml +++ b/kustomize/base/flagger/crd.yaml @@ -78,7 +78,7 @@ spec: description: Deployment progress deadline type: number targetRef: - description: Deployment selector + description: Target selector type: object required: ["apiVersion", "kind", "name"] properties: @@ -86,32 +86,35 @@ spec: type: string kind: type: string + enum: + - Deployment + - Service name: type: string autoscalerRef: description: HPA selector - anyOf: - - type: string - - type: object + type: object required: ["apiVersion", "kind", "name"] properties: apiVersion: type: string kind: type: string + enum: + - HorizontalPodAutoscaler name: type: string ingressRef: description: NGINX ingress selector - anyOf: - - type: string - - type: object + type: object required: ["apiVersion", "kind", "name"] properties: apiVersion: type: string kind: type: string + enum: + - Ingress name: type: string service: @@ -136,6 +139,9 @@ spec: portDiscovery: description: Enable port dicovery type: boolean + timeout: + description: HTTP or gRPC request timeout + type: string meshName: description: AppMesh mesh name type: string @@ -144,33 +150,190 @@ spec: type: array items: type: string - timeout: - description: Istio HTTP or gRPC request timeout - type: string + hosts: + description: The list of host names for this service + type: array + items: + type: string trafficPolicy: - description: Istio traffic policy + description: Istio mTLS traffic policy type: object + properties: + tls: + description: TLS related settings for connections to the upstream service + type: object + properties: + caCertificates: + format: string + type: string + clientCertificate: + description: REQUIRED if mode is `MUTUAL`. + format: string + type: string + mode: + enum: + - DISABLE + - SIMPLE + - MUTUAL + - ISTIO_MUTUAL + type: string + privateKey: + description: REQUIRED if mode is `MUTUAL`. + format: string + type: string + sni: + description: SNI string to present to the server + during TLS handshake. + format: string + type: string + subjectAltNames: + items: + format: string + type: string + type: array match: - description: URL match conditions + description: URI match conditions type: array items: type: object + properties: + uri: + type: object + oneOf: + - required: ["exact"] + - required: ["prefix"] + - required: ["suffix"] + - required: ["regex"] + properties: + exact: + format: string + type: string + prefix: + format: string + type: string + suffix: + format: string + type: string + regex: + format: string + type: string + retries: + description: Retry policy for HTTP requests + type: object + properties: + attempts: + description: Number of retries for a given request + format: int32 + type: integer + perTryTimeout: + description: Timeout per retry attempt for a given request + type: string + retryOn: + description: Specifies the conditions under which retry takes place + format: string + type: string rewrite: - description: URL rewrite + description: Rewrite HTTP URIs type: object + properties: + uri: + format: string + type: string headers: description: Headers operations type: object + properties: + request: + properties: + add: + additionalProperties: + format: string + type: string + type: object + remove: + items: + format: string + type: string + type: array + set: + additionalProperties: + format: string + type: string + type: object + type: object + response: + properties: + add: + additionalProperties: + format: string + type: string + type: object + remove: + items: + format: string + type: string + type: array + set: + additionalProperties: + format: string + type: string + type: object + type: object corsPolicy: - description: CORS policy + description: Cross-Origin Resource Sharing policy (CORS) type: object + properties: + allowCredentials: + type: boolean + allowHeaders: + items: + format: string + type: string + type: array + allowMethods: + description: List of HTTP methods allowed to access the resource + items: + format: string + type: string + type: array + allowOrigin: + description: The list of origins that are allowed to perform + CORS requests. + items: + format: string + type: string + type: array + allowOrigins: + description: String patterns that match allowed origins + type: array + items: + type: object + oneOf: + - required: + - exact + - required: + - prefix + - required: + - regex + properties: + exact: + format: string + type: string + prefix: + format: string + type: string + regex: + format: string + type: string + exposeHeaders: + items: + format: string + type: string + type: array + maxAge: + type: string gateways: - description: Gateway list - type: array - items: - type: string - hosts: - description: Host list + description: The list of Istio gateway for this virtual service type: array items: type: string @@ -205,12 +368,35 @@ spec: type: array items: type: object + properties: + headers: + type: object + additionalProperties: + oneOf: + - required: ["exact"] + - required: ["prefix"] + - required: ["suffix"] + - required: ["regex"] + type: object + properties: + exact: + format: string + type: string + prefix: + format: string + type: string + suffix: + format: string + type: string + regex: + format: string + type: string metrics: description: Metric check list for this canary type: array items: type: object - required: ["name", "threshold"] + required: ["name"] properties: name: description: Name of the metric @@ -222,6 +408,16 @@ spec: threshold: description: Max value accepted for this metric type: number + thresholdRange: + description: Range accepted for this metric + type: object + properties: + min: + description: Min value accepted for this metric + type: number + max: + description: Max value accepted for this metric + type: number query: description: Prometheus query type: string @@ -268,9 +464,9 @@ spec: pattern: "^[0-9]+(m|s)" metadata: description: Metadata (key-value pairs) for this webhook - anyOf: - - type: string - - type: object + type: object + additionalProperties: + type: string status: properties: phase: From e4e92b33537ff671b4b4fcfddce7a534459d81bd Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Sat, 8 Feb 2020 15:14:52 +0200 Subject: [PATCH 3/5] Add metric threshold range to e2e tests --- test/e2e-kubernetes-tests.sh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/e2e-kubernetes-tests.sh b/test/e2e-kubernetes-tests.sh index 6e50b07c8..65bde77bc 100755 --- a/test/e2e-kubernetes-tests.sh +++ b/test/e2e-kubernetes-tests.sh @@ -40,12 +40,14 @@ spec: threshold: 10 iterations: 5 metrics: - - name: request-success-rate - threshold: 99 - interval: 1m - - name: request-duration - threshold: 500 - interval: 30s + - name: request-success-rate + interval: 1m + thresholdRange: + min: 99 + - name: request-duration + interval: 30s + thresholdRange: + max: 500 webhooks: - name: "gate" type: confirm-rollout From 8f99e589a60da7ac422a94d7b80d65cb81db22eb Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Sat, 8 Feb 2020 19:05:25 +0200 Subject: [PATCH 4/5] Add metrics to controller tests Fix: #387 --- pkg/controller/controller_test.go | 38 ++++++++++++++++++++++------- pkg/controller/scheduler_test.go | 36 ++++++++++++++++++++++++--- pkg/metrics/providers/prometheus.go | 2 +- 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 49c0ef5bc..ef3006e52 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -250,14 +250,26 @@ func newTestCanary() *flaggerv1.Canary { MaxWeight: 50, Metrics: []flaggerv1.CanaryMetric{ { - Name: "istio_requests_total", + Name: "request-success-rate", Threshold: 99, Interval: "1m", }, { - Name: "istio_request_duration_seconds_bucket", - Threshold: 500, - Interval: "1m", + Name: "request-duration", + ThresholdRange: &flaggerv1.CanaryThresholdRange{ + Min: toFloatPtr(0), + Max: toFloatPtr(500000), + }, + Interval: "1m", + }, + { + Name: "custom", + ThresholdRange: &flaggerv1.CanaryThresholdRange{ + Min: toFloatPtr(0), + Max: toFloatPtr(500000), + }, + Interval: "1m", + Query: "fake", }, }, }, @@ -266,6 +278,11 @@ func newTestCanary() *flaggerv1.Canary { return cd } +func toFloatPtr(val int) *float64 { + v := float64(val) + return &v +} + func newTestCanaryMirror() *flaggerv1.Canary { cd := newTestCanary() cd.Spec.CanaryAnalysis.Mirror = true @@ -305,13 +322,16 @@ func newTestCanaryAB() *flaggerv1.Canary { }, Metrics: []flaggerv1.CanaryMetric{ { - Name: "istio_requests_total", - Threshold: 99, - Interval: "1m", + Name: "request-success-rate", + ThresholdRange: &flaggerv1.CanaryThresholdRange{ + Min: toFloatPtr(99), + Max: toFloatPtr(100), + }, + Interval: "1m", }, { - Name: "istio_request_duration_seconds_bucket", - Threshold: 500, + Name: "request-duration", + Threshold: 500000, Interval: "1m", }, }, diff --git a/pkg/controller/scheduler_test.go b/pkg/controller/scheduler_test.go index 5d66f641b..96ee6d898 100644 --- a/pkg/controller/scheduler_test.go +++ b/pkg/controller/scheduler_test.go @@ -50,15 +50,45 @@ func TestScheduler_Rollback(t *testing.T) { mocks.ctrl.advanceCanary("podinfo", "default", true) // update failed checks to max - err := mocks.deployer.SyncStatus(mocks.canary, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryPhaseProgressing, FailedChecks: 11}) + err := mocks.deployer.SyncStatus(mocks.canary, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryPhaseProgressing, FailedChecks: 10}) if err != nil { t.Fatal(err.Error()) } - // detect changes + // set a metric check to fail + c, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + cd := c.DeepCopy() + cd.Spec.CanaryAnalysis.Metrics = append(c.Spec.CanaryAnalysis.Metrics, flaggerv1.CanaryMetric{ + Name: "fail", + Interval: "1m", + ThresholdRange: &flaggerv1.CanaryThresholdRange{ + Min: toFloatPtr(0), + Max: toFloatPtr(50), + }, + Query: "fail", + }) + _, err = mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Update(cd) + if err != nil { + t.Fatal(err.Error()) + } + + // run metric checks mocks.ctrl.advanceCanary("podinfo", "default", true) + if err != nil { + t.Fatal(err.Error()) + } - c, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get("podinfo", metav1.GetOptions{}) + // finalise analysis + mocks.ctrl.advanceCanary("podinfo", "default", true) + if err != nil { + t.Fatal(err.Error()) + } + + // check status + c, err = mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get("podinfo", metav1.GetOptions{}) if err != nil { t.Fatal(err.Error()) } diff --git a/pkg/metrics/providers/prometheus.go b/pkg/metrics/providers/prometheus.go index 9823cb61d..8fe215d8c 100644 --- a/pkg/metrics/providers/prometheus.go +++ b/pkg/metrics/providers/prometheus.go @@ -67,7 +67,7 @@ func NewPrometheusProvider(provider flaggerv1.MetricTemplateProvider, credential // RunQuery executes the promQL query and returns the the first result as float64 func (p *PrometheusProvider) RunQuery(query string) (float64, error) { - if p.url.Host == "fake" { + if p.url.String() == "fake" { return 100, nil } From 5c479d9d80ef1ad40afeb94897b321471541e988 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Sat, 8 Feb 2020 20:02:09 +0200 Subject: [PATCH 5/5] Add metric templates to controller tests --- pkg/controller/controller_test.go | 49 +++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index ef3006e52..c6f8e0e18 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -49,7 +49,7 @@ func SetupMocks(c *flaggerv1.Canary) Mocks { if c == nil { c = newTestCanary() } - flaggerClient := fakeFlagger.NewSimpleClientset(c) + flaggerClient := fakeFlagger.NewSimpleClientset(c, newTestMetricTemplate()) // init kube clientset and register mock objects kubeClient := fake.NewSimpleClientset( @@ -178,7 +178,9 @@ func NewTestSecret() *corev1.Secret { }, Type: corev1.SecretTypeOpaque, Data: map[string][]byte{ - "apiKey": []byte("test"), + "apiKey": []byte("test"), + "username": []byte("test"), + "password": []byte("test"), }, } } @@ -192,7 +194,9 @@ func NewTestSecretV2() *corev1.Secret { }, Type: corev1.SecretTypeOpaque, Data: map[string][]byte{ - "apiKey": []byte("test2"), + "apiKey": []byte("test2"), + "username": []byte("test"), + "password": []byte("test"), }, } } @@ -266,10 +270,13 @@ func newTestCanary() *flaggerv1.Canary { Name: "custom", ThresholdRange: &flaggerv1.CanaryThresholdRange{ Min: toFloatPtr(0), - Max: toFloatPtr(500000), + Max: toFloatPtr(100), }, Interval: "1m", - Query: "fake", + TemplateRef: &flaggerv1.MetricTemplateRef{ + Name: "envoy", + Namespace: "default", + }, }, }, }, @@ -334,6 +341,15 @@ func newTestCanaryAB() *flaggerv1.Canary { Threshold: 500000, Interval: "1m", }, + { + Name: "custom", + ThresholdRange: &flaggerv1.CanaryThresholdRange{ + Min: toFloatPtr(0), + Max: toFloatPtr(500000), + }, + Interval: "1m", + Query: "fake", + }, }, }, }, @@ -657,3 +673,26 @@ func newTestHPA() *hpav2.HorizontalPodAutoscaler { return h } + +func newTestMetricTemplate() *flaggerv1.MetricTemplate { + provider := flaggerv1.MetricTemplateProvider{ + Type: "prometheus", + Address: "fake", + SecretRef: &corev1.LocalObjectReference{ + Name: "podinfo-secret-env", + }, + } + + template := &flaggerv1.MetricTemplate{ + TypeMeta: metav1.TypeMeta{APIVersion: flaggerv1.SchemeGroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "envoy", + }, + Spec: flaggerv1.MetricTemplateSpec{ + Provider: provider, + Query: `sum(envoy_cluster_upstream_rq{envoy_cluster_name=~"{{ namespace }}_{{ target }}"})`, + }, + } + return template +}