From 2fb0f7831dcac6dcfd1256510be7c280738d59b1 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Tue, 29 Mar 2022 13:08:17 -0400 Subject: [PATCH 1/7] Support eventual consistency in request-response helper --- conformance/utils/http/http.go | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index d6173d0b15..c9fb448a45 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -20,6 +20,7 @@ import ( "net/url" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -72,8 +73,29 @@ func MakeRequestAndExpectResponse(t *testing.T, r roundtripper.RoundTripper, gwA req.Headers[name] = []string{value} } } - cReq, cRes, err := r.CaptureRoundTrip(req) - require.NoErrorf(t, err, "error making request") + + var cReq *roundtripper.CapturedRequest + var cRes *roundtripper.CapturedResponse + + // We expect eventual consistency once the resources are ready, so wait until + // we've made a successful request and gotten a response with desired status + // before making assertions on the response body. + require.Eventually(t, func() bool { + var err error + + cReq, cRes, err = r.CaptureRoundTrip(req) + if err != nil { + t.Log("request failed, not ready yet") + return false + } + + if cRes.StatusCode != expected.StatusCode { + t.Log("response missing expected status, not ready yet") + return false + } + + return true + }, 60*time.Second, 1*time.Second, "error making request, never got expected status") ExpectResponse(t, cReq, cRes, expected) } From 85d58a1c18476d1d30f3557174574af788e2c34a Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Tue, 29 Mar 2022 13:09:00 -0400 Subject: [PATCH 2/7] Use request-response helper for all tests --- conformance/tests/httproute-cross-namespace.go | 18 ++---------------- .../tests/httproute-simple-same-namespace.go | 18 ++---------------- 2 files changed, 4 insertions(+), 32 deletions(-) diff --git a/conformance/tests/httproute-cross-namespace.go b/conformance/tests/httproute-cross-namespace.go index 044114ca20..a826efaeb6 100644 --- a/conformance/tests/httproute-cross-namespace.go +++ b/conformance/tests/httproute-cross-namespace.go @@ -17,15 +17,12 @@ limitations under the License. package tests import ( - "net/url" "testing" - "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/gateway-api/conformance/utils/http" "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" - "sigs.k8s.io/gateway-api/conformance/utils/roundtripper" "sigs.k8s.io/gateway-api/conformance/utils/suite" ) @@ -43,19 +40,8 @@ var HTTPRouteCrossNamespace = suite.ConformanceTest{ gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeReady(t, suite.Client, suite.ControllerName, gwNN, routeNN) t.Run("Simple HTTP request should reach web-backend", func(t *testing.T) { - t.Logf("Making request to http://%s", gwAddr) - cReq, cRes, err := suite.RoundTripper.CaptureRoundTrip(roundtripper.Request{ - URL: url.URL{Scheme: "http", Host: gwAddr}, - Protocol: "HTTP", - }) - - require.NoErrorf(t, err, "error making request") - - http.ExpectResponse(t, cReq, cRes, http.ExpectedResponse{ - Request: http.ExpectedRequest{ - Method: "GET", - Path: "/", - }, + http.MakeRequestAndExpectResponse(t, suite.RoundTripper, gwAddr, http.ExpectedResponse{ + Request: http.ExpectedRequest{Path: "/"}, StatusCode: 200, Backend: "web-backend", Namespace: "gateway-conformance-web-backend", diff --git a/conformance/tests/httproute-simple-same-namespace.go b/conformance/tests/httproute-simple-same-namespace.go index 3cf702f42c..5279e03a93 100644 --- a/conformance/tests/httproute-simple-same-namespace.go +++ b/conformance/tests/httproute-simple-same-namespace.go @@ -17,16 +17,13 @@ limitations under the License. package tests import ( - "net/url" "testing" - "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/conformance/utils/http" "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" - "sigs.k8s.io/gateway-api/conformance/utils/roundtripper" "sigs.k8s.io/gateway-api/conformance/utils/suite" ) @@ -45,19 +42,8 @@ var HTTPRouteSimpleSameNamespace = suite.ConformanceTest{ gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeReady(t, suite.Client, suite.ControllerName, gwNN, routeNN) t.Run("Simple HTTP request should reach infra-backend", func(t *testing.T) { - t.Logf("Making request to http://%s", gwAddr) - cReq, cRes, err := suite.RoundTripper.CaptureRoundTrip(roundtripper.Request{ - URL: url.URL{Scheme: "http", Host: gwAddr}, - Protocol: "HTTP", - }) - - require.NoErrorf(t, err, "error making request") - - http.ExpectResponse(t, cReq, cRes, http.ExpectedResponse{ - Request: http.ExpectedRequest{ - Method: "GET", - Path: "/", - }, + http.MakeRequestAndExpectResponse(t, suite.RoundTripper, gwAddr, http.ExpectedResponse{ + Request: http.ExpectedRequest{Path: "/"}, StatusCode: 200, Backend: "infra-backend-v1", Namespace: "gateway-conformance-infra", From 824539bd33e6e6e5b543c533ff4265f0ee129829 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Tue, 29 Mar 2022 16:26:53 -0400 Subject: [PATCH 3/7] Log expected vs. actual while waiting for consistency --- conformance/utils/http/http.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index c9fb448a45..fb80fe21e2 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -85,12 +85,12 @@ func MakeRequestAndExpectResponse(t *testing.T, r roundtripper.RoundTripper, gwA cReq, cRes, err = r.CaptureRoundTrip(req) if err != nil { - t.Log("request failed, not ready yet") + t.Logf("Request failed, not ready yet: %v", err.Error()) return false } if cRes.StatusCode != expected.StatusCode { - t.Log("response missing expected status, not ready yet") + t.Logf("Expected response to have status %d but got %d, not ready yet", expected.StatusCode, cRes.StatusCode) return false } From d01bd47d6650c16a2ef3cfdb66247697447cd414 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Tue, 29 Mar 2022 16:59:32 -0400 Subject: [PATCH 4/7] Assert consistency once initial request succeeds --- .../tests/httproute-cross-namespace.go | 2 +- .../tests/httproute-matching-across-routes.go | 2 +- conformance/tests/httproute-matching.go | 2 +- .../tests/httproute-simple-same-namespace.go | 2 +- conformance/utils/http/http.go | 31 +++++++++++++------ 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/conformance/tests/httproute-cross-namespace.go b/conformance/tests/httproute-cross-namespace.go index a826efaeb6..a645437ec9 100644 --- a/conformance/tests/httproute-cross-namespace.go +++ b/conformance/tests/httproute-cross-namespace.go @@ -40,7 +40,7 @@ var HTTPRouteCrossNamespace = suite.ConformanceTest{ gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeReady(t, suite.Client, suite.ControllerName, gwNN, routeNN) t.Run("Simple HTTP request should reach web-backend", func(t *testing.T) { - http.MakeRequestAndExpectResponse(t, suite.RoundTripper, gwAddr, http.ExpectedResponse{ + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, gwAddr, http.ExpectedResponse{ Request: http.ExpectedRequest{Path: "/"}, StatusCode: 200, Backend: "web-backend", diff --git a/conformance/tests/httproute-matching-across-routes.go b/conformance/tests/httproute-matching-across-routes.go index bd7b259ffe..8989390515 100644 --- a/conformance/tests/httproute-matching-across-routes.go +++ b/conformance/tests/httproute-matching-across-routes.go @@ -109,7 +109,7 @@ var HTTPRouteMatchingAcrossRoutes = suite.ConformanceTest{ tc := testCases[i] t.Run(testName(tc, i), func(t *testing.T) { t.Parallel() - http.MakeRequestAndExpectResponse(t, suite.RoundTripper, gwAddr, tc) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, gwAddr, tc) }) } }, diff --git a/conformance/tests/httproute-matching.go b/conformance/tests/httproute-matching.go index 7fc404fc8e..3ec318f10d 100644 --- a/conformance/tests/httproute-matching.go +++ b/conformance/tests/httproute-matching.go @@ -72,7 +72,7 @@ var HTTPRouteMatching = suite.ConformanceTest{ tc := testCases[i] t.Run(testName(tc, i), func(t *testing.T) { t.Parallel() - http.MakeRequestAndExpectResponse(t, suite.RoundTripper, gwAddr, tc) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, gwAddr, tc) }) } }, diff --git a/conformance/tests/httproute-simple-same-namespace.go b/conformance/tests/httproute-simple-same-namespace.go index 5279e03a93..3cc971237f 100644 --- a/conformance/tests/httproute-simple-same-namespace.go +++ b/conformance/tests/httproute-simple-same-namespace.go @@ -42,7 +42,7 @@ var HTTPRouteSimpleSameNamespace = suite.ConformanceTest{ gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeReady(t, suite.Client, suite.ControllerName, gwNN, routeNN) t.Run("Simple HTTP request should reach infra-backend", func(t *testing.T) { - http.MakeRequestAndExpectResponse(t, suite.RoundTripper, gwAddr, http.ExpectedResponse{ + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, gwAddr, http.ExpectedResponse{ Request: http.ExpectedRequest{Path: "/"}, StatusCode: 200, Backend: "infra-backend-v1", diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index fb80fe21e2..5bf3f8149e 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -17,6 +17,7 @@ limitations under the License. package http import ( + "fmt" "net/url" "strings" "testing" @@ -45,9 +46,15 @@ type ExpectedRequest struct { Headers map[string]string } -// MakeRequestAndExpectResponse makes a request with the given parameters and -// verifies the response matches the provided ExpectedResponse. -func MakeRequestAndExpectResponse(t *testing.T, r roundtripper.RoundTripper, gwAddr string, expected ExpectedResponse) { +const maxConsistencyPeriodPerRequest = 60 * time.Second +const numConsistencyChecksPerRequest = 3 + +// MakeRequestAndExpectEventuallyConsistentResponse makes a request with the given parameters, +// understanding that the request may fail for some amount of time. +// +// Once the request succeeds and the response has the expected status code, assert that +// a response matching the provided ExpectedResponse is returned consistently. +func MakeRequestAndExpectEventuallyConsistentResponse(t *testing.T, r roundtripper.RoundTripper, gwAddr string, expected ExpectedResponse) { t.Helper() if expected.Request.Method == "" { @@ -74,16 +81,13 @@ func MakeRequestAndExpectResponse(t *testing.T, r roundtripper.RoundTripper, gwA } } - var cReq *roundtripper.CapturedRequest - var cRes *roundtripper.CapturedResponse - // We expect eventual consistency once the resources are ready, so wait until // we've made a successful request and gotten a response with desired status // before making assertions on the response body. require.Eventually(t, func() bool { var err error - cReq, cRes, err = r.CaptureRoundTrip(req) + _, cRes, err := r.CaptureRoundTrip(req) if err != nil { t.Logf("Request failed, not ready yet: %v", err.Error()) return false @@ -95,9 +99,18 @@ func MakeRequestAndExpectResponse(t *testing.T, r roundtripper.RoundTripper, gwA } return true - }, 60*time.Second, 1*time.Second, "error making request, never got expected status") + }, maxConsistencyPeriodPerRequest, 1*time.Second, "error making request, never got expected status") - ExpectResponse(t, cReq, cRes, expected) + // Once we've made a successful request and gotten a response with the + // desired status, assert that we get the expected response consistently. + for i := 1; i <= numConsistencyChecksPerRequest; i++ { + t.Run(fmt.Sprintf("consistency_check_%d", i), func(t *testing.T) { + cReq, cRes, err := r.CaptureRoundTrip(req) + require.NoError(t, err, "error making request after previous success") + + ExpectResponse(t, cReq, cRes, expected) + }) + } } // ExpectResponse verifies that a captured request and response match the From 41097d5fc7d3c0d12c3fa86db10cc0c2bd2d08a7 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Fri, 1 Apr 2022 14:50:42 -0400 Subject: [PATCH 5/7] Allow for intermittent success prior to reaching consistency --- conformance/utils/http/http.go | 46 ++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index 5bf3f8149e..7570ccff88 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -17,7 +17,6 @@ limitations under the License. package http import ( - "fmt" "net/url" "strings" "testing" @@ -52,8 +51,8 @@ const numConsistencyChecksPerRequest = 3 // MakeRequestAndExpectEventuallyConsistentResponse makes a request with the given parameters, // understanding that the request may fail for some amount of time. // -// Once the request succeeds and the response has the expected status code, assert that -// a response matching the provided ExpectedResponse is returned consistently. +// Once the request succeeds consistently with the response having the expected status code, make +// additional assertions on the response body using the provided ExpectedResponse. func MakeRequestAndExpectEventuallyConsistentResponse(t *testing.T, r roundtripper.RoundTripper, gwAddr string, expected ExpectedResponse) { t.Helper() @@ -81,36 +80,45 @@ func MakeRequestAndExpectEventuallyConsistentResponse(t *testing.T, r roundtripp } } - // We expect eventual consistency once the resources are ready, so wait until - // we've made a successful request and gotten a response with desired status - // before making assertions on the response body. - require.Eventually(t, func() bool { - var err error + cReq, cRes := WaitForConsistency(t, r, req, expected, numConsistencyChecksPerRequest) + ExpectResponse(t, cReq, cRes, expected) +} + +// WaitForConsistency repeats the provided request until it completes with a response having +// the expected status code consistently. The provided threshold determines how many times in +// a row this must occur to be considered "consistent". +func WaitForConsistency(t *testing.T, r roundtripper.RoundTripper, req roundtripper.Request, expected ExpectedResponse, threshold int) (*roundtripper.CapturedRequest, *roundtripper.CapturedResponse) { + var ( + cReq *roundtripper.CapturedRequest + cRes *roundtripper.CapturedResponse + err error + numSuccesses int + ) - _, cRes, err := r.CaptureRoundTrip(req) + require.Eventually(t, func() bool { + cReq, cRes, err = r.CaptureRoundTrip(req) if err != nil { + numSuccesses = 0 t.Logf("Request failed, not ready yet: %v", err.Error()) return false } if cRes.StatusCode != expected.StatusCode { + numSuccesses = 0 t.Logf("Expected response to have status %d but got %d, not ready yet", expected.StatusCode, cRes.StatusCode) return false } + numSuccesses++ + if numSuccesses < threshold { + t.Logf("Request has passed %d times in a row of the desired %d, not ready yet", numSuccesses, threshold) + return false + } + return true }, maxConsistencyPeriodPerRequest, 1*time.Second, "error making request, never got expected status") - // Once we've made a successful request and gotten a response with the - // desired status, assert that we get the expected response consistently. - for i := 1; i <= numConsistencyChecksPerRequest; i++ { - t.Run(fmt.Sprintf("consistency_check_%d", i), func(t *testing.T) { - cReq, cRes, err := r.CaptureRoundTrip(req) - require.NoError(t, err, "error making request after previous success") - - ExpectResponse(t, cReq, cRes, expected) - }) - } + return cReq, cRes } // ExpectResponse verifies that a captured request and response match the From 736fd4697d2cdc193d96f2435eb43dd7b5fb6969 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Fri, 1 Apr 2022 14:58:21 -0400 Subject: [PATCH 6/7] Log when we've reached the consistency threshold for a request --- conformance/utils/http/http.go | 1 + 1 file changed, 1 insertion(+) diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index 7570ccff88..492c11cdf6 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -115,6 +115,7 @@ func WaitForConsistency(t *testing.T, r roundtripper.RoundTripper, req roundtrip return false } + t.Logf("Request has passed %d times in a row of the desired %d, ready!", numSuccesses, threshold) return true }, maxConsistencyPeriodPerRequest, 1*time.Second, "error making request, never got expected status") From 7d3c918fdc6980b969e0b89bd9cc3bee218471fa Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Tue, 5 Apr 2022 12:56:11 -0400 Subject: [PATCH 7/7] Improve constant names + add docstrings --- conformance/utils/http/http.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index 492c11cdf6..19297d0a36 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -45,8 +45,15 @@ type ExpectedRequest struct { Headers map[string]string } -const maxConsistencyPeriodPerRequest = 60 * time.Second -const numConsistencyChecksPerRequest = 3 +// maxTimeToConsistency is the maximum time that WaitForConsistency will wait for +// requiredConsecutiveSuccesses requests to succeed in a row before failing the test. +const maxTimeToConsistency = 30 * time.Second + +// requiredConsecutiveSuccesses is the number of requests that must succeed in a row +// for MakeRequestAndExpectEventuallyConsistentResponse to consider the response "consistent" +// before making additional assertions on the response body. If this number is not reached within +// maxTimeToConsistency, the test will fail. +const requiredConsecutiveSuccesses = 3 // MakeRequestAndExpectEventuallyConsistentResponse makes a request with the given parameters, // understanding that the request may fail for some amount of time. @@ -80,7 +87,7 @@ func MakeRequestAndExpectEventuallyConsistentResponse(t *testing.T, r roundtripp } } - cReq, cRes := WaitForConsistency(t, r, req, expected, numConsistencyChecksPerRequest) + cReq, cRes := WaitForConsistency(t, r, req, expected, requiredConsecutiveSuccesses) ExpectResponse(t, cReq, cRes, expected) } @@ -117,7 +124,7 @@ func WaitForConsistency(t *testing.T, r roundtripper.RoundTripper, req roundtrip t.Logf("Request has passed %d times in a row of the desired %d, ready!", numSuccesses, threshold) return true - }, maxConsistencyPeriodPerRequest, 1*time.Second, "error making request, never got expected status") + }, maxTimeToConsistency, 1*time.Second, "error making request, never got expected status") return cReq, cRes }