Add RetryPolicy on VirtualHost for upstream transient connection issues

pkg/envoy/api/virtual_host.go

@@ -21,39 +21,47 @@ import (
 	extAuthService "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_authz/v3"
 	"github.com/envoyproxy/go-control-plane/pkg/wellknown"
 	"google.golang.org/protobuf/types/known/anypb"
+	"google.golang.org/protobuf/types/known/wrapperspb"
 )
 
+type VirtualHostOption func(*route.VirtualHost)
+
 // NewVirtualHost creates a new VirtualHost.
-func NewVirtualHost(name string, domains []string, routes []*route.Route) *route.VirtualHost {
-	return &route.VirtualHost{
+func NewVirtualHost(name string, domains []string, routes []*route.Route, options ...VirtualHostOption) *route.VirtualHost {
+	virtualHost := &route.VirtualHost{
 		Name:    name,
 		Domains: domains,
 		Routes:  routes,
 	}
+
+	for _, opt := range options {
+		opt(virtualHost)
+	}
+
+	return virtualHost
 }
 
-// NewVirtualHostWithExtAuthz creates a new VirtualHost with ExtAuthz settings.
-func NewVirtualHostWithExtAuthz(
-	name string,
-	contextExtensions map[string]string,
-	domains []string,
-	routes []*route.Route) *route.VirtualHost {
-
-	filter, _ := anypb.New(&extAuthService.ExtAuthzPerRoute{
-		Override: &extAuthService.ExtAuthzPerRoute_CheckSettings{
-			CheckSettings: &extAuthService.CheckSettings{
-				ContextExtensions: contextExtensions,
+func WithExtAuthz(contextExtensions map[string]string) VirtualHostOption {
+	return func(virtualHost *route.VirtualHost) {
+		filter, _ := anypb.New(&extAuthService.ExtAuthzPerRoute{
+			Override: &extAuthService.ExtAuthzPerRoute_CheckSettings{
+				CheckSettings: &extAuthService.CheckSettings{
+					ContextExtensions: contextExtensions,
+				},
 			},
-		},
-	})
+		})
 
-	return &route.VirtualHost{
-		Name:    name,
-		Domains: domains,
-		Routes:  routes,
-		TypedPerFilterConfig: map[string]*anypb.Any{
+		virtualHost.TypedPerFilterConfig = map[string]*anypb.Any{
 			wellknown.HTTPExternalAuthorization: filter,
-		},
+		}
 	}
+}
 
+func WithRetryOnTransientUpstreamFailure() VirtualHostOption {
+	return func(virtualHost *route.VirtualHost) {
+		virtualHost.RetryPolicy = &route.RetryPolicy{
+			RetryOn:    "reset,connect-failure",
+			NumRetries: wrapperspb.UInt32(1),
+		}
+	}
 }

pkg/envoy/api/virtual_host_test.go

@@ -22,6 +22,7 @@ import (
 	route "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
 	"github.com/envoyproxy/go-control-plane/pkg/wellknown"
 	"google.golang.org/protobuf/testing/protocmp"
+	"google.golang.org/protobuf/types/known/wrapperspb"
 	"gotest.tools/v3/assert"
 )
 
@@ -45,7 +46,7 @@ func TestVirtualHostWithExtAuthz(t *testing.T) {
 	domains := []string{"foo", "bar"}
 	routes := []*route.Route{{Name: "baz"}}
 
-	got := NewVirtualHostWithExtAuthz(name, nil, domains, routes)
+	got := NewVirtualHost(name, domains, routes, WithExtAuthz(nil))
 	want := &route.VirtualHost{
 		Name:    name,
 		Domains: domains,
@@ -57,3 +58,25 @@ func TestVirtualHostWithExtAuthz(t *testing.T) {
 	assert.DeepEqual(t, got.Routes, want.Routes, protocmp.Transform())
 	assert.Assert(t, got.TypedPerFilterConfig[wellknown.HTTPExternalAuthorization] != nil)
 }
+
+func TestVirtualHostWithRetryOnTransientUpstreamFailure(t *testing.T) {
+	name := "test"
+	domains := []string{"foo", "bar"}
+	routes := []*route.Route{{Name: "baz"}}
+
+	got := NewVirtualHost(name, domains, routes, WithRetryOnTransientUpstreamFailure())
+	want := &route.VirtualHost{
+		Name:    name,
+		Domains: domains,
+		Routes:  routes,
+		RetryPolicy: &route.RetryPolicy{
+			RetryOn:    "reset,connect-failure",
+			NumRetries: wrapperspb.UInt32(1),
+		},
+	}
+
+	assert.Equal(t, got.Name, want.Name)
+	assert.DeepEqual(t, got.Domains, want.Domains)
+	assert.DeepEqual(t, got.Routes, want.Routes, protocmp.Transform())
+	assert.DeepEqual(t, got.RetryPolicy, want.RetryPolicy, protocmp.Transform())
+}

pkg/generator/ingress_translator.go

@@ -255,20 +255,22 @@ func (translator *IngressTranslator) translateIngress(ctx context.Context, ingre
 		}
 
 		var virtualHost, virtualTLSHost *route.VirtualHost
+		// TODO(norbjd): do we want to enable this by default?
+		virtualHostOptions := []envoy.VirtualHostOption{
+			envoy.WithRetryOnTransientUpstreamFailure(),
+		}
+
 		if extAuthzEnabled {
 			contextExtensions := kmeta.UnionMaps(map[string]string{
 				"client":     "kourier",
 				"visibility": string(rule.Visibility),
 			}, ingress.GetLabels())
-			virtualHost = envoy.NewVirtualHostWithExtAuthz(ruleName, contextExtensions, domainsForRule(rule), routes)
-			if len(tlsRoutes) != 0 {
-				virtualTLSHost = envoy.NewVirtualHostWithExtAuthz(ruleName, contextExtensions, domainsForRule(rule), tlsRoutes)
-			}
-		} else {
-			virtualHost = envoy.NewVirtualHost(ruleName, domainsForRule(rule), routes)
-			if len(tlsRoutes) != 0 {
-				virtualTLSHost = envoy.NewVirtualHost(ruleName, domainsForRule(rule), tlsRoutes)
-			}
+			virtualHostOptions = append(virtualHostOptions, envoy.WithExtAuthz(contextExtensions))
+		}
+
+		virtualHost = envoy.NewVirtualHost(ruleName, domainsForRule(rule), routes, virtualHostOptions...)
+		if len(tlsRoutes) != 0 {
+			virtualTLSHost = envoy.NewVirtualHost(ruleName, domainsForRule(rule), tlsRoutes, virtualHostOptions...)
 		}
 
 		localHosts = append(localHosts, virtualHost)

pkg/generator/ingress_translator_test.go

@@ -87,6 +87,7 @@ func TestIngressTranslator(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"rewritten.example.com"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -152,6 +153,7 @@ func TestIngressTranslator(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"rewritten.example.com"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -227,6 +229,7 @@ func TestIngressTranslator(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"rewritten.example.com"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -301,6 +304,7 @@ func TestIngressTranslator(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"rewritten.example.com"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 			vHostsRedirect := []*route.VirtualHost{
@@ -321,6 +325,7 @@ func TestIngressTranslator(t *testing.T) {
 						}},
 						"/test"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -397,6 +402,7 @@ func TestIngressTranslator(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"rewritten.example.com"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -520,6 +526,7 @@ func TestIngressTranslator(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"rewritten.example.com"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -596,6 +603,7 @@ func TestIngressTranslator(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"rewritten.example.com"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -656,6 +664,7 @@ func TestIngressTranslator(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"rewritten.example.com"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -717,6 +726,7 @@ func TestIngressTranslator(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"rewritten.example.com"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -854,6 +864,7 @@ func TestIngressTranslatorWithHTTPOptionDisabled(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"rewritten.example.com"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 			return &translatedIngress{
@@ -929,6 +940,7 @@ func TestIngressTranslatorWithHTTPOptionDisabled(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"rewritten.example.com"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -1041,6 +1053,7 @@ func TestIngressTranslatorWithUpstreamTLS(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						""),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -1115,6 +1128,7 @@ func TestIngressTranslatorWithUpstreamTLS(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						""),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -1190,6 +1204,7 @@ func TestIngressTranslatorWithUpstreamTLS(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						""),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -1265,6 +1280,7 @@ func TestIngressTranslatorWithUpstreamTLS(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						""),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -1331,6 +1347,7 @@ func TestIngressTranslatorWithUpstreamTLS(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						""),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -1401,6 +1418,7 @@ func TestIngressTranslatorWithUpstreamTLS(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						""),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}
 
@@ -1525,9 +1543,8 @@ func TestIngressTranslatorHTTP01Challenge(t *testing.T) {
 		},
 		want: func() *translatedIngress {
 			vHosts := []*route.VirtualHost{
-				envoy.NewVirtualHostWithExtAuthz(
+				envoy.NewVirtualHost(
 					"(simplens/simplename).Rules[0]",
-					map[string]string{"client": "kourier", "visibility": "ExternalIP"},
 					[]string{"foo.example.com", "foo.example.com:*"},
 					[]*route.Route{envoy.NewRouteExtAuthzDisabled(
 						"(simplens/simplename).Rules[0].Paths[/.well-known/acme-challenge/-VwB1vAXWaN6mVl3-6JVFTEvf7acguaFDUxsP9UzRkE]",
@@ -1540,6 +1557,8 @@ func TestIngressTranslatorHTTP01Challenge(t *testing.T) {
 						nil,
 						""),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
+					envoy.WithExtAuthz(map[string]string{"client": "kourier", "visibility": "ExternalIP"}),
 				),
 			}
 
@@ -1652,6 +1671,7 @@ func TestIngressTranslatorDomainMappingDisableHTTP2(t *testing.T) {
 						map[string]string{"foo": "bar"},
 						"bar.default.svc.cluster.local"),
 					},
+					envoy.WithRetryOnTransientUpstreamFailure(),
 				),
 			}