Tidy up status package a bit

Signed-off-by: Nick Young <[email protected]>

internal/contour/metrics.go

@@ -19,6 +19,7 @@ package contour
 import (
 	"time"
 
+	contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
 	"github.com/projectcontour/contour/internal/dag"
 	"github.com/projectcontour/contour/internal/k8s"
 	"github.com/projectcontour/contour/internal/metrics"
@@ -81,22 +82,22 @@ func (m *RebuildMetricsObserver) OnChange(d *dag.DAG) {
 	select {
 	// If we are leader, the IsLeader channel is closed.
 	case <-m.IsLeader:
-		m.Metrics.SetHTTPProxyMetric(calculateRouteMetric(d.StatusCache.GetProxyStatusMetrics()))
+		m.Metrics.SetHTTPProxyMetric(calculateRouteMetric(d.StatusCache.GetProxyUpdates()))
 	default:
 	}
 }
 
-func calculateRouteMetric(statuses []status.Metric) metrics.RouteMetric {
+func calculateRouteMetric(updates []*status.ProxyUpdate) metrics.RouteMetric {
 	proxyMetricTotal := make(map[metrics.Meta]int)
 	proxyMetricValid := make(map[metrics.Meta]int)
 	proxyMetricInvalid := make(map[metrics.Meta]int)
 	proxyMetricOrphaned := make(map[metrics.Meta]int)
 	proxyMetricRoots := make(map[metrics.Meta]int)
 
-	for _, s := range statuses {
-		calcMetrics(s, proxyMetricValid, proxyMetricInvalid, proxyMetricOrphaned, proxyMetricTotal)
-		if s.Vhost != "" {
-			proxyMetricRoots[metrics.Meta{Namespace: s.Namespace}]++
+	for _, u := range updates {
+		calcMetrics(u, proxyMetricValid, proxyMetricInvalid, proxyMetricOrphaned, proxyMetricTotal)
+		if u.Vhost != "" {
+			proxyMetricRoots[metrics.Meta{Namespace: u.Fullname.Namespace}]++
 		}
 	}
 
@@ -109,14 +110,18 @@ func calculateRouteMetric(statuses []status.Metric) metrics.RouteMetric {
 	}
 }
 
-func calcMetrics(s status.Metric, metricValid map[metrics.Meta]int, metricInvalid map[metrics.Meta]int, metricOrphaned map[metrics.Meta]int, metricTotal map[metrics.Meta]int) {
-	switch s.Status {
-	case k8s.StatusValid:
-		metricValid[metrics.Meta{VHost: s.Vhost, Namespace: s.Namespace}]++
-	case k8s.StatusInvalid:
-		metricInvalid[metrics.Meta{VHost: s.Vhost, Namespace: s.Namespace}]++
-	case k8s.StatusOrphaned:
-		metricOrphaned[metrics.Meta{Namespace: s.Namespace}]++
+func calcMetrics(u *status.ProxyUpdate, metricValid map[metrics.Meta]int, metricInvalid map[metrics.Meta]int, metricOrphaned map[metrics.Meta]int, metricTotal map[metrics.Meta]int) {
+	validCond := u.ConditionFor(status.ValidCondition)
+	switch validCond.Status {
+	case contour_api_v1.ConditionTrue:
+		metricValid[metrics.Meta{VHost: u.Vhost, Namespace: u.Fullname.Namespace}]++
+	case contour_api_v1.ConditionFalse:
+		_, ok := validCond.GetError(status.OrphanedConditionType)
+		if ok {
+			metricOrphaned[metrics.Meta{Namespace: u.Fullname.Namespace}]++
+			break
+		}
+		metricInvalid[metrics.Meta{VHost: u.Vhost, Namespace: u.Fullname.Namespace}]++
 	}
-	metricTotal[metrics.Meta{Namespace: s.Namespace}]++
+	metricTotal[metrics.Meta{Namespace: u.Fullname.Namespace}]++
 }

internal/contour/metrics_test.go

@@ -58,7 +58,7 @@ func TestHTTPProxyMetrics(t *testing.T) {
 
 			dag := builder.Build()
 
-			gotProxy := calculateRouteMetric(dag.StatusCache.GetProxyStatusMetrics())
+			gotProxy := calculateRouteMetric(dag.StatusCache.GetProxyUpdates())
 
 			if tc.wantProxy != nil {
 				assert.Equal(t, *tc.wantProxy, gotProxy)

internal/dag/dag.go

@@ -23,7 +23,7 @@ import (
 	"time"
 
 	envoy_api_v2_auth "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth"
-	projectcontour "github.com/projectcontour/contour/apis/projectcontour/v1"
+	contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
 	"github.com/projectcontour/contour/internal/status"
 	"github.com/projectcontour/contour/internal/timeout"
 	"github.com/projectcontour/contour/internal/xds"
@@ -80,10 +80,16 @@ func (d *DAG) Visit(fn func(Vertex)) {
 	}
 }
 
-// Statuses returns a slice of Status objects associated with
-// the computation of this DAG.
-func (d *DAG) Statuses() map[types.NamespacedName]projectcontour.DetailedCondition {
-	return d.StatusCache.GetProxyValidConditions()
+// GetTestStatuses returns a slice of Status objects associated with
+// the computation of this DAG, for testing status output.
+func (d *DAG) GetTestStatuses() map[types.NamespacedName]contour_api_v1.DetailedCondition {
+	validConds := make(map[types.NamespacedName]contour_api_v1.DetailedCondition)
+
+	for _, pu := range d.StatusCache.GetProxyUpdates() {
+		validConds[pu.Fullname] = *pu.Conditions[status.ValidCondition]
+	}
+
+	return validConds
 }
 
 // AddRoot appends the given root to the DAG's roots.

internal/dag/httpproxy_processor.go

@@ -92,7 +92,7 @@ func (p *HTTPProxyProcessor) Run(dag *DAG, source *KubernetesCache) {
 		proxy, ok := p.source.httpproxies[meta]
 		if ok {
 			pa, commit := p.dag.StatusCache.ProxyAccessor(proxy)
-			pa.ConditionFor(status.ValidCondition).AddError(k8s.StatusOrphaned,
+			pa.ConditionFor(status.ValidCondition).AddError(status.OrphanedConditionType,
 				"Orphaned",
 				"this HTTPProxy is not part of a delegation chain from a root HTTPProxy")
 			commit()

internal/dag/status_test.go

@@ -58,7 +58,7 @@ func TestDAGStatus(t *testing.T) {
 			}
 			dag := builder.Build()
 			t.Logf("%#v\n", dag.StatusCache)
-			got := dag.Statuses()
+			got := dag.GetTestStatuses()
 			assert.Equal(t, tc.want, got)
 		})
 	}

internal/fixture/detailedcondition.go

@@ -49,7 +49,7 @@ func (dcb *DetailedConditionBuilder) Valid() v1.DetailedCondition {
 func (dcb *DetailedConditionBuilder) Orphaned() v1.DetailedCondition {
 
 	dc := (*v1.DetailedCondition)(dcb)
-	dc.AddError("orphaned", "Orphaned", "this HTTPProxy is not part of a delegation chain from a root HTTPProxy")
+	dc.AddError("Orphaned", "Orphaned", "this HTTPProxy is not part of a delegation chain from a root HTTPProxy")
 
 	return *dc
 }

internal/status/cache.go

@@ -30,6 +30,8 @@ const (
 	ProxyStatusValid    ProxyStatus = "valid"
 	ProxyStatusInvalid  ProxyStatus = "invalid"
 	ProxyStatusOrphaned ProxyStatus = "orphaned"
+
+	OrphanedConditionType string = "Orphaned"
 )
 
 // NewCache creates a new Cache for holding status updates.
@@ -113,44 +115,15 @@ func (c Cache) getProxyStatusUpdates() []k8s.StatusUpdate {
 
 }
 
-func (c Cache) GetProxyValidConditions() map[types.NamespacedName]contour_api_v1.DetailedCondition {
-	validConds := make(map[types.NamespacedName]contour_api_v1.DetailedCondition)
-
-	for fullname, pu := range c.proxyUpdates {
-		validConds[fullname] = *pu.Conditions[ValidCondition]
-	}
-
-	return validConds
-}
-
-type Metric struct {
-	Vhost     string
-	Status    ProxyStatus
-	Namespace string
-}
-
-func (c Cache) GetProxyStatusMetrics() []Metric {
-	var statusMetrics []Metric
-	for name, pu := range c.proxyUpdates {
-		metric := Metric{
-			Vhost:     pu.Vhost,
-			Namespace: name.Namespace,
-		}
-		validCond := pu.ConditionFor(ValidCondition)
-		switch validCond.Status {
-		case contour_api_v1.ConditionTrue:
-			metric.Status = ProxyStatusValid
-		case contour_api_v1.ConditionFalse:
-			_, ok := validCond.GetError("orphaned")
-			if ok {
-				metric.Status = ProxyStatusOrphaned
-			} else {
-				metric.Status = ProxyStatusInvalid
-			}
-		}
-
-		statusMetrics = append(statusMetrics, metric)
+// GetProxyUpdates gets the underlying ProxyUpdate objects
+// from the cache, used by various things (`internal/contour/metrics.go` and `internal/dag/status_test.go`)
+// to retrieve info they need.
+// TODO(youngnick): This could conceivably be replaced with a Walk pattern.
+func (c Cache) GetProxyUpdates() []*ProxyUpdate {
 
+	var allUpdates []*ProxyUpdate
+	for _, pu := range c.proxyUpdates {
+		allUpdates = append(allUpdates, pu)
 	}
-	return statusMetrics
+	return allUpdates
 }

internal/status/proxystatus.go

@@ -98,7 +98,7 @@ func (pu *ProxyUpdate) Mutate(obj interface{}) interface{} {
 		proxy.Status.CurrentStatus = k8s.StatusValid
 		proxy.Status.Description = validCond.Message
 	case projectcontour.ConditionFalse:
-		orphanCond, orphaned := validCond.GetError(k8s.StatusOrphaned)
+		orphanCond, orphaned := validCond.GetError(OrphanedConditionType)
 		if orphaned {
 			proxy.Status.CurrentStatus = k8s.StatusOrphaned
 			proxy.Status.Description = orphanCond.Message

internal/status/proxystatus_test.go

@@ -208,12 +208,12 @@ func TestStatusMutator(t *testing.T) {
 					Condition: contour_api_v1.Condition{
 						Type:    string(ValidCondition),
 						Status:  contour_api_v1.ConditionFalse,
-						Reason:  "orphaned",
+						Reason:  "Orphaned",
 						Message: "this HTTPProxy is not part of a delegation chain from a root HTTPProxy",
 					},
 					Errors: []contour_api_v1.SubCondition{
 						{
-							Type:    "orphaned",
+							Type:    "Orphaned",
 							Reason:  "Orphaned",
 							Message: "this HTTPProxy is not part of a delegation chain from a root HTTPProxy",
 						},
@@ -228,12 +228,12 @@ func TestStatusMutator(t *testing.T) {
 					Status:             contour_api_v1.ConditionFalse,
 					ObservedGeneration: testGeneration,
 					LastTransitionTime: testTransitionTime,
-					Reason:             "orphaned",
+					Reason:             "Orphaned",
 					Message:            "this HTTPProxy is not part of a delegation chain from a root HTTPProxy",
 				},
 				Errors: []contour_api_v1.SubCondition{
 					{
-						Type:    "orphaned",
+						Type:    "Orphaned",
 						Reason:  "Orphaned",
 						Message: "this HTTPProxy is not part of a delegation chain from a root HTTPProxy",
 					},