Skip to content

Commit

Permalink
Support Envoy's MaxEjectionPercent and BaseEjectionTime config entrie…
Browse files Browse the repository at this point in the history
…s for passive health checks (#15979)

* Add MaxEjectionPercent to config entry

* Add BaseEjectionTime to config entry

* Add MaxEjectionPercent and BaseEjectionTime to protobufs

* Add MaxEjectionPercent and BaseEjectionTime to api

* Fix integration test breakage

* Verify MaxEjectionPercent and BaseEjectionTime in integration test upstream confings

* Website docs for MaxEjectionPercent and BaseEjection time

* Add `make docs` to browse docs at http://localhost:3000

* Changelog entry

* so that is the difference between consul-docker and dev-docker

* blah

* update proto funcs

* update proto

---------

Co-authored-by: Maliz <maliheh.monshizadeh@hashicorp.com>
  • Loading branch information
analogue and Maliz authored Apr 26, 2023
1 parent 80b1dbc commit 5eaeb7b
Show file tree
Hide file tree
Showing 21 changed files with 775 additions and 493 deletions.
3 changes: 3 additions & 0 deletions .changelog/15979.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
envoy: add `MaxEjectionPercent` and `BaseEjectionTime` to passive health check configs.
```
6 changes: 6 additions & 0 deletions GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ ui-build-image:
@echo "Building UI build container"
@docker build $(NOCACHE) $(QUIET) -t $(UI_BUILD_TAG) - < build-support/docker/Build-UI.dockerfile

# Builds consul in a docker container and then dumps executable into ./pkg/bin/...
consul-docker: go-build-image
@$(SHELL) $(CURDIR)/build-support/scripts/build-docker.sh consul

Expand Down Expand Up @@ -538,6 +539,11 @@ envoy-regen:
@find "command/connect/envoy/testdata" -name '*.golden' -delete
@go test -tags '$(GOTAGS)' ./command/connect/envoy -update

# Point your web browser to http://localhost:3000/consul to live render docs from ./website/
.PHONY: docs
docs:
make -C website

.PHONY: help
help:
$(info available make targets)
Expand Down
10 changes: 10 additions & 0 deletions agent/consul/config_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1484,6 +1484,8 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) {
Interval: 10,
MaxFailures: 2,
EnforcingConsecutive5xx: uintPointer(60),
MaxEjectionPercent: uintPointer(61),
BaseEjectionTime: durationPointer(62 * time.Second),
},
},
Overrides: []*structs.UpstreamConfig{
Expand Down Expand Up @@ -1518,6 +1520,8 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) {
"Interval": int64(10),
"MaxFailures": int64(2),
"EnforcingConsecutive5xx": int64(60),
"MaxEjectionPercent": int64(61),
"BaseEjectionTime": uint64(62 * time.Second),
},
"mesh_gateway": map[string]interface{}{
"Mode": "none",
Expand All @@ -1532,6 +1536,8 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) {
"Interval": int64(10),
"MaxFailures": int64(2),
"EnforcingConsecutive5xx": int64(60),
"MaxEjectionPercent": int64(61),
"BaseEjectionTime": uint64(62 * time.Second),
},
"mesh_gateway": map[string]interface{}{
"Mode": "local",
Expand Down Expand Up @@ -2639,3 +2645,7 @@ func Test_gateWriteToSecondary_AllowedKinds(t *testing.T) {
func uintPointer(v uint32) *uint32 {
return &v
}

func durationPointer(d time.Duration) *time.Duration {
return &d
}
9 changes: 9 additions & 0 deletions agent/proxycfg/proxycfg.deepcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/proto/private/pbpeering"
"github.com/hashicorp/consul/types"
"time"
)

// DeepCopy generates a deep copy of *ConfigSnapshot
Expand Down Expand Up @@ -452,6 +453,14 @@ func (o *configSnapshotIngressGateway) DeepCopy() *configSnapshotIngressGateway
cp.Defaults.PassiveHealthCheck.EnforcingConsecutive5xx = new(uint32)
*cp.Defaults.PassiveHealthCheck.EnforcingConsecutive5xx = *o.Defaults.PassiveHealthCheck.EnforcingConsecutive5xx
}
if o.Defaults.PassiveHealthCheck.MaxEjectionPercent != nil {
cp.Defaults.PassiveHealthCheck.MaxEjectionPercent = new(uint32)
*cp.Defaults.PassiveHealthCheck.MaxEjectionPercent = *o.Defaults.PassiveHealthCheck.MaxEjectionPercent
}
if o.Defaults.PassiveHealthCheck.BaseEjectionTime != nil {
cp.Defaults.PassiveHealthCheck.BaseEjectionTime = new(time.Duration)
*cp.Defaults.PassiveHealthCheck.BaseEjectionTime = *o.Defaults.PassiveHealthCheck.BaseEjectionTime
}
}
return &cp
}
Expand Down
19 changes: 19 additions & 0 deletions agent/structs/config_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,16 @@ type PassiveHealthCheck struct {
// when an outlier status is detected through consecutive 5xx.
// This setting can be used to disable ejection or to ramp it up slowly. Defaults to 100.
EnforcingConsecutive5xx *uint32 `json:",omitempty" alias:"enforcing_consecutive_5xx"`

// The maximum % of an upstream cluster that can be ejected due to outlier detection.
// Defaults to 10% but will eject at least one host regardless of the value.
// TODO: remove me
MaxEjectionPercent *uint32 `json:",omitempty" alias:"max_ejection_percent"`

// The base time that a host is ejected for. The real time is equal to the base time
// multiplied by the number of times the host has been ejected and is capped by
// max_ejection_time (Default 300s). Defaults to 30000ms or 30s.
BaseEjectionTime *time.Duration `json:",omitempty" alias:"base_ejection_time"`
}

func (chk *PassiveHealthCheck) Clone() *PassiveHealthCheck {
Expand All @@ -1120,6 +1130,15 @@ func (chk PassiveHealthCheck) Validate() error {
if chk.Interval < 0*time.Second {
return fmt.Errorf("passive health check interval cannot be negative")
}
if chk.EnforcingConsecutive5xx != nil && *chk.EnforcingConsecutive5xx > 100 {
return fmt.Errorf("passive health check enforcing_consecutive_5xx must be a percentage between 0 and 100")
}
if chk.MaxEjectionPercent != nil && *chk.MaxEjectionPercent > 100 {
return fmt.Errorf("passive health check max_ejection_percent must be a percentage between 0 and 100")
}
if chk.BaseEjectionTime != nil && *chk.BaseEjectionTime < 0*time.Second {
return fmt.Errorf("passive health check base_ejection_time cannot be negative")
}
return nil
}

Expand Down
115 changes: 93 additions & 22 deletions agent/structs/config_entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,9 @@ func TestDecodeConfigEntry(t *testing.T) {
passive_health_check {
interval = "2s"
max_failures = 3
enforcing_consecutive_5xx = 4
max_ejection_percent = 5
base_ejection_time = "6s"
}
},
{
Expand Down Expand Up @@ -460,6 +463,9 @@ func TestDecodeConfigEntry(t *testing.T) {
PassiveHealthCheck {
MaxFailures = 3
Interval = "2s"
EnforcingConsecutive5xx = 4
MaxEjectionPercent = 5
BaseEjectionTime = "6s"
}
},
{
Expand Down Expand Up @@ -502,8 +508,11 @@ func TestDecodeConfigEntry(t *testing.T) {
{
Name: "redis",
PassiveHealthCheck: &PassiveHealthCheck{
MaxFailures: 3,
Interval: 2 * time.Second,
MaxFailures: 3,
Interval: 2 * time.Second,
EnforcingConsecutive5xx: uintPointer(4),
MaxEjectionPercent: uintPointer(5),
BaseEjectionTime: durationPointer(6 * time.Second),
},
},
{
Expand Down Expand Up @@ -2351,6 +2360,39 @@ func TestPassiveHealthCheck_Validate(t *testing.T) {
wantErr: true,
wantMsg: "cannot be negative",
},
{
name: "valid enforcing_consecutive_5xx",
input: PassiveHealthCheck{EnforcingConsecutive5xx: uintPointer(100)},
wantErr: false,
},
{
name: "invalid enforcing_consecutive_5xx",
input: PassiveHealthCheck{EnforcingConsecutive5xx: uintPointer(101)},
wantErr: true,
wantMsg: "must be a percentage",
},
{
name: "valid max_ejection_percent",
input: PassiveHealthCheck{MaxEjectionPercent: uintPointer(100)},
wantErr: false,
},
{
name: "invalid max_ejection_percent",
input: PassiveHealthCheck{MaxEjectionPercent: uintPointer(101)},
wantErr: true,
wantMsg: "must be a percentage",
},
{
name: "valid base_ejection_time",
input: PassiveHealthCheck{BaseEjectionTime: durationPointer(0 * time.Second)},
wantErr: false,
},
{
name: "negative base_ejection_time",
input: PassiveHealthCheck{BaseEjectionTime: durationPointer(-1 * time.Second)},
wantErr: true,
wantMsg: "cannot be negative",
},
}

for _, tc := range tt {
Expand Down Expand Up @@ -2890,8 +2932,11 @@ func TestUpstreamConfig_MergeInto(t *testing.T) {
MaxConcurrentRequests: intPointer(5),
},
PassiveHealthCheck: &PassiveHealthCheck{
MaxFailures: 3,
Interval: 2 * time.Second,
Interval: 2 * time.Second,
MaxFailures: 3,
EnforcingConsecutive5xx: uintPointer(4),
MaxEjectionPercent: uintPointer(5),
BaseEjectionTime: durationPointer(6 * time.Second),
},
MeshGateway: MeshGatewayConfig{Mode: MeshGatewayModeRemote},
},
Expand All @@ -2908,8 +2953,11 @@ func TestUpstreamConfig_MergeInto(t *testing.T) {
MaxConcurrentRequests: intPointer(5),
},
"passive_health_check": &PassiveHealthCheck{
MaxFailures: 3,
Interval: 2 * time.Second,
Interval: 2 * time.Second,
MaxFailures: 3,
EnforcingConsecutive5xx: uintPointer(4),
MaxEjectionPercent: uintPointer(5),
BaseEjectionTime: durationPointer(6 * time.Second),
},
"mesh_gateway": MeshGatewayConfig{Mode: MeshGatewayModeRemote},
},
Expand All @@ -2928,8 +2976,11 @@ func TestUpstreamConfig_MergeInto(t *testing.T) {
MaxConcurrentRequests: intPointer(5),
},
PassiveHealthCheck: &PassiveHealthCheck{
MaxFailures: 3,
Interval: 2 * time.Second,
Interval: 2 * time.Second,
MaxFailures: 3,
EnforcingConsecutive5xx: uintPointer(4),
MaxEjectionPercent: uintPointer(5),
BaseEjectionTime: durationPointer(6 * time.Second),
},
MeshGateway: MeshGatewayConfig{Mode: MeshGatewayModeRemote},
},
Expand All @@ -2945,8 +2996,11 @@ func TestUpstreamConfig_MergeInto(t *testing.T) {
MaxConcurrentRequests: intPointer(12),
},
"passive_health_check": &PassiveHealthCheck{
MaxFailures: 13,
Interval: 14 * time.Second,
MaxFailures: 13,
Interval: 14 * time.Second,
EnforcingConsecutive5xx: uintPointer(15),
MaxEjectionPercent: uintPointer(16),
BaseEjectionTime: durationPointer(17 * time.Second),
},
"mesh_gateway": MeshGatewayConfig{Mode: MeshGatewayModeLocal},
},
Expand All @@ -2962,8 +3016,11 @@ func TestUpstreamConfig_MergeInto(t *testing.T) {
MaxConcurrentRequests: intPointer(5),
},
"passive_health_check": &PassiveHealthCheck{
MaxFailures: 3,
Interval: 2 * time.Second,
Interval: 2 * time.Second,
MaxFailures: 3,
EnforcingConsecutive5xx: uintPointer(4),
MaxEjectionPercent: uintPointer(5),
BaseEjectionTime: durationPointer(6 * time.Second),
},
"mesh_gateway": MeshGatewayConfig{Mode: MeshGatewayModeRemote},
},
Expand All @@ -2985,7 +3042,9 @@ func TestUpstreamConfig_MergeInto(t *testing.T) {
"passive_health_check": &PassiveHealthCheck{
MaxFailures: 13,
Interval: 14 * time.Second,
EnforcingConsecutive5xx: uintPointer(80),
EnforcingConsecutive5xx: uintPointer(15),
MaxEjectionPercent: uintPointer(16),
BaseEjectionTime: durationPointer(17 * time.Second),
},
"mesh_gateway": MeshGatewayConfig{Mode: MeshGatewayModeLocal},
},
Expand All @@ -3003,7 +3062,9 @@ func TestUpstreamConfig_MergeInto(t *testing.T) {
"passive_health_check": &PassiveHealthCheck{
MaxFailures: 13,
Interval: 14 * time.Second,
EnforcingConsecutive5xx: uintPointer(80),
EnforcingConsecutive5xx: uintPointer(15),
MaxEjectionPercent: uintPointer(16),
BaseEjectionTime: durationPointer(17 * time.Second),
},
"mesh_gateway": MeshGatewayConfig{Mode: MeshGatewayModeLocal},
},
Expand Down Expand Up @@ -3138,15 +3199,21 @@ func TestParseUpstreamConfig(t *testing.T) {
name: "passive health check map",
input: map[string]interface{}{
"passive_health_check": map[string]interface{}{
"interval": "22s",
"max_failures": 7,
"interval": "22s",
"max_failures": 7,
"enforcing_consecutive_5xx": 8,
"max_ejection_percent": 9,
"base_ejection_time": "10s",
},
},
want: UpstreamConfig{
ConnectTimeoutMs: 5000,
PassiveHealthCheck: &PassiveHealthCheck{
Interval: 22 * time.Second,
MaxFailures: 7,
Interval: 22 * time.Second,
MaxFailures: 7,
EnforcingConsecutive5xx: uintPointer(8),
MaxEjectionPercent: uintPointer(9),
BaseEjectionTime: durationPointer(10 * time.Second),
},
Protocol: "tcp",
},
Expand Down Expand Up @@ -3273,10 +3340,6 @@ func requireContainsLower(t *testing.T, haystack, needle string) {
require.Contains(t, strings.ToLower(haystack), strings.ToLower(needle))
}

func intPointer(i int) *int {
return &i
}

func TestConfigEntryQuery_CacheInfoKey(t *testing.T) {
assertCacheInfoKeyIsComplete(t, &ConfigEntryQuery{})
}
Expand Down Expand Up @@ -3371,6 +3434,14 @@ func testConfigEntryNormalizeAndValidate(t *testing.T, cases map[string]configEn
}
}

func intPointer(i int) *int {
return &i
}

func uintPointer(v uint32) *uint32 {
return &v
}

func durationPointer(d time.Duration) *time.Duration {
return &d
}
24 changes: 24 additions & 0 deletions agent/structs/structs.deepcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,14 @@ func (o *IngressListener) DeepCopy() *IngressListener {
cp.Services[i2].PassiveHealthCheck.EnforcingConsecutive5xx = new(uint32)
*cp.Services[i2].PassiveHealthCheck.EnforcingConsecutive5xx = *o.Services[i2].PassiveHealthCheck.EnforcingConsecutive5xx
}
if o.Services[i2].PassiveHealthCheck.MaxEjectionPercent != nil {
cp.Services[i2].PassiveHealthCheck.MaxEjectionPercent = new(uint32)
*cp.Services[i2].PassiveHealthCheck.MaxEjectionPercent = *o.Services[i2].PassiveHealthCheck.MaxEjectionPercent
}
if o.Services[i2].PassiveHealthCheck.BaseEjectionTime != nil {
cp.Services[i2].PassiveHealthCheck.BaseEjectionTime = new(time.Duration)
*cp.Services[i2].PassiveHealthCheck.BaseEjectionTime = *o.Services[i2].PassiveHealthCheck.BaseEjectionTime
}
}
if o.Services[i2].Meta != nil {
cp.Services[i2].Meta = make(map[string]string, len(o.Services[i2].Meta))
Expand Down Expand Up @@ -1134,6 +1142,14 @@ func (o *UpstreamConfiguration) DeepCopy() *UpstreamConfiguration {
cp.Overrides[i2].PassiveHealthCheck.EnforcingConsecutive5xx = new(uint32)
*cp.Overrides[i2].PassiveHealthCheck.EnforcingConsecutive5xx = *o.Overrides[i2].PassiveHealthCheck.EnforcingConsecutive5xx
}
if o.Overrides[i2].PassiveHealthCheck.MaxEjectionPercent != nil {
cp.Overrides[i2].PassiveHealthCheck.MaxEjectionPercent = new(uint32)
*cp.Overrides[i2].PassiveHealthCheck.MaxEjectionPercent = *o.Overrides[i2].PassiveHealthCheck.MaxEjectionPercent
}
if o.Overrides[i2].PassiveHealthCheck.BaseEjectionTime != nil {
cp.Overrides[i2].PassiveHealthCheck.BaseEjectionTime = new(time.Duration)
*cp.Overrides[i2].PassiveHealthCheck.BaseEjectionTime = *o.Overrides[i2].PassiveHealthCheck.BaseEjectionTime
}
}
}
}
Expand Down Expand Up @@ -1164,6 +1180,14 @@ func (o *UpstreamConfiguration) DeepCopy() *UpstreamConfiguration {
cp.Defaults.PassiveHealthCheck.EnforcingConsecutive5xx = new(uint32)
*cp.Defaults.PassiveHealthCheck.EnforcingConsecutive5xx = *o.Defaults.PassiveHealthCheck.EnforcingConsecutive5xx
}
if o.Defaults.PassiveHealthCheck.MaxEjectionPercent != nil {
cp.Defaults.PassiveHealthCheck.MaxEjectionPercent = new(uint32)
*cp.Defaults.PassiveHealthCheck.MaxEjectionPercent = *o.Defaults.PassiveHealthCheck.MaxEjectionPercent
}
if o.Defaults.PassiveHealthCheck.BaseEjectionTime != nil {
cp.Defaults.PassiveHealthCheck.BaseEjectionTime = new(time.Duration)
*cp.Defaults.PassiveHealthCheck.BaseEjectionTime = *o.Defaults.PassiveHealthCheck.BaseEjectionTime
}
}
}
return &cp
Expand Down
Loading

0 comments on commit 5eaeb7b

Please sign in to comment.