Add and update metric status

pkg/apis/autoscaling/v1alpha1/metric_lifecycle.go

@@ -18,9 +18,56 @@ package v1alpha1
 
 import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"knative.dev/pkg/apis"
+	duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1"
+)
+
+const (
+	// MetricConditionReady is set when the Metric's latest
+	// underlying revision has reported readiness.
+	MetricConditionReady = apis.ConditionReady
+)
+
+var condSet = apis.NewLivingConditionSet(
+	MetricConditionReady,
 )
 
 // GetGroupVersionKind implements OwnerRefable.
 func (m *Metric) GetGroupVersionKind() schema.GroupVersionKind {
 	return SchemeGroupVersion.WithKind("Metric")
 }
+
+// GetCondition gets the condition `t`.
+func (ms *MetricStatus) GetCondition(t apis.ConditionType) *apis.Condition {
+	return condSet.Manage(ms).GetCondition(t)
+}
+
+// InitializeConditions initializes the conditions of the Metric.
+func (ms *MetricStatus) InitializeConditions() {
+	condSet.Manage(ms).InitializeConditions()
+}
+
+// MarkMetricReady marks the metric status as ready
+func (ms *MetricStatus) MarkMetricReady() {
+	condSet.Manage(ms).MarkTrue(MetricConditionReady)
+}
+
+// MarkMetricNotReady marks the metric status as ready == Unknown
+func (ms *MetricStatus) MarkMetricNotReady(reason, message string) {
+	condSet.Manage(ms).MarkUnknown(MetricConditionReady, reason, message)
+}
+
+// MarkMetricFailed marks the metric status as failed
+func (ms *MetricStatus) MarkMetricFailed(reason, message string) {
+	condSet.Manage(ms).MarkFalse(MetricConditionReady, reason, message)
+}
+
+// IsReady looks at the conditions and if the condition MetricConditionReady
+// is true
+func (ms *MetricStatus) IsReady() bool {
+	return condSet.Manage(ms.duck()).IsHappy()
+}
+
+func (ms *MetricStatus) duck() *duckv1beta1.Status {
+	return (*duckv1beta1.Status)(&ms.Status)
+}

pkg/apis/autoscaling/v1alpha1/metric_lifecycle_test.go

@@ -18,19 +18,158 @@ package v1alpha1
 import (
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-
+	"knative.dev/pkg/apis"
+	"knative.dev/pkg/apis/duck"
+	duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1"
+	apitest "knative.dev/pkg/apis/testing"
 	"knative.dev/serving/pkg/apis/autoscaling"
 )
 
+func TestMetricDuckTypes(t *testing.T) {
+	tests := []struct {
+		name string
+		t    duck.Implementable
+	}{{
+		name: "conditions",
+		t:    &duckv1beta1.Conditions{},
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err := duck.VerifyType(&Metric{}, test.t)
+			if err != nil {
+				t.Errorf("VerifyType(Metric, %T) = %v", test.t, err)
+			}
+		})
+	}
+}
+
+func TestMetricIsReady(t *testing.T) {
+	cases := []struct {
+		name    string
+		status  MetricStatus
+		isReady bool
+	}{{
+		name:    "empty status should not be ready",
+		status:  MetricStatus{},
+		isReady: false,
+	}, {
+		name: "Different condition type should not be ready",
+		status: MetricStatus{
+			Status: duckv1beta1.Status{
+				Conditions: duckv1beta1.Conditions{{
+					Type:   "FooCondition",
+					Status: corev1.ConditionTrue,
+				}},
+			},
+		},
+		isReady: false,
+	}, {
+		name: "False condition status should not be ready",
+		status: MetricStatus{
+			Status: duckv1beta1.Status{
+				Conditions: duckv1beta1.Conditions{{
+					Type:   MetricConditionReady,
+					Status: corev1.ConditionFalse,
+				}},
+			},
+		},
+		isReady: false,
+	}, {
+		name: "Unknown condition status should not be ready",
+		status: MetricStatus{
+			Status: duckv1beta1.Status{
+				Conditions: duckv1beta1.Conditions{{
+					Type:   MetricConditionReady,
+					Status: corev1.ConditionUnknown,
+				}},
+			},
+		},
+		isReady: false,
+	}, {
+		name: "Missing condition status should not be ready",
+		status: MetricStatus{
+			Status: duckv1beta1.Status{
+				Conditions: duckv1beta1.Conditions{{
+					Type: MetricConditionReady,
+				}},
+			},
+		},
+		isReady: false,
+	}, {
+		name: "True condition status should be ready",
+		status: MetricStatus{
+			Status: duckv1beta1.Status{
+				Conditions: duckv1beta1.Conditions{{
+					Type:   MetricConditionReady,
+					Status: corev1.ConditionTrue,
+				}},
+			},
+		},
+		isReady: true,
+	}}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if e, a := tc.isReady, tc.status.IsReady(); e != a {
+				t.Errorf("Ready = %v, want: %v", a, e)
+			}
+		})
+	}
+}
+
+func TestMetricGetSetCondition(t *testing.T) {
+	ms := &MetricStatus{}
+	if a := ms.GetCondition(MetricConditionReady); a != nil {
+		t.Errorf("empty MetricStatus returned %v when expected nil", a)
+	}
+	mc := &apis.Condition{
+		Type:   MetricConditionReady,
+		Status: corev1.ConditionTrue,
+	}
+	ms.MarkMetricReady()
+	if diff := cmp.Diff(mc, ms.GetCondition(MetricConditionReady), cmpopts.IgnoreFields(apis.Condition{}, "LastTransitionTime")); diff != "" {
+		t.Errorf("GetCondition refs diff (-want +got): %v", diff)
+	}
+}
+
+func TestTypicalFlowWithMetricCondition(t *testing.T) {
+	m := &MetricStatus{}
+	m.InitializeConditions()
+	apitest.CheckConditionOngoing(m.duck(), MetricConditionReady, t)
+
+	const (
+		wantReason  = "reason"
+		wantMessage = "the error message"
+	)
+	m.MarkMetricFailed(wantReason, wantMessage)
+	apitest.CheckConditionFailed(m.duck(), MetricConditionReady, t)
+	if got := m.GetCondition(MetricConditionReady); got == nil || got.Reason != wantReason || got.Message != wantMessage {
+		t.Errorf("MarkMetricFailed = %v, wantReason %v, wantMessage %v", got, wantReason, wantMessage)
+	}
+
+	m.MarkMetricNotReady(wantReason, wantMessage)
+	apitest.CheckConditionOngoing(m.duck(), MetricConditionReady, t)
+	if got := m.GetCondition(MetricConditionReady); got == nil || got.Reason != wantReason || got.Message != wantMessage {
+		t.Errorf("MarkMetricNotReady = %v, wantReason %v, wantMessage %v", got, wantReason, wantMessage)
+	}
+
+	m.MarkMetricReady()
+	apitest.CheckConditionSucceeded(m.duck(), MetricConditionReady, t)
+}
+
 func TestMetricGetGroupVersionKind(t *testing.T) {
-	m := &Metric{}
+	r := &Metric{}
 	want := schema.GroupVersionKind{
 		Group:   autoscaling.InternalGroupName,
 		Version: "v1alpha1",
 		Kind:    "Metric",
 	}
-	if got := m.GetGroupVersionKind(); got != want {
+	if got := r.GetGroupVersionKind(); got != want {
 		t.Errorf("got: %v, want: %v", got, want)
 	}
 }

pkg/apis/autoscaling/v1alpha1/metric_types.go

@@ -21,6 +21,7 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/apis"
+	duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1"
 	"knative.dev/pkg/kmeta"
 )
 
@@ -63,7 +64,9 @@ type MetricSpec struct {
 }
 
 // MetricStatus reflects the status of metric collection for this specific entity.
-type MetricStatus struct{}
+type MetricStatus struct {
+	duckv1beta1.Status `json:",inline"`
+}
 
 // MetricList is a list of Metric resources
 //

pkg/autoscaler/collector.go

@@ -259,9 +259,19 @@ func newCollection(metric *av1alpha1.Metric, scraper StatsScraper, logger *zap.S
 				scrapeTicker.Stop()
 				return
 			case <-scrapeTicker.C:
-				message, err := c.getScraper().Scrape()
+				message, err := c.getScraper().Scrape(logger)
 				if err != nil {
+					copy := *(metric.DeepCopy())
+					switch {
+					case err == ErrFailedGetEndpoints:
+						copy.Status.MarkMetricNotReady("NoEndpoints", ErrFailedGetEndpoints.Error())
+					case err == ErrDidNotReceiveStat:
+						copy.Status.MarkMetricFailed("DidNotReceiveStat", ErrDidNotReceiveStat.Error())
+					default:
+						copy.Status.MarkMetricNotReady("CreateOrUpdateFailed", "Collector has failed.")
+					}
 					logger.Errorw("Failed to scrape metrics", zap.Error(err))
+					c.updateMetric(&copy)
 				}
 				if message != nil {
 					c.record(message.Stat)