Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instrument the HAProxy router with metrics that contain route info #13337

Merged
merged 9 commits into from
Apr 2, 2017
6 changes: 6 additions & 0 deletions contrib/completions/bash/openshift
Original file line number Diff line number Diff line change
Expand Up @@ -22266,6 +22266,8 @@ _openshift_infra_f5-router()
local_nonpersistent_flags+=("--kubernetes=")
flags+=("--labels=")
local_nonpersistent_flags+=("--labels=")
flags+=("--listen-addr=")
local_nonpersistent_flags+=("--listen-addr=")
flags+=("--master=")
local_nonpersistent_flags+=("--master=")
flags+=("--name=")
Expand Down Expand Up @@ -22437,10 +22439,14 @@ _openshift_infra_router()
local_nonpersistent_flags+=("--kubernetes=")
flags+=("--labels=")
local_nonpersistent_flags+=("--labels=")
flags+=("--listen-addr=")
local_nonpersistent_flags+=("--listen-addr=")
flags+=("--master=")
local_nonpersistent_flags+=("--master=")
flags+=("--max-connections=")
local_nonpersistent_flags+=("--max-connections=")
flags+=("--metrics-type=")
local_nonpersistent_flags+=("--metrics-type=")
flags+=("--name=")
local_nonpersistent_flags+=("--name=")
flags+=("--namespace=")
Expand Down
6 changes: 6 additions & 0 deletions contrib/completions/zsh/openshift
Original file line number Diff line number Diff line change
Expand Up @@ -22414,6 +22414,8 @@ _openshift_infra_f5-router()
local_nonpersistent_flags+=("--kubernetes=")
flags+=("--labels=")
local_nonpersistent_flags+=("--labels=")
flags+=("--listen-addr=")
local_nonpersistent_flags+=("--listen-addr=")
flags+=("--master=")
local_nonpersistent_flags+=("--master=")
flags+=("--name=")
Expand Down Expand Up @@ -22585,10 +22587,14 @@ _openshift_infra_router()
local_nonpersistent_flags+=("--kubernetes=")
flags+=("--labels=")
local_nonpersistent_flags+=("--labels=")
flags+=("--listen-addr=")
local_nonpersistent_flags+=("--listen-addr=")
flags+=("--master=")
local_nonpersistent_flags+=("--master=")
flags+=("--max-connections=")
local_nonpersistent_flags+=("--max-connections=")
flags+=("--metrics-type=")
local_nonpersistent_flags+=("--metrics-type=")
flags+=("--name=")
local_nonpersistent_flags+=("--name=")
flags+=("--namespace=")
Expand Down
68 changes: 34 additions & 34 deletions images/router/haproxy/conf/haproxy-config.template
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ frontend public
# Search from most specific to general path (host case).
# Note: If no match, haproxy uses the default_backend, no other
# use_backend directives below this will be processed.
use_backend be_http_%[base,map_reg(/var/lib/haproxy/conf/os_http_be.map)]
use_backend be_http:%[base,map_reg(/var/lib/haproxy/conf/os_http_be.map)]

default_backend openshift_default

Expand All @@ -142,7 +142,7 @@ frontend public_ssl
# if the connection is SNI and the route is a passthrough don't use the termination backend, just use the tcp backend
acl sni req.ssl_sni -m found
acl sni_passthrough req.ssl_sni,map_reg(/var/lib/haproxy/conf/os_sni_passthrough.map) -m found
use_backend be_tcp_%[req.ssl_sni,map_reg(/var/lib/haproxy/conf/os_tcp_be.map)] if sni sni_passthrough
use_backend be_tcp:%[req.ssl_sni,map_reg(/var/lib/haproxy/conf/os_tcp_be.map)] if sni sni_passthrough

# if the route is SNI and NOT passthrough enter the termination flow
use_backend be_sni if sni
Expand Down Expand Up @@ -174,13 +174,13 @@ frontend fe_sni
acl reencrypt base,map_reg(/var/lib/haproxy/conf/os_reencrypt.map) -m found

# Search from most specific to general path (host case).
use_backend be_secure_%[base,map_reg(/var/lib/haproxy/conf/os_reencrypt.map)] if reencrypt
use_backend be_secure:%[base,map_reg(/var/lib/haproxy/conf/os_reencrypt.map)] if reencrypt

# map to http backend
# Search from most specific to general path (host case).
# Note: If no match, haproxy uses the default_backend, no other
# use_backend directives below this will be processed.
use_backend be_edge_http_%[base,map_reg(/var/lib/haproxy/conf/os_edge_http_be.map)]
use_backend be_edge_http:%[base,map_reg(/var/lib/haproxy/conf/os_edge_http_be.map)]

default_backend openshift_default

Expand Down Expand Up @@ -208,13 +208,13 @@ frontend fe_no_sni
acl reencrypt base,map_reg(/var/lib/haproxy/conf/os_reencrypt.map) -m found

# Search from most specific to general path (host case).
use_backend be_secure_%[base,map_reg(/var/lib/haproxy/conf/os_reencrypt.map)] if reencrypt
use_backend be_secure:%[base,map_reg(/var/lib/haproxy/conf/os_reencrypt.map)] if reencrypt

# map to http backend
# Search from most specific to general path (host case).
# Note: If no match, haproxy uses the default_backend, no other
# use_backend directives below this will be processed.
use_backend be_edge_http_%[base,map_reg(/var/lib/haproxy/conf/os_edge_http_be.map)]
use_backend be_edge_http:%[base,map_reg(/var/lib/haproxy/conf/os_edge_http_be.map)]

default_backend openshift_default

Expand All @@ -230,17 +230,17 @@ backend openshift_default

##-------------- app level backends ----------------
{{/*
1. If termination is not set: This is plain http -> http. Create a be_http_<service> backend.
1. If termination is not set: This is plain http -> http. Create a be_http:<service> backend.
Incoming http traffic is terminated and sent as http to the pods.

2. If termination is type 'edge': This is https -> http. Create a be_edge_http_<service> backend.
2. If termination is type 'edge': This is https -> http. Create a be_edge_http:<service> backend.
Incoming https traffic is terminated and sent as http to the pods.

3. If termination is type 'reencrypt': This is https -> https. Create a be_secure_<service> backend.
3. If termination is type 'reencrypt': This is https -> https. Create a be_secure:<service> backend.
Incoming https traffic is terminated and then sent as https to the pods.

4. If termination is type 'passthrough': This is https (or any SNI TLS connection) passthrough.
Create a be_tcp_<service> backend.
Create a be_tcp:<service> backend.
Incoming traffic is inspected to get the hostname from the SNI header, but then all traffic is
passed through to the backend pod by just looking at the TCP headers.
*/}}
Expand All @@ -249,11 +249,11 @@ backend openshift_default
{{- if (eq $cfg.TLSTermination "") }}

# Plain http backend
backend be_http_{{$cfgIdx}}
backend be_http:{{$cfgIdx}}
{{- else }}

# Plain http backend but request is TLS, terminated at edge
backend be_edge_http_{{$cfgIdx}}
backend be_edge_http:{{$cfgIdx}}
{{- end }}
mode http
option redispatch
Expand Down Expand Up @@ -315,19 +315,19 @@ backend be_edge_http_{{$cfgIdx}}
{{- with $serviceUnit := index $.ServiceUnits $serviceUnitName }}
{{- range $idx, $endpoint := endpointsForAlias $cfg $serviceUnit }}
{{- if $endpoint.NoHealthCheck }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} cookie {{$endpoint.IdHash}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} cookie {{$endpoint.IdHash}} weight {{$weight}}
{{- else }}
{{- with $healthIntv := index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval" }}
{{- if (matchPattern "[1-9][0-9]*(us|ms|s|m|h|d)?" $healthIntv) }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter {{$healthIntv}} cookie {{$endpoint.IdHash}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter {{$healthIntv}} cookie {{$endpoint.IdHash}} weight {{$weight}}
{{- else }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter 5000ms cookie {{$endpoint.IdHash}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter 5000ms cookie {{$endpoint.IdHash}} weight {{$weight}}
{{- end }}
{{- else }}
{{- if (matchPattern "[1-9][0-9]*(us|ms|s|m|h|d)?" (env "ROUTER_BACKEND_CHECK_INTERVAL" "")) }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter {{env "ROUTER_BACKEND_CHECK_INTERVAL" "5000ms"}} cookie {{$endpoint.IdHash}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter {{env "ROUTER_BACKEND_CHECK_INTERVAL" "5000ms"}} cookie {{$endpoint.IdHash}} weight {{$weight}}
{{- else }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter 5000ms cookie {{$endpoint.IdHash}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter 5000ms cookie {{$endpoint.IdHash}} weight {{$weight}}
{{- end }}
{{- end }}
{{- end }}
Expand All @@ -340,7 +340,7 @@ backend be_edge_http_{{$cfgIdx}}
{{- if eq $cfg.TLSTermination "passthrough" }}

# Secure backend, pass through
backend be_tcp_{{$cfgIdx}}
backend be_tcp:{{$cfgIdx}}
{{- if ne (env "ROUTER_SYSLOG_ADDRESS" "") ""}}
option tcplog
{{- end }}
Expand Down Expand Up @@ -384,19 +384,19 @@ backend be_tcp_{{$cfgIdx}}
{{- with $serviceUnit := index $.ServiceUnits $serviceUnitName }}
{{- range $idx, $endpoint := endpointsForAlias $cfg $serviceUnit }}
{{- if $endpoint.NoHealthCheck }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} weight {{$weight}}
{{- else }}
{{- with $healthIntv := index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval" }}
{{- if (matchPattern "[1-9][0-9]*(us|ms|s|m|h|d)?" $healthIntv) }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter {{$healthIntv}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter {{$healthIntv}} weight {{$weight}}
{{- else }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter 5000ms weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter 5000ms weight {{$weight}}
{{- end }}
{{- else }}
{{- if (matchPattern "[1-9][0-9]*(us|ms|s|m|h|d)?" (env "ROUTER_BACKEND_CHECK_INTERVAL" "")) }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter {{env "ROUTER_BACKEND_CHECK_INTERVAL" "5000ms"}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter {{env "ROUTER_BACKEND_CHECK_INTERVAL" "5000ms"}} weight {{$weight}}
{{- else }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter 5000ms weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter 5000ms weight {{$weight}}
{{- end }}
{{- end }}
{{- end }}
Expand All @@ -409,7 +409,7 @@ backend be_tcp_{{$cfgIdx}}
{{- if eq $cfg.TLSTermination "reencrypt" }}

# Secure backend which requires re-encryption
backend be_secure_{{$cfgIdx}}
backend be_secure:{{$cfgIdx}}
mode http
option redispatch
{{- with $balanceAlgo := index $cfg.Annotations "haproxy.router.openshift.io/balance" }}
Expand Down Expand Up @@ -469,19 +469,19 @@ backend be_secure_{{$cfgIdx}}
{{- with $serviceUnit := index $.ServiceUnits $serviceUnitName }}
{{- range $idx, $endpoint := endpointsForAlias $cfg $serviceUnit }}
{{- if $endpoint.NoHealthCheck }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl verify required ca-file {{ $workingDir }}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl verify required ca-file {{ $workingDir }}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
{{- else }}
{{- with $healthIntv := index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval" }}
{{- if (matchPattern "[1-9][0-9]*(us|ms|s|m|h|d)?" $healthIntv) }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter {{$healthIntv}} verify required ca-file {{ $workingDir }}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter {{$healthIntv}} verify required ca-file {{ $workingDir }}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
{{- else }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter 5000ms verify required ca-file {{$workingDir}}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter 5000ms verify required ca-file {{$workingDir}}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
{{- end }}
{{- else }}
{{- if (matchPattern "[1-9][0-9]*(us|ms|s|m|h|d)?" (env "ROUTER_BACKEND_CHECK_INTERVAL" "")) }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter {{env "ROUTER_BACKEND_CHECK_INTERVAL" "5000ms"}} verify required ca-file {{$workingDir}}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter {{env "ROUTER_BACKEND_CHECK_INTERVAL" "5000ms"}} verify required ca-file {{$workingDir}}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
{{- else }}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter 5000ms verify required ca-file {{$workingDir}}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter 5000ms verify required ca-file {{$workingDir}}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
{{- end }}
{{- end }}{{/* end get health interval annotation */}}
{{- end }}{{/* end else no health check */}}
Expand Down Expand Up @@ -516,7 +516,7 @@ backend be_secure_{{$cfgIdx}}

{{/*
os_http_be.map: contains a mapping of www.example.com -> <service name>. This map is used to discover the correct backend
by attaching a prefix (be_http_) by use_backend statements if acls are matched.
by attaching a prefix (be_http:) by use_backend statements if acls are matched.
*/}}
{{ define "/var/lib/haproxy/conf/os_http_be.map" -}}
{{ range $idx, $cfg := .State -}}
Expand Down Expand Up @@ -547,9 +547,9 @@ backend be_secure_{{$cfgIdx}}
{{ range $idx, $cfg := .State -}}
{{ if and (ne $cfg.Host "") (and (or (eq $cfg.TLSTermination "edge") (eq $cfg.TLSTermination "reencrypt")) (eq $cfg.InsecureEdgeTerminationPolicy "Allow")) -}}
{{ if (eq $cfg.TLSTermination "edge") -}}
{{generateRouteRegexp $cfg.Host $cfg.Path $cfg.IsWildcard}} be_edge_http_{{$idx}}
{{generateRouteRegexp $cfg.Host $cfg.Path $cfg.IsWildcard}} be_edge_http:{{$idx}}
{{ else -}}
{{generateRouteRegexp $cfg.Host $cfg.Path $cfg.IsWildcard}} be_secure_{{$idx}}
{{generateRouteRegexp $cfg.Host $cfg.Path $cfg.IsWildcard}} be_secure:{{$idx}}
{{ end -}}
{{ end -}}
{{ end -}}
Expand All @@ -571,7 +571,7 @@ backend be_secure_{{$cfgIdx}}

{{/*
os_tcp_be.map: contains a mapping of www.example.com -> <service name>. This map is used to discover the correct backend
by attaching a prefix (be_tcp_ or be_secure_) by use_backend statements if acls are matched.
by attaching a prefix (be_tcp: or be_secure:) by use_backend statements if acls are matched.
*/}}
{{ define "/var/lib/haproxy/conf/os_tcp_be.map" -}}
{{ range $idx, $cfg := .State -}}
Expand Down Expand Up @@ -624,4 +624,4 @@ backend be_secure_{{$cfgIdx}}
{{ end -}}
{{ end -}}
{{ end -}}
{{ end -}}{{/* end cert_config map template */}}
{{ end }}{{/* end cert_config map template */}}
10 changes: 10 additions & 0 deletions pkg/cmd/admin/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,16 @@ func RunCmdRouter(f *clientcmd.Factory, cmd *cobra.Command, out, errout io.Write
}
env["ROUTER_CANONICAL_HOSTNAME"] = cfg.RouterCanonicalHostname
}
// automatically start the internal metrics agent if we are handling a known type
if cfg.Type == "haproxy-router" {
env["ROUTER_LISTEN_ADDR"] = fmt.Sprintf("0.0.0.0:%d", defaultStatsPort-1)
env["ROUTER_METRICS_TYPE"] = "haproxy"
ports = append(ports, kapi.ContainerPort{
Name: "router-stats",
ContainerPort: int32(defaultStatsPort - 1),
Protocol: kapi.ProtocolTCP,
})
}
env.Add(secretEnv)
if len(defaultCert) > 0 {
if cfg.SecretsAsEnv {
Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/infra/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ type RouterSelection struct {
DisableNamespaceOwnershipCheck bool

EnableIngress bool

ListenAddr string
}

// Bind sets the appropriate labels
Expand All @@ -73,6 +75,7 @@ func (o *RouterSelection) Bind(flag *pflag.FlagSet) {
flag.BoolVar(&o.AllowWildcardRoutes, "allow-wildcard-routes", cmdutil.Env("ROUTER_ALLOW_WILDCARD_ROUTES", "") == "true", "Allow wildcard host names for routes")
flag.BoolVar(&o.DisableNamespaceOwnershipCheck, "disable-namespace-ownership-check", cmdutil.Env("ROUTER_DISABLE_NAMESPACE_OWNERSHIP_CHECK", "") == "true", "Disables the namespace ownership checks for a route host with different paths or for overlapping host names in the case of wildcard routes. Please be aware that if namespace ownership checks are disabled, routes in a different namespace can use this mechanism to 'steal' sub-paths for existing domains. This is only safe if route creation privileges are restricted, or if all the users can be trusted.")
flag.BoolVar(&o.EnableIngress, "enable-ingress", cmdutil.Env("ROUTER_ENABLE_INGRESS", "") == "true", "Enable configuration via ingress resources")
flag.StringVar(&o.ListenAddr, "listen-addr", cmdutil.Env("ROUTER_LISTEN_ADDR", ""), "The name of an interface to listen on to expose metrics and health checking. If not specified, will not listen.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: --listen-addr is a bit generic-sounding -- shouldn't this be something like --metrics-listen-addr?

Copy link
Contributor Author

@smarterclayton smarterclayton Mar 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure - it's also the health check and debugging ports. This is consistent with what we do with the controller and kubelet, and wasn't inclined to be too different. Technically, any future API we expose from the routers themselves would be on this address, so it is the generic listen address on the process.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's fair, I suppose

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Listen address seem a little general. How about metrics-listen-address? saw the above discussion. This needs to distinguish among the other listening ports. How about management-listening port? Or some such.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the previous discussion between me and solly about why

}

// RouteSelectionFunc returns a func that identifies the host for a route.
Expand Down
Loading