Simplify provider interfaces to set error messages

Makefile

@@ -75,7 +75,7 @@ lint:
 
 .PHONY: test
 test:
-	go test -failfast -v -covermode=count -coverprofile=coverage.out `go list ./...`
+	go test -failfast -covermode=count -coverprofile=coverage.out `go list ./...`
 
 .PHONY: mocks
 mocks:

analysis/analysis.go

@@ -169,7 +169,6 @@ func (c *AnalysisController) runMeasurements(run *v1alpha1.AnalysisRun, tasks []
 			}
 
 			var newMeasurement v1alpha1.Measurement
-			var newMessage string
 			provider, err := c.newProvider(*log, t.metric)
 			if err != nil {
 				if t.incompleteMeasurement != nil {
@@ -179,19 +178,20 @@ func (c *AnalysisController) runMeasurements(run *v1alpha1.AnalysisRun, tasks []
 					newMeasurement.StartedAt = &startedAt
 				}
 				newMeasurement.Status = v1alpha1.AnalysisStatusError
+				newMeasurement.Message = err.Error()
 			} else {
 				if t.incompleteMeasurement == nil {
-					newMeasurement, err = provider.Run(run, t.metric, run.Spec.Arguments)
+					newMeasurement = provider.Run(run, t.metric, run.Spec.Arguments)
 				} else {
 					// metric is incomplete. either terminate or resume it
 					if terminating {
-						log.Infof("terminating measurement")
-						newMeasurement, err = provider.Terminate(run, t.metric, run.Spec.Arguments, *t.incompleteMeasurement)
-						if err == nil && newMeasurement.Status == v1alpha1.AnalysisStatusSuccessful {
-							newMessage = "metric terminated prematurely"
+						log.Infof("terminating in-progress measurement")
+						newMeasurement = provider.Terminate(run, t.metric, run.Spec.Arguments, *t.incompleteMeasurement)
+						if newMeasurement.Status == v1alpha1.AnalysisStatusSuccessful {
+							newMeasurement.Message = "metric terminated"
 						}
 					} else {
-						newMeasurement, err = provider.Resume(run, t.metric, run.Spec.Arguments, *t.incompleteMeasurement)
+						newMeasurement = provider.Resume(run, t.metric, run.Spec.Arguments, *t.incompleteMeasurement)
 					}
 				}
 			}
@@ -213,18 +213,14 @@ func (c *AnalysisController) runMeasurements(run *v1alpha1.AnalysisRun, tasks []
 					metricResult.Count++
 				case v1alpha1.AnalysisStatusError:
 					metricResult.Error++
+					log.Warnf("metric errored: %s", newMeasurement.Message)
 				}
 			}
 			if t.incompleteMeasurement == nil {
 				metricResult.Measurements = append(metricResult.Measurements, newMeasurement)
 			} else {
 				metricResult.Measurements[len(metricResult.Measurements)-1] = newMeasurement
 			}
-			if err != nil {
-				newMessage = err.Error()
-				log.Warnf("metric errored: %s", metricResult.Message)
-			}
-			metricResult.Message = newMessage
 
 			resultsLock.Lock()
 			analysisutil.SetResult(run, *metricResult)
@@ -258,6 +254,9 @@ func (c *AnalysisController) asssessRunStatus(run *v1alpha1.AnalysisRun) v1alpha
 						c.recorder.Eventf(run, corev1.EventTypeNormal, EventReasonStatusCompleted, "metric '%s' completed %s", metric.Name, metricStatus)
 					}
 				}
+				if lastMeasurement := analysisutil.LastMeasurement(run, metric.Name); lastMeasurement != nil {
+					result.Message = lastMeasurement.Message
+				}
 				result.Status = metricStatus
 				analysisutil.SetResult(run, *result)
 			}

analysis/analysis_test.go

@@ -34,13 +34,13 @@ func newTerminatingRun(status v1alpha1.AnalysisStatus) *v1alpha1.AnalysisRun {
 				Metrics: []v1alpha1.Metric{
 					{
 						Name: "run-forever",
-						Provider: v1alpha1.AnalysisProvider{
+						Provider: v1alpha1.MetricProvider{
 							Job: &v1alpha1.JobMetric{},
 						},
 					},
 					{
 						Name: "failed-metric",
-						Provider: v1alpha1.AnalysisProvider{
+						Provider: v1alpha1.MetricProvider{
 							Job: &v1alpha1.JobMetric{},
 						},
 					},
@@ -353,13 +353,13 @@ func TestAssessRunStatusUpdateResult(t *testing.T) {
 				Metrics: []v1alpha1.Metric{
 					{
 						Name: "sleep-infinity",
-						Provider: v1alpha1.AnalysisProvider{
+						Provider: v1alpha1.MetricProvider{
 							Job: &v1alpha1.JobMetric{},
 						},
 					},
 					{
 						Name: "fail-after-30",
-						Provider: v1alpha1.AnalysisProvider{
+						Provider: v1alpha1.MetricProvider{
 							Job: &v1alpha1.JobMetric{},
 						},
 					},
@@ -738,7 +738,7 @@ func TestReconcileAnalysisRunInitial(t *testing.T) {
 					{
 						Name:     "success-rate",
 						Interval: pointer.Int32Ptr(60),
-						Provider: v1alpha1.AnalysisProvider{
+						Provider: v1alpha1.MetricProvider{
 							Prometheus: &v1alpha1.PrometheusMetric{},
 						},
 					},
@@ -814,6 +814,7 @@ func TestReconcileAnalysisRunTerminateSiblingAfterFail(t *testing.T) {
 		// ensure the inProgress measurement is now terminated
 		assert.Equal(t, v1alpha1.AnalysisStatusSuccessful, newRun.Status.MetricResults[0].Measurements[0].Status)
 		assert.NotNil(t, newRun.Status.MetricResults[0].Measurements[0].FinishedAt)
-		assert.Equal(t, "metric terminated prematurely", newRun.Status.MetricResults[0].Message)
+		assert.Equal(t, "metric terminated", newRun.Status.MetricResults[0].Message)
+		assert.Equal(t, "metric terminated", newRun.Status.MetricResults[0].Measurements[0].Message)
 	}
 }

manifests/crds/analysis-run-crd.yaml

@@ -1877,6 +1877,8 @@ spec:
                         finishedAt:
                           format: date-time
                           type: string
+                        message:
+                          type: string
                         metadata:
                           additionalProperties:
                             type: string
@@ -1889,6 +1891,7 @@ spec:
                         value:
                           type: string
                       required:
+                      - message
                       - status
                       type: object
                     type: array

manifests/install.yaml

@@ -1878,6 +1878,8 @@ spec:
                         finishedAt:
                           format: date-time
                           type: string
+                        message:
+                          type: string
                         metadata:
                           additionalProperties:
                             type: string
@@ -1890,6 +1892,7 @@ spec:
                         value:
                           type: string
                       required:
+                      - message
                       - status
                       type: object
                     type: array

manifests/namespace-install.yaml

@@ -1878,6 +1878,8 @@ spec:
                         finishedAt:
                           format: date-time
                           type: string
+                        message:
+                          type: string
                         metadata:
                           additionalProperties:
                             type: string
@@ -1890,6 +1892,7 @@ spec:
                         value:
                           type: string
                       required:
+                      - message
                       - status
                       type: object
                     type: array

pkg/apis/rollouts/v1alpha1/analysis_types.go

@@ -64,7 +64,7 @@ type Metric struct {
 	// 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"`
+	Provider MetricProvider `json:"provider"`
 }
 
 // EffectiveCount is the effective count based on whether or not count/interval is specified
@@ -82,9 +82,9 @@ func (m *Metric) EffectiveCount() *int32 {
 	return &m.Count
 }
 
-// AnalysisProvider which external system to use to verify the analysis
+// MetricProvider which external system to use to verify the analysis
 // Only one of the fields in this struct should be non-nil
-type AnalysisProvider struct {
+type MetricProvider struct {
 	// Prometheus specifies the prometheus metric to query
 	Prometheus *PrometheusMetric `json:"prometheus,omitempty"`
 	// Job specifies the job metric run
@@ -203,6 +203,8 @@ type MetricResult struct {
 type Measurement struct {
 	// Status is the status of this single measurement
 	Status AnalysisStatus `json:"status"`
+	// Message contains a message describing current condition (e.g. error messages)
+	Message string `json:"message"`
 	// StartedAt is the timestamp in which this measurement started to be measured
 	StartedAt *metav1.Time `json:"startedAt,omitempty"`
 	// FinishedAt is the timestamp in which this measurement completed and value was collected