From f91e3ba067581f11e7f3aab2f3906c76a2f84fc1 Mon Sep 17 00:00:00 2001 From: RaviHari Date: Thu, 28 Apr 2022 12:22:28 +0530 Subject: [PATCH] feat: Allow prometheus server address to be centrally configured (#1956) Signed-off-by: Ravi Hari --- analysis/analysis.go | 8 +- metricproviders/prometheus/prometheus.go | 35 ++++++- metricproviders/prometheus/prometheus_test.go | 99 ++++++++++++++++++- 3 files changed, 133 insertions(+), 9 deletions(-) diff --git a/analysis/analysis.go b/analysis/analysis.go index 1ebee1ec2c..5bae4a8892 100644 --- a/analysis/analysis.go +++ b/analysis/analysis.go @@ -316,7 +316,7 @@ func (c *Controller) runMeasurements(run *v1alpha1.AnalysisRun, tasks []metricTa for _, task := range tasks { wg.Add(1) - go func(t metricTask) { + go func(t metricTask) error { defer wg.Done() //redact secret values from logs logger := logutil.WithRedactor(*logutil.WithAnalysisRun(run).WithField("metric", t.metric.Name), secrets) @@ -326,6 +326,10 @@ func (c *Controller) runMeasurements(run *v1alpha1.AnalysisRun, tasks []metricTa resultsLock.Unlock() provider, err := c.newProvider(*logger, t.metric) + if err != nil { + log.Errorf("Error in getting provider :%v", err) + return err + } if metricResult == nil { metricResult = &v1alpha1.MetricResult{ Name: t.metric.Name, @@ -404,7 +408,7 @@ func (c *Controller) runMeasurements(run *v1alpha1.AnalysisRun, tasks []metricTa resultsLock.Lock() analysisutil.SetResult(run, *metricResult) resultsLock.Unlock() - + return nil }(task) } wg.Wait() diff --git a/metricproviders/prometheus/prometheus.go b/metricproviders/prometheus/prometheus.go index 6646086eec..2dadaf8aa2 100644 --- a/metricproviders/prometheus/prometheus.go +++ b/metricproviders/prometheus/prometheus.go @@ -2,7 +2,10 @@ package prometheus import ( "context" + "errors" "fmt" + "net/url" + "os" "time" "github.com/prometheus/client_golang/api" @@ -21,7 +24,8 @@ const ( ProviderType = "Prometheus" // ResolvedPrometheusQuery is used as the key for storing the resolved prometheus query in the metrics result // metadata object. - ResolvedPrometheusQuery = "ResolvedPrometheusQuery" + ResolvedPrometheusQuery = "ResolvedPrometheusQuery" + EnvVarArgoRolloutsPrometheusAddress = "ARGO_ROLLOUTS_PROMETHEUS_ADDRESS" ) // Provider contains all the required components to run a prometheus query @@ -140,12 +144,39 @@ func NewPrometheusProvider(api v1.API, logCtx log.Entry) *Provider { // NewPrometheusAPI generates a prometheus API from the metric configuration func NewPrometheusAPI(metric v1alpha1.Metric) (v1.API, error) { + envValuesByKey := make(map[string]string) + if value, ok := os.LookupEnv(fmt.Sprintf("%s", EnvVarArgoRolloutsPrometheusAddress)); ok { + envValuesByKey[EnvVarArgoRolloutsPrometheusAddress] = value + log.Debugf("ARGO_ROLLOUTS_PROMETHEUS_ADDRESS: %v", envValuesByKey[EnvVarArgoRolloutsPrometheusAddress]) + } + if len(metric.Provider.Prometheus.Address) != 0 { + if !IsUrl(metric.Provider.Prometheus.Address) { + return nil, errors.New("prometheus address is not is url format") + } + } else if envValuesByKey[EnvVarArgoRolloutsPrometheusAddress] != "" { + if IsUrl(envValuesByKey[EnvVarArgoRolloutsPrometheusAddress]) { + metric.Provider.Prometheus.Address = envValuesByKey[EnvVarArgoRolloutsPrometheusAddress] + } else { + return nil, errors.New("prometheus address is not is url format") + } + } else { + return nil, errors.New("prometheus address is not configured") + } client, err := api.NewClient(api.Config{ Address: metric.Provider.Prometheus.Address, }) if err != nil { + log.Errorf("Error in getting prometheus client: %v", err) return nil, err } - return v1.NewAPI(client), nil } + +func IsUrl(str string) bool { + u, err := url.Parse(str) + if err != nil { + log.Errorf("Error in parsing url: %v", err) + } + log.Debugf("Parsed url: %v", u) + return err == nil && u.Scheme != "" && u.Host != "" +} diff --git a/metricproviders/prometheus/prometheus_test.go b/metricproviders/prometheus/prometheus_test.go index 4268fd92c2..d62f080bcd 100644 --- a/metricproviders/prometheus/prometheus_test.go +++ b/metricproviders/prometheus/prometheus_test.go @@ -3,16 +3,15 @@ package prometheus import ( "fmt" "math" + "os" "testing" + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" v1 "github.com/prometheus/client_golang/api/prometheus/v1" - "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 { @@ -58,6 +57,31 @@ func TestRunSuccessfully(t *testing.T) { assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, measurement.Phase) } +func TestRunSuccessfullyWithEnv(t *testing.T) { + e := log.Entry{} + mock := mockAPI{ + value: newScalar(10), + } + address := "http://127.0.0.1:9090" + os.Setenv(EnvVarArgoRolloutsPrometheusAddress, address) + p := NewPrometheusProvider(mock, e) + metric := v1alpha1.Metric{ + Name: "foo", + SuccessCondition: "result == 10", + FailureCondition: "result != 10", + Provider: v1alpha1.MetricProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Query: "test", + }, + }, + } + measurement := p.Run(newAnalysisRun(), metric) + assert.NotNil(t, measurement.StartedAt) + assert.Equal(t, "10", measurement.Value) + assert.NotNil(t, measurement.FinishedAt) + assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, measurement.Phase) +} + func TestRunSuccessfullyWithWarning(t *testing.T) { e := log.NewEntry(log.New()) mock := mockAPI{ @@ -83,6 +107,31 @@ func TestRunSuccessfullyWithWarning(t *testing.T) { assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, measurement.Phase) } +func TestRunSuccessfullyWithWarningWithEnv(t *testing.T) { + e := log.NewEntry(log.New()) + mock := mockAPI{ + value: newScalar(10), + warnings: v1.Warnings([]string{"warning", "warning2"}), + } + p := NewPrometheusProvider(mock, *e) + metric := v1alpha1.Metric{ + Name: "foo", + SuccessCondition: "result == 10", + FailureCondition: "result != 10", + Provider: v1alpha1.MetricProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Query: "test", + }, + }, + } + measurement := p.Run(newAnalysisRun(), metric) + assert.NotNil(t, measurement.StartedAt) + assert.Equal(t, "10", measurement.Value) + assert.NotNil(t, measurement.FinishedAt) + assert.Equal(t, `"warning", "warning2"`, measurement.Metadata["warnings"]) + assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, measurement.Phase) +} + func TestRunWithQueryError(t *testing.T) { e := log.NewEntry(log.New()) expectedErr := fmt.Errorf("bad big bug :(") @@ -362,17 +411,57 @@ func TestProcessInvalidResponse(t *testing.T) { } func TestNewPrometheusAPI(t *testing.T) { + os.Unsetenv(EnvVarArgoRolloutsPrometheusAddress) + address := ":invalid::url" metric := v1alpha1.Metric{ Provider: v1alpha1.MetricProvider{ Prometheus: &v1alpha1.PrometheusMetric{ - Address: ":invalid::url", + Address: address, }, }, } - _, err := NewPrometheusAPI(metric) + api, err := NewPrometheusAPI(metric) assert.NotNil(t, err) + log.Infof("api:%v", api) metric.Provider.Prometheus.Address = "https://www.example.com" _, err = NewPrometheusAPI(metric) assert.Nil(t, err) } + +func TestNewPrometheusAPIWithEnv(t *testing.T) { + os.Unsetenv(EnvVarArgoRolloutsPrometheusAddress) + os.Setenv(EnvVarArgoRolloutsPrometheusAddress, ":invalid::url") + address := "" + metric := v1alpha1.Metric{ + Provider: v1alpha1.MetricProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Address: address, + }, + }, + } + api, err := NewPrometheusAPI(metric) + assert.NotNil(t, err) + log.Infof("api:%v", api) + + os.Unsetenv(EnvVarArgoRolloutsPrometheusAddress) + os.Setenv(EnvVarArgoRolloutsPrometheusAddress, "https://www.example.com") + _, err = NewPrometheusAPI(metric) + assert.Nil(t, err) +} + +func TestNewPrometheusAddressNotConfigured(t *testing.T) { + os.Unsetenv(EnvVarArgoRolloutsPrometheusAddress) + os.Setenv(EnvVarArgoRolloutsPrometheusAddress, "") + address := "" + metric := v1alpha1.Metric{ + Provider: v1alpha1.MetricProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Address: address, + }, + }, + } + api, err := NewPrometheusAPI(metric) + assert.NotNil(t, err) + log.Infof("api:%v", api) +}