diff --git a/images/router/haproxy/conf/haproxy-config.template b/images/router/haproxy/conf/haproxy-config.template index 0f6063e67..1e9f29cb8 100644 --- a/images/router/haproxy/conf/haproxy-config.template +++ b/images/router/haproxy/conf/haproxy-config.template @@ -749,7 +749,7 @@ backend {{ genBackendNamePrefix $cfg.TLSTermination }}:{{ $cfgIdx }} {{- end }} {{- end }}{{/* end type specific options*/}} - {{- if and (not $endpoint.NoHealthCheck) (gt $cfg.ActiveEndpoints 1) }} check inter {{firstMatch $timeSpecPattern (index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval") (env "ROUTER_BACKEND_CHECK_INTERVAL") "5000ms" }} + {{- if and (not $endpoint.NoHealthCheck) (gt $cfg.ActiveEndpoints 1) }} check inter {{ clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval") (env "ROUTER_BACKEND_CHECK_INTERVAL") "5000ms") }} {{- end }}{{/* end else no health check */}} {{- with $podMaxConn := index $cfg.Annotations "haproxy.router.openshift.io/pod-concurrent-connections" }} {{- if (isInteger (index $cfg.Annotations "haproxy.router.openshift.io/pod-concurrent-connections")) }} maxconn {{$podMaxConn }} {{- end }} @@ -764,7 +764,7 @@ backend {{ genBackendNamePrefix $cfg.TLSTermination }}:{{ $cfgIdx }} {{- if (eq $cfg.TLSTermination "reencrypt") }} dynamic-cookie-key {{ $cfg.RoutingKeyName }} {{- range $idx, $serverName := $dynamicConfigManager.GenerateDynamicServerNames $cfgIdx }} - server {{ $serverName }} 172.4.0.4:8765 weight 0 ssl disabled check inter {{ firstMatch $timeSpecPattern (index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval") (env "ROUTER_BACKEND_CHECK_INTERVAL") "5000ms" }} + server {{ $serverName }} 172.4.0.4:8765 weight 0 ssl disabled check inter {{ clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval") (env "ROUTER_BACKEND_CHECK_INTERVAL") "5000ms") }} {{- if gt (len (index $cfg.Certificates (printf "%s_pod" $cfg.Host)).Contents) 0 }} verify required ca-file {{ $workingDir }}/router/cacerts/{{$cfgIdx }}.pem {{- else }} {{- if not (isTrue $router_disable_http2) }} alpn h2,http/1.1 @@ -845,7 +845,7 @@ backend {{ genBackendNamePrefix $cfg.TLSTermination }}:{{ $cfgIdx }} {{- with $serviceUnit := index $.ServiceUnits $serviceUnitName }} {{- range $idx, $endpoint := processEndpointsForAlias $cfg $serviceUnit (env "ROUTER_BACKEND_PROCESS_ENDPOINTS" "") }} server {{ $endpoint.ID }} {{ $endpoint.IP }}:{{ $endpoint.Port }} weight {{ $weight }} - {{- if and (not $endpoint.NoHealthCheck) (gt $cfg.ActiveEndpoints 1) }} check inter {{firstMatch $timeSpecPattern (index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval") (env "ROUTER_BACKEND_CHECK_INTERVAL") "5000ms" }} + {{- if and (not $endpoint.NoHealthCheck) (gt $cfg.ActiveEndpoints 1) }} check inter {{ clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval") (env "ROUTER_BACKEND_CHECK_INTERVAL") "5000ms") }} {{- end }}{{/* end else no health check */}} {{- with $podMaxConn := index $cfg.Annotations "haproxy.router.openshift.io/pod-concurrent-connections" }} {{- if (isInteger (index $cfg.Annotations "haproxy.router.openshift.io/pod-concurrent-connections")) }} maxconn {{$podMaxConn }} {{- end }} diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index 889a1289e..5821d75eb 100644 --- a/pkg/router/router_test.go +++ b/pkg/router/router_test.go @@ -802,6 +802,115 @@ func TestConfigTemplate(t *testing.T) { }, }, }, + "valid route health check interval annotation": { + mustCreateWithConfig{ + mustCreateEndpointSlices: []mustCreateEndpointSlice{ + { + name: "servicer1", + serviceName: "servicer1", + addresses: []string{"1.1.1.1", "1.1.1.2"}, + }, + }, + mustCreateRoute: mustCreateRoute{ + name: "r1", + host: "r1example.com", + targetServiceName: "servicer1", + weight: 1, + time: start, + annotations: map[string]string{ + "router.openshift.io/haproxy.health.check.interval": "10s", + }, + }, + mustMatchConfig: mustMatchConfig{ + section: "backend", + sectionName: insecureBackendName(h.namespace, "r1"), + attribute: "server", + value: "inter 10s", + }, + }, + }, + "route health check interval annotation exceeds the haproxy maximum": { + mustCreateWithConfig{ + mustCreateEndpointSlices: []mustCreateEndpointSlice{ + { + name: "servicer2", + serviceName: "servicer2", + addresses: []string{"1.1.1.1", "1.1.1.2"}, + }, + }, + mustCreateRoute: mustCreateRoute{ + name: "r2", + host: "r2example.com", + targetServiceName: "servicer2", + weight: 1, + time: start, + annotations: map[string]string{ + "router.openshift.io/haproxy.health.check.interval": "5000d", + }, + }, + mustMatchConfig: mustMatchConfig{ + section: "backend", + sectionName: insecureBackendName(h.namespace, "r2"), + attribute: "server", + value: "inter 2147483647ms", + }, + }, + }, + "invalid route health check interval annotation": { + mustCreateWithConfig{ + mustCreateEndpointSlices: []mustCreateEndpointSlice{ + { + name: "servicer3", + serviceName: "servicer3", + addresses: []string{"1.1.1.1", "1.1.1.2"}, + }, + }, + mustCreateRoute: mustCreateRoute{ + name: "r3", + host: "r3example.com", + targetServiceName: "servicer3", + weight: 1, + time: start, + annotations: map[string]string{ + "router.openshift.io/haproxy.health.check.interval": "abc", + }, + }, + mustMatchConfig: mustMatchConfig{ + section: "backend", + sectionName: insecureBackendName(h.namespace, "r3"), + attribute: "server", + value: "inter 5000ms", + }, + }, + }, + "passthrough route health check interval annotation exceeds the haproxy maximum": { + mustCreateWithConfig{ + mustCreateEndpointSlices: []mustCreateEndpointSlice{ + { + name: "servicer4", + serviceName: "servicer4", + addresses: []string{"1.1.1.1", "1.1.1.2"}, + }, + }, + mustCreateRoute: mustCreateRoute{ + name: "r4", + host: "r4example.com", + targetServiceName: "servicer4", + weight: 1, + time: start, + annotations: map[string]string{ + "router.openshift.io/haproxy.health.check.interval": "9000d", + }, + tlsTermination: routev1.TLSTerminationPassthrough, + }, + mustMatchConfig: mustMatchConfig{ + section: "backend", + sectionName: passthroughBackendName(h.namespace, "r4"), + attribute: "server", + value: "inter 2147483647ms", + }, + }, + }, } defer cleanUpRoutes(t) @@ -963,6 +1072,8 @@ type mustCreateEndpointSlice struct { serviceName string // appProtocol is the appProtocol of the endpointslice. appProtocol string + // addresses is the addresses field of the endpoint + addresses []string } func (e mustCreateEndpointSlice) Apply(h *harness) error { @@ -973,6 +1084,10 @@ func (e mustCreateEndpointSlice) Apply(h *harness) error { if e.appProtocol != "" { appProtocol = &e.appProtocol } + addresses := []string{"1.1.1.1"} + if len(e.addresses) > 0 { + addresses = e.addresses + } ep := &discoveryv1.EndpointSlice{ ObjectMeta: metav1.ObjectMeta{ Namespace: h.namespace, @@ -983,7 +1098,7 @@ func (e mustCreateEndpointSlice) Apply(h *harness) error { UID: h.nextUID(), }, Endpoints: []discoveryv1.Endpoint{{ - Addresses: []string{"1.1.1.1"}, + Addresses: addresses, }}, Ports: []discoveryv1.EndpointPort{{ AppProtocol: appProtocol, @@ -1246,3 +1361,8 @@ func insecureBackendName(ns, route string) string { func reencryptBackendName(ns, route string) string { return "be_secure:" + ns + ":" + route } + +// passthroughBackendName contructs the HAProxy config's backend name for a passthrough route. +func passthroughBackendName(ns, route string) string { + return "be_tcp:" + ns + ":" + route +} diff --git a/pkg/router/template/template_helper.go b/pkg/router/template/template_helper.go index 153e2b82c..df0024b91 100644 --- a/pkg/router/template/template_helper.go +++ b/pkg/router/template/template_helper.go @@ -320,14 +320,14 @@ func generateHAProxyMap(name string, td templateData) []string { } // clipHAProxyTimeoutValue prevents the HAProxy config file -// from using timeout values specified via the haproxy.router.openshift.io/timeout -// annotation that exceed the maximum value allowed by HAProxy, or by +// from using time values specified via the annotations +// that exceed the maximum value allowed by HAProxy, or by // haproxytime.ParseDuration. // // Return the empty string instead of an error in the event that a -// timeout string value is not parsable as a valid time duration. -// Return the largest HAProxy timeout if the input value exceeds it. -// Return the default timeout (5s) if there is another error. +// time string value is not parsable as a valid time duration. +// Return the largest HAProxy time if the input value exceeds it. +// Return the default time (5s) if there is another error. func clipHAProxyTimeoutValue(val string) string { // If the empty string is passed in, // simply return the empty string. @@ -340,20 +340,20 @@ func clipHAProxyTimeoutValue(val string) string { switch err { case nil: case haproxytime.OverflowError: - log.Info("route annotation timeout exceeds maximum allowable format, clipping to "+templateutil.HaproxyMaxTimeout, "input", val) + log.Info("route annotation time value exceeds maximum allowable format, clipping to "+templateutil.HaproxyMaxTimeout, "input", val) return templateutil.HaproxyMaxTimeout case haproxytime.SyntaxError: - log.Error(err, "route annotation timeout ignored because value is invalid", "input", val) + log.Error(err, "route annotation time value ignored or defaulted because value is invalid", "input", val) return "" default: // This is not used at the moment - log.Info("invalid route annotation timeout, setting to "+templateutil.HaproxyDefaultTimeout, "input", val) + log.Info("invalid route annotation time value, setting to "+templateutil.HaproxyDefaultTimeout, "input", val) return templateutil.HaproxyDefaultTimeout } - // Then check to see if the timeout is larger than what HAProxy allows + // Then check to see if the time is larger than what HAProxy allows if duration > templateutil.HaproxyMaxTimeoutDuration { - log.Info("route annotation timeout exceeds maximum allowable by HAProxy, clipping to "+templateutil.HaproxyMaxTimeout, "input", val) + log.Info("route annotation time value exceeds maximum allowable by HAProxy, clipping to "+templateutil.HaproxyMaxTimeout, "input", val) return templateutil.HaproxyMaxTimeout }