From 1a1535d859784c32b8c5d7346827d3462c4a28b3 Mon Sep 17 00:00:00 2001 From: charlie Date: Wed, 31 May 2023 12:20:23 +0800 Subject: [PATCH 1/7] resolve conflict Signed-off-by: charlie --- .../tests/httproute-canary-weight.go | 92 +++++++++++++------ .../tests/httproute-canary-weight.yaml | 42 +++++++++ test/ingress/conformance/utils/http/http.go | 67 ++++++++++++++ 3 files changed, 173 insertions(+), 28 deletions(-) diff --git a/test/ingress/conformance/tests/httproute-canary-weight.go b/test/ingress/conformance/tests/httproute-canary-weight.go index cc8fdb743..1aef757a5 100644 --- a/test/ingress/conformance/tests/httproute-canary-weight.go +++ b/test/ingress/conformance/tests/httproute-canary-weight.go @@ -30,48 +30,84 @@ var HTTPRouteCanaryWeight = suite.ConformanceTest{ Description: "The Ingress in the higress-conformance-infra namespace uses the canary weight traffic split.", Manifests: []string{"tests/httproute-canary-weight.yaml"}, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { - testcases := []http.Assertion{ - // test if the weight is 0 + tt := []struct { + assertion http.Assertion + totReq int // total request + fn func(succ int, fail int) bool // the results are determined by the number of successful and failed requests + }{ { - Meta: http.AssertionMeta{ - TargetBackend: "infra-backend-v1", - TargetNamespace: "higress-conformance-infra", + totReq: 100, + fn: func(succ int, fail int) bool { // avoid accidentally incorrect request failures + return succ > 90 }, - Request: http.AssertionRequest{ - ActualRequest: http.Request{ - Path: "/weight-0", - Host: "canary.higress.io", + assertion: http.Assertion{ + // test if the weight is 0 + Meta: http.AssertionMeta{ + TargetBackend: "infra-backend-v1", + TargetNamespace: "higress-conformance-infra", }, - }, - Response: http.AssertionResponse{ - ExpectedResponse: http.Response{ - StatusCode: 200, + Request: http.AssertionRequest{ + ActualRequest: http.Request{ + Path: "/weight-0", + Host: "canary.higress.io", + }, + }, + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 200, + }, }, }, - }, - // test if the weight is 100 - { - Meta: http.AssertionMeta{ - TargetBackend: "infra-backend-v2", - TargetNamespace: "higress-conformance-infra", + }, { // test if the weight is 100 + totReq: 100, + fn: func(succ int, fail int) bool { + return succ > 90 }, - Request: http.AssertionRequest{ - ActualRequest: http.Request{ - Path: "/weight-100", - Host: "canary.higress.io", + assertion: http.Assertion{ + Meta: http.AssertionMeta{ + TargetBackend: "infra-backend-v2", + TargetNamespace: "higress-conformance-infra", + }, + Request: http.AssertionRequest{ + ActualRequest: http.Request{ + Path: "/weight-100", + Host: "canary.higress.io", + }, }, + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 200, + }, + }, + }, + }, { + totReq: 100, + fn: func(succ int, fail int) bool { + return succ > 40 }, - Response: http.AssertionResponse{ - ExpectedResponse: http.Response{ - StatusCode: 200, + assertion: http.Assertion{ + Meta: http.AssertionMeta{ + TargetBackend: "infra-backend-v2", + TargetNamespace: "higress-conformance-infra", + }, + Request: http.AssertionRequest{ + ActualRequest: http.Request{ + Path: "/weight-50", + Host: "canary.higress.io", + }, + }, + Response: http.AssertionResponse{ + ExpectedResponse: http.Response{ + StatusCode: 200, + }, }, }, }, } t.Run("Canary HTTPRoute Traffic Split", func(t *testing.T) { - for _, testcase := range testcases { - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, suite.GatewayAddress, testcase) + for _, testcase := range tt { + http.MakeRequestAndCountExpectedResponse(t, suite.RoundTripper, suite.GatewayAddress, testcase.assertion, testcase.totReq, testcase.fn) } }) }, diff --git a/test/ingress/conformance/tests/httproute-canary-weight.yaml b/test/ingress/conformance/tests/httproute-canary-weight.yaml index 49f2134cb..74796de71 100644 --- a/test/ingress/conformance/tests/httproute-canary-weight.yaml +++ b/test/ingress/conformance/tests/httproute-canary-weight.yaml @@ -95,3 +95,45 @@ spec: name: infra-backend-v1 port: number: 8080 +--- +# Test if the weight is 50 +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + nginx.ingress.kubernetes.io/canary: "true" + nginx.ingress.kubernetes.io/canary-weight: "50" + name: ingress-echo-canary-weight-50 + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - host: canary.higress.io + http: + paths: + - path: /weight-50 + pathType: Exact + backend: + service: + name: infra-backend-v2 + port: + number: 8080 +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: ingress-echo-weight-50 + namespace: higress-conformance-infra +spec: + ingressClassName: higress + rules: + - host: canary.higress.io + http: + paths: + - path: /weight-50 + pathType: Exact + backend: + service: + name: infra-backend-v1 + port: + number: 8080 \ No newline at end of file diff --git a/test/ingress/conformance/utils/http/http.go b/test/ingress/conformance/utils/http/http.go index 75d3cd485..80017c9c7 100644 --- a/test/ingress/conformance/utils/http/http.go +++ b/test/ingress/conformance/utils/http/http.go @@ -423,3 +423,70 @@ func setRedirectRequestDefaults(req *roundtripper.Request, cRes *roundtripper.Ca expected.Request.RedirectRequest.Path = req.URL.Path } } + +func getRequest(gwAddr string, expected Assertion) roundtripper.Request { + path, query, _ := strings.Cut(expected.Request.ActualRequest.Path, "?") + + req := roundtripper.Request{ + Method: expected.Request.ActualRequest.Method, + Host: expected.Request.ActualRequest.Host, + URL: url.URL{Scheme: "http", Host: gwAddr, Path: path, RawQuery: query}, + Protocol: "HTTP", + Headers: map[string][]string{}, + UnfollowRedirect: expected.Request.ActualRequest.UnfollowRedirect, + } + + if expected.Request.ActualRequest.Headers != nil { + for name, value := range expected.Request.ActualRequest.Headers { + req.Headers[name] = []string{value} + } + } + + backendSetHeaders := []string{} + for name, val := range expected.Response.AdditionalResponseHeaders { + backendSetHeaders = append(backendSetHeaders, name+":"+val) + } + req.Headers["X-Echo-Set-Header"] = []string{strings.Join(backendSetHeaders, ",")} + + return req +} + +// MakeRequestAndCountExpectedResponse make 'totReq' requests and determine whether to test results according to 'fn' callback function +func MakeRequestAndCountExpectedResponse(t *testing.T, r roundtripper.RoundTripper, gwAddr string, expected Assertion, totReq int, fn func(int, int) bool) { + t.Helper() + + if expected.Request.ActualRequest.Method == "" { + expected.Request.ActualRequest.Method = "GET" + } + + if expected.Response.ExpectedResponse.StatusCode == 0 { + expected.Response.ExpectedResponse.StatusCode = 200 + } + + t.Logf("Making %s request to http://%s%s", expected.Request.ActualRequest.Method, gwAddr, expected.Request.ActualRequest.Path) + + req := getRequest(gwAddr, expected) + + succ, fail := 0, 0 + for i := 0; i < totReq; i++ { + cReq, cRes, err := r.CaptureRoundTrip(req) + if err != nil { + fail += 1 + t.Logf("Request failed, not ready yet: %v (failed count: %v)", err.Error(), fail) + continue + } + + if err := CompareRequest(&req, cReq, cRes, expected); err != nil { + fail += 1 + t.Logf("Response expectation failed for request: %v not ready yet: %v (failed count: %v)", req, err, fail) + continue + } + + succ += 1 + } + + if !fn(succ, fail) { + t.Logf("Test failed, the num of succ: %d, the num of fail: %d, the sum of req: %d", succ, fail, totReq) + } + t.Logf("Test passed") +} From bfb23917bf292b109b3650948cf40e2a76f14820 Mon Sep 17 00:00:00 2001 From: charlie Date: Sun, 26 Mar 2023 17:50:23 +0800 Subject: [PATCH 2/7] fix: add a blank line --- test/ingress/conformance/tests/httproute-canary-weight.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ingress/conformance/tests/httproute-canary-weight.yaml b/test/ingress/conformance/tests/httproute-canary-weight.yaml index 74796de71..1a1c1b7c7 100644 --- a/test/ingress/conformance/tests/httproute-canary-weight.yaml +++ b/test/ingress/conformance/tests/httproute-canary-weight.yaml @@ -136,4 +136,4 @@ spec: service: name: infra-backend-v1 port: - number: 8080 \ No newline at end of file + number: 8080 From eba4ed52d1bcd00308f7fa838297a2cfa4495fc3 Mon Sep 17 00:00:00 2001 From: charlie Date: Mon, 27 Mar 2023 19:14:13 +0800 Subject: [PATCH 3/7] fix --- .../tests/httproute-canary-weight.go | 26 +++++++------------ test/ingress/conformance/utils/http/http.go | 17 +++++++++--- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/test/ingress/conformance/tests/httproute-canary-weight.go b/test/ingress/conformance/tests/httproute-canary-weight.go index 1aef757a5..59dc91600 100644 --- a/test/ingress/conformance/tests/httproute-canary-weight.go +++ b/test/ingress/conformance/tests/httproute-canary-weight.go @@ -31,15 +31,13 @@ var HTTPRouteCanaryWeight = suite.ConformanceTest{ Manifests: []string{"tests/httproute-canary-weight.yaml"}, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { tt := []struct { - assertion http.Assertion - totReq int // total request - fn func(succ int, fail int) bool // the results are determined by the number of successful and failed requests + assertion http.Assertion + minSuccRate float32 + maxSuccRate float32 }{ { - totReq: 100, - fn: func(succ int, fail int) bool { // avoid accidentally incorrect request failures - return succ > 90 - }, + minSuccRate: 0.9, + maxSuccRate: 1.0, assertion: http.Assertion{ // test if the weight is 0 Meta: http.AssertionMeta{ @@ -59,10 +57,8 @@ var HTTPRouteCanaryWeight = suite.ConformanceTest{ }, }, }, { // test if the weight is 100 - totReq: 100, - fn: func(succ int, fail int) bool { - return succ > 90 - }, + minSuccRate: 0.9, + maxSuccRate: 1.0, assertion: http.Assertion{ Meta: http.AssertionMeta{ TargetBackend: "infra-backend-v2", @@ -81,10 +77,8 @@ var HTTPRouteCanaryWeight = suite.ConformanceTest{ }, }, }, { - totReq: 100, - fn: func(succ int, fail int) bool { - return succ > 40 - }, + minSuccRate: 0.4, + maxSuccRate: 0.6, assertion: http.Assertion{ Meta: http.AssertionMeta{ TargetBackend: "infra-backend-v2", @@ -107,7 +101,7 @@ var HTTPRouteCanaryWeight = suite.ConformanceTest{ t.Run("Canary HTTPRoute Traffic Split", func(t *testing.T) { for _, testcase := range tt { - http.MakeRequestAndCountExpectedResponse(t, suite.RoundTripper, suite.GatewayAddress, testcase.assertion, testcase.totReq, testcase.fn) + http.MakeRequestAndCountExpectedResponse(t, suite.RoundTripper, suite.GatewayAddress, testcase.assertion, testcase.minSuccRate, testcase.maxSuccRate) } }) }, diff --git a/test/ingress/conformance/utils/http/http.go b/test/ingress/conformance/utils/http/http.go index 80017c9c7..60d570c47 100644 --- a/test/ingress/conformance/utils/http/http.go +++ b/test/ingress/conformance/utils/http/http.go @@ -126,6 +126,8 @@ type Response struct { // maxTimeToConsistency, the test will fail. const requiredConsecutiveSuccesses = 3 +const totalRequest = 100 + // MakeRequestAndExpectEventuallyConsistentResponse makes a request with the given parameters, // understanding that the request may fail for some amount of time. // @@ -452,7 +454,7 @@ func getRequest(gwAddr string, expected Assertion) roundtripper.Request { } // MakeRequestAndCountExpectedResponse make 'totReq' requests and determine whether to test results according to 'fn' callback function -func MakeRequestAndCountExpectedResponse(t *testing.T, r roundtripper.RoundTripper, gwAddr string, expected Assertion, totReq int, fn func(int, int) bool) { +func MakeRequestAndCountExpectedResponse(t *testing.T, r roundtripper.RoundTripper, gwAddr string, expected Assertion, minSuccRate, maxSuccRate float32) { t.Helper() if expected.Request.ActualRequest.Method == "" { @@ -468,7 +470,7 @@ func MakeRequestAndCountExpectedResponse(t *testing.T, r roundtripper.RoundTripp req := getRequest(gwAddr, expected) succ, fail := 0, 0 - for i := 0; i < totReq; i++ { + for i := 0; i < totalRequest; i++ { cReq, cRes, err := r.CaptureRoundTrip(req) if err != nil { fail += 1 @@ -485,8 +487,15 @@ func MakeRequestAndCountExpectedResponse(t *testing.T, r roundtripper.RoundTripp succ += 1 } - if !fn(succ, fail) { - t.Logf("Test failed, the num of succ: %d, the num of fail: %d, the sum of req: %d", succ, fail, totReq) + rate := float32(succ) / totalRequest + if 0 < minSuccRate && rate < minSuccRate { + t.Errorf("Test failed, expect the minSuccRate is %v, got %v", minSuccRate, rate) + return + } + if 0 < maxSuccRate && maxSuccRate > rate { + t.Errorf("Test failed, expect the maxSuccRate is %v, got %v", maxSuccRate, rate) + return } + t.Logf("Test passed") } From 44eb2935419eba7caa4f631bdf50404d76066353 Mon Sep 17 00:00:00 2001 From: charlie Date: Mon, 27 Mar 2023 19:31:18 +0800 Subject: [PATCH 4/7] add rateDeviation --- .../conformance/tests/httproute-canary-weight.go | 16 ++++++---------- test/ingress/conformance/utils/http/http.go | 10 +++++++--- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/test/ingress/conformance/tests/httproute-canary-weight.go b/test/ingress/conformance/tests/httproute-canary-weight.go index 59dc91600..e89472010 100644 --- a/test/ingress/conformance/tests/httproute-canary-weight.go +++ b/test/ingress/conformance/tests/httproute-canary-weight.go @@ -31,13 +31,11 @@ var HTTPRouteCanaryWeight = suite.ConformanceTest{ Manifests: []string{"tests/httproute-canary-weight.yaml"}, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { tt := []struct { - assertion http.Assertion - minSuccRate float32 - maxSuccRate float32 + assertion http.Assertion + succRate float32 }{ { - minSuccRate: 0.9, - maxSuccRate: 1.0, + succRate: 1.0, assertion: http.Assertion{ // test if the weight is 0 Meta: http.AssertionMeta{ @@ -57,8 +55,7 @@ var HTTPRouteCanaryWeight = suite.ConformanceTest{ }, }, }, { // test if the weight is 100 - minSuccRate: 0.9, - maxSuccRate: 1.0, + succRate: 1.0, assertion: http.Assertion{ Meta: http.AssertionMeta{ TargetBackend: "infra-backend-v2", @@ -77,8 +74,7 @@ var HTTPRouteCanaryWeight = suite.ConformanceTest{ }, }, }, { - minSuccRate: 0.4, - maxSuccRate: 0.6, + succRate: 0.5, assertion: http.Assertion{ Meta: http.AssertionMeta{ TargetBackend: "infra-backend-v2", @@ -101,7 +97,7 @@ var HTTPRouteCanaryWeight = suite.ConformanceTest{ t.Run("Canary HTTPRoute Traffic Split", func(t *testing.T) { for _, testcase := range tt { - http.MakeRequestAndCountExpectedResponse(t, suite.RoundTripper, suite.GatewayAddress, testcase.assertion, testcase.minSuccRate, testcase.maxSuccRate) + http.MakeRequestAndCountExpectedResponse(t, suite.RoundTripper, suite.GatewayAddress, testcase.assertion, testcase.succRate) } }) }, diff --git a/test/ingress/conformance/utils/http/http.go b/test/ingress/conformance/utils/http/http.go index 60d570c47..ea5b2c8cf 100644 --- a/test/ingress/conformance/utils/http/http.go +++ b/test/ingress/conformance/utils/http/http.go @@ -16,6 +16,7 @@ package http import ( "fmt" + "math" "net/url" "strings" "testing" @@ -128,6 +129,8 @@ const requiredConsecutiveSuccesses = 3 const totalRequest = 100 +const rateDeviation = 0.1 + // MakeRequestAndExpectEventuallyConsistentResponse makes a request with the given parameters, // understanding that the request may fail for some amount of time. // @@ -453,8 +456,8 @@ func getRequest(gwAddr string, expected Assertion) roundtripper.Request { return req } -// MakeRequestAndCountExpectedResponse make 'totReq' requests and determine whether to test results according to 'fn' callback function -func MakeRequestAndCountExpectedResponse(t *testing.T, r roundtripper.RoundTripper, gwAddr string, expected Assertion, minSuccRate, maxSuccRate float32) { +// MakeRequestAndCountExpectedResponse make 'totReq' requests and determine whether to test results according to the succRate +func MakeRequestAndCountExpectedResponse(t *testing.T, r roundtripper.RoundTripper, gwAddr string, expected Assertion, succRate float64) { t.Helper() if expected.Request.ActualRequest.Method == "" { @@ -487,7 +490,8 @@ func MakeRequestAndCountExpectedResponse(t *testing.T, r roundtripper.RoundTripp succ += 1 } - rate := float32(succ) / totalRequest + rate := float64(succ) / totalRequest + minSuccRate, maxSuccRate := math.Min(succRate-rateDeviation, 0), math.Max(succRate+rateDeviation, 1.0) if 0 < minSuccRate && rate < minSuccRate { t.Errorf("Test failed, expect the minSuccRate is %v, got %v", minSuccRate, rate) return From 8d04756ceefbf059228c2a9a114488c468e065e5 Mon Sep 17 00:00:00 2001 From: charlie Date: Mon, 27 Mar 2023 19:36:02 +0800 Subject: [PATCH 5/7] fix --- test/ingress/conformance/tests/httproute-canary-weight.go | 2 +- test/ingress/conformance/utils/http/http.go | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/test/ingress/conformance/tests/httproute-canary-weight.go b/test/ingress/conformance/tests/httproute-canary-weight.go index e89472010..bcdf622a9 100644 --- a/test/ingress/conformance/tests/httproute-canary-weight.go +++ b/test/ingress/conformance/tests/httproute-canary-weight.go @@ -32,7 +32,7 @@ var HTTPRouteCanaryWeight = suite.ConformanceTest{ Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { tt := []struct { assertion http.Assertion - succRate float32 + succRate float64 }{ { succRate: 1.0, diff --git a/test/ingress/conformance/utils/http/http.go b/test/ingress/conformance/utils/http/http.go index ea5b2c8cf..b4cec03f9 100644 --- a/test/ingress/conformance/utils/http/http.go +++ b/test/ingress/conformance/utils/http/http.go @@ -492,12 +492,8 @@ func MakeRequestAndCountExpectedResponse(t *testing.T, r roundtripper.RoundTripp rate := float64(succ) / totalRequest minSuccRate, maxSuccRate := math.Min(succRate-rateDeviation, 0), math.Max(succRate+rateDeviation, 1.0) - if 0 < minSuccRate && rate < minSuccRate { - t.Errorf("Test failed, expect the minSuccRate is %v, got %v", minSuccRate, rate) - return - } - if 0 < maxSuccRate && maxSuccRate > rate { - t.Errorf("Test failed, expect the maxSuccRate is %v, got %v", maxSuccRate, rate) + if rate < minSuccRate || maxSuccRate > rate { + t.Errorf("Test failed, expect the minSuccRate is %v, the maxSuccRate is %v, but got %v", minSuccRate, maxSuccRate, rate) return } From 30f90203275c9a6765f7fa14c9171daefa7c425d Mon Sep 17 00:00:00 2001 From: charlie Date: Thu, 30 Mar 2023 16:33:17 +0800 Subject: [PATCH 6/7] fix --- test/ingress/conformance/utils/http/http.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/ingress/conformance/utils/http/http.go b/test/ingress/conformance/utils/http/http.go index b4cec03f9..ddddef324 100644 --- a/test/ingress/conformance/utils/http/http.go +++ b/test/ingress/conformance/utils/http/http.go @@ -491,8 +491,8 @@ func MakeRequestAndCountExpectedResponse(t *testing.T, r roundtripper.RoundTripp } rate := float64(succ) / totalRequest - minSuccRate, maxSuccRate := math.Min(succRate-rateDeviation, 0), math.Max(succRate+rateDeviation, 1.0) - if rate < minSuccRate || maxSuccRate > rate { + minSuccRate, maxSuccRate := math.Max(succRate-rateDeviation, 0), math.Min(succRate+rateDeviation, 1.0) + if rate < minSuccRate || maxSuccRate < rate { t.Errorf("Test failed, expect the minSuccRate is %v, the maxSuccRate is %v, but got %v", minSuccRate, maxSuccRate, rate) return } From 01d5dd73e9b01df60cf7f8896a6a07bed297a23d Mon Sep 17 00:00:00 2001 From: charlie Date: Thu, 30 Mar 2023 19:08:22 +0800 Subject: [PATCH 7/7] trigger ci Signed-off-by: charlie --- test/ingress/conformance/utils/http/http.go | 1 - 1 file changed, 1 deletion(-) diff --git a/test/ingress/conformance/utils/http/http.go b/test/ingress/conformance/utils/http/http.go index ddddef324..55e01dbd7 100644 --- a/test/ingress/conformance/utils/http/http.go +++ b/test/ingress/conformance/utils/http/http.go @@ -496,6 +496,5 @@ func MakeRequestAndCountExpectedResponse(t *testing.T, r roundtripper.RoundTripp t.Errorf("Test failed, expect the minSuccRate is %v, the maxSuccRate is %v, but got %v", minSuccRate, maxSuccRate, rate) return } - t.Logf("Test passed") }