From 1d8c08083dbd43b0e9bf5d279b7fcd624847bee1 Mon Sep 17 00:00:00 2001 From: Martin Gencur Date: Thu, 26 Mar 2020 14:55:17 +0100 Subject: [PATCH 1/2] Provide test flag --ingressRetries for retrying route inconsistency --- test/e2e_flags.go | 4 ++++ test/v1/route.go | 16 ++++++++++++++-- test/v1alpha1/route.go | 17 ++++++++++++++--- test/v1beta1/route.go | 16 ++++++++++++++-- 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/test/e2e_flags.go b/test/e2e_flags.go index 153e3e328123..67bc1a3faafe 100644 --- a/test/e2e_flags.go +++ b/test/e2e_flags.go @@ -33,6 +33,8 @@ type ServingEnvironmentFlags struct { ResolvableDomain bool // Resolve Route controller's `domainSuffix` Https bool // Indicates where the test service will be created with https IngressClass string // Indicates the class of Ingress provider to test. + IngressRetries int // Indicates how many consecutive requests sent against the ingress should be + // successful before claiming the Ingress ready. CertificateClass string // Indicates the class of Certificate provider to test. SystemNamespace string // Indicates the system namespace, in which Knative Serving is installed. } @@ -48,6 +50,8 @@ func initializeServingFlags() *ServingEnvironmentFlags { flag.StringVar(&f.IngressClass, "ingressClass", network.IstioIngressClassName, "Set this flag to the ingress class to test against.") + flag.IntVar(&f.IngressRetries, "ingressRetries", 0, + "Set this flag to a positive number to enforce retrying ingress consistency in tests.") flag.StringVar(&f.CertificateClass, "certificateClass", network.CertManagerCertificateClassName, "Set this flag to the certificate class to test against.") diff --git a/test/v1/route.go b/test/v1/route.go index 740a49ab1d41..d22bdd338669 100644 --- a/test/v1/route.go +++ b/test/v1/route.go @@ -114,10 +114,22 @@ func IsRouteNotReady(r *v1.Route) (bool, error) { return !r.Status.IsReady(), nil } -// RetryingRouteInconsistency retries common requests seen when creating a new route +// RetryingRouteInconsistency conditionally retries common requests seen when creating a new route. func RetryingRouteInconsistency(innerCheck spoof.ResponseChecker) spoof.ResponseChecker { + var successes int + if test.ServingFlags.IngressRetries > 0 { + return func(resp *spoof.Response) (bool, error) { + success, err := innerCheck(resp) + if !success { + successes = 0 + return false, err + } + successes++ + return successes == test.ServingFlags.IngressRetries, err + } + } + // By default only wrap the innerCheck. return func(resp *spoof.Response) (bool, error) { - // If we didn't match any retryable codes, invoke the ResponseChecker that we wrapped. return innerCheck(resp) } } diff --git a/test/v1alpha1/route.go b/test/v1alpha1/route.go index 4ff55e2ac515..c050488a488c 100644 --- a/test/v1alpha1/route.go +++ b/test/v1alpha1/route.go @@ -50,11 +50,22 @@ func CreateRoute(t pkgTest.T, clients *test.Clients, names test.ResourceNames, f return clients.ServingAlphaClient.Routes.Create(route) } -// RetryingRouteInconsistency retries common requests seen when creating a new route -// TODO(5573): Remove this. +// RetryingRouteInconsistency conditionally retries common requests seen when creating a new route. func RetryingRouteInconsistency(innerCheck spoof.ResponseChecker) spoof.ResponseChecker { + var successes int + if test.ServingFlags.IngressRetries > 0 { + return func(resp *spoof.Response) (bool, error) { + success, err := innerCheck(resp) + if !success { + successes = 0 + return false, err + } + successes++ + return successes == test.ServingFlags.IngressRetries, err + } + } + // By default only wrap the innerCheck. return func(resp *spoof.Response) (bool, error) { - // If we didn't match any retryable codes, invoke the ResponseChecker that we wrapped. return innerCheck(resp) } } diff --git a/test/v1beta1/route.go b/test/v1beta1/route.go index 4ee7dc21318a..294723b439c3 100644 --- a/test/v1beta1/route.go +++ b/test/v1beta1/route.go @@ -116,10 +116,22 @@ func IsRouteFailed(r *v1beta1.Route) (bool, error) { return r.IsFailed(), nil } -// RetryingRouteInconsistency retries common requests seen when creating a new route +// RetryingRouteInconsistency conditionally retries common requests seen when creating a new route. func RetryingRouteInconsistency(innerCheck spoof.ResponseChecker) spoof.ResponseChecker { + var successes int + if test.ServingFlags.IngressRetries > 0 { + return func(resp *spoof.Response) (bool, error) { + success, err := innerCheck(resp) + if !success { + successes = 0 + return false, err + } + successes++ + return successes == test.ServingFlags.IngressRetries, err + } + } + // By default only wrap the innerCheck. return func(resp *spoof.Response) (bool, error) { - // If we didn't match any retryable codes, invoke the ResponseChecker that we wrapped. return innerCheck(resp) } } From 69f5ec0b468b557f905f2e10f4c81a302395804b Mon Sep 17 00:00:00 2001 From: Martin Gencur Date: Thu, 4 Jun 2020 19:14:50 +0200 Subject: [PATCH 2/2] Fixes --- test/e2e_flags.go | 3 ++- test/v1/route.go | 2 +- test/v1alpha1/route.go | 2 +- test/v1beta1/route.go | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/test/e2e_flags.go b/test/e2e_flags.go index 67bc1a3faafe..a492ae3c6e79 100644 --- a/test/e2e_flags.go +++ b/test/e2e_flags.go @@ -33,8 +33,9 @@ type ServingEnvironmentFlags struct { ResolvableDomain bool // Resolve Route controller's `domainSuffix` Https bool // Indicates where the test service will be created with https IngressClass string // Indicates the class of Ingress provider to test. - IngressRetries int // Indicates how many consecutive requests sent against the ingress should be + // Indicates how many consecutive requests sent against the ingress should be // successful before claiming the Ingress ready. + IngressRetries int CertificateClass string // Indicates the class of Certificate provider to test. SystemNamespace string // Indicates the system namespace, in which Knative Serving is installed. } diff --git a/test/v1/route.go b/test/v1/route.go index d22bdd338669..6b86ac291a6e 100644 --- a/test/v1/route.go +++ b/test/v1/route.go @@ -116,8 +116,8 @@ func IsRouteNotReady(r *v1.Route) (bool, error) { // RetryingRouteInconsistency conditionally retries common requests seen when creating a new route. func RetryingRouteInconsistency(innerCheck spoof.ResponseChecker) spoof.ResponseChecker { - var successes int if test.ServingFlags.IngressRetries > 0 { + var successes int return func(resp *spoof.Response) (bool, error) { success, err := innerCheck(resp) if !success { diff --git a/test/v1alpha1/route.go b/test/v1alpha1/route.go index c050488a488c..99326ab2edc1 100644 --- a/test/v1alpha1/route.go +++ b/test/v1alpha1/route.go @@ -52,8 +52,8 @@ func CreateRoute(t pkgTest.T, clients *test.Clients, names test.ResourceNames, f // RetryingRouteInconsistency conditionally retries common requests seen when creating a new route. func RetryingRouteInconsistency(innerCheck spoof.ResponseChecker) spoof.ResponseChecker { - var successes int if test.ServingFlags.IngressRetries > 0 { + var successes int return func(resp *spoof.Response) (bool, error) { success, err := innerCheck(resp) if !success { diff --git a/test/v1beta1/route.go b/test/v1beta1/route.go index 294723b439c3..fcd980c7b184 100644 --- a/test/v1beta1/route.go +++ b/test/v1beta1/route.go @@ -118,8 +118,8 @@ func IsRouteFailed(r *v1beta1.Route) (bool, error) { // RetryingRouteInconsistency conditionally retries common requests seen when creating a new route. func RetryingRouteInconsistency(innerCheck spoof.ResponseChecker) spoof.ResponseChecker { - var successes int if test.ServingFlags.IngressRetries > 0 { + var successes int return func(resp *spoof.Response) (bool, error) { success, err := innerCheck(resp) if !success {