Add a Retrying modifier for spoof.ResponseChecker

This is a strawman for the alternative to #1298 I had in mind.

test/conformance/route_test.go

@@ -19,6 +19,7 @@ package conformance
 
 import (
 	"fmt"
+	"net/http"
 	"strings"
 	"testing"
 
@@ -83,7 +84,12 @@ func assertResourcesUpdatedWhenRevisionIsReady(t *testing.T, logger *zap.Sugared
 		t.Fatalf("Error fetching Route %s: %v", names.Route, err)
 	}
 
-	err = test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, updatedRoute.Status.Domain, test.EventuallyMatchesBody(expectedText), "WaitForEndpointToServeText")
+	err = test.WaitForEndpointState(
+		clients.Kube,
+		logger,
+		updatedRoute.Status.Domain,
+		test.Retrying(test.EventuallyMatchesBody(expectedText), http.StatusServiceUnavailable),
+		"WaitForEndpointToServeText")
 	if err != nil {
 		t.Fatalf("The endpoint for Route %s at domain %s didn't serve the expected text \"%s\": %v", names.Route, updatedRoute.Status.Domain, expectedText, err)
 	}

test/conformance/service_test.go

@@ -64,7 +64,7 @@ func updateServiceWithImage(clients *test.Clients, names test.ResourceNames, ima
 // Shamelessly cribbed from route_test. We expect the Route and Configuration to be ready if the Service is ready.
 func assertServiceResourcesUpdated(t *testing.T, logger *zap.SugaredLogger, clients *test.Clients, names test.ResourceNames, routeDomain, expectedText string) {
 	// TODO(#1178): Remove "Wait" from all checks below this point.
-	err := test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, routeDomain, test.EventuallyMatchesBody(expectedText), "WaitForEndpointToServeText")
+	err := test.WaitForEndpointState(clients.Kube, logger, routeDomain, test.EventuallyMatchesBody(expectedText), "WaitForEndpointToServeText")
 	if err != nil {
 		t.Fatalf("The endpoint for Route %s at domain %s didn't serve the expected text \"%s\": %v", names.Route, routeDomain, expectedText, err)
 	}

test/e2e/autoscale_test.go

@@ -18,6 +18,7 @@ limitations under the License.
 package e2e
 
 import (
+	"net/http"
 	"strings"
 	"testing"
 
@@ -56,7 +57,6 @@ func generateTrafficBurst(clients *test.Clients, logger *zap.SugaredLogger, num
 		go func() {
 			test.WaitForEndpointState(clients.Kube,
 				logger,
-				test.Flags.ResolvableDomain,
 				domain,
 				test.EventuallyMatchesBody(autoscaleExpectedOutput),
 				"MakingConcurrentRequests")
@@ -157,9 +157,8 @@ func TestAutoscaleUpDownUp(t *testing.T) {
 	err = test.WaitForEndpointState(
 		clients.Kube,
 		logger,
-		test.Flags.ResolvableDomain,
 		domain,
-		test.EventuallyMatchesBody(autoscaleExpectedOutput),
+		test.Retrying(test.EventuallyMatchesBody(autoscaleExpectedOutput), http.StatusServiceUnavailable),
 		"CheckingEndpointAfterUpdating")
 	if err != nil {
 		t.Fatalf(`The endpoint for Route %s at domain %s didn't serve

test/e2e/helloworld_test.go

@@ -57,7 +57,7 @@ func TestHelloWorld(t *testing.T) {
 	}
 	domain := route.Status.Domain
 
-	err = test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, domain, test.MatchesBody(helloWorldExpectedOutput), "HelloWorldServesText")
+	err = test.WaitForEndpointState(clients.Kube, logger, domain, test.MatchesBody(helloWorldExpectedOutput), "HelloWorldServesText")
 	if err != nil {
 		t.Fatalf("The endpoint for Route %s at domain %s didn't serve the expected text \"%s\": %v", names.Route, domain, helloWorldExpectedOutput, err)
 	}

test/request.go

@@ -34,6 +34,21 @@ func MatchesAny(_ *spoof.Response) (bool, error) {
 	return true, nil
 }
 
+// Retrying modifies a ResponseChecker to retry certain response codes.
+func Retrying(rc spoof.ResponseChecker, codes ...int) spoof.ResponseChecker {
+	return func(resp *spoof.Response) (bool, error) {
+		for _, code := range codes {
+			if resp.StatusCode == code {
+				// Returning (false, nil) causes SpoofingClient.Poll to retry.
+				return false, nil
+			}
+		}
+
+		// If we didn't match any retryable codes, invoke the ResponseChecker that we wrapped.
+		return rc(resp)
+	}
+}
+
 // MatchesBody checks that the *first* response body matches the "expected" body, otherwise failing.
 func MatchesBody(expected string) spoof.ResponseChecker {
 	return func(resp *spoof.Response) (bool, error) {
@@ -64,18 +79,19 @@ func EventuallyMatchesBody(expected string) spoof.ResponseChecker {
 // the domain in the request headers, otherwise it will make the request directly to domain.
 // desc will be used to name the metric that is emitted to track how long it took for the
 // domain to get into the state checked by inState.  Commas in `desc` must be escaped.
-func WaitForEndpointState(kubeClientset *kubernetes.Clientset, logger *zap.SugaredLogger, resolvableDomain bool, domain string, inState spoof.ResponseChecker, desc string) error {
+func WaitForEndpointState(kubeClientset *kubernetes.Clientset, logger *zap.SugaredLogger, domain string, inState spoof.ResponseChecker, desc string) error {
 	metricName := fmt.Sprintf("WaitForEndpointState/%s", desc)
 	_, span := trace.StartSpan(context.Background(), metricName)
 	defer span.End()
 
-	client, err := spoof.New(kubeClientset, logger, domain, resolvableDomain)
+	client, err := spoof.New(kubeClientset, logger, domain, Flags.ResolvableDomain)
 	if err != nil {
 		return err
 	}
 
-	// TODO(#348): The ingress endpoint tends to return 503's and 404's
-	client.RetryCodes = []int{http.StatusServiceUnavailable, http.StatusNotFound}
+	// TODO(#348): The ingress endpoint tends to return 503's and 404's.
+	// Since there is currently a workaround for 503's, only retry 404's.
+	client.RetryCodes = []int{http.StatusNotFound}
 
 	req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("http://%s", domain), nil)
 	if err != nil {