diff --git a/config/schema.json b/config/schema.json index 1691affe..8c38dd07 100644 --- a/config/schema.json +++ b/config/schema.json @@ -389,6 +389,23 @@ "maximum": 100, "default": 100 }, + "consecutive5xx": { + "description": "Number of consecutive 5xx responses before ejecting an unhealthy host.", + "type": "integer", + "minimum": 0 + }, + "enforcingConsecutiveGatewayFailure": { + "description": "Percentage of consecutive gateway failures that trigger host ejection. Value between 0 and 100.", + "type": "integer", + "minimum": 0, + "maximum": 100, + "default": 100 + }, + "consecutiveGatewayFailure": { + "description": "Number of consecutive gateway failures before ejecting an unhealthy host.", + "type": "integer", + "minimum": 0 + }, "maxEjectionPercent": { "description": "Maximum percentage of hosts that can be ejected from the load balancing pool. Value between 0 and 100.", "type": "integer", diff --git a/config/types.go b/config/types.go index 2ab4547b..0e0d8c28 100644 --- a/config/types.go +++ b/config/types.go @@ -51,6 +51,7 @@ const ( DefaultOutlierDetectionInterval = 10 * time.Second DefaultOutlierDetectionMaxFailures uint32 = 5 DefaultOutlierDetectionEnforcingConsecutive5xx uint32 = 100 + DefaultOutlierDetectionEnforcingGatewayFailure uint32 = 100 DefaultOutlierDetectionMaxEjectionPercent uint32 = 50 ) @@ -637,11 +638,22 @@ func (cfg *Config) ConsulDNSEnabled() bool { } // OutlierDetectionConfig defines the passive health check settings for outlier detection. +// These fields map directly to the Consul API PassiveHealthCheck struct: +// - Interval maps to PassiveHealthCheck.Interval +// - MaxFailures maps to PassiveHealthCheck.MaxFailures +// - EnforcingConsecutive5xx maps to PassiveHealthCheck.EnforcingConsecutive5xx +// - Consecutive5xx maps to PassiveHealthCheck.Consecutive5xx +// - EnforcingConsecutiveGatewayFailure maps to PassiveHealthCheck.EnforcingConsecutiveGatewayFailure +// - ConsecutiveGatewayFailure maps to PassiveHealthCheck.ConsecutiveGatewayFailure +// - MaxEjectionPercent maps to PassiveHealthCheck.MaxEjectionPercent type OutlierDetectionConfig struct { - Interval time.Duration `json:"interval,omitempty"` - MaxFailures uint32 `json:"maxFailures,omitempty"` - EnforcingConsecutive5xx *uint32 `json:"enforcingConsecutive5xx,omitempty"` - MaxEjectionPercent *uint32 `json:"maxEjectionPercent,omitempty"` + Interval time.Duration `json:"interval,omitempty"` + MaxFailures uint32 `json:"maxFailures,omitempty"` + EnforcingConsecutive5xx *uint32 `json:"enforcingConsecutive5xx,omitempty"` + Consecutive5xx *uint32 `json:"consecutive5xx,omitempty"` + EnforcingConsecutiveGatewayFailure *uint32 `json:"enforcingConsecutiveGatewayFailure,omitempty"` + ConsecutiveGatewayFailure *uint32 `json:"consecutiveGatewayFailure,omitempty"` + MaxEjectionPercent *uint32 `json:"maxEjectionPercent,omitempty"` } // NetworkPartitionResilienceConfig defines the configuration for network partition resilience mode @@ -654,13 +666,13 @@ type NetworkPartitionResilienceConfig struct { // NewOutlierDetectionConfig returns a new OutlierDetectionConfig with default values // for network partition resilience mode. func NewOutlierDetectionConfig() *OutlierDetectionConfig { - enforcingConsecutive5xx := DefaultOutlierDetectionEnforcingConsecutive5xx + enforcingConsecutiveGatewayFailure := DefaultOutlierDetectionEnforcingGatewayFailure maxEjectionPercent := DefaultOutlierDetectionMaxEjectionPercent return &OutlierDetectionConfig{ - Interval: DefaultOutlierDetectionInterval, - MaxFailures: DefaultOutlierDetectionMaxFailures, - EnforcingConsecutive5xx: &enforcingConsecutive5xx, - MaxEjectionPercent: &maxEjectionPercent, + Interval: DefaultOutlierDetectionInterval, + MaxFailures: DefaultOutlierDetectionMaxFailures, + EnforcingConsecutiveGatewayFailure: &enforcingConsecutiveGatewayFailure, + MaxEjectionPercent: &maxEjectionPercent, } } diff --git a/go.mod b/go.mod index 8dafe307..e248c978 100644 --- a/go.mod +++ b/go.mod @@ -1,16 +1,16 @@ module github.com/hashicorp/consul-ecs -go 1.25.5 +go 1.25.7 require ( github.com/aws/aws-sdk-go v1.55.5 github.com/cenkalti/backoff/v4 v4.1.3 github.com/deckarep/golang-set v1.7.1 - github.com/google/go-cmp v0.5.9 + github.com/google/go-cmp v0.6.0 github.com/hashicorp/consul-awsauth v0.0.0-20250130185352-0a5f57fe920a github.com/hashicorp/consul-server-connection-manager v0.1.2 github.com/hashicorp/consul/api v1.26.1-rc1 - github.com/hashicorp/consul/sdk v0.16.0 + github.com/hashicorp/consul/sdk v0.17.0 github.com/hashicorp/go-hclog v1.6.3 github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-rootcerts v1.0.2 @@ -19,7 +19,7 @@ require ( github.com/miekg/dns v1.1.41 github.com/mitchellh/cli v1.1.5 github.com/mitchellh/mapstructure v1.5.0 - github.com/stretchr/testify v1.8.3 + github.com/stretchr/testify v1.8.4 github.com/xeipuuv/gojsonschema v1.2.0 ) @@ -34,6 +34,7 @@ require ( github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/fatih/color v1.16.0 // indirect + github.com/go-viper/mapstructure/v2 v2.4.0 // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/google/uuid v1.3.0 // indirect github.com/hashicorp/consul/proto-public v0.1.0 // indirect @@ -65,7 +66,7 @@ require ( github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect golang.org/x/crypto v0.45.0 // indirect - golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 // indirect + golang.org/x/exp v0.0.0-20250808145144-a408d31f581a // indirect golang.org/x/net v0.47.0 // indirect golang.org/x/sys v0.38.0 // indirect golang.org/x/text v0.31.0 // indirect @@ -74,3 +75,5 @@ require ( google.golang.org/protobuf v1.33.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +replace github.com/hashicorp/consul/api => /Users/mukul/hashicorp/consul-enterprise/api diff --git a/go.sum b/go.sum index cb5e5e3c..38377f9b 100644 --- a/go.sum +++ b/go.sum @@ -54,6 +54,8 @@ github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9 github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= +github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= +github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= @@ -75,8 +77,8 @@ github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= -github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= @@ -86,12 +88,10 @@ github.com/hashicorp/consul-awsauth v0.0.0-20250130185352-0a5f57fe920a h1:PlTwRw github.com/hashicorp/consul-awsauth v0.0.0-20250130185352-0a5f57fe920a/go.mod h1:kmtdeOrrLwhUY6PoKQf0RRYuOA8xJmalhVJBOcMsyNs= github.com/hashicorp/consul-server-connection-manager v0.1.2 h1:tNVQHUPuMbd+cMdD8kd+qkZUYpmLmrHMAV/49f4L53I= github.com/hashicorp/consul-server-connection-manager v0.1.2/go.mod h1:NzQoVi1KcxGI2SangsDue8+ZPuXZWs+6BKAKrDNyg+w= -github.com/hashicorp/consul/api v1.26.1-rc1 h1:eO+53vwWxEV2TMbTVrJbXp2TJjJNv+Nxij8j+xi3yOc= -github.com/hashicorp/consul/api v1.26.1-rc1/go.mod h1:ZKnaWXL2r23i+SPmEj1H5YsRNUS/uUzB2uhmD2QY4IY= github.com/hashicorp/consul/proto-public v0.1.0 h1:O0LSmCqydZi363hsqc6n2v5sMz3usQMXZF6ziK3SzXU= github.com/hashicorp/consul/proto-public v0.1.0/go.mod h1:vs2KkuWwtjkIgA5ezp4YKPzQp4GitV+q/+PvksrA92k= -github.com/hashicorp/consul/sdk v0.16.0 h1:SE9m0W6DEfgIVCJX7xU+iv/hUl4m/nxqMTnCdMxDpJ8= -github.com/hashicorp/consul/sdk v0.16.0/go.mod h1:7pxqqhqoaPqnBnzXD1StKed62LqJeClzVsUEy85Zr0A= +github.com/hashicorp/consul/sdk v0.17.0 h1:N/JigV6y1yEMfTIhXoW0DXUecM2grQnFuRpY7PcLHLI= +github.com/hashicorp/consul/sdk v0.17.0/go.mod h1:8dgIhY6VlPUprRH7o7UenVuFEgq017qUn3k9wS5mCt4= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= @@ -262,8 +262,8 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= -github.com/stretchr/testify v1.8.3 h1:RP3t2pwF7cMEbC1dqtB6poj3niw/9gnV4Cjg5oW5gtY= -github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= @@ -279,8 +279,8 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.45.0 h1:jMBrvKuj23MTlT0bQEOBcAE0mjg8mK9RXFhRH6nyF3Q= golang.org/x/crypto v0.45.0/go.mod h1:XTGrrkGJve7CYK7J8PEww4aY7gM3qMCElcJQ8n8JdX4= -golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 h1:m64FZMko/V45gv0bNmrNYoDEq8U5YUhetc9cBWKS1TQ= -golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63/go.mod h1:0v4NqG35kSWCMzLaMeX+IQrlSnVE/bqGSyC2cz/9Le8= +golang.org/x/exp v0.0.0-20250808145144-a408d31f581a h1:Y+7uR/b1Mw2iSXZ3G//1haIiSElDQZ8KWh0h+sZPG90= +golang.org/x/exp v0.0.0-20250808145144-a408d31f581a/go.mod h1:rT6SFzZ7oxADUDx58pcaKFTcZ+inxAa9fTrYx/uVYwg= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= diff --git a/subcommand/mesh-init/command.go b/subcommand/mesh-init/command.go index 9ff7aa43..36482aa2 100644 --- a/subcommand/mesh-init/command.go +++ b/subcommand/mesh-init/command.go @@ -582,12 +582,6 @@ func getLocalityParams(taskMeta awsutil.ECSTaskMeta) *api.Locality { // If upstreams are defined in the proxy configuration, passive health check is also // applied to those specific upstreams via UpstreamConfig.Overrides. func (c *Command) registerServiceDefaults(consulClient *api.Client, service *api.AgentService, resilience *config.NetworkPartitionResilienceConfig) error { - // Get the outlier detection config or use defaults - outlierDetectionCfg := resilience.OutlierDetection - if outlierDetectionCfg == nil { - outlierDetectionCfg = config.NewOutlierDetectionConfig() - } - // Fetch existing service defaults configuration queryOpts := &api.QueryOptions{} if service.Partition != "" { @@ -638,15 +632,42 @@ func (c *Command) registerServiceDefaults(consulClient *api.Client, service *api // Only set PassiveHealthCheck if it doesn't already exist if serviceDefaults.UpstreamConfig.Defaults.PassiveHealthCheck == nil { - // Normalize outlier detection config by applying defaults for nil pointer fields + // Get the outlier detection config or use defaults + outlierDetectionCfg := resilience.OutlierDetection + // Normalize outlier detection config by ensuring pointer fields have valid values normalizedCfg := normalizeOutlierDetectionConfig(outlierDetectionCfg) + // Build PassiveHealthCheck with only non-nil fields passiveHealthCheck := &api.PassiveHealthCheck{ - Interval: normalizedCfg.Interval, - MaxFailures: normalizedCfg.MaxFailures, - EnforcingConsecutive5xx: normalizedCfg.EnforcingConsecutive5xx, - MaxEjectionPercent: normalizedCfg.MaxEjectionPercent, + Interval: normalizedCfg.Interval, + MaxFailures: normalizedCfg.MaxFailures, + } + + // Set EnforcingConsecutive5xx if not nil and valid + if normalizedCfg.EnforcingConsecutive5xx != nil { + passiveHealthCheck.EnforcingConsecutive5xx = normalizedCfg.EnforcingConsecutive5xx + } + + // Set Consecutive5xx if not nil and valid + if normalizedCfg.Consecutive5xx != nil { + passiveHealthCheck.Consecutive5xx = normalizedCfg.Consecutive5xx + } + + // Set EnforcingConsecutiveGatewayFailure if not nil and valid + if normalizedCfg.EnforcingConsecutiveGatewayFailure != nil { + passiveHealthCheck.EnforcingConsecutiveGatewayFailure = normalizedCfg.EnforcingConsecutiveGatewayFailure + } + + // Set ConsecutiveGatewayFailure if not nil and valid + if normalizedCfg.ConsecutiveGatewayFailure != nil { + passiveHealthCheck.ConsecutiveGatewayFailure = normalizedCfg.ConsecutiveGatewayFailure + } + + // Set MaxEjectionPercent if not nil and valid + if normalizedCfg.MaxEjectionPercent != nil { + passiveHealthCheck.MaxEjectionPercent = normalizedCfg.MaxEjectionPercent } + serviceDefaults.UpstreamConfig.Defaults.PassiveHealthCheck = passiveHealthCheck c.log.Info("adding passive health check to service defaults", "service", service.Service) } else { @@ -684,37 +705,13 @@ func isConfigEntryNotFoundError(err error) bool { strings.Contains(errMsg, "no such config entry") } -// normalizeOutlierDetectionConfig ensures all pointer fields in the OutlierDetectionConfig -// have non-nil values by applying defaults where necessary. This is important because: -// 1. User-provided configurations may have nil pointer fields if not specified in JSON -// 2. The PassiveHealthCheck struct expects these values to be properly initialized -// 3. Consul API requires valid values for outlier detection parameters +// normalizeOutlierDetectionConfig ensures the OutlierDetectionConfig is properly initialized. +// If the config is nil, it returns a new config with default values. +// Otherwise, it returns the provided config as-is, allowing the caller to check +// if individual fields are nil before setting them in PassiveHealthCheck. func normalizeOutlierDetectionConfig(cfg *config.OutlierDetectionConfig) *config.OutlierDetectionConfig { if cfg == nil { return config.NewOutlierDetectionConfig() } - - // Create a new normalized config with all fields properly initialized - normalized := &config.OutlierDetectionConfig{ - Interval: cfg.Interval, - MaxFailures: cfg.MaxFailures, - } - - // Apply default for EnforcingConsecutive5xx if nil - if cfg.EnforcingConsecutive5xx == nil { - defaultVal := config.DefaultOutlierDetectionEnforcingConsecutive5xx - normalized.EnforcingConsecutive5xx = &defaultVal - } else { - normalized.EnforcingConsecutive5xx = cfg.EnforcingConsecutive5xx - } - - // Apply default for MaxEjectionPercent if nil - if cfg.MaxEjectionPercent == nil { - defaultVal := config.DefaultOutlierDetectionMaxEjectionPercent - normalized.MaxEjectionPercent = &defaultVal - } else { - normalized.MaxEjectionPercent = cfg.MaxEjectionPercent - } - - return normalized + return cfg } diff --git a/subcommand/mesh-init/command_test.go b/subcommand/mesh-init/command_test.go index f2a84a16..d03f4f44 100644 --- a/subcommand/mesh-init/command_test.go +++ b/subcommand/mesh-init/command_test.go @@ -1088,7 +1088,7 @@ func TestRegisterServiceDefaults(t *testing.T) { expectErrorMessage string expectServiceName string expectPassiveHealthCheckSet bool - expectedInterval time.Duration + validatePassiveHealthCheck func(t *testing.T, phc *api.PassiveHealthCheck) }{ "creates new service defaults when not found": { setupConsul: func(t *testing.T, client *api.Client) { @@ -1097,7 +1097,19 @@ func TestRegisterServiceDefaults(t *testing.T) { expectError: false, expectServiceName: "test-service", expectPassiveHealthCheckSet: true, - expectedInterval: 10 * time.Second, // Default from config + validatePassiveHealthCheck: func(t *testing.T, phc *api.PassiveHealthCheck) { + // When PassiveHealthCheck is newly created, all fields must have their default values + require.Equal(t, config.DefaultOutlierDetectionInterval, phc.Interval, "Interval must be set to default") + require.Equal(t, config.DefaultOutlierDetectionMaxFailures, phc.MaxFailures, "MaxFailures must be set to default") + + // EnforcingConsecutiveGatewayFailure is always set (never nil) via normalization + require.NotNil(t, phc.EnforcingConsecutiveGatewayFailure, "EnforcingConsecutiveGatewayFailure must never be nil in new PassiveHealthCheck") + require.Equal(t, config.DefaultOutlierDetectionEnforcingGatewayFailure, *phc.EnforcingConsecutiveGatewayFailure, "EnforcingConsecutiveGatewayFailure must use default value") + + // MaxEjectionPercent is always set (never nil) via normalization + require.NotNil(t, phc.MaxEjectionPercent, "MaxEjectionPercent must never be nil in new PassiveHealthCheck") + require.Equal(t, config.DefaultOutlierDetectionMaxEjectionPercent, *phc.MaxEjectionPercent, "MaxEjectionPercent must use default value") + }, }, "merges with existing service defaults without passive health check": { setupConsul: func(t *testing.T, client *api.Client) { @@ -1117,7 +1129,19 @@ func TestRegisterServiceDefaults(t *testing.T) { expectError: false, expectServiceName: "test-service-merge", expectPassiveHealthCheckSet: true, - expectedInterval: 10 * time.Second, + validatePassiveHealthCheck: func(t *testing.T, phc *api.PassiveHealthCheck) { + // When merging and adding PassiveHealthCheck, defaults are applied + require.Equal(t, config.DefaultOutlierDetectionInterval, phc.Interval, "Interval must be set to default when adding to merged config") + require.Equal(t, config.DefaultOutlierDetectionMaxFailures, phc.MaxFailures, "MaxFailures must be set to default when adding to merged config") + + // EnforcingConsecutiveGatewayFailure is always set via normalization + require.NotNil(t, phc.EnforcingConsecutiveGatewayFailure, "EnforcingConsecutiveGatewayFailure must never be nil when adding PassiveHealthCheck") + require.Equal(t, config.DefaultOutlierDetectionEnforcingGatewayFailure, *phc.EnforcingConsecutiveGatewayFailure, "EnforcingConsecutiveGatewayFailure must use default value") + + // MaxEjectionPercent is always set via normalization + require.NotNil(t, phc.MaxEjectionPercent, "MaxEjectionPercent must never be nil when adding PassiveHealthCheck") + require.Equal(t, config.DefaultOutlierDetectionMaxEjectionPercent, *phc.MaxEjectionPercent, "MaxEjectionPercent must use default value") + }, }, "preserves existing passive health check config": { setupConsul: func(t *testing.T, client *api.Client) { @@ -1131,8 +1155,53 @@ func TestRegisterServiceDefaults(t *testing.T) { UpstreamConfig: &api.UpstreamConfiguration{ Defaults: &api.UpstreamConfig{ PassiveHealthCheck: &api.PassiveHealthCheck{ - Interval: 30 * time.Second, - MaxFailures: 10, + Interval: 30 * time.Second, + MaxFailures: 10, + EnforcingConsecutiveGatewayFailure: &enforcingVal, + ConsecutiveGatewayFailure: &ejectVal, + MaxEjectionPercent: &ejectVal, + }, + }, + }, + } + _, _, err := client.ConfigEntries().Set(existing, nil) + require.NoError(t, err) + }, + expectError: false, + expectServiceName: "test-service-preserve", + expectPassiveHealthCheckSet: true, + validatePassiveHealthCheck: func(t *testing.T, phc *api.PassiveHealthCheck) { + // When PassiveHealthCheck already exists, it MUST be preserved as-is (NOT regenerated) + // The registerServiceDefaults function should skip creation and keep the original + require.Equal(t, 30*time.Second, phc.Interval, "Existing Interval must be preserved (30s), not overwritten with default (10s)") + require.Equal(t, uint32(10), phc.MaxFailures, "Existing MaxFailures must be preserved (10), not overwritten with default (5)") + + // The existing EnforcingConsecutiveGatewayFailure value must be preserved + require.NotNil(t, phc.EnforcingConsecutiveGatewayFailure, "EnforcingConsecutiveGatewayFailure must be preserved") + require.Equal(t, uint32(100), *phc.EnforcingConsecutiveGatewayFailure, "Existing EnforcingConsecutiveGatewayFailure (100) must be preserved, not replaced with default") + + // The existing consecutiveGatewayFailure must be preserved + require.NotNil(t, phc.ConsecutiveGatewayFailure, "ConsecutiveGatewayFailure must be preserved") + require.Equal(t, uint32(50), *phc.ConsecutiveGatewayFailure, "Existing ConsecutiveGatewayFailure (50) must be preserved, not replaced with default") + // The existing MaxEjectionPercent must be preserved + require.NotNil(t, phc.MaxEjectionPercent, "MaxEjectionPercent must be preserved") + require.Equal(t, uint32(50), *phc.MaxEjectionPercent, "Existing MaxEjectionPercent (50) must be preserved, not replaced with default") + }, + }, + "preserves existing passive health check with EnforcingConsecutive5xx": { + setupConsul: func(t *testing.T, client *api.Client) { + // Pre-create service defaults WITH existing PassiveHealthCheck using the legacy EnforcingConsecutive5xx field + // This should be preserved and NOT overwritten + enforcingVal := uint32(85) + ejectVal := uint32(45) + existing := &api.ServiceConfigEntry{ + Kind: api.ServiceDefaults, + Name: "test-service-preserve-5xx", + UpstreamConfig: &api.UpstreamConfiguration{ + Defaults: &api.UpstreamConfig{ + PassiveHealthCheck: &api.PassiveHealthCheck{ + Interval: 25 * time.Second, + MaxFailures: 9, EnforcingConsecutive5xx: &enforcingVal, MaxEjectionPercent: &ejectVal, }, @@ -1143,9 +1212,22 @@ func TestRegisterServiceDefaults(t *testing.T) { require.NoError(t, err) }, expectError: false, - expectServiceName: "test-service-preserve", + expectServiceName: "test-service-preserve-5xx", expectPassiveHealthCheckSet: true, - expectedInterval: 30 * time.Second, // Should preserve the existing interval, not apply default + validatePassiveHealthCheck: func(t *testing.T, phc *api.PassiveHealthCheck) { + // When PassiveHealthCheck with legacy EnforcingConsecutive5xx already exists, it MUST be preserved as-is + // The registerServiceDefaults function should skip creation and keep the original 5xx field + require.Equal(t, 25*time.Second, phc.Interval, "Existing Interval must be preserved (25s), not overwritten with default (10s)") + require.Equal(t, uint32(9), phc.MaxFailures, "Existing MaxFailures must be preserved (9), not overwritten with default (5)") + + // The existing EnforcingConsecutive5xx (legacy field) must be preserved + require.NotNil(t, phc.EnforcingConsecutive5xx, "Legacy EnforcingConsecutive5xx must be preserved when existing") + require.Equal(t, uint32(85), *phc.EnforcingConsecutive5xx, "Existing EnforcingConsecutive5xx (85) must be preserved, not replaced") + + // The existing MaxEjectionPercent must be preserved + require.NotNil(t, phc.MaxEjectionPercent, "MaxEjectionPercent must be preserved") + require.Equal(t, uint32(45), *phc.MaxEjectionPercent, "Existing MaxEjectionPercent (45) must be preserved, not replaced with default (50)") + }, }, } @@ -1203,8 +1285,8 @@ func TestRegisterServiceDefaults(t *testing.T) { // Verify PassiveHealthCheck is set if tc.expectPassiveHealthCheckSet { require.NotNil(t, svcDefaults.UpstreamConfig.Defaults.PassiveHealthCheck) - // Verify the interval matches expected value (either preserved or newly set) - require.Equal(t, tc.expectedInterval, svcDefaults.UpstreamConfig.Defaults.PassiveHealthCheck.Interval) + // Run the comprehensive validation function for this test case + tc.validatePassiveHealthCheck(t, svcDefaults.UpstreamConfig.Defaults.PassiveHealthCheck) } } }) @@ -1266,30 +1348,30 @@ func TestIsConfigEntryNotFoundErrorHelper(t *testing.T) { // TestNormalizeOutlierDetectionConfig tests the normalization of outlier detection configuration func TestNormalizeOutlierDetectionConfig(t *testing.T) { cases := map[string]struct { - input *config.OutlierDetectionConfig - expectNilEnforcingConsec5xx bool - expectNilMaxEjectionPercent bool - expectedEnforcingConsec5xx uint32 - expectedMaxEjectionPercent uint32 + input *config.OutlierDetectionConfig + expectNilEnforcingGatewayFailure bool + expectNilMaxEjectionPercent bool + expectedEnforcingGatewayFailure uint32 + expectedMaxEjectionPercent uint32 }{ "nil config returns defaults": { - input: nil, - expectNilEnforcingConsec5xx: false, - expectNilMaxEjectionPercent: false, - expectedEnforcingConsec5xx: config.DefaultOutlierDetectionEnforcingConsecutive5xx, - expectedMaxEjectionPercent: config.DefaultOutlierDetectionMaxEjectionPercent, + input: nil, + expectNilEnforcingGatewayFailure: false, + expectNilMaxEjectionPercent: false, + expectedEnforcingGatewayFailure: config.DefaultOutlierDetectionEnforcingGatewayFailure, + expectedMaxEjectionPercent: config.DefaultOutlierDetectionMaxEjectionPercent, }, "config with nil pointer fields gets defaults applied": { input: &config.OutlierDetectionConfig{ Interval: 10 * time.Second, MaxFailures: 5, - EnforcingConsecutive5xx: nil, // Should get default + EnforcingGatewayFailure: nil, // Should get default MaxEjectionPercent: nil, // Should get default }, - expectNilEnforcingConsec5xx: false, - expectNilMaxEjectionPercent: false, - expectedEnforcingConsec5xx: config.DefaultOutlierDetectionEnforcingConsecutive5xx, - expectedMaxEjectionPercent: config.DefaultOutlierDetectionMaxEjectionPercent, + expectNilEnforcingGatewayFailure: false, + expectNilMaxEjectionPercent: false, + expectedEnforcingGatewayFailure: config.DefaultOutlierDetectionEnforcingGatewayFailure, + expectedMaxEjectionPercent: config.DefaultOutlierDetectionMaxEjectionPercent, }, "config with set pointer fields is preserved": { input: func() *config.OutlierDetectionConfig { @@ -1298,14 +1380,14 @@ func TestNormalizeOutlierDetectionConfig(t *testing.T) { return &config.OutlierDetectionConfig{ Interval: 15 * time.Second, MaxFailures: 8, - EnforcingConsecutive5xx: &customEnforcing, + EnforcingGatewayFailure: &customEnforcing, MaxEjectionPercent: &customEjection, } }(), - expectNilEnforcingConsec5xx: false, - expectNilMaxEjectionPercent: false, - expectedEnforcingConsec5xx: 80, - expectedMaxEjectionPercent: 30, + expectNilEnforcingGatewayFailure: false, + expectNilMaxEjectionPercent: false, + expectedEnforcingGatewayFailure: 80, + expectedMaxEjectionPercent: 30, }, "config with partial nil fields applies selective defaults": { input: func() *config.OutlierDetectionConfig { @@ -1313,14 +1395,14 @@ func TestNormalizeOutlierDetectionConfig(t *testing.T) { return &config.OutlierDetectionConfig{ Interval: 12 * time.Second, MaxFailures: 6, - EnforcingConsecutive5xx: &customEnforcing, // Has value + EnforcingGatewayFailure: &customEnforcing, // Has value MaxEjectionPercent: nil, // Will get default } }(), - expectNilEnforcingConsec5xx: false, - expectNilMaxEjectionPercent: false, - expectedEnforcingConsec5xx: 90, - expectedMaxEjectionPercent: config.DefaultOutlierDetectionMaxEjectionPercent, + expectNilEnforcingGatewayFailure: false, + expectNilMaxEjectionPercent: false, + expectedEnforcingGatewayFailure: 90, + expectedMaxEjectionPercent: config.DefaultOutlierDetectionMaxEjectionPercent, }, } @@ -1329,12 +1411,12 @@ func TestNormalizeOutlierDetectionConfig(t *testing.T) { normalized := normalizeOutlierDetectionConfig(tc.input) require.NotNil(t, normalized) - // Verify EnforcingConsecutive5xx - if tc.expectNilEnforcingConsec5xx { - require.Nil(t, normalized.EnforcingConsecutive5xx) + // Verify EnforcingGatewayFailure + if tc.expectNilEnforcingGatewayFailure { + require.Nil(t, normalized.EnforcingGatewayFailure) } else { - require.NotNil(t, normalized.EnforcingConsecutive5xx) - require.Equal(t, tc.expectedEnforcingConsec5xx, *normalized.EnforcingConsecutive5xx) + require.NotNil(t, normalized.EnforcingGatewayFailure) + require.Equal(t, tc.expectedEnforcingGatewayFailure, *normalized.EnforcingGatewayFailure) } // Verify MaxEjectionPercent