From d06573de6c4cb9f0f19e02b8c31d5c56eadfe0d8 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Thu, 15 Sep 2022 11:00:09 +0200 Subject: [PATCH 01/17] plugins: allow empty config --- pkg/plugins/middlewares.go | 3 +++ pkg/server/middleware/plugins.go | 5 ----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/plugins/middlewares.go b/pkg/plugins/middlewares.go index 09edbc9543..e46dbae344 100644 --- a/pkg/plugins/middlewares.go +++ b/pkg/plugins/middlewares.go @@ -84,6 +84,9 @@ func (p middlewareBuilder) createConfig(config map[string]interface{}) (reflect. } vConfig := results[0] + if len(config) == 0 { + return vConfig, nil + } cfg := &mapstructure.DecoderConfig{ DecodeHook: mapstructure.StringToSliceHookFunc(","), diff --git a/pkg/server/middleware/plugins.go b/pkg/server/middleware/plugins.go index a401791514..3a092f1408 100644 --- a/pkg/server/middleware/plugins.go +++ b/pkg/server/middleware/plugins.go @@ -2,7 +2,6 @@ package middleware import ( "errors" - "fmt" "github.com/traefik/traefik/v2/pkg/config/dynamic" "github.com/traefik/traefik/v2/pkg/plugins" @@ -30,9 +29,5 @@ func findPluginConfig(rawConfig map[string]dynamic.PluginConf) (string, map[stri return "", nil, errors.New("missing plugin type") } - if len(rawPluginConfig) == 0 { - return "", nil, fmt.Errorf("missing plugin configuration: %s", pluginType) - } - return pluginType, rawPluginConfig, nil } From 1c9a7b8c618f012ea66dc3ae6a6e71defec3eb4e Mon Sep 17 00:00:00 2001 From: Romain Date: Fri, 16 Sep 2022 09:54:09 +0200 Subject: [PATCH 02/17] Add documentation for json schema usage to validate config in the FAQ Co-authored-by: Kevin Pollet --- docs/content/getting-started/faq.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/content/getting-started/faq.md b/docs/content/getting-started/faq.md index 0875044525..01b33f6398 100644 --- a/docs/content/getting-started/faq.md +++ b/docs/content/getting-started/faq.md @@ -157,3 +157,27 @@ By default, the following headers are automatically added when proxying requests For more details, please check out the [forwarded header](../routing/entrypoints.md#forwarded-headers) documentation. + +## What does the "field not found" error mean? + +```shell +error: field not found, node: -badField- +``` + +The "field not found" error occurs, when an unknown property is encountered in the dynamic or static configuration. + +One easy way to check whether a configuration file is well-formed, is to validate it with: + +- [JSON Schema of the static configuration](https://json.schemastore.org/traefik-v2.json) +- [JSON Schema of the dynamic configuration](https://json.schemastore.org/traefik-v2-file-provider.json) + +## Why are some resources (routers, middlewares, services...) not created/applied? + +As a common tip, if a resource is dropped/not created by Traefik after the dynamic configuration was evaluated, +one should look for an error in the logs. + +If found, the error obviously confirms that something went wrong while creating the resource, +and the message should help in figuring out the mistake(s) in the configuration, and how to fix it. + +When using the file provider, +one easy way to check if the dynamic configuration is well-formed is to validate it with the [JSON Schema of the dynamic configuration](https://json.schemastore.org/traefik-v2-file-provider.json). From a4b447256be0bb2f77b38c7aa61696af5812524d Mon Sep 17 00:00:00 2001 From: NEwa-05 Date: Fri, 16 Sep 2022 12:16:09 +0200 Subject: [PATCH 03/17] Add a note on case insensitive regex matching --- docs/content/routing/routers/index.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/docs/content/routing/routers/index.md b/docs/content/routing/routers/index.md index 5e80ef04b2..535a0e6381 100644 --- a/docs/content/routing/routers/index.md +++ b/docs/content/routing/routers/index.md @@ -233,18 +233,18 @@ If the rule is verified, the router becomes active, calls middlewares, and then The table below lists all the available matchers: -| Rule | Description | -|------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------| -| ```Headers(`key`, `value`)``` | Check if there is a key `key`defined in the headers, with the value `value` | -| ```HeadersRegexp(`key`, `regexp`)``` | Check if there is a key `key`defined in the headers, with a value that matches the regular expression `regexp` | -| ```Host(`example.com`, ...)``` | Check if the request domain (host header value) targets one of the given `domains`. | -| ```HostHeader(`example.com`, ...)``` | Same as `Host`, only exists for historical reasons. | -| ```HostRegexp(`example.com`, `{subdomain:[a-z]+}.example.com`, ...)``` | Match the request domain. See "Regexp Syntax" below. | -| ```Method(`GET`, ...)``` | Check if the request method is one of the given `methods` (`GET`, `POST`, `PUT`, `DELETE`, `PATCH`, `HEAD`) | -| ```Path(`/path`, `/articles/{cat:[a-z]+}/{id:[0-9]+}`, ...)``` | Match exact request path. See "Regexp Syntax" below. | -| ```PathPrefix(`/products/`, `/articles/{cat:[a-z]+}/{id:[0-9]+}`)``` | Match request prefix path. See "Regexp Syntax" below. | -| ```Query(`foo=bar`, `bar=baz`)``` | Match Query String parameters. It accepts a sequence of key=value pairs. | -| ```ClientIP(`10.0.0.0/16`, `::1`)``` | Match if the request client IP is one of the given IP/CIDR. It accepts IPv4, IPv6 and CIDR formats. | +| Rule | Description | +|--------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------| +| ```Headers(`key`, `value`)``` | Check if there is a key `key`defined in the headers, with the value `value` | +| ```HeadersRegexp(`key`, `regexp`)``` | Check if there is a key `key`defined in the headers, with a value that matches the regular expression `regexp` | +| ```Host(`example.com`, ...)``` | Check if the request domain (host header value) targets one of the given `domains`. | +| ```HostHeader(`example.com`, ...)``` | Same as `Host`, only exists for historical reasons. | +| ```HostRegexp(`example.com`, `{subdomain:[a-z]+}.example.com`, ...)``` | Match the request domain. See "Regexp Syntax" below. | +| ```Method(`GET`, ...)``` | Check if the request method is one of the given `methods` (`GET`, `POST`, `PUT`, `DELETE`, `PATCH`, `HEAD`) | +| ```Path(`/path`, `/articles/{cat:[a-z]+}/{id:[0-9]+}`, ...)``` | Match exact request path. See "Regexp Syntax" below. | +| ```PathPrefix(`/products/`, `/articles/{cat:[a-z]+}/{id:[0-9]+}`)``` | Match request prefix path. See "Regexp Syntax" below. | +| ```Query(`foo=bar`, `bar=baz`)``` | Match Query String parameters. It accepts a sequence of key=value pairs. | +| ```ClientIP(`10.0.0.0/16`, `::1`)``` | Match if the request client IP is one of the given IP/CIDR. It accepts IPv4, IPv6 and CIDR formats. | !!! important "Non-ASCII Domain Names" @@ -259,6 +259,7 @@ The table below lists all the available matchers: The regexp name (`name` in the above example) is an arbitrary value, that exists only for historical reasons. Any `regexp` supported by [Go's regexp package](https://golang.org/pkg/regexp/) may be used. + For example, here is a case insensitive path matcher syntax: ```Path(`/{path:(?i:Products)}`)```. !!! info "Combining Matchers Using Operators and Parenthesis" From 5bc03af75fe6edbb5600e427df2ccc41323ca8a9 Mon Sep 17 00:00:00 2001 From: Kevin Pollet Date: Fri, 16 Sep 2022 16:00:08 +0200 Subject: [PATCH 04/17] Prepare release v2.9.0-rc3 --- .semaphore/semaphore.yml | 2 +- CHANGELOG.md | 6 ++++++ script/gcg/traefik-rc-new.toml | 10 +++++----- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.semaphore/semaphore.yml b/.semaphore/semaphore.yml index 128b5656ec..f43ec07d77 100644 --- a/.semaphore/semaphore.yml +++ b/.semaphore/semaphore.yml @@ -64,7 +64,7 @@ blocks: - name: GH_VERSION value: 1.12.1 - name: CODENAME - value: "beaufort" + value: "banon" - name: IN_DOCKER value: "" prologue: diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f0007d01e..39e8622472 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## [v2.9.0-rc3](https://github.com/traefik/traefik/tree/v2.9.0-rc3) (2022-09-16) +[All Commits](https://github.com/traefik/traefik/compare/v2.9.0-rc2...v2.9.0-rc3) + +**Misc:** +- Merge current v2.8 into v2.9 ([#9343](https://github.com/traefik/traefik/pull/9343) by [kevinpollet](https://github.com/kevinpollet)) + ## [v2.9.0-rc1](https://github.com/traefik/traefik/tree/v2.9.0-rc2) (2022-09-14) [All Commits](https://github.com/traefik/traefik/compare/v2.8.0-rc1...v2.9.0-rc2) diff --git a/script/gcg/traefik-rc-new.toml b/script/gcg/traefik-rc-new.toml index 9e01d10e00..2e8ec3d34a 100644 --- a/script/gcg/traefik-rc-new.toml +++ b/script/gcg/traefik-rc-new.toml @@ -4,11 +4,11 @@ RepositoryName = "traefik" OutputType = "file" FileName = "traefik_changelog.md" -# example RC2 of v2.8.0 -CurrentRef = "v2.8" -PreviousRef = "v2.8.0-rc1" -BaseBranch = "v2.8" -FutureCurrentRefName = "v2.8.0-rc2" +# example RC3 of v2.9.0 +CurrentRef = "v2.9" +PreviousRef = "v2.9.0-rc2" +BaseBranch = "v2.9" +FutureCurrentRefName = "v2.9.0-rc3" ThresholdPreviousRef = 10 ThresholdCurrentRef = 10 From 89870ad539ba0e36d9d2c8f204ff74cb58725370 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Mon, 19 Sep 2022 11:26:08 +0200 Subject: [PATCH 05/17] docs: fix link to RouteNamespaces --- docs/content/migration/v2.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/migration/v2.md b/docs/content/migration/v2.md index 9fda2a6513..f96af26a31 100644 --- a/docs/content/migration/v2.md +++ b/docs/content/migration/v2.md @@ -445,7 +445,7 @@ To enable HTTP/3 on an EntryPoint, please check out the [HTTP/3 configuration](. ### Kubernetes Gateway API Provider In `v2.6`, the [Kubernetes Gateway API provider](../providers/kubernetes-gateway.md) now only supports the version [v1alpha2](https://gateway-api.sigs.k8s.io/v1alpha2/guides/getting-started/) of the specification and -[route namespaces](https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1alpha2.RouteNamespaces) selectors, which requires Traefik to fetch and watch the cluster namespaces. +[route namespaces](https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1beta1.RouteNamespaces) selectors, which requires Traefik to fetch and watch the cluster namespaces. Therefore, the [RBAC](../reference/dynamic-configuration/kubernetes-gateway.md#rbac) and [CRD](../reference/dynamic-configuration/kubernetes-gateway.md#definitions) definitions must be updated. ## v2.6.0 to v2.6.1 From d6b69e13477e6feee2da16b1538bdc200fd70983 Mon Sep 17 00:00:00 2001 From: Thomas Harris Date: Mon, 19 Sep 2022 16:26:08 +0200 Subject: [PATCH 06/17] Support multiple namespaces in the Nomad Provider --- .golangci.toml | 1 + docs/content/deprecation/features.md | 6 ++ docs/content/migration/v2.md | 6 ++ docs/content/providers/nomad.md | 49 ++++++++++- .../reference/static-configuration/cli-ref.md | 3 + .../reference/static-configuration/env-ref.md | 3 + .../reference/static-configuration/file.toml | 1 + .../reference/static-configuration/file.yaml | 3 + pkg/config/static/static_config.go | 10 ++- pkg/provider/aggregator/aggregator.go | 4 +- pkg/provider/nomad/config_test.go | 53 ++++++++++++ pkg/provider/nomad/nomad.go | 83 +++++++++++++++---- pkg/provider/nomad/nomad_test.go | 9 +- .../components/_commons/PanelMiddlewares.vue | 3 + .../_commons/PanelMirroringServices.vue | 3 + .../_commons/PanelRouterDetails.vue | 3 + .../_commons/PanelServiceDetails.vue | 3 + .../_commons/PanelWeightedServices.vue | 3 + .../src/components/_commons/ProviderIcon.vue | 3 + .../components/dashboard/PanelProvider.vue | 3 + 20 files changed, 227 insertions(+), 25 deletions(-) diff --git a/.golangci.toml b/.golangci.toml index 318e5cace2..f867578b0e 100644 --- a/.golangci.toml +++ b/.golangci.toml @@ -178,6 +178,7 @@ "SA1019: cfg.FeaturePolicy is deprecated", "SA1019: c.Providers.ConsulCatalog.Namespace is deprecated", "SA1019: c.Providers.Consul.Namespace is deprecated", + "SA1019: c.Providers.Nomad.Namespace is deprecated", ] [[issues.exclude-rules]] path = "(.+)_test.go" diff --git a/docs/content/deprecation/features.md b/docs/content/deprecation/features.md index 8837af814f..5b32fc1bf3 100644 --- a/docs/content/deprecation/features.md +++ b/docs/content/deprecation/features.md @@ -7,6 +7,7 @@ This page is maintained and updated periodically to reflect our roadmap and any | [Pilot](#pilot) | 2.7 | 2.8 | 2.9 | | [Consul Enterprise Namespace](#consul-enterprise-namespace) | 2.8 | N/A | 3.0 | | [TLS 1.0 and 1.1 Support](#tls-10-and-11) | N/A | 2.8 | N/A | +| [Nomad Namespace](#nomad-namespace) | 2.10 | N/A | 3.0 | ## Impact @@ -26,3 +27,8 @@ please use the `namespaces` options instead. ### TLS 1.0 and 1.1 Starting on 2.8 the default TLS options will use the minimum version of TLS 1.2. Of course, it can still be overridden with custom configuration. + +### Nomad Namespace + +Starting on 2.10 the `namespace` option of the Nomad provider is deprecated, +please use the `namespaces` options instead. diff --git a/docs/content/migration/v2.md b/docs/content/migration/v2.md index 00827cb28e..d769107539 100644 --- a/docs/content/migration/v2.md +++ b/docs/content/migration/v2.md @@ -490,3 +490,9 @@ In `v2.8.2`, Traefik now reject certificates signed with the SHA-1 hash function ### Traefik Pilot In `v2.9`, Traefik Pilot support has been removed. + +## v2.10 + +### Nomad Namespace + +In `v2.10`, the `namespace` option of the Nomad provider is deprecated, please use the `namespaces` options instead. diff --git a/docs/content/providers/nomad.md b/docs/content/providers/nomad.md index 92ce542adf..572ccc0d62 100644 --- a/docs/content/providers/nomad.md +++ b/docs/content/providers/nomad.md @@ -442,24 +442,65 @@ For additional information, refer to [Restrict the Scope of Service Discovery](. ### `namespace` +??? warning "Deprecated in favor of the [`namespaces`](#namespaces) option." + + _Optional, Default=""_ + + The `namespace` option defines the namespace in which the Nomad services will be discovered. + + !!! warning + + One should only define either the `namespaces` option or the `namespace` option. + + ```yaml tab="File (YAML)" + providers: + nomad: + namespace: "production" + # ... + ``` + + ```toml tab="File (TOML)" + [providers.nomad] + namespace = "production" + # ... + ``` + + ```bash tab="CLI" + --providers.nomad.namespace=production + # ... + ``` + +### `namespaces` + _Optional, Default=""_ -The `namespace` option defines the namespace in which the Nomad services will be discovered. +The `namespaces` option defines the namespaces in which the nomad services will be discovered. +When using the `namespaces` option, the discovered object names will be suffixed as shown below: + +```text +@nomad- +``` + +!!! warning + + One should only define either the `namespaces` option or the `namespace` option. ```yaml tab="File (YAML)" providers: nomad: - namespace: "production" + namespaces: + - "ns1" + - "ns2" # ... ``` ```toml tab="File (TOML)" [providers.nomad] - namespace = "production" + namespaces = ["ns1", "ns2"] # ... ``` ```bash tab="CLI" ---providers.nomad.namespace=production +--providers.nomad.namespaces=ns1,ns2 # ... ``` diff --git a/docs/content/reference/static-configuration/cli-ref.md b/docs/content/reference/static-configuration/cli-ref.md index aa0638d085..d1c60130b0 100644 --- a/docs/content/reference/static-configuration/cli-ref.md +++ b/docs/content/reference/static-configuration/cli-ref.md @@ -855,6 +855,9 @@ Expose Nomad services by default. (Default: ```true```) `--providers.nomad.namespace`: Sets the Nomad namespace used to discover services. +`--providers.nomad.namespaces`: +Sets the Nomad namespaces used to discover services. + `--providers.nomad.prefix`: Prefix for nomad service tags. (Default: ```traefik```) diff --git a/docs/content/reference/static-configuration/env-ref.md b/docs/content/reference/static-configuration/env-ref.md index b496ce97ae..5b2313d21c 100644 --- a/docs/content/reference/static-configuration/env-ref.md +++ b/docs/content/reference/static-configuration/env-ref.md @@ -855,6 +855,9 @@ Expose Nomad services by default. (Default: ```true```) `TRAEFIK_PROVIDERS_NOMAD_NAMESPACE`: Sets the Nomad namespace used to discover services. +`TRAEFIK_PROVIDERS_NOMAD_NAMESPACES`: +Sets the Nomad namespaces used to discover services. + `TRAEFIK_PROVIDERS_NOMAD_PREFIX`: Prefix for nomad service tags. (Default: ```traefik```) diff --git a/docs/content/reference/static-configuration/file.toml b/docs/content/reference/static-configuration/file.toml index 3ad3d95c59..33297ecc9e 100644 --- a/docs/content/reference/static-configuration/file.toml +++ b/docs/content/reference/static-configuration/file.toml @@ -181,6 +181,7 @@ prefix = "foobar" stale = true namespace = "foobar" + namespaces = ["foobar", "foobar"] exposedByDefault = true refreshInterval = "42s" [providers.nomad.endpoint] diff --git a/docs/content/reference/static-configuration/file.yaml b/docs/content/reference/static-configuration/file.yaml index dbe316d4ad..fc9363f85e 100644 --- a/docs/content/reference/static-configuration/file.yaml +++ b/docs/content/reference/static-configuration/file.yaml @@ -195,6 +195,9 @@ providers: prefix: foobar stale: true namespace: foobar + namespaces: + - foobar + - foobar exposedByDefault: true refreshInterval: 42s endpoint: diff --git a/pkg/config/static/static_config.go b/pkg/config/static/static_config.go index 090a1e058d..03b6eae45a 100644 --- a/pkg/config/static/static_config.go +++ b/pkg/config/static/static_config.go @@ -186,7 +186,7 @@ type Providers struct { Rest *rest.Provider `description:"Enable Rest backend with default settings." json:"rest,omitempty" toml:"rest,omitempty" yaml:"rest,omitempty" export:"true" label:"allowEmpty" file:"allowEmpty"` Rancher *rancher.Provider `description:"Enable Rancher backend with default settings." json:"rancher,omitempty" toml:"rancher,omitempty" yaml:"rancher,omitempty" export:"true" label:"allowEmpty" file:"allowEmpty"` ConsulCatalog *consulcatalog.ProviderBuilder `description:"Enable ConsulCatalog backend with default settings." json:"consulCatalog,omitempty" toml:"consulCatalog,omitempty" yaml:"consulCatalog,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` - Nomad *nomad.Provider `description:"Enable Nomad backend with default settings." json:"nomad,omitempty" toml:"nomad,omitempty" yaml:"nomad,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` + Nomad *nomad.ProviderBuilder `description:"Enable Nomad backend with default settings." json:"nomad,omitempty" toml:"nomad,omitempty" yaml:"nomad,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` Ecs *ecs.Provider `description:"Enable AWS ECS backend with default settings." json:"ecs,omitempty" toml:"ecs,omitempty" yaml:"ecs,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` Consul *consul.ProviderBuilder `description:"Enable Consul backend with default settings." json:"consul,omitempty" toml:"consul,omitempty" yaml:"consul,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` @@ -326,11 +326,15 @@ func (c *Configuration) ValidateConfiguration() error { } if c.Providers.ConsulCatalog != nil && c.Providers.ConsulCatalog.Namespace != "" && len(c.Providers.ConsulCatalog.Namespaces) > 0 { - return fmt.Errorf("consul catalog provider cannot have both namespace and namespaces options configured") + return fmt.Errorf("Consul Catalog provider cannot have both namespace and namespaces options configured") } if c.Providers.Consul != nil && c.Providers.Consul.Namespace != "" && len(c.Providers.Consul.Namespaces) > 0 { - return fmt.Errorf("consul provider cannot have both namespace and namespaces options configured") + return fmt.Errorf("Consul provider cannot have both namespace and namespaces options configured") + } + + if c.Providers.Nomad != nil && c.Providers.Nomad.Namespace != "" && len(c.Providers.Nomad.Namespaces) > 0 { + return fmt.Errorf("Nomad provider cannot have both namespace and namespaces options configured") } return nil diff --git a/pkg/provider/aggregator/aggregator.go b/pkg/provider/aggregator/aggregator.go index 07d4488fb1..3ddb1c3f25 100644 --- a/pkg/provider/aggregator/aggregator.go +++ b/pkg/provider/aggregator/aggregator.go @@ -115,7 +115,9 @@ func NewProviderAggregator(conf static.Providers) ProviderAggregator { } if conf.Nomad != nil { - p.quietAddProvider(conf.Nomad) + for _, pvd := range conf.Nomad.BuildProviders() { + p.quietAddProvider(pvd) + } } if conf.Consul != nil { diff --git a/pkg/provider/nomad/config_test.go b/pkg/provider/nomad/config_test.go index 4c72ee8548..9d0aa21b89 100644 --- a/pkg/provider/nomad/config_test.go +++ b/pkg/provider/nomad/config_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/traefik/traefik/v2/pkg/config/dynamic" ) @@ -2509,5 +2510,57 @@ func Test_keepItem(t *testing.T) { } } +func TestNamespaces(t *testing.T) { + testCases := []struct { + desc string + namespace string + namespaces []string + expectedNamespaces []string + }{ + { + desc: "no defined namespaces", + expectedNamespaces: []string{""}, + }, + { + desc: "deprecated: use of defined namespace", + namespace: "test-ns", + expectedNamespaces: []string{"test-ns"}, + }, + { + desc: "use of 1 defined namespaces", + namespaces: []string{"test-ns"}, + expectedNamespaces: []string{"test-ns"}, + }, + { + desc: "use of multiple defined namespaces", + namespaces: []string{"test-ns1", "test-ns2", "test-ns3", "test-ns4"}, + expectedNamespaces: []string{"test-ns1", "test-ns2", "test-ns3", "test-ns4"}, + }, + } + + for _, test := range testCases { + test := test + + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + pb := &ProviderBuilder{ + Namespace: test.namespace, + Namespaces: test.namespaces, + } + + assert.Equal(t, test.expectedNamespaces, extractNamespacesFromProvider(pb.BuildProviders())) + }) + } +} + +func extractNamespacesFromProvider(providers []*Provider) []string { + res := make([]string, len(providers)) + for i, p := range providers { + res[i] = p.namespace + } + return res +} + func Int(v int) *int { return &v } func Bool(v bool) *bool { return &v } diff --git a/pkg/provider/nomad/nomad.go b/pkg/provider/nomad/nomad.go index 34877fb915..e321c05a5f 100644 --- a/pkg/provider/nomad/nomad.go +++ b/pkg/provider/nomad/nomad.go @@ -2,6 +2,7 @@ package nomad import ( "context" + "errors" "fmt" "strings" "text/template" @@ -46,17 +47,68 @@ type item struct { ExtraConf configuration // global options } -// Provider holds configurations of the provider. -type Provider struct { +// ProviderBuilder is responsible for constructing namespaced instances of the Nomad provider. +type ProviderBuilder struct { + Configuration `yaml:",inline" export:"true"` + + // Deprecated: Use Namespaces option instead + Namespace string `description:"Sets the Nomad namespace used to discover services." json:"namespace,omitempty" toml:"namespace,omitempty" yaml:"namespace,omitempty"` + Namespaces []string `description:"Sets the Nomad namespaces used to discover services." json:"namespaces,omitempty" toml:"namespaces,omitempty" yaml:"namespaces,omitempty"` +} + +// BuildProviders builds Nomad provider instances for the given namespaces configuration. +func (p *ProviderBuilder) BuildProviders() []*Provider { + if p.Namespace != "" { + log.WithoutContext().Warnf("Namespace option is deprecated, please use the Namespaces option instead.") + } + + if len(p.Namespaces) == 0 { + return []*Provider{{ + Configuration: p.Configuration, + name: providerName, + // p.Namespace could be empty + namespace: p.Namespace, + }} + } + + var providers []*Provider + for _, namespace := range p.Namespaces { + providers = append(providers, &Provider{ + Configuration: p.Configuration, + name: providerName + "-" + namespace, + namespace: namespace, + }) + } + + return providers +} + +// Configuration represents the Nomad provider configuration. +type Configuration struct { DefaultRule string `description:"Default rule." json:"defaultRule,omitempty" toml:"defaultRule,omitempty" yaml:"defaultRule,omitempty"` Constraints string `description:"Constraints is an expression that Traefik matches against the Nomad service's tags to determine whether to create route(s) for that service." json:"constraints,omitempty" toml:"constraints,omitempty" yaml:"constraints,omitempty" export:"true"` Endpoint *EndpointConfig `description:"Nomad endpoint settings" json:"endpoint,omitempty" toml:"endpoint,omitempty" yaml:"endpoint,omitempty" export:"true"` Prefix string `description:"Prefix for nomad service tags." json:"prefix,omitempty" toml:"prefix,omitempty" yaml:"prefix,omitempty" export:"true"` Stale bool `description:"Use stale consistency for catalog reads." json:"stale,omitempty" toml:"stale,omitempty" yaml:"stale,omitempty" export:"true"` - Namespace string `description:"Sets the Nomad namespace used to discover services." json:"namespace,omitempty" toml:"namespace,omitempty" yaml:"namespace,omitempty" export:"true"` ExposedByDefault bool `description:"Expose Nomad services by default." json:"exposedByDefault,omitempty" toml:"exposedByDefault,omitempty" yaml:"exposedByDefault,omitempty" export:"true"` RefreshInterval ptypes.Duration `description:"Interval for polling Nomad API." json:"refreshInterval,omitempty" toml:"refreshInterval,omitempty" yaml:"refreshInterval,omitempty" export:"true"` +} + +// SetDefaults sets the default values for the Nomad Traefik Provider Configuration. +func (c *Configuration) SetDefaults() { + c.Endpoint = &EndpointConfig{} + c.Prefix = defaultPrefix + c.ExposedByDefault = true + c.RefreshInterval = ptypes.Duration(15 * time.Second) + c.DefaultRule = defaultTemplateRule +} +// Provider holds configuration along with the namespace it will discover services in. +type Provider struct { + Configuration + + name string + namespace string client *api.Client // client for Nomad API defaultRuleTpl *template.Template // default routing rule } @@ -72,22 +124,23 @@ type EndpointConfig struct { EndpointWaitTime ptypes.Duration `description:"WaitTime limits how long a Watch will block. If not provided, the agent default values will be used" json:"endpointWaitTime,omitempty" toml:"endpointWaitTime,omitempty" yaml:"endpointWaitTime,omitempty" export:"true"` } -// SetDefaults sets the default values for the Nomad Traefik Provider. -func (p *Provider) SetDefaults() { - p.Endpoint = &EndpointConfig{} - p.Prefix = defaultPrefix - p.ExposedByDefault = true - p.RefreshInterval = ptypes.Duration(15 * time.Second) - p.DefaultRule = defaultTemplateRule -} - // Init the Nomad Traefik Provider. func (p *Provider) Init() error { + if p.namespace == api.AllNamespacesNamespace { + return errors.New("wildcard namespace not supported") + } + defaultRuleTpl, err := provider.MakeDefaultRuleTemplate(p.DefaultRule, nil) if err != nil { return fmt.Errorf("error while parsing default rule: %w", err) } p.defaultRuleTpl = defaultRuleTpl + + // In case they didn't initialize Provider with BuildProviders + if p.name == "" { + p.name = providerName + } + return nil } @@ -95,13 +148,13 @@ func (p *Provider) Init() error { // using the given configuration channel. func (p *Provider) Provide(configurationChan chan<- dynamic.Message, pool *safe.Pool) error { var err error - p.client, err = createClient(p.Namespace, p.Endpoint) + p.client, err = createClient(p.namespace, p.Endpoint) if err != nil { return fmt.Errorf("failed to create nomad API client: %w", err) } pool.GoCtx(func(routineCtx context.Context) { - ctxLog := log.With(routineCtx, log.Str(log.ProviderName, providerName)) + ctxLog := log.With(routineCtx, log.Str(log.ProviderName, p.name)) logger := log.FromContext(ctxLog) operation := func() error { @@ -154,7 +207,7 @@ func (p *Provider) loadConfiguration(ctx context.Context, configurationC chan<- return err } configurationC <- dynamic.Message{ - ProviderName: providerName, + ProviderName: p.name, Configuration: p.buildConfig(ctx, items), } diff --git a/pkg/provider/nomad/nomad_test.go b/pkg/provider/nomad/nomad_test.go index 02e1217efd..b9d2bfa39c 100644 --- a/pkg/provider/nomad/nomad_test.go +++ b/pkg/provider/nomad/nomad_test.go @@ -64,7 +64,12 @@ func Test_globalConfig(t *testing.T) { for _, test := range cases { t.Run(test.Name, func(t *testing.T) { - p := Provider{ExposedByDefault: test.ExposedByDefault, Prefix: test.Prefix} + p := Provider{ + Configuration: Configuration{ + ExposedByDefault: test.ExposedByDefault, + Prefix: test.Prefix, + }, + } result := p.getExtraConf(test.Tags) require.Equal(t, test.exp, result) }) @@ -91,7 +96,7 @@ func Test_getNomadServiceData(t *testing.T) { require.NoError(t, err) // fudge client, avoid starting up via Provide - p.client, err = createClient(p.Namespace, p.Endpoint) + p.client, err = createClient(p.namespace, p.Endpoint) require.NoError(t, err) // make the query for services diff --git a/webui/src/components/_commons/PanelMiddlewares.vue b/webui/src/components/_commons/PanelMiddlewares.vue index 3437f8bef1..ef0a56e0d4 100644 --- a/webui/src/components/_commons/PanelMiddlewares.vue +++ b/webui/src/components/_commons/PanelMiddlewares.vue @@ -1151,6 +1151,9 @@ export default { if (name.startsWith('consulcatalog-')) { return `statics/providers/consulcatalog.svg` } + if (name.startsWith('nomad-')) { + return `statics/providers/nomad.svg` + } return `statics/providers/${name}.svg` } diff --git a/webui/src/components/_commons/PanelMirroringServices.vue b/webui/src/components/_commons/PanelMirroringServices.vue index 849d995593..4ddb6a5c79 100644 --- a/webui/src/components/_commons/PanelMirroringServices.vue +++ b/webui/src/components/_commons/PanelMirroringServices.vue @@ -75,6 +75,9 @@ export default { if (name.startsWith('consulcatalog-')) { return `statics/providers/consulcatalog.svg` } + if (name.startsWith('nomad-')) { + return `statics/providers/nomad.svg` + } return `statics/providers/${name}.svg` } diff --git a/webui/src/components/_commons/PanelRouterDetails.vue b/webui/src/components/_commons/PanelRouterDetails.vue index 582d131330..4b5d2029bc 100644 --- a/webui/src/components/_commons/PanelRouterDetails.vue +++ b/webui/src/components/_commons/PanelRouterDetails.vue @@ -141,6 +141,9 @@ export default { if (name.startsWith('consulcatalog-')) { return `statics/providers/consulcatalog.svg` } + if (name.startsWith('nomad-')) { + return `statics/providers/nomad.svg` + } return `statics/providers/${name}.svg` } diff --git a/webui/src/components/_commons/PanelServiceDetails.vue b/webui/src/components/_commons/PanelServiceDetails.vue index 6d8de0857a..b734361729 100644 --- a/webui/src/components/_commons/PanelServiceDetails.vue +++ b/webui/src/components/_commons/PanelServiceDetails.vue @@ -155,6 +155,9 @@ export default { if (name.startsWith('consulcatalog-')) { return `statics/providers/consulcatalog.svg` } + if (name.startsWith('nomad-')) { + return `statics/providers/nomad.svg` + } return `statics/providers/${name}.svg` } diff --git a/webui/src/components/_commons/PanelWeightedServices.vue b/webui/src/components/_commons/PanelWeightedServices.vue index 2b6f539d63..e5b4c27ff2 100644 --- a/webui/src/components/_commons/PanelWeightedServices.vue +++ b/webui/src/components/_commons/PanelWeightedServices.vue @@ -75,6 +75,9 @@ export default { if (name.startsWith('consulcatalog-')) { return `statics/providers/consulcatalog.svg` } + if (name.startsWith('nomad-')) { + return `statics/providers/nomad.svg` + } return `statics/providers/${name}.svg` } diff --git a/webui/src/components/_commons/ProviderIcon.vue b/webui/src/components/_commons/ProviderIcon.vue index 6b3bac4098..2d792ada55 100644 --- a/webui/src/components/_commons/ProviderIcon.vue +++ b/webui/src/components/_commons/ProviderIcon.vue @@ -20,6 +20,9 @@ export default { if (name.startsWith('consulcatalog-')) { return `statics/providers/consulcatalog.svg` } + if (name.startsWith('nomad-')) { + return `statics/providers/nomad.svg` + } return `statics/providers/${name}.svg` } diff --git a/webui/src/components/dashboard/PanelProvider.vue b/webui/src/components/dashboard/PanelProvider.vue index a4d451866f..59fbd47826 100644 --- a/webui/src/components/dashboard/PanelProvider.vue +++ b/webui/src/components/dashboard/PanelProvider.vue @@ -37,6 +37,9 @@ export default { if (name.startsWith('consulcatalog-')) { return `statics/providers/consulcatalog.svg` } + if (name.startsWith('nomad-')) { + return `statics/providers/nomad.svg` + } return `statics/providers/${name}.svg` } From df99a9fb578f9c47865fca48b3553f3ea5cb1faf Mon Sep 17 00:00:00 2001 From: Michael Hampton <24950358+Michampt@users.noreply.github.com> Date: Tue, 20 Sep 2022 06:42:08 -0700 Subject: [PATCH 07/17] Add option to keep only healthy ECS tasks --- docs/content/providers/ecs.md | 24 ++++++++++++++++++ .../reference/static-configuration/cli-ref.md | 19 ++++++++------ .../reference/static-configuration/env-ref.md | 19 ++++++++------ .../reference/static-configuration/file.toml | 1 + .../reference/static-configuration/file.yaml | 1 + pkg/provider/ecs/ecs.go | 25 +++++++++++-------- 6 files changed, 63 insertions(+), 26 deletions(-) diff --git a/docs/content/providers/ecs.md b/docs/content/providers/ecs.md index 31e5e324db..a9d0064567 100644 --- a/docs/content/providers/ecs.md +++ b/docs/content/providers/ecs.md @@ -169,6 +169,30 @@ providers: # ... ``` +### `healthyTasksOnly` + +_Optional, Default=false_ + +Determines whether Traefik discovers only healthy tasks (`HEALTHY` healthStatus). + +```yaml tab="File (YAML)" +providers: + ecs: + healthyTasksOnly: true + # ... +``` + +```toml tab="File (TOML)" +[providers.ecs] + healthyTasksOnly = true + # ... +``` + +```bash tab="CLI" +--providers.ecs.healthyTasksOnly=true +# ... +``` + ### `defaultRule` _Optional, Default=```Host(`{{ normalize .Name }}`)```_ diff --git a/docs/content/reference/static-configuration/cli-ref.md b/docs/content/reference/static-configuration/cli-ref.md index d1c60130b0..ef9f2a1259 100644 --- a/docs/content/reference/static-configuration/cli-ref.md +++ b/docs/content/reference/static-configuration/cli-ref.md @@ -559,13 +559,13 @@ Watch Docker Swarm events. (Default: ```true```) Enable AWS ECS backend with default settings. (Default: ```false```) `--providers.ecs.accesskeyid`: -The AWS credentials access key to use for making requests +AWS credentials access key ID to use for making requests. `--providers.ecs.autodiscoverclusters`: -Auto discover cluster (Default: ```false```) +Auto discover cluster. (Default: ```false```) `--providers.ecs.clusters`: -ECS Clusters name (Default: ```default```) +ECS Cluster names. (Default: ```default```) `--providers.ecs.constraints`: Constraints is an expression that Traefik matches against the container's labels to determine whether to create any route for that container. @@ -574,19 +574,22 @@ Constraints is an expression that Traefik matches against the container's labels Default rule. (Default: ```Host(`{{ normalize .Name }}`)```) `--providers.ecs.ecsanywhere`: -Enable ECS Anywhere support (Default: ```false```) +Enable ECS Anywhere support. (Default: ```false```) `--providers.ecs.exposedbydefault`: -Expose services by default (Default: ```true```) +Expose services by default. (Default: ```true```) + +`--providers.ecs.healthytasksonly`: +Determines whether to discover only healthy tasks. (Default: ```false```) `--providers.ecs.refreshseconds`: -Polling interval (in seconds) (Default: ```15```) +Polling interval (in seconds). (Default: ```15```) `--providers.ecs.region`: -The AWS region to use for requests +AWS region to use for requests. `--providers.ecs.secretaccesskey`: -The AWS credentials access key to use for making requests +AWS credentials access key to use for making requests. `--providers.etcd`: Enable Etcd backend with default settings. (Default: ```false```) diff --git a/docs/content/reference/static-configuration/env-ref.md b/docs/content/reference/static-configuration/env-ref.md index 5b2313d21c..18c54f58e2 100644 --- a/docs/content/reference/static-configuration/env-ref.md +++ b/docs/content/reference/static-configuration/env-ref.md @@ -559,13 +559,13 @@ Watch Docker Swarm events. (Default: ```true```) Enable AWS ECS backend with default settings. (Default: ```false```) `TRAEFIK_PROVIDERS_ECS_ACCESSKEYID`: -The AWS credentials access key to use for making requests +AWS credentials access key ID to use for making requests. `TRAEFIK_PROVIDERS_ECS_AUTODISCOVERCLUSTERS`: -Auto discover cluster (Default: ```false```) +Auto discover cluster. (Default: ```false```) `TRAEFIK_PROVIDERS_ECS_CLUSTERS`: -ECS Clusters name (Default: ```default```) +ECS Cluster names. (Default: ```default```) `TRAEFIK_PROVIDERS_ECS_CONSTRAINTS`: Constraints is an expression that Traefik matches against the container's labels to determine whether to create any route for that container. @@ -574,19 +574,22 @@ Constraints is an expression that Traefik matches against the container's labels Default rule. (Default: ```Host(`{{ normalize .Name }}`)```) `TRAEFIK_PROVIDERS_ECS_ECSANYWHERE`: -Enable ECS Anywhere support (Default: ```false```) +Enable ECS Anywhere support. (Default: ```false```) `TRAEFIK_PROVIDERS_ECS_EXPOSEDBYDEFAULT`: -Expose services by default (Default: ```true```) +Expose services by default. (Default: ```true```) + +`TRAEFIK_PROVIDERS_ECS_HEALTHYTASKSONLY`: +Determines whether to discover only healthy tasks. (Default: ```false```) `TRAEFIK_PROVIDERS_ECS_REFRESHSECONDS`: -Polling interval (in seconds) (Default: ```15```) +Polling interval (in seconds). (Default: ```15```) `TRAEFIK_PROVIDERS_ECS_REGION`: -The AWS region to use for requests +AWS region to use for requests. `TRAEFIK_PROVIDERS_ECS_SECRETACCESSKEY`: -The AWS credentials access key to use for making requests +AWS credentials access key to use for making requests. `TRAEFIK_PROVIDERS_ETCD`: Enable Etcd backend with default settings. (Default: ```false```) diff --git a/docs/content/reference/static-configuration/file.toml b/docs/content/reference/static-configuration/file.toml index 33297ecc9e..20af99cc3e 100644 --- a/docs/content/reference/static-configuration/file.toml +++ b/docs/content/reference/static-configuration/file.toml @@ -206,6 +206,7 @@ accessKeyID = "foobar" secretAccessKey = "foobar" ecsAnywhere = true + healthyTasksOnly = true [providers.consul] rootKey = "foobar" endpoints = ["foobar", "foobar"] diff --git a/docs/content/reference/static-configuration/file.yaml b/docs/content/reference/static-configuration/file.yaml index fc9363f85e..ba5c35cb9c 100644 --- a/docs/content/reference/static-configuration/file.yaml +++ b/docs/content/reference/static-configuration/file.yaml @@ -224,6 +224,7 @@ providers: accessKeyID: foobar secretAccessKey: foobar ecsAnywhere: true + healthyTasksOnly: true consul: rootKey: foobar endpoints: diff --git a/pkg/provider/ecs/ecs.go b/pkg/provider/ecs/ecs.go index 1958723dba..fa7d2185ab 100644 --- a/pkg/provider/ecs/ecs.go +++ b/pkg/provider/ecs/ecs.go @@ -27,17 +27,18 @@ import ( // Provider holds configurations of the provider. type Provider struct { Constraints string `description:"Constraints is an expression that Traefik matches against the container's labels to determine whether to create any route for that container." json:"constraints,omitempty" toml:"constraints,omitempty" yaml:"constraints,omitempty" export:"true"` - ExposedByDefault bool `description:"Expose services by default" json:"exposedByDefault,omitempty" toml:"exposedByDefault,omitempty" yaml:"exposedByDefault,omitempty" export:"true"` - RefreshSeconds int `description:"Polling interval (in seconds)" json:"refreshSeconds,omitempty" toml:"refreshSeconds,omitempty" yaml:"refreshSeconds,omitempty" export:"true"` + ExposedByDefault bool `description:"Expose services by default." json:"exposedByDefault,omitempty" toml:"exposedByDefault,omitempty" yaml:"exposedByDefault,omitempty" export:"true"` + RefreshSeconds int `description:"Polling interval (in seconds)." json:"refreshSeconds,omitempty" toml:"refreshSeconds,omitempty" yaml:"refreshSeconds,omitempty" export:"true"` DefaultRule string `description:"Default rule." json:"defaultRule,omitempty" toml:"defaultRule,omitempty" yaml:"defaultRule,omitempty"` // Provider lookup parameters. - Clusters []string `description:"ECS Clusters name" json:"clusters,omitempty" toml:"clusters,omitempty" yaml:"clusters,omitempty" export:"true"` - AutoDiscoverClusters bool `description:"Auto discover cluster" json:"autoDiscoverClusters,omitempty" toml:"autoDiscoverClusters,omitempty" yaml:"autoDiscoverClusters,omitempty" export:"true"` - ECSAnywhere bool `description:"Enable ECS Anywhere support" json:"ecsAnywhere,omitempty" toml:"ecsAnywhere,omitempty" yaml:"ecsAnywhere,omitempty" export:"true"` - Region string `description:"The AWS region to use for requests" json:"region,omitempty" toml:"region,omitempty" yaml:"region,omitempty" export:"true"` - AccessKeyID string `description:"The AWS credentials access key to use for making requests" json:"accessKeyID,omitempty" toml:"accessKeyID,omitempty" yaml:"accessKeyID,omitempty" loggable:"false"` - SecretAccessKey string `description:"The AWS credentials access key to use for making requests" json:"secretAccessKey,omitempty" toml:"secretAccessKey,omitempty" yaml:"secretAccessKey,omitempty" loggable:"false"` + Clusters []string `description:"ECS Cluster names." json:"clusters,omitempty" toml:"clusters,omitempty" yaml:"clusters,omitempty" export:"true"` + AutoDiscoverClusters bool `description:"Auto discover cluster." json:"autoDiscoverClusters,omitempty" toml:"autoDiscoverClusters,omitempty" yaml:"autoDiscoverClusters,omitempty" export:"true"` + HealthyTasksOnly bool `description:"Determines whether to discover only healthy tasks." json:"healthyTasksOnly,omitempty" toml:"healthyTasksOnly,omitempty" yaml:"healthyTasksOnly,omitempty" export:"true"` + ECSAnywhere bool `description:"Enable ECS Anywhere support." json:"ecsAnywhere,omitempty" toml:"ecsAnywhere,omitempty" yaml:"ecsAnywhere,omitempty" export:"true"` + Region string `description:"AWS region to use for requests." json:"region,omitempty" toml:"region,omitempty" yaml:"region,omitempty" export:"true"` + AccessKeyID string `description:"AWS credentials access key ID to use for making requests." json:"accessKeyID,omitempty" toml:"accessKeyID,omitempty" yaml:"accessKeyID,omitempty" loggable:"false"` + SecretAccessKey string `description:"AWS credentials access key to use for making requests." json:"secretAccessKey,omitempty" toml:"secretAccessKey,omitempty" yaml:"secretAccessKey,omitempty" loggable:"false"` defaultRuleTpl *template.Template } @@ -81,6 +82,7 @@ var ( func (p *Provider) SetDefaults() { p.Clusters = []string{"default"} p.AutoDiscoverClusters = false + p.HealthyTasksOnly = false p.ExposedByDefault = true p.RefreshSeconds = 15 p.DefaultRule = DefaultTemplateRule @@ -258,9 +260,12 @@ func (p *Provider) listInstances(ctx context.Context, client *awsClient) ([]ecsI logger.Errorf("Unable to describe tasks for %v", page.TaskArns) } else { for _, t := range resp.Tasks { - if aws.StringValue(t.LastStatus) == ecs.DesiredStatusRunning { - tasks[aws.StringValue(t.TaskArn)] = t + if p.HealthyTasksOnly && aws.StringValue(t.HealthStatus) != ecs.HealthStatusHealthy { + logger.Debugf("Skipping unhealthy task %s", aws.StringValue(t.TaskArn)) + continue } + + tasks[aws.StringValue(t.TaskArn)] = t } } } From fe415c12a00915f2ad7fd37225ba27da6d524288 Mon Sep 17 00:00:00 2001 From: Jeremy JACQUE Date: Mon, 15 Nov 2021 23:05:15 +0100 Subject: [PATCH 08/17] feat: add gRPC support to healthcheck --- docs/content/routing/services/index.md | 3 +- pkg/healthcheck/healthcheck.go | 56 +++++++- pkg/healthcheck/healthcheck_test.go | 192 ++++++++++++++++++++++++- 3 files changed, 246 insertions(+), 5 deletions(-) diff --git a/docs/content/routing/services/index.md b/docs/content/routing/services/index.md index f7b1e8a596..66f76b6c84 100644 --- a/docs/content/routing/services/index.md +++ b/docs/content/routing/services/index.md @@ -316,7 +316,8 @@ On subsequent requests, to keep the session alive with the same server, the clie #### Health Check Configure health check to remove unhealthy servers from the load balancing rotation. -Traefik will consider your servers healthy as long as they return status codes between `2XX` and `3XX` to the health check requests (carried out every `interval`). +Traefik will consider your HTTP(s) servers healthy as long as they return status codes between `2XX` and `3XX` to the health check requests (carried out every `interval`). +For gRPC servers, Traefik will consider them healthy as long as they return `SERVING` to gRPC health check v1 requests). You have to define the `scheme`to `grpc`. To propagate status changes (e.g. all servers of this service are down) upwards, HealthCheck must also be enabled on the parent(s) of this service. diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index 77a7743bb5..630f50ed22 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/pkg/healthcheck/healthcheck.go @@ -19,6 +19,10 @@ import ( "github.com/traefik/traefik/v2/pkg/metrics" "github.com/traefik/traefik/v2/pkg/safe" "github.com/vulcand/oxy/roundrobin" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + healthpb "google.golang.org/grpc/health/grpc_health_v1" + "google.golang.org/grpc/status" ) const ( @@ -245,9 +249,10 @@ func NewBackendConfig(options Options, backendName string) *BackendConfig { } } -// checkHealth returns a nil error in case it was successful and otherwise +// checkHealthHTTP returns a nil error in case it was successful and otherwise // a non-nil error with a meaningful description why the health check failed. -func checkHealth(serverURL *url.URL, backend *BackendConfig) error { +// Dedicatted to HTTP servers. +func checkHealthHTTP(serverURL *url.URL, backend *BackendConfig) error { req, err := backend.newRequest(serverURL) if err != nil { return fmt.Errorf("failed to create HTTP request: %w", err) @@ -280,6 +285,53 @@ func checkHealth(serverURL *url.URL, backend *BackendConfig) error { return nil } +// checkHealthGrpc returns a nil error in case it was successful and otherwise +// a non-nil error with a meaningful description why the health check failed. +// Dedicatted to gRPC servers implementing gRPC Health Checking Protocol v1. +func checkHealthGrpc(serverURL *url.URL, backend *BackendConfig) error { + u, err := serverURL.Parse(backend.Path) + if err != nil { + return fmt.Errorf("failed to parse serverURL: %w", err) + } + + opts := []grpc.DialOption{ + grpc.WithInsecure(), + } + + conn, err := grpc.Dial(u.Hostname()+":"+u.Port(), opts...) + if err != nil { + return fmt.Errorf("fail to dial: %w", err) + } + + defer conn.Close() + + grpcCtx := context.Background() + + resp, err := healthpb.NewHealthClient(conn).Check(grpcCtx, &healthpb.HealthCheckRequest{}) + if err != nil { + if stat, ok := status.FromError(err); ok && stat.Code() == codes.Unimplemented { + return fmt.Errorf("the server doesn't implement the grpc health protocol") + } + return fmt.Errorf("gRPC request failed %w", err) + } + + if resp.Status != healthpb.HealthCheckResponse_SERVING { + return fmt.Errorf("received gRPC status code: %v", resp.Status) + } + + return nil +} + +// checkHealth calls the proper health check function depending on the +// scheme declared in the backend config options. +// defaults to HTTP. +func checkHealth(serverURL *url.URL, backend *BackendConfig) error { + if strings.Compare(backend.Options.Scheme, "grpc") == 0 { + return checkHealthGrpc(serverURL, backend) + } + return checkHealthHTTP(serverURL, backend) +} + // StatusUpdater should be implemented by a service that, when its status // changes (e.g. all if its children are down), needs to propagate upwards (to // their parent(s)) that change. diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go index 914f48291f..f943365299 100644 --- a/pkg/healthcheck/healthcheck_test.go +++ b/pkg/healthcheck/healthcheck_test.go @@ -2,6 +2,7 @@ package healthcheck import ( "context" + "net" "net/http" "net/http/httptest" "net/url" @@ -14,13 +15,24 @@ import ( "github.com/traefik/traefik/v2/pkg/config/runtime" "github.com/traefik/traefik/v2/pkg/testhelpers" "github.com/vulcand/oxy/roundrobin" + "google.golang.org/grpc" + healthpb "google.golang.org/grpc/health/grpc_health_v1" ) const ( - healthCheckInterval = 200 * time.Millisecond - healthCheckTimeout = 100 * time.Millisecond + healthCheckInterval = 200 * time.Millisecond + gRPChealthCheckInterval = 500 * time.Millisecond + healthCheckTimeout = 100 * time.Millisecond ) +type gRPCCheckStatus = healthpb.HealthCheckResponse_ServingStatus + +type GrpcHealthChecker struct { + mu sync.RWMutex + status []gRPCCheckStatus + statusIdx int +} + type testHandler struct { done func() healthSequence []int @@ -154,6 +166,133 @@ func TestSetBackendsConfiguration(t *testing.T) { } } +func TestSetGrpcBackendsConfiguration(t *testing.T) { + testCases := []struct { + desc string + startHealthy bool + healthSequence []gRPCCheckStatus + expectedNumRemovedServers int + expectedNumUpsertedServers int + expectedGaugeValue float64 + }{ + { + desc: "healthy server staying healthy", + startHealthy: true, + healthSequence: []gRPCCheckStatus{ + healthpb.HealthCheckResponse_SERVING, + }, + expectedNumRemovedServers: 0, + expectedNumUpsertedServers: 0, + expectedGaugeValue: 1, + }, + { + desc: "healthy server becoming sick", + startHealthy: true, + healthSequence: []gRPCCheckStatus{ + healthpb.HealthCheckResponse_NOT_SERVING, + }, + expectedNumRemovedServers: 1, + expectedNumUpsertedServers: 0, + expectedGaugeValue: 0, + }, + { + desc: "sick server becoming healthy", + startHealthy: false, + healthSequence: []gRPCCheckStatus{ + healthpb.HealthCheckResponse_SERVING, + }, + expectedNumRemovedServers: 0, + expectedNumUpsertedServers: 1, + expectedGaugeValue: 1, + }, + { + desc: "sick server staying sick", + startHealthy: false, + healthSequence: []gRPCCheckStatus{ + healthpb.HealthCheckResponse_NOT_SERVING, + }, + expectedNumRemovedServers: 0, + expectedNumUpsertedServers: 0, + expectedGaugeValue: 0, + }, + { + desc: "healthy server toggling to sick and back to healthy", + startHealthy: true, + healthSequence: []gRPCCheckStatus{ + healthpb.HealthCheckResponse_NOT_SERVING, + healthpb.HealthCheckResponse_SERVING, + }, + expectedNumRemovedServers: 1, + expectedNumUpsertedServers: 1, + expectedGaugeValue: 1, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + // The context is passed to the health check and canonically canceled by + // the test server once all expected requests have been received. + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + gRPCListener, err := net.Listen("tcp4", "127.0.0.1:0") + assert.NoError(t, err) + defer gRPCListener.Close() + ts := newTestServerGrpc(t, test.healthSequence, &gRPCListener, cancel) + defer ts.Stop() + addr := gRPCListener.Addr().String() + + lb := &testLoadBalancer{RWMutex: &sync.RWMutex{}} + backend := NewBackendConfig(Options{ + Scheme: "grpc", + Path: "check", + Interval: gRPChealthCheckInterval, + Timeout: healthCheckTimeout, + LB: lb, + }, "gRPCbackendName") + + serverURL := testhelpers.MustParseURL("http://" + addr) + if test.startHealthy { + lb.servers = append(lb.servers, serverURL) + } else { + backend.disabledURLs = append(backend.disabledURLs, backendURL{url: serverURL, weight: 1}) + } + + collectingMetrics := &testhelpers.CollectingGauge{} + check := HealthCheck{ + Backends: make(map[string]*BackendConfig), + metrics: metricsHealthcheck{serverUpGauge: collectingMetrics}, + } + + wg := sync.WaitGroup{} + wg.Add(1) + + go func() { + check.execute(ctx, backend) + wg.Done() + }() + + // Make test timeout dependent on number of expected requests, health + // check interval, and a safety margin. + timeout := time.Duration(len(test.healthSequence)*int(gRPChealthCheckInterval) + int(time.Second)) + select { + case <-time.After(timeout): + t.Fatal("test did not complete in time") + case <-ctx.Done(): + wg.Wait() + } + lb.Lock() + defer lb.Unlock() + + assert.Equal(t, test.expectedNumRemovedServers, lb.numRemovedServers, "removed servers") + assert.Equal(t, test.expectedNumUpsertedServers, lb.numUpsertedServers, "upserted servers") + assert.Equal(t, test.expectedGaugeValue, collectingMetrics.GaugeValue, "ServerUp Gauge") + }) + } +} + func TestNewRequest(t *testing.T) { type expected struct { err bool @@ -518,6 +657,55 @@ func newTestServer(done func(), healthSequence []int) *httptest.Server { return httptest.NewServer(handler) } +func (s *GrpcHealthChecker) Check(ctx context.Context, req *healthpb.HealthCheckRequest) (*healthpb.HealthCheckResponse, error) { + s.mu.Lock() + defer s.mu.Unlock() + stat := s.status[s.statusIdx] + if s.statusIdx < len(s.status)-1 { + s.statusIdx++ + } + return &healthpb.HealthCheckResponse{ + Status: stat, + }, nil +} + +func (s *GrpcHealthChecker) Watch(req *healthpb.HealthCheckRequest, server healthpb.Health_WatchServer) error { + s.mu.Lock() + defer s.mu.Unlock() + stat := s.status[s.statusIdx] + if s.statusIdx < len(s.status)-1 { + s.statusIdx++ + } + return server.Send(&healthpb.HealthCheckResponse{ + Status: stat, + }) +} + +func newTestServerGrpc(t *testing.T, healthSequence []gRPCCheckStatus, gRPCListener *net.Listener, done func()) *grpc.Server { + t.Helper() + server := grpc.NewServer() + gRPCService := &GrpcHealthChecker{status: healthSequence, statusIdx: 0} + healthpb.RegisterHealthServer(server, gRPCService) + go func() { + err := server.Serve((*gRPCListener)) + assert.NoError(t, err) + }() + // Waitt for gRPC server to be ready + time.Sleep(800 * time.Millisecond) + // Once the test server received tthe expectted number of queries, cancel the context + go func() { + for { + time.Sleep(gRPChealthCheckInterval + healthCheckTimeout) + gRPCService.mu.RLock() + if gRPCService.statusIdx == len(gRPCService.status)-1 { + done() + } + gRPCService.mu.RUnlock() + } + }() + return server +} + // ServeHTTP returns HTTP response codes following a status sequences. // It calls the given 'done' function once all request health indicators have been depleted. func (th *testHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { From 764480065fc2cb9b7aa0082965db3bdee14d5db4 Mon Sep 17 00:00:00 2001 From: Jeremy JACQUE Date: Sun, 28 Nov 2021 10:25:45 +0100 Subject: [PATCH 09/17] Honor healthcheck timeout for gRPC --- pkg/healthcheck/healthcheck.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index 630f50ed22..f70af2cd27 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/pkg/healthcheck/healthcheck.go @@ -293,24 +293,31 @@ func checkHealthGrpc(serverURL *url.URL, backend *BackendConfig) error { if err != nil { return fmt.Errorf("failed to parse serverURL: %w", err) } + grpcSrvAddr := u.Hostname() + ":" + u.Port() opts := []grpc.DialOption{ grpc.WithInsecure(), } - conn, err := grpc.Dial(u.Hostname()+":"+u.Port(), opts...) + grpcCtx, grpcCancel := context.WithTimeout(context.Background(), backend.Options.Timeout) + defer grpcCancel() + + conn, err := grpc.DialContext(grpcCtx, grpcSrvAddr, opts...) if err != nil { - return fmt.Errorf("fail to dial: %w", err) + if errors.Is(err, context.DeadlineExceeded) { + return fmt.Errorf("fail to connect to %s within %s", grpcSrvAddr, backend.Options.Timeout) + } + return fmt.Errorf("fail to connect to %s: %w", grpcSrvAddr, err) } - defer conn.Close() - grpcCtx := context.Background() - resp, err := healthpb.NewHealthClient(conn).Check(grpcCtx, &healthpb.HealthCheckRequest{}) if err != nil { if stat, ok := status.FromError(err); ok && stat.Code() == codes.Unimplemented { - return fmt.Errorf("the server doesn't implement the grpc health protocol") + return fmt.Errorf("the server doesn't implement the gRPC health protocol") + } + if stat, ok := status.FromError(err); ok && stat.Code() == codes.DeadlineExceeded { + return fmt.Errorf("gRPC health check timeout") } return fmt.Errorf("gRPC request failed %w", err) } From d7190b593acc79eed036adf3fe7a04a730565729 Mon Sep 17 00:00:00 2001 From: Jeremy JACQUE Date: Sun, 28 Nov 2021 15:32:04 +0100 Subject: [PATCH 10/17] fix typos and improve healthcheck doc for gRPC --- docs/content/routing/services/index.md | 2 +- pkg/healthcheck/healthcheck_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/content/routing/services/index.md b/docs/content/routing/services/index.md index 66f76b6c84..133b2bf881 100644 --- a/docs/content/routing/services/index.md +++ b/docs/content/routing/services/index.md @@ -317,7 +317,7 @@ On subsequent requests, to keep the session alive with the same server, the clie Configure health check to remove unhealthy servers from the load balancing rotation. Traefik will consider your HTTP(s) servers healthy as long as they return status codes between `2XX` and `3XX` to the health check requests (carried out every `interval`). -For gRPC servers, Traefik will consider them healthy as long as they return `SERVING` to gRPC health check v1 requests). You have to define the `scheme`to `grpc`. +For gRPC servers, Traefik will consider them healthy as long as they return `SERVING` to [gRPC health check v1](https://github.com/grpc/grpc/blob/master/doc/health-checking.md) requests. You have to define the `scheme` to `grpc`. To propagate status changes (e.g. all servers of this service are down) upwards, HealthCheck must also be enabled on the parent(s) of this service. diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go index f943365299..857256a3a6 100644 --- a/pkg/healthcheck/healthcheck_test.go +++ b/pkg/healthcheck/healthcheck_test.go @@ -690,9 +690,9 @@ func newTestServerGrpc(t *testing.T, healthSequence []gRPCCheckStatus, gRPCListe err := server.Serve((*gRPCListener)) assert.NoError(t, err) }() - // Waitt for gRPC server to be ready + // Wait for gRPC server to be ready time.Sleep(800 * time.Millisecond) - // Once the test server received tthe expectted number of queries, cancel the context + // Once the test server received the expected number of queries, cancel the context go func() { for { time.Sleep(gRPChealthCheckInterval + healthCheckTimeout) From cd7de06c4145db7ba66146caa446a4ad28c274e5 Mon Sep 17 00:00:00 2001 From: Jeremy JACQUE Date: Thu, 15 Sep 2022 19:48:54 +0200 Subject: [PATCH 11/17] Add a new mode option for LB healthCheck, document and test it --- docs/content/reference/dynamic-configuration/file.toml | 1 + docs/content/reference/dynamic-configuration/kv-ref.md | 1 + docs/content/routing/services/index.md | 3 ++- pkg/config/dynamic/fixtures/sample.toml | 1 + pkg/config/dynamic/http_config.go | 1 + pkg/config/label/label_test.go | 4 ++++ pkg/healthcheck/healthcheck.go | 10 +++++++--- pkg/healthcheck/healthcheck_test.go | 2 +- pkg/provider/kv/kv_test.go | 2 ++ pkg/server/service/service.go | 10 ++++++++++ 10 files changed, 30 insertions(+), 5 deletions(-) diff --git a/docs/content/reference/dynamic-configuration/file.toml b/docs/content/reference/dynamic-configuration/file.toml index 895a1f97f1..4634afea2c 100644 --- a/docs/content/reference/dynamic-configuration/file.toml +++ b/docs/content/reference/dynamic-configuration/file.toml @@ -53,6 +53,7 @@ url = "foobar" [http.services.Service01.loadBalancer.healthCheck] scheme = "foobar" + mode = "foobar" path = "foobar" method = "foobar" port = 42 diff --git a/docs/content/reference/dynamic-configuration/kv-ref.md b/docs/content/reference/dynamic-configuration/kv-ref.md index c12162ccc8..533350cbbd 100644 --- a/docs/content/reference/dynamic-configuration/kv-ref.md +++ b/docs/content/reference/dynamic-configuration/kv-ref.md @@ -211,6 +211,7 @@ | `traefik/http/services/Service01/loadBalancer/healthCheck/path` | `foobar` | | `traefik/http/services/Service01/loadBalancer/healthCheck/port` | `42` | | `traefik/http/services/Service01/loadBalancer/healthCheck/scheme` | `foobar` | +| `traefik/http/services/Service01/loadBalancer/healthCheck/mode` | `foobar` | | `traefik/http/services/Service01/loadBalancer/healthCheck/timeout` | `foobar` | | `traefik/http/services/Service01/loadBalancer/passHostHeader` | `true` | | `traefik/http/services/Service01/loadBalancer/responseForwarding/flushInterval` | `foobar` | diff --git a/docs/content/routing/services/index.md b/docs/content/routing/services/index.md index 133b2bf881..e9bcdfc492 100644 --- a/docs/content/routing/services/index.md +++ b/docs/content/routing/services/index.md @@ -317,7 +317,7 @@ On subsequent requests, to keep the session alive with the same server, the clie Configure health check to remove unhealthy servers from the load balancing rotation. Traefik will consider your HTTP(s) servers healthy as long as they return status codes between `2XX` and `3XX` to the health check requests (carried out every `interval`). -For gRPC servers, Traefik will consider them healthy as long as they return `SERVING` to [gRPC health check v1](https://github.com/grpc/grpc/blob/master/doc/health-checking.md) requests. You have to define the `scheme` to `grpc`. +For gRPC servers, Traefik will consider them healthy as long as they return `SERVING` to [gRPC health check v1](https://github.com/grpc/grpc/blob/master/doc/health-checking.md) requests. You have to define the `mode` to `grpc`. To propagate status changes (e.g. all servers of this service are down) upwards, HealthCheck must also be enabled on the parent(s) of this service. @@ -325,6 +325,7 @@ Below are the available options for the health check mechanism: - `path` (required), defines the server URL path for the health check endpoint . - `scheme` (optional), replaces the server URL `scheme` for the health check endpoint. +- `mode` (default: http), if defined to `grpc`, will use the gRPC health check protocol to probe the server. - `hostname` (optional), sets the value of `hostname` in the `Host` header of the health check request. - `port` (optional), replaces the server URL `port` for the health check endpoint. - `interval` (default: 30s), defines the frequency of the health check calls. diff --git a/pkg/config/dynamic/fixtures/sample.toml b/pkg/config/dynamic/fixtures/sample.toml index fdde014c76..11e586ea0a 100644 --- a/pkg/config/dynamic/fixtures/sample.toml +++ b/pkg/config/dynamic/fixtures/sample.toml @@ -422,6 +422,7 @@ url = "foobar" [http.services.Service0.loadBalancer.healthCheck] scheme = "foobar" + mode = "foobar" path = "foobar" port = 42 interval = "foobar" diff --git a/pkg/config/dynamic/http_config.go b/pkg/config/dynamic/http_config.go index 0fa03feba9..85b6b92c27 100644 --- a/pkg/config/dynamic/http_config.go +++ b/pkg/config/dynamic/http_config.go @@ -213,6 +213,7 @@ func (s *Server) SetDefaults() { // ServerHealthCheck holds the HealthCheck configuration. type ServerHealthCheck struct { Scheme string `json:"scheme,omitempty" toml:"scheme,omitempty" yaml:"scheme,omitempty" export:"true"` + Mode string `json:"mode,omitempty" toml:"mode,omitempty" yaml:"mode,omitempty" export:"true"` Path string `json:"path,omitempty" toml:"path,omitempty" yaml:"path,omitempty" export:"true"` Method string `json:"method,omitempty" toml:"method,omitempty" yaml:"method,omitempty" export:"true"` Port int `json:"port,omitempty" toml:"port,omitempty,omitzero" yaml:"port,omitempty" export:"true"` diff --git a/pkg/config/label/label_test.go b/pkg/config/label/label_test.go index 04bce464ee..d9b40ffb86 100644 --- a/pkg/config/label/label_test.go +++ b/pkg/config/label/label_test.go @@ -153,6 +153,7 @@ func TestDecodeConfiguration(t *testing.T) { "traefik.http.services.Service0.loadbalancer.healthcheck.method": "foobar", "traefik.http.services.Service0.loadbalancer.healthcheck.port": "42", "traefik.http.services.Service0.loadbalancer.healthcheck.scheme": "foobar", + "traefik.http.services.Service0.loadbalancer.healthcheck.mode": "foobar", "traefik.http.services.Service0.loadbalancer.healthcheck.timeout": "foobar", "traefik.http.services.Service0.loadbalancer.healthcheck.followredirects": "true", "traefik.http.services.Service0.loadbalancer.passhostheader": "true", @@ -169,6 +170,7 @@ func TestDecodeConfiguration(t *testing.T) { "traefik.http.services.Service1.loadbalancer.healthcheck.method": "foobar", "traefik.http.services.Service1.loadbalancer.healthcheck.port": "42", "traefik.http.services.Service1.loadbalancer.healthcheck.scheme": "foobar", + "traefik.http.services.Service1.loadbalancer.healthcheck.mode": "foobar", "traefik.http.services.Service1.loadbalancer.healthcheck.timeout": "foobar", "traefik.http.services.Service1.loadbalancer.healthcheck.followredirects": "true", "traefik.http.services.Service1.loadbalancer.passhostheader": "true", @@ -650,6 +652,7 @@ func TestDecodeConfiguration(t *testing.T) { }, HealthCheck: &dynamic.ServerHealthCheck{ Scheme: "foobar", + Mode: "foobar", Path: "foobar", Method: "foobar", Port: 42, @@ -678,6 +681,7 @@ func TestDecodeConfiguration(t *testing.T) { }, HealthCheck: &dynamic.ServerHealthCheck{ Scheme: "foobar", + Mode: "foobar", Path: "foobar", Method: "foobar", Port: 42, diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index f70af2cd27..8838a74228 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/pkg/healthcheck/healthcheck.go @@ -64,6 +64,7 @@ type Options struct { Headers map[string]string Hostname string Scheme string + Mode string Path string Method string Port int @@ -295,8 +296,11 @@ func checkHealthGrpc(serverURL *url.URL, backend *BackendConfig) error { } grpcSrvAddr := u.Hostname() + ":" + u.Port() - opts := []grpc.DialOption{ - grpc.WithInsecure(), + opts := []grpc.DialOption{} + for _, insecureScheme := range []string{"http", "h2c", ""} { + if strings.Compare(backend.Options.Scheme, insecureScheme) == 0 { + opts = append(opts, grpc.WithInsecure()) + } } grpcCtx, grpcCancel := context.WithTimeout(context.Background(), backend.Options.Timeout) @@ -333,7 +337,7 @@ func checkHealthGrpc(serverURL *url.URL, backend *BackendConfig) error { // scheme declared in the backend config options. // defaults to HTTP. func checkHealth(serverURL *url.URL, backend *BackendConfig) error { - if strings.Compare(backend.Options.Scheme, "grpc") == 0 { + if strings.Compare(backend.Options.Mode, "grpc") == 0 { return checkHealthGrpc(serverURL, backend) } return checkHealthHTTP(serverURL, backend) diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go index 857256a3a6..e35e085fd7 100644 --- a/pkg/healthcheck/healthcheck_test.go +++ b/pkg/healthcheck/healthcheck_test.go @@ -246,7 +246,7 @@ func TestSetGrpcBackendsConfiguration(t *testing.T) { lb := &testLoadBalancer{RWMutex: &sync.RWMutex{}} backend := NewBackendConfig(Options{ - Scheme: "grpc", + Mode: "grpc", Path: "check", Interval: gRPChealthCheckInterval, Timeout: healthCheckTimeout, diff --git a/pkg/provider/kv/kv_test.go b/pkg/provider/kv/kv_test.go index 535a48bab6..83cea3ad9e 100644 --- a/pkg/provider/kv/kv_test.go +++ b/pkg/provider/kv/kv_test.go @@ -48,6 +48,7 @@ func Test_buildConfiguration(t *testing.T) { "traefik/http/services/Service01/loadBalancer/healthCheck/headers/name0": "foobar", "traefik/http/services/Service01/loadBalancer/healthCheck/headers/name1": "foobar", "traefik/http/services/Service01/loadBalancer/healthCheck/scheme": "foobar", + "traefik/http/services/Service01/loadBalancer/healthCheck/mode": "foobar", "traefik/http/services/Service01/loadBalancer/healthCheck/followredirects": "true", "traefik/http/services/Service01/loadBalancer/responseForwarding/flushInterval": "foobar", "traefik/http/services/Service01/loadBalancer/passHostHeader": "true", @@ -642,6 +643,7 @@ func Test_buildConfiguration(t *testing.T) { }, HealthCheck: &dynamic.ServerHealthCheck{ Scheme: "foobar", + Mode: "foobar", Path: "foobar", Port: 42, Interval: "foobar", diff --git a/pkg/server/service/service.go b/pkg/server/service/service.go index 67c2d97d12..fef481c3f7 100644 --- a/pkg/server/service/service.go +++ b/pkg/server/service/service.go @@ -360,8 +360,18 @@ func buildHealthCheckOptions(ctx context.Context, lb healthcheck.Balancer, backe followRedirects = *hc.FollowRedirects } + mode := "http" + if hc.Mode != "" { + if hc.Mode != "grpc" && hc.Mode != "http" { + logger.Errorf("Illegal health check mode for backend '%s'", backend) + } else { + mode = hc.Mode + } + } + return &healthcheck.Options{ Scheme: hc.Scheme, + Mode: mode, Path: hc.Path, Method: hc.Method, Port: hc.Port, From 885f87e53b33d021ee23ce5cefeee3bda7b6b1fe Mon Sep 17 00:00:00 2001 From: Julien Salleyron Date: Fri, 16 Sep 2022 17:28:31 +0200 Subject: [PATCH 12/17] review. --- pkg/healthcheck/healthcheck.go | 4 +- pkg/healthcheck/healthcheck_test.go | 305 +++++++++++----------------- 2 files changed, 124 insertions(+), 185 deletions(-) diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index 8838a74228..47b86abd8e 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/pkg/healthcheck/healthcheck.go @@ -298,7 +298,7 @@ func checkHealthGrpc(serverURL *url.URL, backend *BackendConfig) error { opts := []grpc.DialOption{} for _, insecureScheme := range []string{"http", "h2c", ""} { - if strings.Compare(backend.Options.Scheme, insecureScheme) == 0 { + if backend.Options.Scheme == insecureScheme { opts = append(opts, grpc.WithInsecure()) } } @@ -337,7 +337,7 @@ func checkHealthGrpc(serverURL *url.URL, backend *BackendConfig) error { // scheme declared in the backend config options. // defaults to HTTP. func checkHealth(serverURL *url.URL, backend *BackendConfig) error { - if strings.Compare(backend.Options.Mode, "grpc") == 0 { + if backend.Options.Mode == "grpc" { return checkHealthGrpc(serverURL, backend) } return checkHealthHTTP(serverURL, backend) diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go index e35e085fd7..f640884d8f 100644 --- a/pkg/healthcheck/healthcheck_test.go +++ b/pkg/healthcheck/healthcheck_test.go @@ -20,28 +20,102 @@ import ( ) const ( - healthCheckInterval = 200 * time.Millisecond - gRPChealthCheckInterval = 500 * time.Millisecond - healthCheckTimeout = 100 * time.Millisecond + healthCheckInterval = 200 * time.Millisecond + healthCheckTimeout = 100 * time.Millisecond ) -type gRPCCheckStatus = healthpb.HealthCheckResponse_ServingStatus +func newTestGrpcServer(t *testing.T, done func(), healthSequence []int) string { + t.Helper() + + gRPCListener, err := net.Listen("tcp4", "127.0.0.1:0") + assert.NoError(t, err) + t.Cleanup(func() { gRPCListener.Close() }) -type GrpcHealthChecker struct { - mu sync.RWMutex - status []gRPCCheckStatus - statusIdx int + server := grpc.NewServer() + t.Cleanup(server.Stop) + + gRPCService := &HealthChecker{ + done: done, + healthSequence: healthSequence, + } + + healthpb.RegisterHealthServer(server, gRPCService) + go func() { + err := server.Serve(gRPCListener) + assert.NoError(t, err) + }() + + return "http://" + gRPCListener.Addr().String() } -type testHandler struct { +func newTestServer(t *testing.T, done func(), healthSequence []int) string { + t.Helper() + + handler := &HealthChecker{ + done: done, + healthSequence: healthSequence, + } + ts := httptest.NewServer(handler) + t.Cleanup(ts.Close) + + return ts.URL +} + +type HealthChecker struct { done func() healthSequence []int } +func (s *HealthChecker) popStatus() int { + if len(s.healthSequence) == 0 { + panic("received unexpected request") + } + + stat := s.healthSequence[0] + s.healthSequence = s.healthSequence[1:] + + return stat +} + +func (s *HealthChecker) Check(ctx context.Context, req *healthpb.HealthCheckRequest) (*healthpb.HealthCheckResponse, error) { + stat := s.popStatus() + if len(s.healthSequence) == 0 { + s.done() + } + + return &healthpb.HealthCheckResponse{ + Status: healthpb.HealthCheckResponse_ServingStatus(stat), + }, nil +} + +func (s *HealthChecker) Watch(req *healthpb.HealthCheckRequest, server healthpb.Health_WatchServer) error { + stat := s.popStatus() + if len(s.healthSequence) == 0 { + s.done() + } + + return server.Send(&healthpb.HealthCheckResponse{ + Status: healthpb.HealthCheckResponse_ServingStatus(stat), + }) +} + +// ServeHTTP returns HTTP response codes following a status sequences. +// It calls the given 'done' function once all request health indicators have been depleted. +func (s *HealthChecker) ServeHTTP(w http.ResponseWriter, r *http.Request) { + stat := s.popStatus() + + w.WriteHeader(stat) + + if len(s.healthSequence) == 0 { + s.done() + } +} + func TestSetBackendsConfiguration(t *testing.T) { testCases := []struct { desc string startHealthy bool + grpc bool healthSequence []int expectedNumRemovedServers int expectedNumUpsertedServers int @@ -103,124 +177,57 @@ func TestSetBackendsConfiguration(t *testing.T) { expectedNumUpsertedServers: 1, expectedGaugeValue: 1, }, - } - - for _, test := range testCases { - test := test - t.Run(test.desc, func(t *testing.T) { - t.Parallel() - - // The context is passed to the health check and canonically canceled by - // the test server once all expected requests have been received. - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - ts := newTestServer(cancel, test.healthSequence) - defer ts.Close() - - lb := &testLoadBalancer{RWMutex: &sync.RWMutex{}} - backend := NewBackendConfig(Options{ - Path: "/path", - Interval: healthCheckInterval, - Timeout: healthCheckTimeout, - LB: lb, - }, "backendName") - - serverURL := testhelpers.MustParseURL(ts.URL) - if test.startHealthy { - lb.servers = append(lb.servers, serverURL) - } else { - backend.disabledURLs = append(backend.disabledURLs, backendURL{url: serverURL, weight: 1}) - } - - collectingMetrics := &testhelpers.CollectingGauge{} - check := HealthCheck{ - Backends: make(map[string]*BackendConfig), - metrics: metricsHealthcheck{serverUpGauge: collectingMetrics}, - } - - wg := sync.WaitGroup{} - wg.Add(1) - - go func() { - check.execute(ctx, backend) - wg.Done() - }() - - // Make test timeout dependent on number of expected requests, health - // check interval, and a safety margin. - timeout := time.Duration(len(test.healthSequence)*int(healthCheckInterval) + 500) - select { - case <-time.After(timeout): - t.Fatal("test did not complete in time") - case <-ctx.Done(): - wg.Wait() - } - - lb.Lock() - defer lb.Unlock() - - assert.Equal(t, test.expectedNumRemovedServers, lb.numRemovedServers, "removed servers") - assert.Equal(t, test.expectedNumUpsertedServers, lb.numUpsertedServers, "upserted servers") - assert.Equal(t, test.expectedGaugeValue, collectingMetrics.GaugeValue, "ServerUp Gauge") - }) - } -} - -func TestSetGrpcBackendsConfiguration(t *testing.T) { - testCases := []struct { - desc string - startHealthy bool - healthSequence []gRPCCheckStatus - expectedNumRemovedServers int - expectedNumUpsertedServers int - expectedGaugeValue float64 - }{ { - desc: "healthy server staying healthy", + desc: "healthy grpc server staying healthy", + grpc: true, startHealthy: true, - healthSequence: []gRPCCheckStatus{ - healthpb.HealthCheckResponse_SERVING, + healthSequence: []int{ + int(healthpb.HealthCheckResponse_SERVING), }, expectedNumRemovedServers: 0, expectedNumUpsertedServers: 0, expectedGaugeValue: 1, }, { - desc: "healthy server becoming sick", + desc: "healthy grpc server becoming sick", + grpc: true, startHealthy: true, - healthSequence: []gRPCCheckStatus{ - healthpb.HealthCheckResponse_NOT_SERVING, + healthSequence: []int{ + int(healthpb.HealthCheckResponse_NOT_SERVING), }, expectedNumRemovedServers: 1, expectedNumUpsertedServers: 0, expectedGaugeValue: 0, }, { - desc: "sick server becoming healthy", + desc: "sick grpc server becoming healthy", + grpc: true, startHealthy: false, - healthSequence: []gRPCCheckStatus{ - healthpb.HealthCheckResponse_SERVING, + healthSequence: []int{ + int(healthpb.HealthCheckResponse_SERVING), }, expectedNumRemovedServers: 0, expectedNumUpsertedServers: 1, expectedGaugeValue: 1, }, { - desc: "sick server staying sick", + desc: "sick grpc server staying sick", + grpc: true, startHealthy: false, - healthSequence: []gRPCCheckStatus{ - healthpb.HealthCheckResponse_NOT_SERVING, + healthSequence: []int{ + int(healthpb.HealthCheckResponse_NOT_SERVING), }, expectedNumRemovedServers: 0, expectedNumUpsertedServers: 0, expectedGaugeValue: 0, }, { - desc: "healthy server toggling to sick and back to healthy", + desc: "healthy grpc server toggling to sick and back to healthy", + grpc: true, startHealthy: true, - healthSequence: []gRPCCheckStatus{ - healthpb.HealthCheckResponse_NOT_SERVING, - healthpb.HealthCheckResponse_SERVING, + healthSequence: []int{ + int(healthpb.HealthCheckResponse_NOT_SERVING), + int(healthpb.HealthCheckResponse_SERVING), }, expectedNumRemovedServers: 1, expectedNumUpsertedServers: 1, @@ -235,25 +242,28 @@ func TestSetGrpcBackendsConfiguration(t *testing.T) { // The context is passed to the health check and canonically canceled by // the test server once all expected requests have been received. - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - gRPCListener, err := net.Listen("tcp4", "127.0.0.1:0") - assert.NoError(t, err) - defer gRPCListener.Close() - ts := newTestServerGrpc(t, test.healthSequence, &gRPCListener, cancel) - defer ts.Stop() - addr := gRPCListener.Addr().String() lb := &testLoadBalancer{RWMutex: &sync.RWMutex{}} backend := NewBackendConfig(Options{ - Mode: "grpc", - Path: "check", - Interval: gRPChealthCheckInterval, + Path: "/path", + Interval: healthCheckInterval, Timeout: healthCheckTimeout, LB: lb, - }, "gRPCbackendName") + }, "backendName") + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + var url string + if test.grpc { + backend.Mode = "grpc" + url = newTestGrpcServer(t, cancel, test.healthSequence) + } else { + url = newTestServer(t, cancel, test.healthSequence) - serverURL := testhelpers.MustParseURL("http://" + addr) + } + + serverURL := testhelpers.MustParseURL(url) if test.startHealthy { lb.servers = append(lb.servers, serverURL) } else { @@ -276,13 +286,14 @@ func TestSetGrpcBackendsConfiguration(t *testing.T) { // Make test timeout dependent on number of expected requests, health // check interval, and a safety margin. - timeout := time.Duration(len(test.healthSequence)*int(gRPChealthCheckInterval) + int(time.Second)) + timeout := time.Duration(len(test.healthSequence)*int(healthCheckInterval) + 500) select { case <-time.After(timeout): t.Fatal("test did not complete in time") case <-ctx.Done(): wg.Wait() } + lb.Lock() defer lb.Unlock() @@ -649,78 +660,6 @@ func (lb *testLoadBalancer) removeServer(u *url.URL) { lb.servers = append(lb.servers[:i], lb.servers[i+1:]...) } -func newTestServer(done func(), healthSequence []int) *httptest.Server { - handler := &testHandler{ - done: done, - healthSequence: healthSequence, - } - return httptest.NewServer(handler) -} - -func (s *GrpcHealthChecker) Check(ctx context.Context, req *healthpb.HealthCheckRequest) (*healthpb.HealthCheckResponse, error) { - s.mu.Lock() - defer s.mu.Unlock() - stat := s.status[s.statusIdx] - if s.statusIdx < len(s.status)-1 { - s.statusIdx++ - } - return &healthpb.HealthCheckResponse{ - Status: stat, - }, nil -} - -func (s *GrpcHealthChecker) Watch(req *healthpb.HealthCheckRequest, server healthpb.Health_WatchServer) error { - s.mu.Lock() - defer s.mu.Unlock() - stat := s.status[s.statusIdx] - if s.statusIdx < len(s.status)-1 { - s.statusIdx++ - } - return server.Send(&healthpb.HealthCheckResponse{ - Status: stat, - }) -} - -func newTestServerGrpc(t *testing.T, healthSequence []gRPCCheckStatus, gRPCListener *net.Listener, done func()) *grpc.Server { - t.Helper() - server := grpc.NewServer() - gRPCService := &GrpcHealthChecker{status: healthSequence, statusIdx: 0} - healthpb.RegisterHealthServer(server, gRPCService) - go func() { - err := server.Serve((*gRPCListener)) - assert.NoError(t, err) - }() - // Wait for gRPC server to be ready - time.Sleep(800 * time.Millisecond) - // Once the test server received the expected number of queries, cancel the context - go func() { - for { - time.Sleep(gRPChealthCheckInterval + healthCheckTimeout) - gRPCService.mu.RLock() - if gRPCService.statusIdx == len(gRPCService.status)-1 { - done() - } - gRPCService.mu.RUnlock() - } - }() - return server -} - -// ServeHTTP returns HTTP response codes following a status sequences. -// It calls the given 'done' function once all request health indicators have been depleted. -func (th *testHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - if len(th.healthSequence) == 0 { - panic("received unexpected request") - } - - w.WriteHeader(th.healthSequence[0]) - - th.healthSequence = th.healthSequence[1:] - if len(th.healthSequence) == 0 { - th.done() - } -} - func TestLBStatusUpdater(t *testing.T) { lb := &testLoadBalancer{RWMutex: &sync.RWMutex{}} svInfo := &runtime.ServiceInfo{} From 63d2664b102cb651401cf3747933d8f20fd22265 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 17 Sep 2022 09:37:26 +0200 Subject: [PATCH 13/17] review --- pkg/healthcheck/healthcheck_test.go | 245 +++++----------------------- pkg/healthcheck/mock_test.go | 205 +++++++++++++++++++++++ 2 files changed, 245 insertions(+), 205 deletions(-) create mode 100644 pkg/healthcheck/mock_test.go diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go index f640884d8f..43941033a4 100644 --- a/pkg/healthcheck/healthcheck_test.go +++ b/pkg/healthcheck/healthcheck_test.go @@ -2,7 +2,6 @@ package healthcheck import ( "context" - "net" "net/http" "net/http/httptest" "net/url" @@ -15,7 +14,6 @@ import ( "github.com/traefik/traefik/v2/pkg/config/runtime" "github.com/traefik/traefik/v2/pkg/testhelpers" "github.com/vulcand/oxy/roundrobin" - "google.golang.org/grpc" healthpb "google.golang.org/grpc/health/grpc_health_v1" ) @@ -24,99 +22,12 @@ const ( healthCheckTimeout = 100 * time.Millisecond ) -func newTestGrpcServer(t *testing.T, done func(), healthSequence []int) string { - t.Helper() - - gRPCListener, err := net.Listen("tcp4", "127.0.0.1:0") - assert.NoError(t, err) - t.Cleanup(func() { gRPCListener.Close() }) - - server := grpc.NewServer() - t.Cleanup(server.Stop) - - gRPCService := &HealthChecker{ - done: done, - healthSequence: healthSequence, - } - - healthpb.RegisterHealthServer(server, gRPCService) - go func() { - err := server.Serve(gRPCListener) - assert.NoError(t, err) - }() - - return "http://" + gRPCListener.Addr().String() -} - -func newTestServer(t *testing.T, done func(), healthSequence []int) string { - t.Helper() - - handler := &HealthChecker{ - done: done, - healthSequence: healthSequence, - } - ts := httptest.NewServer(handler) - t.Cleanup(ts.Close) - - return ts.URL -} - -type HealthChecker struct { - done func() - healthSequence []int -} - -func (s *HealthChecker) popStatus() int { - if len(s.healthSequence) == 0 { - panic("received unexpected request") - } - - stat := s.healthSequence[0] - s.healthSequence = s.healthSequence[1:] - - return stat -} - -func (s *HealthChecker) Check(ctx context.Context, req *healthpb.HealthCheckRequest) (*healthpb.HealthCheckResponse, error) { - stat := s.popStatus() - if len(s.healthSequence) == 0 { - s.done() - } - - return &healthpb.HealthCheckResponse{ - Status: healthpb.HealthCheckResponse_ServingStatus(stat), - }, nil -} - -func (s *HealthChecker) Watch(req *healthpb.HealthCheckRequest, server healthpb.Health_WatchServer) error { - stat := s.popStatus() - if len(s.healthSequence) == 0 { - s.done() - } - - return server.Send(&healthpb.HealthCheckResponse{ - Status: healthpb.HealthCheckResponse_ServingStatus(stat), - }) -} - -// ServeHTTP returns HTTP response codes following a status sequences. -// It calls the given 'done' function once all request health indicators have been depleted. -func (s *HealthChecker) ServeHTTP(w http.ResponseWriter, r *http.Request) { - stat := s.popStatus() - - w.WriteHeader(stat) - - if len(s.healthSequence) == 0 { - s.done() - } -} - func TestSetBackendsConfiguration(t *testing.T) { testCases := []struct { desc string startHealthy bool - grpc bool - healthSequence []int + mode string + server StartTestServer expectedNumRemovedServers int expectedNumUpsertedServers int expectedGaugeValue float64 @@ -124,7 +35,7 @@ func TestSetBackendsConfiguration(t *testing.T) { { desc: "healthy server staying healthy", startHealthy: true, - healthSequence: []int{http.StatusOK}, + server: newHTTPServer(http.StatusOK), expectedNumRemovedServers: 0, expectedNumUpsertedServers: 0, expectedGaugeValue: 1, @@ -132,7 +43,7 @@ func TestSetBackendsConfiguration(t *testing.T) { { desc: "healthy server staying healthy (StatusNoContent)", startHealthy: true, - healthSequence: []int{http.StatusNoContent}, + server: newHTTPServer(http.StatusNoContent), expectedNumRemovedServers: 0, expectedNumUpsertedServers: 0, expectedGaugeValue: 1, @@ -140,7 +51,7 @@ func TestSetBackendsConfiguration(t *testing.T) { { desc: "healthy server staying healthy (StatusPermanentRedirect)", startHealthy: true, - healthSequence: []int{http.StatusPermanentRedirect}, + server: newHTTPServer(http.StatusPermanentRedirect), expectedNumRemovedServers: 0, expectedNumUpsertedServers: 0, expectedGaugeValue: 1, @@ -148,7 +59,7 @@ func TestSetBackendsConfiguration(t *testing.T) { { desc: "healthy server becoming sick", startHealthy: true, - healthSequence: []int{http.StatusServiceUnavailable}, + server: newHTTPServer(http.StatusServiceUnavailable), expectedNumRemovedServers: 1, expectedNumUpsertedServers: 0, expectedGaugeValue: 0, @@ -156,7 +67,7 @@ func TestSetBackendsConfiguration(t *testing.T) { { desc: "sick server becoming healthy", startHealthy: false, - healthSequence: []int{http.StatusOK}, + server: newHTTPServer(http.StatusOK), expectedNumRemovedServers: 0, expectedNumUpsertedServers: 1, expectedGaugeValue: 1, @@ -164,7 +75,7 @@ func TestSetBackendsConfiguration(t *testing.T) { { desc: "sick server staying sick", startHealthy: false, - healthSequence: []int{http.StatusServiceUnavailable}, + server: newHTTPServer(http.StatusServiceUnavailable), expectedNumRemovedServers: 0, expectedNumUpsertedServers: 0, expectedGaugeValue: 0, @@ -172,63 +83,52 @@ func TestSetBackendsConfiguration(t *testing.T) { { desc: "healthy server toggling to sick and back to healthy", startHealthy: true, - healthSequence: []int{http.StatusServiceUnavailable, http.StatusOK}, + server: newHTTPServer(http.StatusServiceUnavailable, http.StatusOK), expectedNumRemovedServers: 1, expectedNumUpsertedServers: 1, expectedGaugeValue: 1, }, { - desc: "healthy grpc server staying healthy", - grpc: true, - startHealthy: true, - healthSequence: []int{ - int(healthpb.HealthCheckResponse_SERVING), - }, + desc: "healthy grpc server staying healthy", + mode: "grpc", + startHealthy: true, + server: newGRPCServer(healthpb.HealthCheckResponse_SERVING), expectedNumRemovedServers: 0, expectedNumUpsertedServers: 0, expectedGaugeValue: 1, }, { - desc: "healthy grpc server becoming sick", - grpc: true, - startHealthy: true, - healthSequence: []int{ - int(healthpb.HealthCheckResponse_NOT_SERVING), - }, + desc: "healthy grpc server becoming sick", + mode: "grpc", + startHealthy: true, + server: newGRPCServer(healthpb.HealthCheckResponse_NOT_SERVING), expectedNumRemovedServers: 1, expectedNumUpsertedServers: 0, expectedGaugeValue: 0, }, { - desc: "sick grpc server becoming healthy", - grpc: true, - startHealthy: false, - healthSequence: []int{ - int(healthpb.HealthCheckResponse_SERVING), - }, + desc: "sick grpc server becoming healthy", + mode: "grpc", + startHealthy: false, + server: newGRPCServer(healthpb.HealthCheckResponse_SERVING), expectedNumRemovedServers: 0, expectedNumUpsertedServers: 1, expectedGaugeValue: 1, }, { - desc: "sick grpc server staying sick", - grpc: true, - startHealthy: false, - healthSequence: []int{ - int(healthpb.HealthCheckResponse_NOT_SERVING), - }, + desc: "sick grpc server staying sick", + mode: "grpc", + startHealthy: false, + server: newGRPCServer(healthpb.HealthCheckResponse_NOT_SERVING), expectedNumRemovedServers: 0, expectedNumUpsertedServers: 0, expectedGaugeValue: 0, }, { - desc: "healthy grpc server toggling to sick and back to healthy", - grpc: true, - startHealthy: true, - healthSequence: []int{ - int(healthpb.HealthCheckResponse_NOT_SERVING), - int(healthpb.HealthCheckResponse_SERVING), - }, + desc: "healthy grpc server toggling to sick and back to healthy", + mode: "grpc", + startHealthy: true, + server: newGRPCServer(healthpb.HealthCheckResponse_NOT_SERVING, healthpb.HealthCheckResponse_SERVING), expectedNumRemovedServers: 1, expectedNumUpsertedServers: 1, expectedGaugeValue: 1, @@ -240,30 +140,24 @@ func TestSetBackendsConfiguration(t *testing.T) { t.Run(test.desc, func(t *testing.T) { t.Parallel() - // The context is passed to the health check and canonically canceled by - // the test server once all expected requests have been received. + // The context is passed to the health check and + // canonically canceled by the test server once all expected requests have been received. + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + serverURL, timeout := test.server.Start(t, cancel) lb := &testLoadBalancer{RWMutex: &sync.RWMutex{}} - backend := NewBackendConfig(Options{ + + options := Options{ + Mode: test.mode, Path: "/path", Interval: healthCheckInterval, Timeout: healthCheckTimeout, LB: lb, - }, "backendName") - - ctx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) - - var url string - if test.grpc { - backend.Mode = "grpc" - url = newTestGrpcServer(t, cancel, test.healthSequence) - } else { - url = newTestServer(t, cancel, test.healthSequence) - } + backend := NewBackendConfig(options, "backendName") - serverURL := testhelpers.MustParseURL(url) if test.startHealthy { lb.servers = append(lb.servers, serverURL) } else { @@ -271,6 +165,7 @@ func TestSetBackendsConfiguration(t *testing.T) { } collectingMetrics := &testhelpers.CollectingGauge{} + check := HealthCheck{ Backends: make(map[string]*BackendConfig), metrics: metricsHealthcheck{serverUpGauge: collectingMetrics}, @@ -284,9 +179,6 @@ func TestSetBackendsConfiguration(t *testing.T) { wg.Done() }() - // Make test timeout dependent on number of expected requests, health - // check interval, and a safety margin. - timeout := time.Duration(len(test.healthSequence)*int(healthCheckInterval) + 500) select { case <-time.After(timeout): t.Fatal("test did not complete in time") @@ -603,63 +495,6 @@ func TestBalancers_RemoveServer(t *testing.T) { assert.Equal(t, 0, len(balancer2.Servers())) } -type testLoadBalancer struct { - // RWMutex needed due to parallel test execution: Both the system-under-test - // and the test assertions reference the counters. - *sync.RWMutex - numRemovedServers int - numUpsertedServers int - servers []*url.URL - // options is just to make sure that LBStatusUpdater forwards options on Upsert to its BalancerHandler - options []roundrobin.ServerOption -} - -func (lb *testLoadBalancer) ServeHTTP(w http.ResponseWriter, req *http.Request) { - // noop -} - -func (lb *testLoadBalancer) RemoveServer(u *url.URL) error { - lb.Lock() - defer lb.Unlock() - lb.numRemovedServers++ - lb.removeServer(u) - return nil -} - -func (lb *testLoadBalancer) UpsertServer(u *url.URL, options ...roundrobin.ServerOption) error { - lb.Lock() - defer lb.Unlock() - lb.numUpsertedServers++ - lb.servers = append(lb.servers, u) - lb.options = append(lb.options, options...) - return nil -} - -func (lb *testLoadBalancer) Servers() []*url.URL { - return lb.servers -} - -func (lb *testLoadBalancer) Options() []roundrobin.ServerOption { - return lb.options -} - -func (lb *testLoadBalancer) removeServer(u *url.URL) { - var i int - var serverURL *url.URL - found := false - for i, serverURL = range lb.servers { - if *serverURL == *u { - found = true - break - } - } - if !found { - return - } - - lb.servers = append(lb.servers[:i], lb.servers[i+1:]...) -} - func TestLBStatusUpdater(t *testing.T) { lb := &testLoadBalancer{RWMutex: &sync.RWMutex{}} svInfo := &runtime.ServiceInfo{} diff --git a/pkg/healthcheck/mock_test.go b/pkg/healthcheck/mock_test.go new file mode 100644 index 0000000000..19e60b15c6 --- /dev/null +++ b/pkg/healthcheck/mock_test.go @@ -0,0 +1,205 @@ +package healthcheck + +import ( + "context" + "net" + "net/http" + "net/http/httptest" + "net/url" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/traefik/traefik/v2/pkg/testhelpers" + "github.com/vulcand/oxy/roundrobin" + "google.golang.org/grpc" + healthpb "google.golang.org/grpc/health/grpc_health_v1" +) + +type StartTestServer interface { + Start(t *testing.T, done func()) (*url.URL, time.Duration) +} + +type Status interface { + ~int | ~int32 +} + +type HealthSequence[T Status] struct { + sequenceMu sync.Mutex + sequence []T +} + +func (s *HealthSequence[T]) Pop() T { + s.sequenceMu.Lock() + defer s.sequenceMu.Unlock() + + stat := s.sequence[0] + + s.sequence = s.sequence[1:] + + return stat +} + +func (s *HealthSequence[T]) IsEmpty() bool { + s.sequenceMu.Lock() + defer s.sequenceMu.Unlock() + + return len(s.sequence) == 0 +} + +type GRPCServer struct { + status HealthSequence[healthpb.HealthCheckResponse_ServingStatus] + done func() +} + +func newGRPCServer(healthSequence ...healthpb.HealthCheckResponse_ServingStatus) *GRPCServer { + gRPCService := &GRPCServer{ + status: HealthSequence[healthpb.HealthCheckResponse_ServingStatus]{ + sequence: healthSequence, + }, + } + + return gRPCService +} + +func (s *GRPCServer) Check(_ context.Context, _ *healthpb.HealthCheckRequest) (*healthpb.HealthCheckResponse, error) { + stat := s.status.Pop() + if s.status.IsEmpty() { + s.done() + } + + return &healthpb.HealthCheckResponse{ + Status: stat, + }, nil +} + +func (s *GRPCServer) Watch(_ *healthpb.HealthCheckRequest, server healthpb.Health_WatchServer) error { + stat := s.status.Pop() + if s.status.IsEmpty() { + s.done() + } + + return server.Send(&healthpb.HealthCheckResponse{ + Status: stat, + }) +} + +func (s *GRPCServer) Start(t *testing.T, done func()) (*url.URL, time.Duration) { + t.Helper() + + listener, err := net.Listen("tcp4", "127.0.0.1:0") + assert.NoError(t, err) + t.Cleanup(func() { _ = listener.Close() }) + + server := grpc.NewServer() + t.Cleanup(server.Stop) + + s.done = done + + healthpb.RegisterHealthServer(server, s) + + go func() { + err := server.Serve(listener) + assert.NoError(t, err) + }() + + // Make test timeout dependent on number of expected requests, health check interval, and a safety margin. + return testhelpers.MustParseURL("http://" + listener.Addr().String()), time.Duration(len(s.status.sequence)*int(healthCheckInterval) + 500) +} + +type HTTPServer struct { + status HealthSequence[int] + done func() +} + +func newHTTPServer(healthSequence ...int) *HTTPServer { + handler := &HTTPServer{ + status: HealthSequence[int]{ + sequence: healthSequence, + }, + } + + return handler +} + +// ServeHTTP returns HTTP response codes following a status sequences. +// It calls the given 'done' function once all request health indicators have been depleted. +func (s *HTTPServer) ServeHTTP(w http.ResponseWriter, _ *http.Request) { + stat := s.status.Pop() + + w.WriteHeader(stat) + + if s.status.IsEmpty() { + s.done() + } +} + +func (s *HTTPServer) Start(t *testing.T, done func()) (*url.URL, time.Duration) { + t.Helper() + + s.done = done + + ts := httptest.NewServer(s) + t.Cleanup(ts.Close) + + // Make test timeout dependent on number of expected requests, health check interval, and a safety margin. + return testhelpers.MustParseURL(ts.URL), time.Duration(len(s.status.sequence)*int(healthCheckInterval) + 500) +} + +type testLoadBalancer struct { + // RWMutex needed due to parallel test execution: Both the system-under-test + // and the test assertions reference the counters. + *sync.RWMutex + numRemovedServers int + numUpsertedServers int + servers []*url.URL + // options is just to make sure that LBStatusUpdater forwards options on Upsert to its BalancerHandler + options []roundrobin.ServerOption +} + +func (lb *testLoadBalancer) ServeHTTP(w http.ResponseWriter, req *http.Request) { + // noop +} + +func (lb *testLoadBalancer) RemoveServer(u *url.URL) error { + lb.Lock() + defer lb.Unlock() + lb.numRemovedServers++ + lb.removeServer(u) + return nil +} + +func (lb *testLoadBalancer) UpsertServer(u *url.URL, options ...roundrobin.ServerOption) error { + lb.Lock() + defer lb.Unlock() + lb.numUpsertedServers++ + lb.servers = append(lb.servers, u) + lb.options = append(lb.options, options...) + return nil +} + +func (lb *testLoadBalancer) Servers() []*url.URL { + return lb.servers +} + +func (lb *testLoadBalancer) Options() []roundrobin.ServerOption { + return lb.options +} + +func (lb *testLoadBalancer) removeServer(u *url.URL) { + var i int + var serverURL *url.URL + found := false + for i, serverURL = range lb.servers { + if *serverURL == *u { + found = true + break + } + } + if !found { + return + } + + lb.servers = append(lb.servers[:i], lb.servers[i+1:]...) +} From 296cf3db3e0ed09f9362c4b8af3690d4d5df7dfa Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 19 Sep 2022 10:12:27 +0200 Subject: [PATCH 14/17] review: generate --- docs/content/reference/dynamic-configuration/kv-ref.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/reference/dynamic-configuration/kv-ref.md b/docs/content/reference/dynamic-configuration/kv-ref.md index 533350cbbd..f72e024e41 100644 --- a/docs/content/reference/dynamic-configuration/kv-ref.md +++ b/docs/content/reference/dynamic-configuration/kv-ref.md @@ -208,10 +208,10 @@ | `traefik/http/services/Service01/loadBalancer/healthCheck/hostname` | `foobar` | | `traefik/http/services/Service01/loadBalancer/healthCheck/interval` | `foobar` | | `traefik/http/services/Service01/loadBalancer/healthCheck/method` | `foobar` | +| `traefik/http/services/Service01/loadBalancer/healthCheck/mode` | `foobar` | | `traefik/http/services/Service01/loadBalancer/healthCheck/path` | `foobar` | | `traefik/http/services/Service01/loadBalancer/healthCheck/port` | `42` | | `traefik/http/services/Service01/loadBalancer/healthCheck/scheme` | `foobar` | -| `traefik/http/services/Service01/loadBalancer/healthCheck/mode` | `foobar` | | `traefik/http/services/Service01/loadBalancer/healthCheck/timeout` | `foobar` | | `traefik/http/services/Service01/loadBalancer/passHostHeader` | `true` | | `traefik/http/services/Service01/loadBalancer/responseForwarding/flushInterval` | `foobar` | From f1e1b6ab23921f19c3d8eda0336bf2f93163f0cb Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 19 Sep 2022 10:18:26 +0200 Subject: [PATCH 15/17] review --- .../dynamic-configuration/docker-labels.yml | 1 + .../reference/dynamic-configuration/file.yaml | 1 + .../marathon-labels.json | 1 + pkg/healthcheck/healthcheck.go | 56 ++++++++++--------- 4 files changed, 33 insertions(+), 26 deletions(-) diff --git a/docs/content/reference/dynamic-configuration/docker-labels.yml b/docs/content/reference/dynamic-configuration/docker-labels.yml index c413f27bd8..f4ddee380e 100644 --- a/docs/content/reference/dynamic-configuration/docker-labels.yml +++ b/docs/content/reference/dynamic-configuration/docker-labels.yml @@ -154,6 +154,7 @@ - "traefik.http.services.service01.loadbalancer.healthcheck.method=foobar" - "traefik.http.services.service01.loadbalancer.healthcheck.port=42" - "traefik.http.services.service01.loadbalancer.healthcheck.scheme=foobar" +- "traefik.http.services.service01.loadbalancer.healthcheck.mode=foobar" - "traefik.http.services.service01.loadbalancer.healthcheck.timeout=foobar" - "traefik.http.services.service01.loadbalancer.passhostheader=true" - "traefik.http.services.service01.loadbalancer.responseforwarding.flushinterval=foobar" diff --git a/docs/content/reference/dynamic-configuration/file.yaml b/docs/content/reference/dynamic-configuration/file.yaml index ec30817783..807bc8a14e 100644 --- a/docs/content/reference/dynamic-configuration/file.yaml +++ b/docs/content/reference/dynamic-configuration/file.yaml @@ -58,6 +58,7 @@ http: - url: foobar healthCheck: scheme: foobar + mode: foobar path: foobar method: foobar port: 42 diff --git a/docs/content/reference/dynamic-configuration/marathon-labels.json b/docs/content/reference/dynamic-configuration/marathon-labels.json index 5da0048199..7966f27f71 100644 --- a/docs/content/reference/dynamic-configuration/marathon-labels.json +++ b/docs/content/reference/dynamic-configuration/marathon-labels.json @@ -154,6 +154,7 @@ "traefik.http.services.service01.loadbalancer.healthcheck.method": "foobar", "traefik.http.services.service01.loadbalancer.healthcheck.port": "42", "traefik.http.services.service01.loadbalancer.healthcheck.scheme": "foobar", +"traefik.http.services.service01.loadbalancer.healthcheck.mode": "foobar", "traefik.http.services.service01.loadbalancer.healthcheck.timeout": "foobar", "traefik.http.services.service01.loadbalancer.passhostheader": "true", "traefik.http.services.service01.loadbalancer.responseforwarding.flushinterval": "foobar", diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index 47b86abd8e..bd91183fe2 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/pkg/healthcheck/healthcheck.go @@ -250,9 +250,19 @@ func NewBackendConfig(options Options, backendName string) *BackendConfig { } } +// checkHealth calls the proper health check function depending on the +// scheme declared in the backend config options. +// defaults to HTTP. +func checkHealth(serverURL *url.URL, backend *BackendConfig) error { + if backend.Options.Mode == "grpc" { + return checkHealthGRPC(serverURL, backend) + } + return checkHealthHTTP(serverURL, backend) +} + // checkHealthHTTP returns a nil error in case it was successful and otherwise // a non-nil error with a meaningful description why the health check failed. -// Dedicatted to HTTP servers. +// Dedicated to HTTP servers. func checkHealthHTTP(serverURL *url.URL, backend *BackendConfig) error { req, err := backend.newRequest(serverURL) if err != nil { @@ -286,43 +296,47 @@ func checkHealthHTTP(serverURL *url.URL, backend *BackendConfig) error { return nil } -// checkHealthGrpc returns a nil error in case it was successful and otherwise +// checkHealthGRPC returns a nil error in case it was successful and otherwise // a non-nil error with a meaningful description why the health check failed. -// Dedicatted to gRPC servers implementing gRPC Health Checking Protocol v1. -func checkHealthGrpc(serverURL *url.URL, backend *BackendConfig) error { +// Dedicated to gRPC servers implementing gRPC Health Checking Protocol v1. +func checkHealthGRPC(serverURL *url.URL, backend *BackendConfig) error { u, err := serverURL.Parse(backend.Path) if err != nil { return fmt.Errorf("failed to parse serverURL: %w", err) } + grpcSrvAddr := u.Hostname() + ":" + u.Port() - opts := []grpc.DialOption{} + var opts []grpc.DialOption for _, insecureScheme := range []string{"http", "h2c", ""} { if backend.Options.Scheme == insecureScheme { opts = append(opts, grpc.WithInsecure()) } } - grpcCtx, grpcCancel := context.WithTimeout(context.Background(), backend.Options.Timeout) - defer grpcCancel() + ctx, cancel := context.WithTimeout(context.Background(), backend.Options.Timeout) + defer cancel() - conn, err := grpc.DialContext(grpcCtx, grpcSrvAddr, opts...) + conn, err := grpc.DialContext(ctx, grpcSrvAddr, opts...) if err != nil { if errors.Is(err, context.DeadlineExceeded) { - return fmt.Errorf("fail to connect to %s within %s", grpcSrvAddr, backend.Options.Timeout) + return fmt.Errorf("fail to connect to %s within %s: %w", grpcSrvAddr, backend.Options.Timeout, err) } return fmt.Errorf("fail to connect to %s: %w", grpcSrvAddr, err) } - defer conn.Close() + defer func() { _ = conn.Close() }() - resp, err := healthpb.NewHealthClient(conn).Check(grpcCtx, &healthpb.HealthCheckRequest{}) + resp, err := healthpb.NewHealthClient(conn).Check(ctx, &healthpb.HealthCheckRequest{}) if err != nil { - if stat, ok := status.FromError(err); ok && stat.Code() == codes.Unimplemented { - return fmt.Errorf("the server doesn't implement the gRPC health protocol") - } - if stat, ok := status.FromError(err); ok && stat.Code() == codes.DeadlineExceeded { - return fmt.Errorf("gRPC health check timeout") + if stat, ok := status.FromError(err); ok { + switch stat.Code() { + case codes.Unimplemented: + return fmt.Errorf("the server doesn't implement the gRPC health protocol: %w", err) + case codes.DeadlineExceeded: + return fmt.Errorf("gRPC health check timeout: %w", err) + } } + return fmt.Errorf("gRPC request failed %w", err) } @@ -333,16 +347,6 @@ func checkHealthGrpc(serverURL *url.URL, backend *BackendConfig) error { return nil } -// checkHealth calls the proper health check function depending on the -// scheme declared in the backend config options. -// defaults to HTTP. -func checkHealth(serverURL *url.URL, backend *BackendConfig) error { - if backend.Options.Mode == "grpc" { - return checkHealthGrpc(serverURL, backend) - } - return checkHealthHTTP(serverURL, backend) -} - // StatusUpdater should be implemented by a service that, when its status // changes (e.g. all if its children are down), needs to propagate upwards (to // their parent(s)) that change. From f8ac97bf9ce7bc19dea7b0ee612a9bfaed7ba745 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 19 Sep 2022 10:32:42 +0200 Subject: [PATCH 16/17] review --- pkg/config/dynamic/http_config.go | 1 + pkg/healthcheck/healthcheck.go | 11 ++++++----- pkg/server/service/service.go | 10 ++++------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/config/dynamic/http_config.go b/pkg/config/dynamic/http_config.go index 85b6b92c27..376223e1c5 100644 --- a/pkg/config/dynamic/http_config.go +++ b/pkg/config/dynamic/http_config.go @@ -230,6 +230,7 @@ type ServerHealthCheck struct { func (h *ServerHealthCheck) SetDefaults() { fr := true h.FollowRedirects = &fr + h.Mode = "http" } // +k8s:deepcopy-gen=true diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index bd91183fe2..36e90d4e6a 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/pkg/healthcheck/healthcheck.go @@ -302,27 +302,28 @@ func checkHealthHTTP(serverURL *url.URL, backend *BackendConfig) error { func checkHealthGRPC(serverURL *url.URL, backend *BackendConfig) error { u, err := serverURL.Parse(backend.Path) if err != nil { - return fmt.Errorf("failed to parse serverURL: %w", err) + return fmt.Errorf("failed to parse server URL: %w", err) } - grpcSrvAddr := u.Hostname() + ":" + u.Port() + serverAddr := u.Hostname() + ":" + u.Port() var opts []grpc.DialOption for _, insecureScheme := range []string{"http", "h2c", ""} { if backend.Options.Scheme == insecureScheme { opts = append(opts, grpc.WithInsecure()) + break } } ctx, cancel := context.WithTimeout(context.Background(), backend.Options.Timeout) defer cancel() - conn, err := grpc.DialContext(ctx, grpcSrvAddr, opts...) + conn, err := grpc.DialContext(ctx, serverAddr, opts...) if err != nil { if errors.Is(err, context.DeadlineExceeded) { - return fmt.Errorf("fail to connect to %s within %s: %w", grpcSrvAddr, backend.Options.Timeout, err) + return fmt.Errorf("fail to connect to %s within %s: %w", serverAddr, backend.Options.Timeout, err) } - return fmt.Errorf("fail to connect to %s: %w", grpcSrvAddr, err) + return fmt.Errorf("fail to connect to %s: %w", serverAddr, err) } defer func() { _ = conn.Close() }() diff --git a/pkg/server/service/service.go b/pkg/server/service/service.go index fef481c3f7..dce9fcb2fe 100644 --- a/pkg/server/service/service.go +++ b/pkg/server/service/service.go @@ -361,12 +361,10 @@ func buildHealthCheckOptions(ctx context.Context, lb healthcheck.Balancer, backe } mode := "http" - if hc.Mode != "" { - if hc.Mode != "grpc" && hc.Mode != "http" { - logger.Errorf("Illegal health check mode for backend '%s'", backend) - } else { - mode = hc.Mode - } + if hc.Mode != "" && hc.Mode != "grpc" && hc.Mode != "http" { + logger.Errorf("Illegal health check mode for backend '%s'", backend) + } else { + mode = hc.Mode } return &healthcheck.Options{ From 6b8c133a2848b0aeab5eccb5ca5ffe92cb63f24d Mon Sep 17 00:00:00 2001 From: kevinpollet Date: Tue, 20 Sep 2022 16:25:36 +0200 Subject: [PATCH 17/17] review --- docs/content/routing/services/index.md | 2 +- pkg/healthcheck/healthcheck.go | 35 +++++++++++++++----------- pkg/server/service/service.go | 11 +++++--- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/docs/content/routing/services/index.md b/docs/content/routing/services/index.md index e9bcdfc492..7186765f1c 100644 --- a/docs/content/routing/services/index.md +++ b/docs/content/routing/services/index.md @@ -317,7 +317,7 @@ On subsequent requests, to keep the session alive with the same server, the clie Configure health check to remove unhealthy servers from the load balancing rotation. Traefik will consider your HTTP(s) servers healthy as long as they return status codes between `2XX` and `3XX` to the health check requests (carried out every `interval`). -For gRPC servers, Traefik will consider them healthy as long as they return `SERVING` to [gRPC health check v1](https://github.com/grpc/grpc/blob/master/doc/health-checking.md) requests. You have to define the `mode` to `grpc`. +For gRPC servers, Traefik will consider them healthy as long as they return `SERVING` to [gRPC health check v1](https://github.com/grpc/grpc/blob/master/doc/health-checking.md) requests. To propagate status changes (e.g. all servers of this service are down) upwards, HealthCheck must also be enabled on the parent(s) of this service. diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index 36e90d4e6a..36178ce985 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/pkg/healthcheck/healthcheck.go @@ -30,6 +30,11 @@ const ( serverDown = "DOWN" ) +const ( + HTTPMode = "http" + GRPCMode = "grpc" +) + var ( singleton *HealthCheck once sync.Once @@ -251,17 +256,15 @@ func NewBackendConfig(options Options, backendName string) *BackendConfig { } // checkHealth calls the proper health check function depending on the -// scheme declared in the backend config options. -// defaults to HTTP. +// backend config mode, defaults to HTTP. func checkHealth(serverURL *url.URL, backend *BackendConfig) error { - if backend.Options.Mode == "grpc" { + if backend.Options.Mode == GRPCMode { return checkHealthGRPC(serverURL, backend) } return checkHealthHTTP(serverURL, backend) } -// checkHealthHTTP returns a nil error in case it was successful and otherwise -// a non-nil error with a meaningful description why the health check failed. +// checkHealthHTTP returns an error with a meaningful description if the health check failed. // Dedicated to HTTP servers. func checkHealthHTTP(serverURL *url.URL, backend *BackendConfig) error { req, err := backend.newRequest(serverURL) @@ -296,8 +299,7 @@ func checkHealthHTTP(serverURL *url.URL, backend *BackendConfig) error { return nil } -// checkHealthGRPC returns a nil error in case it was successful and otherwise -// a non-nil error with a meaningful description why the health check failed. +// checkHealthGRPC returns an error with a meaningful description if the health check failed. // Dedicated to gRPC servers implementing gRPC Health Checking Protocol v1. func checkHealthGRPC(serverURL *url.URL, backend *BackendConfig) error { u, err := serverURL.Parse(backend.Path) @@ -305,14 +307,17 @@ func checkHealthGRPC(serverURL *url.URL, backend *BackendConfig) error { return fmt.Errorf("failed to parse server URL: %w", err) } - serverAddr := u.Hostname() + ":" + u.Port() + port := u.Port() + if backend.Options.Port != 0 { + port = strconv.Itoa(backend.Options.Port) + } + + serverAddr := net.JoinHostPort(u.Hostname(), port) var opts []grpc.DialOption - for _, insecureScheme := range []string{"http", "h2c", ""} { - if backend.Options.Scheme == insecureScheme { - opts = append(opts, grpc.WithInsecure()) - break - } + switch backend.Options.Scheme { + case "http", "h2c", "": + opts = append(opts, grpc.WithInsecure()) } ctx, cancel := context.WithTimeout(context.Background(), backend.Options.Timeout) @@ -332,13 +337,13 @@ func checkHealthGRPC(serverURL *url.URL, backend *BackendConfig) error { if stat, ok := status.FromError(err); ok { switch stat.Code() { case codes.Unimplemented: - return fmt.Errorf("the server doesn't implement the gRPC health protocol: %w", err) + return fmt.Errorf("gRPC server does not implement the health protocol: %w", err) case codes.DeadlineExceeded: return fmt.Errorf("gRPC health check timeout: %w", err) } } - return fmt.Errorf("gRPC request failed %w", err) + return fmt.Errorf("gRPC health check failed: %w", err) } if resp.Status != healthpb.HealthCheckResponse_SERVING { diff --git a/pkg/server/service/service.go b/pkg/server/service/service.go index dce9fcb2fe..2c46da74f0 100644 --- a/pkg/server/service/service.go +++ b/pkg/server/service/service.go @@ -360,11 +360,14 @@ func buildHealthCheckOptions(ctx context.Context, lb healthcheck.Balancer, backe followRedirects = *hc.FollowRedirects } - mode := "http" - if hc.Mode != "" && hc.Mode != "grpc" && hc.Mode != "http" { - logger.Errorf("Illegal health check mode for backend '%s'", backend) - } else { + mode := healthcheck.HTTPMode + switch hc.Mode { + case "": + mode = healthcheck.HTTPMode + case healthcheck.GRPCMode, healthcheck.HTTPMode: mode = hc.Mode + default: + logger.Errorf("Illegal health check mode for backend '%s'", backend) } return &healthcheck.Options{