Use generic pkg/resources helpers for endpoint counting (#3709)

pkg/reconciler/v1alpha1/revision/helpers.go

@@ -33,15 +33,6 @@ func getBuildDoneCondition(build *duckv1alpha1.KResource) *duckv1alpha1.Conditio
 	return nil
 }
 
-func isServiceReady(e *corev1.Endpoints) bool {
-	for _, es := range e.Subsets {
-		if len(es.Addresses) > 0 {
-			return true
-		}
-	}
-	return false
-}
-
 func hasDeploymentTimedOut(deployment *appsv1.Deployment) bool {
 	// as per https://kubernetes.io/docs/concepts/workloads/controllers/deployment
 	for _, cond := range deployment.Status.Conditions {

pkg/reconciler/v1alpha1/revision/helpers_test.go

@@ -92,43 +92,6 @@ func TestGetBuildDoneCondition(t *testing.T) {
 
 }
 
-func TestGetIsServiceReady(t *testing.T) {
-	tests := []struct {
-		description string
-		endpoints   *corev1.Endpoints
-		ready       bool
-	}{{
-		description: "no subsets",
-		endpoints:   &corev1.Endpoints{},
-	}, {
-		description: "subset no address",
-		endpoints: &corev1.Endpoints{
-			Subsets: []corev1.EndpointSubset{{
-				Addresses: []corev1.EndpointAddress{},
-			}},
-		},
-	}, {
-		description: "subset with address",
-		endpoints: &corev1.Endpoints{
-			Subsets: []corev1.EndpointSubset{{
-				Addresses: []corev1.EndpointAddress{{
-					IP: "127.0.0.1",
-				}},
-			}},
-		},
-		ready: true,
-	}}
-
-	for _, test := range tests {
-		t.Run(test.description, func(t *testing.T) {
-			ready := isServiceReady(test.endpoints)
-			if ready != test.ready {
-				t.Errorf("getIsServiceReady(%v) = %v, want %v", test.endpoints, ready, test.ready)
-			}
-		})
-	}
-}
-
 func TestGetDeploymentProgressCondition(t *testing.T) {
 	tests := []struct {
 		description string

pkg/reconciler/v1alpha1/revision/reconcile_resources.go

@@ -21,6 +21,8 @@ import (
 	"fmt"
 	"time"
 
+	"go.uber.org/zap"
+
 	"github.com/knative/pkg/kmp"
 	"github.com/knative/pkg/logging"
 	"github.com/knative/pkg/logging/logkey"
@@ -29,7 +31,8 @@ import (
 	"github.com/knative/serving/pkg/reconciler/v1alpha1/revision/config"
 	"github.com/knative/serving/pkg/reconciler/v1alpha1/revision/resources"
 	resourcenames "github.com/knative/serving/pkg/reconciler/v1alpha1/revision/resources/names"
-	"go.uber.org/zap"
+	presources "github.com/knative/serving/pkg/resources"
+
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
@@ -237,7 +240,7 @@ func (c *Reconciler) reconcileService(ctx context.Context, rev *v1alpha1.Revisio
 	// If the endpoints resource indicates that the Service it sits in front of is ready,
 	// then surface this in our Revision status as resources available (pods were scheduled)
 	// and container healthy (endpoints should be gated by any provided readiness checks).
-	if isServiceReady(endpoints) {
+	if presources.ReadyAddressCount(endpoints) > 0 {
 		rev.Status.MarkResourcesAvailable()
 		rev.Status.MarkContainerHealthy()
 	} else if !rev.Status.IsActivationRequired() {

pkg/reconciler/v1alpha1/serverlessservice/serverlessservice.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/davecgh/go-spew/spew"
 	"github.com/google/go-cmp/cmp"
+
 	"github.com/knative/pkg/controller"
 	"github.com/knative/pkg/logging"
 	"github.com/knative/pkg/system"
@@ -33,6 +34,7 @@ import (
 	rbase "github.com/knative/serving/pkg/reconciler"
 	"github.com/knative/serving/pkg/reconciler/v1alpha1/serverlessservice/resources"
 	"github.com/knative/serving/pkg/reconciler/v1alpha1/serverlessservice/resources/names"
+	presources "github.com/knative/serving/pkg/resources"
 	"go.uber.org/zap"
 
 	corev1 "k8s.io/api/core/v1"
@@ -280,26 +282,17 @@ func (r *reconciler) reconcilePublicEndpoints(ctx context.Context, sks *netv1alp
 			}
 		}
 	}
-	if hasEndpoints(eps) {
+	if r := presources.ReadyAddressCount(eps); r > 0 {
+		logger.Infof("Endpoints %s/%s has %d ready  endpoints", r)
 		sks.Status.MarkEndpointsReady()
 	} else {
+		logger.Info("Endpoints %s/%s has no ready endpoints")
 		sks.Status.MarkEndpointsNotReady("NoHealthyBackends")
 	}
 	logger.Debugf("Done reconciling public K8s endpoints %s", sn)
 	return nil
 }
 
-// hasEndpoints returns true if Endpoints resource has at least one endpoint.
-func hasEndpoints(eps *corev1.Endpoints) bool {
-	for _, ss := range eps.Subsets {
-		if len(ss.Addresses) > 0 {
-			return true
-		}
-
-	}
-	return false
-}
-
 func (r *reconciler) reconcilePrivateService(ctx context.Context, sks *netv1alpha1.ServerlessService) error {
 	logger := logging.FromContext(ctx)
 	sn := names.PrivateService(sks)