From ba7f997ac4d9689312bdb8b6e07f8a5847538201 Mon Sep 17 00:00:00 2001 From: Zach Aller Date: Thu, 15 Jun 2023 11:21:37 -0500 Subject: [PATCH] fix: resolve args to metric in garbage collection function (#2843) * resolve args to metric in garbage collection function Signed-off-by: zachaller * remove incorrectly added test, this moved to a function Signed-off-by: zachaller * re-trigger Signed-off-by: zachaller * add context to errors Signed-off-by: zachaller * better error message Signed-off-by: zachaller * better error message Signed-off-by: zachaller --------- Signed-off-by: zachaller --- analysis/analysis.go | 7 ++- analysis/analysis_test.go | 98 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/analysis/analysis.go b/analysis/analysis.go index c826280d19..fb307e62e0 100644 --- a/analysis/analysis.go +++ b/analysis/analysis.go @@ -711,8 +711,13 @@ func calculateNextReconcileTime(run *v1alpha1.AnalysisRun, metrics []v1alpha1.Me func (c *Controller) garbageCollectMeasurements(run *v1alpha1.AnalysisRun, measurementRetentionMetricNamesMap map[string]*v1alpha1.MeasurementRetention, limit int) error { var errors []error + resolvedArgsMetric, err := getResolvedMetricsWithoutSecrets(run.Spec.Metrics, run.Spec.Args) + if err != nil { + return fmt.Errorf("failed to resolve args on metrics during garbage collection: %w", err) + } + metricsByName := make(map[string]v1alpha1.Metric) - for _, metric := range run.Spec.Metrics { + for _, metric := range resolvedArgsMetric { metricsByName[metric.Name] = metric } diff --git a/analysis/analysis_test.go b/analysis/analysis_test.go index ed12da2dca..cec3446b89 100644 --- a/analysis/analysis_test.go +++ b/analysis/analysis_test.go @@ -8,6 +8,8 @@ import ( "testing" "time" + "github.com/argoproj/argo-rollouts/metric" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -1099,6 +1101,102 @@ func TestTrimMeasurementHistory(t *testing.T) { } } +func TestGarbageCollectArgResolution(t *testing.T) { + f := newFixture(t) + defer f.Close() + c, _, _ := f.newController(noResyncPeriodFunc) + + c.newProvider = func(logCtx log.Entry, metric v1alpha1.Metric) (metric.Provider, error) { + assert.Equal(t, "https://prometheus.kubeaddons:8080", metric.Provider.Prometheus.Address) + return f.provider, nil + } + + f.provider.On("GarbageCollect", mock.Anything, mock.Anything, mock.Anything).Return(nil) + run := &v1alpha1.AnalysisRun{ + Spec: v1alpha1.AnalysisRunSpec{ + Metrics: []v1alpha1.Metric{ + { + Name: "metric1", + Interval: "60s", + Provider: v1alpha1.MetricProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Address: "https://prometheus.kubeaddons:{{args.port}}", + }, + }, + }, + { + Name: "metric2", + Interval: "60s", + Provider: v1alpha1.MetricProvider{ + Prometheus: &v1alpha1.PrometheusMetric{ + Address: "https://prometheus.kubeaddons:{{args.port}}", + }, + }, + }, + }, + }, + Status: v1alpha1.AnalysisRunStatus{ + Phase: v1alpha1.AnalysisPhaseRunning, + MetricResults: []v1alpha1.MetricResult{ + { + Name: "metric1", + Phase: v1alpha1.AnalysisPhaseRunning, + Measurements: []v1alpha1.Measurement{ + { + Value: "1", + Phase: v1alpha1.AnalysisPhaseSuccessful, + StartedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))), + FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))), + }, + { + Value: "2", + Phase: v1alpha1.AnalysisPhaseSuccessful, + StartedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))), + FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))), + }, + }, + }, + { + Name: "metric2", + Measurements: []v1alpha1.Measurement{ + { + Value: "2", + Phase: v1alpha1.AnalysisPhaseSuccessful, + StartedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))), + FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))), + }, + { + Value: "3", + Phase: v1alpha1.AnalysisPhaseSuccessful, + StartedAt: timePtr(metav1.NewTime(time.Now().Add(-30 * time.Second))), + FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-30 * time.Second))), + }, + { + Value: "4", + Phase: v1alpha1.AnalysisPhaseSuccessful, + StartedAt: timePtr(metav1.NewTime(time.Now().Add(-30 * time.Second))), + FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-30 * time.Second))), + }, + }, + }, + }, + }, + } + run.Spec.Args = append(run.Spec.Args, v1alpha1.Argument{ + Name: "port", + Value: pointer.String("8080"), + }) + var measurementRetentionMetricsMap = map[string]*v1alpha1.MeasurementRetention{} + measurementRetentionMetricsMap["metric2"] = &v1alpha1.MeasurementRetention{MetricName: "metric2", Limit: 2} + err := c.garbageCollectMeasurements(run, measurementRetentionMetricsMap, 1) + assert.Nil(t, err) + assert.Len(t, run.Status.MetricResults[0].Measurements, 1) + assert.Equal(t, "2", run.Status.MetricResults[0].Measurements[0].Value) + assert.Len(t, run.Status.MetricResults[1].Measurements, 2) + assert.Equal(t, "3", run.Status.MetricResults[1].Measurements[0].Value) + assert.Equal(t, "4", run.Status.MetricResults[1].Measurements[1].Value) +} + func TestResolveMetricArgsUnableToSubstitute(t *testing.T) { f := newFixture(t) defer f.Close()