From 97307be36a6a697d95dfd6c40ce4550dc6482814 Mon Sep 17 00:00:00 2001 From: Brenda Chan Date: Wed, 4 Jul 2018 13:05:12 -0400 Subject: [PATCH] Removes 503 as a retryable status code * Only allow 503's for newly created revisions --- test/conformance/route_test.go | 8 +++++++- test/conformance/service_test.go | 3 ++- test/e2e/autoscale_test.go | 11 +++++++++-- test/e2e/helloworld_test.go | 3 ++- test/request.go | 24 +++++++++++++++++++----- 5 files changed, 39 insertions(+), 10 deletions(-) diff --git a/test/conformance/route_test.go b/test/conformance/route_test.go index cf53d236fac1..3c4dfbc4948d 100644 --- a/test/conformance/route_test.go +++ b/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") + expectedEndpointState := test.NewExpectedEndpointstate(updatedRoute.Status.Domain) + // TODO(#348): The ingress endpoint occasionally returns 503's and 404's. + // Explicitly allow 404's and 503's for a newly created revision. + expectedEndpointState.AllowableStatusCodes = []int{http.StatusServiceUnavailable, http.StatusNotFound} + + err = test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, expectedEndpointState, 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) } diff --git a/test/conformance/service_test.go b/test/conformance/service_test.go index 0f1efdba1804..b7832696a2eb 100644 --- a/test/conformance/service_test.go +++ b/test/conformance/service_test.go @@ -63,8 +63,9 @@ 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) { + expectedEndpointState := test.NewExpectedEndpointstate(routeDomain) // 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, test.Flags.ResolvableDomain, expectedEndpointState, 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) } diff --git a/test/e2e/autoscale_test.go b/test/e2e/autoscale_test.go index 844a86690232..88f65fe37576 100644 --- a/test/e2e/autoscale_test.go +++ b/test/e2e/autoscale_test.go @@ -18,6 +18,7 @@ limitations under the License. package e2e import ( + "net/http" "strings" "testing" @@ -50,6 +51,7 @@ func isDeploymentScaledToZero() func(d *v1beta1.Deployment) (bool, error) { func generateTrafficBurst(clients *test.Clients, logger *zap.SugaredLogger, num int, domain string) { concurrentRequests := make(chan bool, num) + expectedEndpointState := test.NewExpectedEndpointstate(domain) logger.Infof("Performing %d concurrent requests.", num) for i := 0; i < num; i++ { @@ -57,7 +59,7 @@ func generateTrafficBurst(clients *test.Clients, logger *zap.SugaredLogger, num test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, - domain, + expectedEndpointState, test.EventuallyMatchesBody(autoscaleExpectedOutput), "MakingConcurrentRequests") concurrentRequests <- true @@ -154,11 +156,16 @@ func TestAutoscaleUpDownUp(t *testing.T) { } domain := route.Status.Domain + expectedEndpointState := test.NewExpectedEndpointstate(domain) + // TODO(#348): The ingress endpoint occasionally returns 503's and 404's. + // Explicitly allow 404's and 503's for a newly created revision. + expectedEndpointState.AllowableStatusCodes = []int{http.StatusServiceUnavailable, http.StatusNotFound} + err = test.WaitForEndpointState( clients.Kube, logger, test.Flags.ResolvableDomain, - domain, + expectedEndpointState, test.EventuallyMatchesBody(autoscaleExpectedOutput), "CheckingEndpointAfterUpdating") if err != nil { diff --git a/test/e2e/helloworld_test.go b/test/e2e/helloworld_test.go index 4fad12aaae04..c673050f07bf 100644 --- a/test/e2e/helloworld_test.go +++ b/test/e2e/helloworld_test.go @@ -57,7 +57,8 @@ func TestHelloWorld(t *testing.T) { } domain := route.Status.Domain - err = test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, domain, test.MatchesBody(helloWorldExpectedOutput), "HelloWorldServesText") + expectedEndpointState := test.NewExpectedEndpointstate(domain) + err = test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, expectedEndpointState, 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) } diff --git a/test/request.go b/test/request.go index 8b6ab599ae4c..b54a46b009d0 100644 --- a/test/request.go +++ b/test/request.go @@ -29,6 +29,21 @@ import ( "k8s.io/client-go/kubernetes" ) +type ExpectedEndpointState struct { + Domain string + AllowableStatusCodes []int +} + +func NewExpectedEndpointstate(domain string) *ExpectedEndpointState { + return &ExpectedEndpointState{ + Domain: domain, + // TODO(#348): The ingress endpoint tends to return 503's and 404's. + // Since there is currently a workaround for 503's, only allow 404's + // as an acceptable status code. + AllowableStatusCodes: []int{http.StatusNotFound}, + } +} + // MatchesAny is a NOP matcher. This is useful for polling until a 200 is returned. func MatchesAny(_ *spoof.Response) (bool, error) { return true, nil @@ -64,20 +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, resolvableDomain bool, expectedEndpointState *ExpectedEndpointState, 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, expectedEndpointState.Domain, 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} + client.RetryCodes = expectedEndpointState.AllowableStatusCodes - req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("http://%s", domain), nil) + req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("http://%s", expectedEndpointState.Domain), nil) if err != nil { return err }