Pull host spoofing logic into an http client

test/adding_tests.md

@@ -165,9 +165,13 @@ ready to serve requests right away. To poll a deployed endpoint and wait for it
 in the state you want it to be in (or timeout) use `WaitForEndpointState`:
 
 ```go
-err = test.WaitForEndpointState(clients.Kube, resolvableDomain, updatedRoute.Status.Domain, func(body string) (bool, error) {
-    return body == expectedText, nil
-}, "SomeDescription")
+err = test.WaitForEndpointState(
+		clients.Kube,
+		logger,
+		test.Flags.ResolvableDomain,
+		updatedRoute.Status.Domain,
+		test.EventuallyMatchesBody(expectedText),
+		"SomeDescription")
 if err != nil {
     t.Fatalf("The endpoint for Route %s at domain %s didn't serve the expected text \"%s\": %v", routeName, updatedRoute.Status.Domain, expectedText, err)
 }
@@ -178,6 +182,24 @@ should be used or the domain should be used directly.
 
 _See [request.go](./request.go)._
 
+If you need more low-level access to the http request or response against a deployed
+service, you can directly use the `SpoofingClient` that `WaitForEndpointState` wraps.
+
+
+```go
+// Error handling elided for brevity, but you know better.
+client, err := spoof.New(clients.Kube, logger, route.Status.Domain, test.Flags.ResolvableDomain)
+req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("http://%s", route.Status.Domain), nil)
+
+// Single request.
+resp, err := client.Do(req)
+
+// Polling until we meet some condition.
+resp, err := client.Poll(req, test.BodyMatches(expectedText))
+```
+
+_See [spoof.go](./spoof/spoof.go)._
+
 ### Check Knative Serving resources
 
 After creating Knative Serving resources or making changes to them, you will need to wait for the system

test/conformance/route_test.go

@@ -83,9 +83,7 @@ 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, func(body string) (bool, error) {
-		return body == expectedText, nil
-	}, "WaitForEndpointToServeText")
+	err = test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, updatedRoute.Status.Domain, 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, updatedRoute.Status.Domain, expectedText, err)
 	}

test/conformance/service_test.go

@@ -64,9 +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, func(body string) (bool, error) {
-		return body == expectedText, nil
-	}, "WaitForEndpointToServeText")
+	err := test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, 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

@@ -36,12 +36,6 @@ var (
 	initialScaleToZeroThreshold string
 )
 
-func isExpectedOutput() func(body string) (bool, error) {
-	return func(body string) (bool, error) {
-		return strings.Contains(body, autoscaleExpectedOutput), nil
-	}
-}
-
 func isDeploymentScaledUp() func(d *v1beta1.Deployment) (bool, error) {
 	return func(d *v1beta1.Deployment) (bool, error) {
 		return d.Status.ReadyReplicas >= 1, nil
@@ -64,7 +58,7 @@ func generateTrafficBurst(clients *test.Clients, logger *zap.SugaredLogger, num
 				logger,
 				test.Flags.ResolvableDomain,
 				domain,
-				isExpectedOutput(),
+				test.EventuallyMatchesBody(autoscaleExpectedOutput),
 				"MakingConcurrentRequests")
 			concurrentRequests <- true
 		}()
@@ -165,7 +159,7 @@ func TestAutoscaleUpDownUp(t *testing.T) {
 		logger,
 		test.Flags.ResolvableDomain,
 		domain,
-		isExpectedOutput(),
+		test.EventuallyMatchesBody(autoscaleExpectedOutput),
 		"CheckingEndpointAfterUpdating")
 	if err != nil {
 		t.Fatalf(`The endpoint for Route %s at domain %s didn't serve

test/e2e/helloworld_test.go

@@ -29,12 +29,6 @@ const (
 	helloWorldExpectedOutput = "Hello World! How about some tasty noodles?"
 )
 
-func isHelloWorldExpectedOutput() func(body string) (bool, error) {
-	return func(body string) (bool, error) {
-		return strings.TrimRight(body, "\n") == helloWorldExpectedOutput, nil
-	}
-}
-
 func TestHelloWorld(t *testing.T) {
 	clients := Setup(t)
 
@@ -63,7 +57,7 @@ func TestHelloWorld(t *testing.T) {
 	}
 	domain := route.Status.Domain
 
-	err = test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, domain, isHelloWorldExpectedOutput(), "HelloWorldServesText")
+	err = test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, 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

@@ -19,96 +19,64 @@ package test
 
 import (
 	"context"
-	"errors"
 	"fmt"
-	"io/ioutil"
-	"net"
 	"net/http"
-	"time"
+	"strings"
 
+	"github.com/knative/serving/test/spoof"
 	"go.opencensus.io/trace"
 	"go.uber.org/zap"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
 )
 
-const (
-	requestInterval = 1 * time.Second
-	requestTimeout  = 5 * time.Minute
-)
-
-func waitForRequestToDomainState(logger *zap.SugaredLogger, address string, spoofDomain string, retryableCodes []int, inState func(body string) (bool, error)) error {
-	h := http.Client{}
-	req, err := http.NewRequest("GET", address, nil)
-	if err != nil {
-		return err
-	}
+// 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) {
+		if !strings.Contains(string(resp.Body), expected) {
+			// Returning (true, err) causes SpoofingClient.Poll to fail.
+			return true, fmt.Errorf("body mismatch: got %q, want %q", string(resp.Body), expected)
+		}
 
-	if spoofDomain != "" {
-		req.Host = spoofDomain
+		return true, nil
 	}
+}
 
-	var body []byte
-	err = wait.PollImmediate(requestInterval, requestTimeout, func() (bool, error) {
-		resp, err := h.Do(req)
-		if err != nil {
-			if err, ok := err.(net.Error); ok && err.Timeout() {
-				logger.Infof("Retrying for TCP timeout %v", err)
-				return false, nil
-			}
-			return true, err
+// EventuallyMatchesBody checks that the response body *eventually* matches the expected body.
+// TODO(#1178): Delete me. We don't want to need this; we should be waiting for an appropriate Status instead.
+func EventuallyMatchesBody(expected string) spoof.ResponseChecker {
+	return func(resp *spoof.Response) (bool, error) {
+		if !strings.Contains(string(resp.Body), expected) {
+			// Returning (false, nil) causes SpoofingClient.Poll to retry.
+			return false, nil
 		}
 
-		if resp.StatusCode != 200 {
-			for _, code := range retryableCodes {
-				if resp.StatusCode == code {
-					logger.Infof("Retrying for code %v", resp.StatusCode)
-					return false, nil
-				}
-			}
-			s := fmt.Sprintf("Status code %d was not a retriable code (%v)", resp.StatusCode, retryableCodes)
-			return true, errors.New(s)
-		}
-		body, err = ioutil.ReadAll(resp.Body)
-		return inState(string(body))
-	})
-	return err
+		return true, nil
+	}
 }
 
 // WaitForEndpointState will poll an endpoint until inState indicates the state is achieved.
 // If resolvableDomain is false, it will use kubeClientset to look up the ingress and spoof
 // 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 func(body string) (bool, error), desc string) error {
+func WaitForEndpointState(kubeClientset *kubernetes.Clientset, logger *zap.SugaredLogger, resolvableDomain bool, domain string, inState spoof.ResponseChecker, desc string) error {
 	metricName := fmt.Sprintf("WaitForEndpointState/%s", desc)
 	_, span := trace.StartSpan(context.Background(), metricName)
 	defer span.End()
 
-	var endpoint, spoofDomain string
-
-	// If the domain that the Route controller is configured to assign to Route.Status.Domain
-	// (the domainSuffix) is not resolvable, we need to retrieve the IP of the endpoint and
-	// spoof the Host in our requests.
-	if !resolvableDomain {
-		ingressName := "knative-ingressgateway"
-		ingressNamespace := "istio-system"
-		ingress, err := kubeClientset.CoreV1().Services(ingressNamespace).Get(ingressName, metav1.GetOptions{})
-		if err != nil {
-			return err
-		}
-		if ingress.Status.LoadBalancer.Ingress[0].IP == "" {
-			return fmt.Errorf("Expected ingress loadbalancer IP for %s to be set, instead was empty", ingressName)
-		}
-		endpoint = fmt.Sprintf("http://%s", ingress.Status.LoadBalancer.Ingress[0].IP)
-		spoofDomain = domain
-	} else {
-		// If the domain is resolvable, we can use it directly when we make requests
-		endpoint = domain
+	client, err := spoof.New(kubeClientset, logger, domain, resolvableDomain)
+	if err != nil {
+		return err
 	}
 
-	logger.Infof("Wait for the endpoint to be up and handling requests")
 	// TODO(#348): The ingress endpoint tends to return 503's and 404's
-	return waitForRequestToDomainState(logger, endpoint, spoofDomain, []int{503, 404}, inState)
+	client.RetryCodes = []int{http.StatusServiceUnavailable, http.StatusNotFound}
+
+	req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("http://%s", domain), nil)
+	if err != nil {
+		return err
+	}
+
+	_, err = client.Poll(req, inState)
+	return err
 }