From 04fea418d158a731ff8eb1b80e05ca190e813896 Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Thu, 26 Sep 2019 21:44:40 -0700 Subject: [PATCH 1/7] Add evaluate conditon util --- Gopkg.lock | 33 +++++++++++++++++++++ utils/evaluate/evaluate.go | 24 ++++++++++++++++ utils/evaluate/evaluate_test.go | 51 +++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+) create mode 100644 utils/evaluate/evaluate.go create mode 100644 utils/evaluate/evaluate_test.go diff --git a/Gopkg.lock b/Gopkg.lock index 793a7a0dbb..86389aadae 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -25,6 +25,33 @@ pruneopts = "" revision = "de5bf2ad457846296e2031421a34e2568e304e35" +[[projects]] + digest = "1:d006584293547a5a28116ac4e02e250a07911e59b3db7fb1b7ab93c25721cdcb" + name = "github.com/antlr/antlr4" + packages = ["runtime/Go/antlr"] + pruneopts = "" + revision = "be58ebffde8e29c154192c019608f0a5b8e6a064" + version = "4.7.2" + +[[projects]] + digest = "1:3fbdd1c39fb005519b1cb4a58723527614d78368868bb7e24a45526e2d5f1740" + name = "github.com/antonmedv/expr" + packages = [ + ".", + "ast", + "checker", + "compiler", + "internal/conf", + "internal/file", + "optimizer", + "parser", + "parser/gen", + "vm", + ] + pruneopts = "" + revision = "4d40254bf70e2ba6f90dea9cc5d36e96336bbe84" + version = "v2.1.2" + [[projects]] branch = "master" digest = "1:c0bec5f9b98d0bc872ff5e834fac186b807b656683bd29cb82fb207a1513fabb" @@ -303,6 +330,8 @@ digest = "1:6f218995d6a74636cfcab45ce03005371e682b4b9bee0e5eb0ccfd83ef85364f" name = "github.com/prometheus/client_golang" packages = [ + "api", + "api/prometheus/v1", "prometheus", "prometheus/internal", "prometheus/promhttp", @@ -1008,14 +1037,18 @@ analyzer-name = "dep" analyzer-version = 1 input-imports = [ + "github.com/antonmedv/expr", "github.com/bouk/monkey", "github.com/emicklei/go-restful", "github.com/ghodss/yaml", "github.com/go-openapi/spec", "github.com/golang/glog", "github.com/pkg/errors", + "github.com/prometheus/client_golang/api", + "github.com/prometheus/client_golang/api/prometheus/v1", "github.com/prometheus/client_golang/prometheus", "github.com/prometheus/client_golang/prometheus/promhttp", + "github.com/prometheus/common/model", "github.com/sirupsen/logrus", "github.com/spf13/cobra", "github.com/stretchr/testify/assert", diff --git a/utils/evaluate/evaluate.go b/utils/evaluate/evaluate.go new file mode 100644 index 0000000000..f2a5d752f2 --- /dev/null +++ b/utils/evaluate/evaluate.go @@ -0,0 +1,24 @@ +package evaluate + +import ( + "github.com/antonmedv/expr" +) + +// EvalCondition evaluates the condition with the resultValue as an input +func EvalCondition(resultValue interface{}, condition string) (bool, error) { + env := map[string]interface{}{ + "result": resultValue, + } + + program, err := expr.Compile(condition, expr.Env(env), expr.AsBool()) + if err != nil { + return false, err + } + + output, err := expr.Run(program, env) + if err != nil { + return false, err + } + + return output.(bool), err +} diff --git a/utils/evaluate/evaluate_test.go b/utils/evaluate/evaluate_test.go new file mode 100644 index 0000000000..9c5fbd22e1 --- /dev/null +++ b/utils/evaluate/evaluate_test.go @@ -0,0 +1,51 @@ +package evaluate + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestEvaluateConditonWithSucces(t *testing.T) { + b, err := EvalCondition(true, "result == true") + assert.Nil(t, err) + assert.True(t, b) +} + +func TestEvaluateConditonWithFailure(t *testing.T) { + b, err := EvalCondition(true, "result == false") + assert.Nil(t, err) + assert.False(t, b) +} + +func TestErrorWithNonBoolReturn(t *testing.T) { + b, err := EvalCondition(true, "1") + assert.Equal(t, fmt.Errorf("expected bool, but got int"), err) + assert.False(t, b) +} + +func TestErrorWithInvalidReference(t *testing.T) { + b, err := EvalCondition(true, "invalidVariable") + assert.Equal(t, fmt.Errorf("unknown name invalidVariable (1:1)\n | invalidVariable\n | ^"), err) + assert.False(t, b) +} + +func TestEvaluateArray(t *testing.T) { + floats := []float64{float64(2), float64(2)} + b, err := EvalCondition(floats, "all(result, {# > 1})") + assert.Nil(t, err) + assert.True(t, b) +} + +func TestEvaluateFloat64(t *testing.T) { + b, err := EvalCondition(float64(5), "result > 1") + assert.Nil(t, err) + assert.True(t, b) +} + +func TestEvaluateInvalidStruct(t *testing.T) { + b, err := EvalCondition(true, "result.Name() == 'hi'") + assert.Errorf(t, err, "") + assert.False(t, b) +} From a671e3f39102bdf4792800f84221ba3b206a6b49 Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Fri, 27 Sep 2019 11:29:01 -0700 Subject: [PATCH 2/7] Add inital provider and prometheus implementation --- manifests/crds/analysis-run-crd.yaml | 19 +- manifests/crds/analysis-template-crd.yaml | 19 +- manifests/install.yaml | 38 ++- manifests/namespace-install.yaml | 38 ++- pkg/apis/rollouts/v1alpha1/analysis_types.go | 9 + .../rollouts/v1alpha1/openapi_generated.go | 45 ++- .../v1alpha1/zz_generated.deepcopy.go | 28 +- providers/prometheus/mock_test.go | 74 +++++ providers/prometheus/prometheus.go | 149 ++++++++++ providers/prometheus/prometheus_test.go | 265 ++++++++++++++++++ providers/provider.go | 33 +++ utils/evaluate/evaluate_test.go | 7 + 12 files changed, 686 insertions(+), 38 deletions(-) create mode 100644 providers/prometheus/mock_test.go create mode 100644 providers/prometheus/prometheus.go create mode 100644 providers/prometheus/prometheus_test.go create mode 100644 providers/provider.go diff --git a/manifests/crds/analysis-run-crd.yaml b/manifests/crds/analysis-run-crd.yaml index 9848b15c97..d5c973df45 100644 --- a/manifests/crds/analysis-run-crd.yaml +++ b/manifests/crds/analysis-run-crd.yaml @@ -29,27 +29,36 @@ spec: count: format: int32 type: integer + failFast: + type: boolean failureCondition: type: string interval: format: int32 type: integer + maxConsecutiveErrors: + format: int32 + type: integer maxFailures: format: int32 type: integer name: type: string - prometheus: + provider: properties: - query: - type: string - server: - type: string + prometheus: + properties: + query: + type: string + server: + type: string + type: object type: object successCondition: type: string required: - name + - provider type: object type: array required: diff --git a/manifests/crds/analysis-template-crd.yaml b/manifests/crds/analysis-template-crd.yaml index f1263a745c..28485ff019 100644 --- a/manifests/crds/analysis-template-crd.yaml +++ b/manifests/crds/analysis-template-crd.yaml @@ -27,27 +27,36 @@ spec: count: format: int32 type: integer + failFast: + type: boolean failureCondition: type: string interval: format: int32 type: integer + maxConsecutiveErrors: + format: int32 + type: integer maxFailures: format: int32 type: integer name: type: string - prometheus: + provider: properties: - query: - type: string - server: - type: string + prometheus: + properties: + query: + type: string + server: + type: string + type: object type: object successCondition: type: string required: - name + - provider type: object type: array required: diff --git a/manifests/install.yaml b/manifests/install.yaml index 99c43eb81e..0fef477026 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -30,27 +30,36 @@ spec: count: format: int32 type: integer + failFast: + type: boolean failureCondition: type: string interval: format: int32 type: integer + maxConsecutiveErrors: + format: int32 + type: integer maxFailures: format: int32 type: integer name: type: string - prometheus: + provider: properties: - query: - type: string - server: - type: string + prometheus: + properties: + query: + type: string + server: + type: string + type: object type: object successCondition: type: string required: - name + - provider type: object type: array required: @@ -173,27 +182,36 @@ spec: count: format: int32 type: integer + failFast: + type: boolean failureCondition: type: string interval: format: int32 type: integer + maxConsecutiveErrors: + format: int32 + type: integer maxFailures: format: int32 type: integer name: type: string - prometheus: + provider: properties: - query: - type: string - server: - type: string + prometheus: + properties: + query: + type: string + server: + type: string + type: object type: object successCondition: type: string required: - name + - provider type: object type: array required: diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index 3dee647f19..f4fe801718 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -30,27 +30,36 @@ spec: count: format: int32 type: integer + failFast: + type: boolean failureCondition: type: string interval: format: int32 type: integer + maxConsecutiveErrors: + format: int32 + type: integer maxFailures: format: int32 type: integer name: type: string - prometheus: + provider: properties: - query: - type: string - server: - type: string + prometheus: + properties: + query: + type: string + server: + type: string + type: object type: object successCondition: type: string required: - name + - provider type: object type: array required: @@ -173,27 +182,36 @@ spec: count: format: int32 type: integer + failFast: + type: boolean failureCondition: type: string interval: format: int32 type: integer + maxConsecutiveErrors: + format: int32 + type: integer maxFailures: format: int32 type: integer name: type: string - prometheus: + provider: properties: - query: - type: string - server: - type: string + prometheus: + properties: + query: + type: string + server: + type: string + type: object type: object successCondition: type: string required: - name + - provider type: object type: array required: diff --git a/pkg/apis/rollouts/v1alpha1/analysis_types.go b/pkg/apis/rollouts/v1alpha1/analysis_types.go index 8b92e35896..ef171358e4 100644 --- a/pkg/apis/rollouts/v1alpha1/analysis_types.go +++ b/pkg/apis/rollouts/v1alpha1/analysis_types.go @@ -55,6 +55,15 @@ type Metric struct { // MaxConsecutiveErrors is the maximum number of times the measurement is allowed to error in // succession, before the metric is considered error (default: 4) MaxConsecutiveErrors *int32 `json:"maxConsecutiveErrors,omitempty"` + // FailFast will fail the entire analysis run prematurely + FailFast bool `json:"failFast,omitempty"` + // Provider configuration to the external system to use to verify the analysis + Provider AnalysisProvider `json:"provider"` +} + +// AnalysisProvider which external system to use to verify the analysis +// Only one of the fields in this struct should be non-nil +type AnalysisProvider struct { // PrometheusMetric specifies the prometheus metric to query Prometheus *PrometheusMetric `json:"prometheus,omitempty"` } diff --git a/pkg/apis/rollouts/v1alpha1/openapi_generated.go b/pkg/apis/rollouts/v1alpha1/openapi_generated.go index 23f25aa49e..9e180b0e79 100644 --- a/pkg/apis/rollouts/v1alpha1/openapi_generated.go +++ b/pkg/apis/rollouts/v1alpha1/openapi_generated.go @@ -29,6 +29,7 @@ import ( func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { return map[string]common.OpenAPIDefinition{ + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.AnalysisProvider": schema_pkg_apis_rollouts_v1alpha1_AnalysisProvider(ref), "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.AnalysisRun": schema_pkg_apis_rollouts_v1alpha1_AnalysisRun(ref), "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.AnalysisRunList": schema_pkg_apis_rollouts_v1alpha1_AnalysisRunList(ref), "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.AnalysisRunSpec": schema_pkg_apis_rollouts_v1alpha1_AnalysisRunSpec(ref), @@ -66,6 +67,26 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA } } +func schema_pkg_apis_rollouts_v1alpha1_AnalysisProvider(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "AnalysisProvider which external system to use to verify the analysis Only one of the fields in this struct should be non-nil", + Properties: map[string]spec.Schema{ + "prometheus": { + SchemaProps: spec.SchemaProps{ + Description: "PrometheusMetric specifies the prometheus metric to query", + Ref: ref("github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.PrometheusMetric"), + }, + }, + }, + }, + }, + Dependencies: []string{ + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.PrometheusMetric"}, + } +} + func schema_pkg_apis_rollouts_v1alpha1_AnalysisRun(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -953,18 +974,32 @@ func schema_pkg_apis_rollouts_v1alpha1_Metric(ref common.ReferenceCallback) comm Format: "int32", }, }, - "prometheus": { + "maxConsecutiveErrors": { SchemaProps: spec.SchemaProps{ - Description: "PrometheusMetric specifies the prometheus metric to query", - Ref: ref("github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.PrometheusMetric"), + Description: "MaxConsecutiveErrors is the maximum number of times the measurement is allowed to error in succession, before the metric is considered error (default: 4)", + Type: []string{"integer"}, + Format: "int32", + }, + }, + "failFast": { + SchemaProps: spec.SchemaProps{ + Description: "FailFast will fail the entire analysis run prematurely", + Type: []string{"boolean"}, + Format: "", + }, + }, + "provider": { + SchemaProps: spec.SchemaProps{ + Description: "Provider configuration to the external system to use to verify the analysis", + Ref: ref("github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.AnalysisProvider"), }, }, }, - Required: []string{"name"}, + Required: []string{"name", "provider"}, }, }, Dependencies: []string{ - "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.PrometheusMetric"}, + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.AnalysisProvider"}, } } diff --git a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go index 821c7e8a90..89a36ec97b 100644 --- a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go @@ -26,6 +26,27 @@ import ( intstr "k8s.io/apimachinery/pkg/util/intstr" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AnalysisProvider) DeepCopyInto(out *AnalysisProvider) { + *out = *in + if in.Prometheus != nil { + in, out := &in.Prometheus, &out.Prometheus + *out = new(PrometheusMetric) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AnalysisProvider. +func (in *AnalysisProvider) DeepCopy() *AnalysisProvider { + if in == nil { + return nil + } + out := new(AnalysisProvider) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AnalysisRun) DeepCopyInto(out *AnalysisRun) { *out = *in @@ -571,11 +592,12 @@ func (in *Metric) DeepCopyInto(out *Metric) { *out = new(int32) **out = **in } - if in.Prometheus != nil { - in, out := &in.Prometheus, &out.Prometheus - *out = new(PrometheusMetric) + if in.MaxConsecutiveErrors != nil { + in, out := &in.MaxConsecutiveErrors, &out.MaxConsecutiveErrors + *out = new(int32) **out = **in } + in.Provider.DeepCopyInto(&out.Provider) return } diff --git a/providers/prometheus/mock_test.go b/providers/prometheus/mock_test.go new file mode 100644 index 0000000000..ff6f6debbc --- /dev/null +++ b/providers/prometheus/mock_test.go @@ -0,0 +1,74 @@ +package prometheus + +import ( + "context" + "time" + + v1 "github.com/prometheus/client_golang/api/prometheus/v1" + "github.com/prometheus/common/model" +) + +type mockAPI struct { + value model.Value + err error +} + +// Query performs a query for the given time. +func (m mockAPI) Query(ctx context.Context, query string, ts time.Time) (model.Value, error) { + if m.err != nil { + return nil, m.err + } + return m.value, nil +} + +// Below methods are not used but required for the interface implementation + +func (m mockAPI) AlertManagers(ctx context.Context) (v1.AlertManagersResult, error) { + return v1.AlertManagersResult{}, nil +} + +// CleanTombstones removes the deleted data from disk and cleans up the existing tombstones. +func (m mockAPI) CleanTombstones(ctx context.Context) error { + return nil +} + +// Config returns the current Prometheus configuration. +func (m mockAPI) Config(ctx context.Context) (v1.ConfigResult, error) { + return v1.ConfigResult{}, nil +} + +// DeleteSeries deletes data for a selection of series in a time range. +func (m mockAPI) DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) error { + return nil +} + +// Flags returns the flag values that Prometheus was launched with. +func (m mockAPI) Flags(ctx context.Context) (v1.FlagsResult, error) { + return v1.FlagsResult{}, nil +} + +// LabelValues performs a query for the values of the given label. +func (m mockAPI) LabelValues(ctx context.Context, label string) (model.LabelValues, error) { + return nil, nil +} + +// QueryRange performs a query for the given range. +func (m mockAPI) QueryRange(ctx context.Context, query string, r v1.Range) (model.Value, error) { + return nil, nil +} + +// Series finds series by label matchers. +func (m mockAPI) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, error) { + return nil, nil +} + +// Snapshot creates a snapshot of all current data into snapshots/- +// under the TSDB's data directory and returns the directory as response. +func (m mockAPI) Snapshot(ctx context.Context, skipHead bool) (v1.SnapshotResult, error) { + return v1.SnapshotResult{}, nil +} + +// Targets returns an overview of the current state of the Prometheus target discovery. +func (m mockAPI) Targets(ctx context.Context) (v1.TargetsResult, error) { + return v1.TargetsResult{}, nil +} diff --git a/providers/prometheus/prometheus.go b/providers/prometheus/prometheus.go new file mode 100644 index 0000000000..b77d5fc846 --- /dev/null +++ b/providers/prometheus/prometheus.go @@ -0,0 +1,149 @@ +package prometheus + +import ( + "context" + "fmt" + "time" + + "github.com/prometheus/client_golang/api" + v1 "github.com/prometheus/client_golang/api/prometheus/v1" + "github.com/prometheus/common/model" + log "github.com/sirupsen/logrus" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" + "github.com/argoproj/argo-rollouts/utils/evaluate" +) + +const ( + //ProviderType indicates the provider is prometheus + ProviderType = "Prometheus" +) + +// Provider contains all the required components to run a prometheus query +type Provider struct { + api v1.API + logCtx log.Entry +} + +// Type incidates provider is a prometheus provider +func (p *Provider) Type() string { + return ProviderType +} + +// Run queries prometheus for the metric +func (p *Provider) Run(metric v1alpha1.AnalysisMetric, args []v1alpha1.Argument) (v1alpha1.Measurement, error) { + startTime := metav1.Now() + newMeasurement := v1alpha1.Measurement{ + StartedAt: &startTime, + } + + //TODO(dthomson) make timeout configuriable + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + //TODO(dthomson) build query with args + response, err := p.api.Query(ctx, metric.Provider.Prometheus.Query, time.Now()) + if err != nil { + finishedTime := metav1.Now() + newMeasurement.Status = v1alpha1.AnalysisStatusError + newMeasurement.FinishedAt = &finishedTime + return newMeasurement, err + } + + newValue, newStatus, err := p.processResponse(metric, response) + if err != nil { + finishedTime := metav1.Now() + newMeasurement.Status = v1alpha1.AnalysisStatusError + newMeasurement.FinishedAt = &finishedTime + return newMeasurement, err + } + newMeasurement.Value = newValue + + newMeasurement.Status = newStatus + // if newStatus.Completed() { + // finishedTime := metav1.Now() + // p.measurement.FinishedAt = &finishedTime + // } + return newMeasurement, nil +} + +// Resume should not be used the prometheus provider since all the work should occur in the Run method +func (p *Provider) Resume(metric v1alpha1.AnalysisMetric, args []v1alpha1.Argument, measurement v1alpha1.Measurement) (v1alpha1.Measurement, error) { + p.logCtx.Warn("Prometheus provider should not execute the Resume method") + return measurement, nil +} + +func (p *Provider) evaluateResult(result interface{}, metric v1alpha1.AnalysisMetric) v1alpha1.AnalysisStatus { + successCondition, err := evaluate.EvalCondition(result, metric.SuccessCondition) + if err != nil { + p.logCtx.Warning(err.Error()) + return v1alpha1.AnalysisStatusError + } + + failCondition, err := evaluate.EvalCondition(result, metric.FailureCondition) + if err != nil { + return v1alpha1.AnalysisStatusError + } + if failCondition { + return v1alpha1.AnalysisStatusFailed + } + + if !failCondition && !successCondition { + return v1alpha1.AnalysisStatusInconclusive + } + + if successCondition { + return v1alpha1.AnalysisStatusSuccessful + } + + return v1alpha1.AnalysisStatusPending +} + +func (p *Provider) processResponse(metric v1alpha1.AnalysisMetric, response model.Value) (string, v1alpha1.AnalysisStatus, error) { + switch value := response.(type) { + case *model.Scalar: + valueStr := value.Value.String() + result := float64(value.Value) + newStatus := p.evaluateResult(result, metric) + return valueStr, newStatus, nil + case model.Vector: + result := make([]float64, len(value)) + valueStr := "[" + for _, s := range value { + if s != nil { + valueStr = valueStr + s.Value.String() + "," + result = append(result, float64(s.Value)) + } + } + // if we appended to the string, we should remove the last comma on the string + if len(valueStr) > 1 { + valueStr = valueStr[:len(valueStr)-1] + } + valueStr = valueStr + "]" + newStatus := p.evaluateResult(result, metric) + return valueStr, newStatus, nil + //TODO(dthomson) add other response types + default: + return "", v1alpha1.AnalysisStatusError, fmt.Errorf("Prometheus metric type not supported") + } +} + +// NewPrometheusProvider Creates a new Prometheus client +func NewPrometheusProvider(api v1.API, logCtx log.Entry) *Provider { + return &Provider{ + logCtx: logCtx, + api: api, + } +} + +// NewPrometheusAPI generates a prometheus API from the metric configuration +func NewPrometheusAPI(metric v1alpha1.AnalysisMetric) (v1.API, error) { + client, err := api.NewClient(api.Config{ + Address: metric.Provider.Prometheus.Server, + }) + if err != nil { + return nil, err + } + + return v1.NewAPI(client), nil +} diff --git a/providers/prometheus/prometheus_test.go b/providers/prometheus/prometheus_test.go new file mode 100644 index 0000000000..e50eb9f457 --- /dev/null +++ b/providers/prometheus/prometheus_test.go @@ -0,0 +1,265 @@ +package prometheus + +import ( + "fmt" + "testing" + + "github.com/prometheus/common/model" + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" +) + +func newScalar(f float64) model.Value { + return &model.Scalar{ + Value: model.SampleValue(f), + Timestamp: model.Time(0), + } +} + +func TestType(t *testing.T) { + e := log.Entry{} + mock := mockAPI{ + value: newScalar(10), + } + p := NewPrometheusProvider(mock, e) + assert.Equal(t, ProviderType, p.Type()) +} + +func TestRunSuccessFully(t *testing.T) { + e := log.Entry{} + mock := mockAPI{ + value: newScalar(10), + } + p := NewPrometheusProvider(mock, e) + metric := v1alpha1.AnalysisMetric{ + Name: "foo", + SuccessCondition: "result == 10", + FailureCondition: "result != 10", + Provider: v1alpha1.AnalysisProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Query: "test", + }, + }, + } + measurement, err := p.Run(metric, []v1alpha1.Argument{}) + assert.Nil(t, err) + assert.NotNil(t, measurement.StartedAt) + assert.Equal(t, "10", measurement.Value) + // assert.NotNil(t, measurement.FinishedAt) + // assert.Equal(t, v1alpha1.AnalysisStatusSuccessful, measurement.Status) +} + +func TestRunWithQueryError(t *testing.T) { + e := log.Entry{} + expectedErr := fmt.Errorf("bad big bug :(") + mock := mockAPI{ + err: expectedErr, + } + p := NewPrometheusProvider(mock, e) + metric := v1alpha1.AnalysisMetric{ + Name: "foo", + SuccessCondition: "result == 10", + FailureCondition: "result != 10", + Provider: v1alpha1.AnalysisProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Query: "test", + }, + }, + } + measurement, err := p.Run(metric, []v1alpha1.Argument{}) + assert.Equal(t, expectedErr, err) + assert.NotNil(t, measurement.StartedAt) + assert.Equal(t, "", measurement.Value) + assert.NotNil(t, measurement.FinishedAt) + assert.Equal(t, v1alpha1.AnalysisStatusError, measurement.Status) +} + +func TestRunWithEvaluationError(t *testing.T) { + e := log.WithField("", "") + mock := mockAPI{} + p := NewPrometheusProvider(mock, *e) + metric := v1alpha1.AnalysisMetric{ + Name: "foo", + SuccessCondition: "result == 10", + FailureCondition: "result != 10", + Provider: v1alpha1.AnalysisProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Query: "test", + }, + }, + } + measurement, err := p.Run(metric, []v1alpha1.Argument{}) + assert.NotNil(t, err) + assert.NotNil(t, measurement.StartedAt) + assert.Equal(t, "", measurement.Value) + assert.NotNil(t, measurement.FinishedAt) + assert.Equal(t, v1alpha1.AnalysisStatusError, measurement.Status) +} + +func TestResume(t *testing.T) { + e := log.WithField("", "") + mock := mockAPI{} + p := NewPrometheusProvider(mock, *e) + metric := v1alpha1.AnalysisMetric{ + Name: "foo", + SuccessCondition: "result == 10", + FailureCondition: "result != 10", + Provider: v1alpha1.AnalysisProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Query: "test", + }, + }, + } + now := metav1.Now() + previousMeasuresment := v1alpha1.Measurement{ + StartedAt: &now, + Status: v1alpha1.AnalysisStatusInconclusive, + } + measurement, err := p.Resume(metric, []v1alpha1.Argument{}, previousMeasuresment) + assert.Nil(t, err) + assert.Equal(t, previousMeasuresment, measurement) +} + +func TestEvaluateResultWithSuccess(t *testing.T) { + p := Provider{} + metric := v1alpha1.AnalysisMetric{ + SuccessCondition: "true", + FailureCondition: "false", + } + status := p.evaluateResult(true, metric) + assert.Equal(t, v1alpha1.AnalysisStatusSuccessful, status) +} + +func TestEvaluateResultWithFailure(t *testing.T) { + p := Provider{} + metric := v1alpha1.AnalysisMetric{ + SuccessCondition: "true", + FailureCondition: "true", + } + status := p.evaluateResult(true, metric) + assert.Equal(t, v1alpha1.AnalysisStatusFailed, status) + +} + +func TestEvaluateResultInconclusive(t *testing.T) { + p := Provider{} + metric := v1alpha1.AnalysisMetric{ + SuccessCondition: "false", + FailureCondition: "false", + } + status := p.evaluateResult(true, metric) + assert.Equal(t, v1alpha1.AnalysisStatusInconclusive, status) +} + +func TestEvaluateResultWithErrorOnSuccessCondition(t *testing.T) { + logCtx := log.WithField("test", "test") + p := Provider{ + logCtx: *logCtx, + } + metric := v1alpha1.AnalysisMetric{ + SuccessCondition: "a == true", + FailureCondition: "true", + } + status := p.evaluateResult(true, metric) + assert.Equal(t, v1alpha1.AnalysisStatusError, status) + +} + +func TestEvaluateResultWithErrorOnFailureCondition(t *testing.T) { + logCtx := log.WithField("test", "test") + p := Provider{ + logCtx: *logCtx, + } + metric := v1alpha1.AnalysisMetric{ + SuccessCondition: "true", + FailureCondition: "a == true", + } + status := p.evaluateResult(true, metric) + assert.Equal(t, v1alpha1.AnalysisStatusError, status) + +} + +func TestProcessScalarResponse(t *testing.T) { + logCtx := log.WithField("test", "test") + p := Provider{ + logCtx: *logCtx, + } + metric := v1alpha1.AnalysisMetric{ + SuccessCondition: "result == 10", + FailureCondition: "result != 10", + } + + response := &model.Scalar{ + Value: model.SampleValue(10), + Timestamp: model.Time(0), + } + + value, status, err := p.processResponse(metric, response) + assert.Nil(t, err) + assert.Equal(t, v1alpha1.AnalysisStatusSuccessful, status) + assert.Equal(t, "10", value) + +} + +func TestProcessVectorResponse(t *testing.T) { + logCtx := log.WithField("test", "test") + p := Provider{ + logCtx: *logCtx, + } + metric := v1alpha1.AnalysisMetric{ + SuccessCondition: "10 in result", + FailureCondition: "false", + } + + response := model.Vector{ + { + Value: model.SampleValue(10), + Timestamp: model.Time(0), + }, + { + Value: model.SampleValue(11), + Timestamp: model.Time(0), + }, + } + value, status, err := p.processResponse(metric, response) + assert.Nil(t, err) + assert.Equal(t, v1alpha1.AnalysisStatusSuccessful, status) + assert.Equal(t, "[10,11]", value) + +} + +func TestProcessInvalidResponse(t *testing.T) { + logCtx := log.WithField("test", "test") + p := Provider{ + logCtx: *logCtx, + } + metric := v1alpha1.AnalysisMetric{ + SuccessCondition: "true", + FailureCondition: "true", + } + + value, status, err := p.processResponse(metric, nil) + assert.NotNil(t, err) + assert.Equal(t, v1alpha1.AnalysisStatusError, status) + assert.Equal(t, "", value) + +} + +func TestNewPrometheusAPI(t *testing.T) { + metric := v1alpha1.AnalysisMetric{ + Provider: v1alpha1.AnalysisProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Server: ":invalid::url", + }, + }, + } + _, err := NewPrometheusAPI(metric) + assert.NotNil(t, err) + + metric.Provider.Prometheus.Server = "https://www.example.com" + _, err = NewPrometheusAPI(metric) + assert.Nil(t, err) +} diff --git a/providers/provider.go b/providers/provider.go new file mode 100644 index 0000000000..750f9e2884 --- /dev/null +++ b/providers/provider.go @@ -0,0 +1,33 @@ +package providers + +import ( + "fmt" + + log "github.com/sirupsen/logrus" + + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" + "github.com/argoproj/argo-rollouts/providers/prometheus" +) + +// Provider methods to query a external systems and generate a measurement +type Provider interface { + // Run start a new external system call for a measurement + // should be idoponent and do nothing is a call has been started + Run(v1alpha1.AnalysisMetric, []v1alpha1.Argument) (v1alpha1.Measurement, error) + // Checks if the external system call is finished and returns the current measuremtn + Resume(v1alpha1.AnalysisMetric, []v1alpha1.Argument, v1alpha1.Measurement) (v1alpha1.Measurement, error) + // Type gets the provider type + Type() string +} + +// NewProvider creates the correct provider based on the provider type of the analysisMetric +func NewProvider(logCtx log.Entry, metric v1alpha1.AnalysisMetric) (Provider, error) { + if metric.Provider.Prometheus != nil { + api, err := prometheus.NewPrometheusAPI(metric) + if err != nil { + return nil, err + } + return prometheus.NewPrometheusProvider(api, logCtx), nil + } + return nil, fmt.Errorf("no valid provider in metric '%s'", metric.Name) +} diff --git a/utils/evaluate/evaluate_test.go b/utils/evaluate/evaluate_test.go index 9c5fbd22e1..796a1718e2 100644 --- a/utils/evaluate/evaluate_test.go +++ b/utils/evaluate/evaluate_test.go @@ -38,6 +38,13 @@ func TestEvaluateArray(t *testing.T) { assert.True(t, b) } +func TestEvaluatInOperator(t *testing.T) { + floats := []float64{float64(2), float64(2)} + b, err := EvalCondition(floats, "2 in result") + assert.Nil(t, err) + assert.True(t, b) +} + func TestEvaluateFloat64(t *testing.T) { b, err := EvalCondition(float64(5), "result > 1") assert.Nil(t, err) From 27a85c096b6f225819c08a3746fbb788b6c37f2f Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Fri, 27 Sep 2019 11:41:22 -0700 Subject: [PATCH 3/7] Remove dead code --- providers/prometheus/prometheus.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/providers/prometheus/prometheus.go b/providers/prometheus/prometheus.go index b77d5fc846..8f573878dc 100644 --- a/providers/prometheus/prometheus.go +++ b/providers/prometheus/prometheus.go @@ -92,11 +92,8 @@ func (p *Provider) evaluateResult(result interface{}, metric v1alpha1.AnalysisMe return v1alpha1.AnalysisStatusInconclusive } - if successCondition { - return v1alpha1.AnalysisStatusSuccessful - } - - return v1alpha1.AnalysisStatusPending + // If we reach this code path, failCondition is false and successCondition is true + return v1alpha1.AnalysisStatusSuccessful } func (p *Provider) processResponse(metric v1alpha1.AnalysisMetric, response model.Value) (string, v1alpha1.AnalysisStatus, error) { From 127ddcfaffe68b2b432a67c980565e232e7a9e0e Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Fri, 27 Sep 2019 13:03:30 -0700 Subject: [PATCH 4/7] Add initial buildQuery functionality --- Gopkg.lock | 17 +++++++++++ providers/prometheus/prometheus.go | 29 ++++++++++++------- providers/prometheus/prometheus_test.go | 23 +++++++++++++++ utils/query/query.go | 38 +++++++++++++++++++++++++ utils/query/query_test.go | 37 ++++++++++++++++++++++++ 5 files changed, 134 insertions(+), 10 deletions(-) create mode 100644 utils/query/query.go create mode 100644 utils/query/query_test.go diff --git a/Gopkg.lock b/Gopkg.lock index 86389aadae..cfb182e478 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -406,6 +406,22 @@ revision = "f35b8ab0b5a2cef36673838d662e249dd9c94686" version = "v1.2.2" +[[projects]] + digest = "1:857a9ecd5cb13379ecc8f798f6e6b6b574c98b9355657d91e068275f1120aaf7" + name = "github.com/valyala/bytebufferpool" + packages = ["."] + pruneopts = "" + revision = "e746df99fe4a3986f4d4f79e13c1e0117ce9c2f7" + version = "v1.0.0" + +[[projects]] + digest = "1:1ad3ae8a5edf223abb8435e16ccc0696ce886c6f889e100809122e931e7423b2" + name = "github.com/valyala/fasttemplate" + packages = ["."] + pruneopts = "" + revision = "8b5e4e491ab636663841c42ea3c5a9adebabaf36" + version = "v1.0.1" + [[projects]] branch = "master" digest = "1:f7be435e0ca22e2cd62b2d2542081a231685837170a87a3662abb7cdf9f3f1cd" @@ -1052,6 +1068,7 @@ "github.com/sirupsen/logrus", "github.com/spf13/cobra", "github.com/stretchr/testify/assert", + "github.com/valyala/fasttemplate", "k8s.io/api/apps/v1", "k8s.io/api/core/v1", "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1", diff --git a/providers/prometheus/prometheus.go b/providers/prometheus/prometheus.go index 8f573878dc..8cbe84d6c6 100644 --- a/providers/prometheus/prometheus.go +++ b/providers/prometheus/prometheus.go @@ -5,6 +5,8 @@ import ( "fmt" "time" + "github.com/argoproj/argo-rollouts/utils/query" + "github.com/prometheus/client_golang/api" v1 "github.com/prometheus/client_golang/api/prometheus/v1" "github.com/prometheus/common/model" @@ -31,6 +33,13 @@ func (p *Provider) Type() string { return ProviderType } +func failOnError(m v1alpha1.Measurement, err error) (v1alpha1.Measurement, error) { + finishedTime := metav1.Now() + m.Status = v1alpha1.AnalysisStatusError + m.FinishedAt = &finishedTime + return m, err +} + // Run queries prometheus for the metric func (p *Provider) Run(metric v1alpha1.AnalysisMetric, args []v1alpha1.Argument) (v1alpha1.Measurement, error) { startTime := metav1.Now() @@ -41,21 +50,21 @@ func (p *Provider) Run(metric v1alpha1.AnalysisMetric, args []v1alpha1.Argument) //TODO(dthomson) make timeout configuriable ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - //TODO(dthomson) build query with args - response, err := p.api.Query(ctx, metric.Provider.Prometheus.Query, time.Now()) + + query, err := query.BuildQuery(metric.Provider.Prometheus.Query, args) if err != nil { - finishedTime := metav1.Now() - newMeasurement.Status = v1alpha1.AnalysisStatusError - newMeasurement.FinishedAt = &finishedTime - return newMeasurement, err + return failOnError(newMeasurement, err) + } + + response, err := p.api.Query(ctx, query, time.Now()) + if err != nil { + return failOnError(newMeasurement, err) } newValue, newStatus, err := p.processResponse(metric, response) if err != nil { - finishedTime := metav1.Now() - newMeasurement.Status = v1alpha1.AnalysisStatusError - newMeasurement.FinishedAt = &finishedTime - return newMeasurement, err + return failOnError(newMeasurement, err) + } newMeasurement.Value = newValue diff --git a/providers/prometheus/prometheus_test.go b/providers/prometheus/prometheus_test.go index e50eb9f457..f345083af3 100644 --- a/providers/prometheus/prometheus_test.go +++ b/providers/prometheus/prometheus_test.go @@ -77,6 +77,29 @@ func TestRunWithQueryError(t *testing.T) { assert.Equal(t, v1alpha1.AnalysisStatusError, measurement.Status) } +func TestRunWithBuildQueryError(t *testing.T) { + e := log.Entry{} + expectedErr := fmt.Errorf("failed to resolve {{inputs.var}}") + mock := mockAPI{ + err: expectedErr, + } + p := NewPrometheusProvider(mock, e) + metric := v1alpha1.AnalysisMetric{ + Name: "foo", + Provider: v1alpha1.AnalysisProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Query: "test-{{inputs.var}}", + }, + }, + } + measurement, err := p.Run(metric, []v1alpha1.Argument{}) + assert.Equal(t, expectedErr, err) + assert.NotNil(t, measurement.StartedAt) + assert.Equal(t, "", measurement.Value) + assert.NotNil(t, measurement.FinishedAt) + assert.Equal(t, v1alpha1.AnalysisStatusError, measurement.Status) +} + func TestRunWithEvaluationError(t *testing.T) { e := log.WithField("", "") mock := mockAPI{} diff --git a/utils/query/query.go b/utils/query/query.go new file mode 100644 index 0000000000..07aeacc043 --- /dev/null +++ b/utils/query/query.go @@ -0,0 +1,38 @@ +package query + +import ( + "fmt" + "io" + + "github.com/valyala/fasttemplate" + + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" +) + +const ( + openBracket = "{{" + closeBracket = "}}" +) + +// BuildQuery starts in a template and injects the provider args +func BuildQuery(template string, args []v1alpha1.Argument) (string, error) { + t, err := fasttemplate.NewTemplate(template, openBracket, closeBracket) + if err != nil { + return "", err + } + argsMap := make(map[string]string) + for i := range args { + arg := args[i] + argsMap[fmt.Sprintf("input.%s", arg.Name)] = arg.Value + } + var unresolvedErr error + s := t.ExecuteFuncString(func(w io.Writer, tag string) (int, error) { + if value, ok := argsMap[tag]; ok { + return w.Write([]byte(value)) + } + unresolvedErr = fmt.Errorf("failed to resolve {{%s}}", tag) + + return w.Write([]byte("")) + }) + return s, unresolvedErr +} diff --git a/utils/query/query_test.go b/utils/query/query_test.go new file mode 100644 index 0000000000..d933a9e7bc --- /dev/null +++ b/utils/query/query_test.go @@ -0,0 +1,37 @@ +package query + +import ( + "fmt" + "testing" + + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" + + "github.com/stretchr/testify/assert" +) + +func TestBuildQueryWithNoSubstitution(t *testing.T) { + query, err := BuildQuery("test", nil) + assert.Nil(t, err) + assert.Equal(t, "test", query) +} + +func TestBuildQueryWithSubstitution(t *testing.T) { + args := []v1alpha1.Argument{{ + Name: "var", + Value: "foo", + }} + query, err := BuildQuery("test-{{input.var}}", args) + assert.Nil(t, err) + assert.Equal(t, "test-foo", query) +} + +func TestInvalidTemplate(t *testing.T) { + _, err := BuildQuery("test-{{input.var", nil) + assert.Equal(t, fmt.Errorf("Cannot find end tag=\"}}\" in the template=\"test-{{input.var\" starting from \"input.var\""), err) +} + +func TestMissingArgs(t *testing.T) { + _, err := BuildQuery("test-{{input.var}}", nil) + assert.NotNil(t, err) + assert.Equal(t, fmt.Errorf("failed to resolve {{input.var}}"), err) +} From b2bb23704a67b3026fe3bcd1d877b98b4c124f51 Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Fri, 27 Sep 2019 18:10:48 -0700 Subject: [PATCH 5/7] Rename AnalysisMetric to Metric --- providers/prometheus/prometheus.go | 10 ++++----- providers/prometheus/prometheus_test.go | 28 ++++++++++++------------- providers/provider.go | 8 +++---- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/providers/prometheus/prometheus.go b/providers/prometheus/prometheus.go index 8cbe84d6c6..47e39aba98 100644 --- a/providers/prometheus/prometheus.go +++ b/providers/prometheus/prometheus.go @@ -41,7 +41,7 @@ func failOnError(m v1alpha1.Measurement, err error) (v1alpha1.Measurement, error } // Run queries prometheus for the metric -func (p *Provider) Run(metric v1alpha1.AnalysisMetric, args []v1alpha1.Argument) (v1alpha1.Measurement, error) { +func (p *Provider) Run(metric v1alpha1.Metric, args []v1alpha1.Argument) (v1alpha1.Measurement, error) { startTime := metav1.Now() newMeasurement := v1alpha1.Measurement{ StartedAt: &startTime, @@ -77,12 +77,12 @@ func (p *Provider) Run(metric v1alpha1.AnalysisMetric, args []v1alpha1.Argument) } // Resume should not be used the prometheus provider since all the work should occur in the Run method -func (p *Provider) Resume(metric v1alpha1.AnalysisMetric, args []v1alpha1.Argument, measurement v1alpha1.Measurement) (v1alpha1.Measurement, error) { +func (p *Provider) Resume(metric v1alpha1.Metric, args []v1alpha1.Argument, measurement v1alpha1.Measurement) (v1alpha1.Measurement, error) { p.logCtx.Warn("Prometheus provider should not execute the Resume method") return measurement, nil } -func (p *Provider) evaluateResult(result interface{}, metric v1alpha1.AnalysisMetric) v1alpha1.AnalysisStatus { +func (p *Provider) evaluateResult(result interface{}, metric v1alpha1.Metric) v1alpha1.AnalysisStatus { successCondition, err := evaluate.EvalCondition(result, metric.SuccessCondition) if err != nil { p.logCtx.Warning(err.Error()) @@ -105,7 +105,7 @@ func (p *Provider) evaluateResult(result interface{}, metric v1alpha1.AnalysisMe return v1alpha1.AnalysisStatusSuccessful } -func (p *Provider) processResponse(metric v1alpha1.AnalysisMetric, response model.Value) (string, v1alpha1.AnalysisStatus, error) { +func (p *Provider) processResponse(metric v1alpha1.Metric, response model.Value) (string, v1alpha1.AnalysisStatus, error) { switch value := response.(type) { case *model.Scalar: valueStr := value.Value.String() @@ -143,7 +143,7 @@ func NewPrometheusProvider(api v1.API, logCtx log.Entry) *Provider { } // NewPrometheusAPI generates a prometheus API from the metric configuration -func NewPrometheusAPI(metric v1alpha1.AnalysisMetric) (v1.API, error) { +func NewPrometheusAPI(metric v1alpha1.Metric) (v1.API, error) { client, err := api.NewClient(api.Config{ Address: metric.Provider.Prometheus.Server, }) diff --git a/providers/prometheus/prometheus_test.go b/providers/prometheus/prometheus_test.go index f345083af3..f70e742d0b 100644 --- a/providers/prometheus/prometheus_test.go +++ b/providers/prometheus/prometheus_test.go @@ -34,7 +34,7 @@ func TestRunSuccessFully(t *testing.T) { value: newScalar(10), } p := NewPrometheusProvider(mock, e) - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ Name: "foo", SuccessCondition: "result == 10", FailureCondition: "result != 10", @@ -59,7 +59,7 @@ func TestRunWithQueryError(t *testing.T) { err: expectedErr, } p := NewPrometheusProvider(mock, e) - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ Name: "foo", SuccessCondition: "result == 10", FailureCondition: "result != 10", @@ -84,7 +84,7 @@ func TestRunWithBuildQueryError(t *testing.T) { err: expectedErr, } p := NewPrometheusProvider(mock, e) - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ Name: "foo", Provider: v1alpha1.AnalysisProvider{ Prometheus: &v1alpha1.PrometheusMetric{ @@ -104,7 +104,7 @@ func TestRunWithEvaluationError(t *testing.T) { e := log.WithField("", "") mock := mockAPI{} p := NewPrometheusProvider(mock, *e) - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ Name: "foo", SuccessCondition: "result == 10", FailureCondition: "result != 10", @@ -126,7 +126,7 @@ func TestResume(t *testing.T) { e := log.WithField("", "") mock := mockAPI{} p := NewPrometheusProvider(mock, *e) - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ Name: "foo", SuccessCondition: "result == 10", FailureCondition: "result != 10", @@ -148,7 +148,7 @@ func TestResume(t *testing.T) { func TestEvaluateResultWithSuccess(t *testing.T) { p := Provider{} - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ SuccessCondition: "true", FailureCondition: "false", } @@ -158,7 +158,7 @@ func TestEvaluateResultWithSuccess(t *testing.T) { func TestEvaluateResultWithFailure(t *testing.T) { p := Provider{} - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ SuccessCondition: "true", FailureCondition: "true", } @@ -169,7 +169,7 @@ func TestEvaluateResultWithFailure(t *testing.T) { func TestEvaluateResultInconclusive(t *testing.T) { p := Provider{} - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ SuccessCondition: "false", FailureCondition: "false", } @@ -182,7 +182,7 @@ func TestEvaluateResultWithErrorOnSuccessCondition(t *testing.T) { p := Provider{ logCtx: *logCtx, } - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ SuccessCondition: "a == true", FailureCondition: "true", } @@ -196,7 +196,7 @@ func TestEvaluateResultWithErrorOnFailureCondition(t *testing.T) { p := Provider{ logCtx: *logCtx, } - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ SuccessCondition: "true", FailureCondition: "a == true", } @@ -210,7 +210,7 @@ func TestProcessScalarResponse(t *testing.T) { p := Provider{ logCtx: *logCtx, } - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ SuccessCondition: "result == 10", FailureCondition: "result != 10", } @@ -232,7 +232,7 @@ func TestProcessVectorResponse(t *testing.T) { p := Provider{ logCtx: *logCtx, } - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ SuccessCondition: "10 in result", FailureCondition: "false", } @@ -259,7 +259,7 @@ func TestProcessInvalidResponse(t *testing.T) { p := Provider{ logCtx: *logCtx, } - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ SuccessCondition: "true", FailureCondition: "true", } @@ -272,7 +272,7 @@ func TestProcessInvalidResponse(t *testing.T) { } func TestNewPrometheusAPI(t *testing.T) { - metric := v1alpha1.AnalysisMetric{ + metric := v1alpha1.Metric{ Provider: v1alpha1.AnalysisProvider{ Prometheus: &v1alpha1.PrometheusMetric{ Server: ":invalid::url", diff --git a/providers/provider.go b/providers/provider.go index 750f9e2884..92447d3377 100644 --- a/providers/provider.go +++ b/providers/provider.go @@ -13,15 +13,15 @@ import ( type Provider interface { // Run start a new external system call for a measurement // should be idoponent and do nothing is a call has been started - Run(v1alpha1.AnalysisMetric, []v1alpha1.Argument) (v1alpha1.Measurement, error) + Run(v1alpha1.Metric, []v1alpha1.Argument) (v1alpha1.Measurement, error) // Checks if the external system call is finished and returns the current measuremtn - Resume(v1alpha1.AnalysisMetric, []v1alpha1.Argument, v1alpha1.Measurement) (v1alpha1.Measurement, error) + Resume(v1alpha1.Metric, []v1alpha1.Argument, v1alpha1.Measurement) (v1alpha1.Measurement, error) // Type gets the provider type Type() string } -// NewProvider creates the correct provider based on the provider type of the analysisMetric -func NewProvider(logCtx log.Entry, metric v1alpha1.AnalysisMetric) (Provider, error) { +// NewProvider creates the correct provider based on the provider type of the Metric +func NewProvider(logCtx log.Entry, metric v1alpha1.Metric) (Provider, error) { if metric.Provider.Prometheus != nil { api, err := prometheus.NewPrometheusAPI(metric) if err != nil { From 77f87e56ccebd98aeb31f79eb2c203770f189f9c Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Fri, 27 Sep 2019 18:13:28 -0700 Subject: [PATCH 6/7] Make prometheus provider always add a finishAt time --- providers/prometheus/prometheus.go | 6 ++---- providers/prometheus/prometheus_test.go | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/providers/prometheus/prometheus.go b/providers/prometheus/prometheus.go index 47e39aba98..4bcd58ea64 100644 --- a/providers/prometheus/prometheus.go +++ b/providers/prometheus/prometheus.go @@ -69,10 +69,8 @@ func (p *Provider) Run(metric v1alpha1.Metric, args []v1alpha1.Argument) (v1alph newMeasurement.Value = newValue newMeasurement.Status = newStatus - // if newStatus.Completed() { - // finishedTime := metav1.Now() - // p.measurement.FinishedAt = &finishedTime - // } + finishedTime := metav1.Now() + newMeasurement.FinishedAt = &finishedTime return newMeasurement, nil } diff --git a/providers/prometheus/prometheus_test.go b/providers/prometheus/prometheus_test.go index f70e742d0b..ab331e340e 100644 --- a/providers/prometheus/prometheus_test.go +++ b/providers/prometheus/prometheus_test.go @@ -28,7 +28,7 @@ func TestType(t *testing.T) { assert.Equal(t, ProviderType, p.Type()) } -func TestRunSuccessFully(t *testing.T) { +func TestRunSuccessfully(t *testing.T) { e := log.Entry{} mock := mockAPI{ value: newScalar(10), @@ -48,8 +48,8 @@ func TestRunSuccessFully(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, measurement.StartedAt) assert.Equal(t, "10", measurement.Value) - // assert.NotNil(t, measurement.FinishedAt) - // assert.Equal(t, v1alpha1.AnalysisStatusSuccessful, measurement.Status) + assert.NotNil(t, measurement.FinishedAt) + assert.Equal(t, v1alpha1.AnalysisStatusSuccessful, measurement.Status) } func TestRunWithQueryError(t *testing.T) { From 13f932162c45eff133fac996c140170a442f17a8 Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Fri, 27 Sep 2019 18:16:45 -0700 Subject: [PATCH 7/7] Address feedback --- providers/provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/provider.go b/providers/provider.go index 92447d3377..774a18e61c 100644 --- a/providers/provider.go +++ b/providers/provider.go @@ -12,7 +12,7 @@ import ( // Provider methods to query a external systems and generate a measurement type Provider interface { // Run start a new external system call for a measurement - // should be idoponent and do nothing is a call has been started + //idempotent and do nothing if a call has been started Run(v1alpha1.Metric, []v1alpha1.Argument) (v1alpha1.Measurement, error) // Checks if the external system call is finished and returns the current measuremtn Resume(v1alpha1.Metric, []v1alpha1.Argument, v1alpha1.Measurement) (v1alpha1.Measurement, error)