From 9e946e2b2aa839d3ccf39d0f386144af4e329872 Mon Sep 17 00:00:00 2001 From: Clayton Gonsalves Date: Wed, 3 Jul 2024 22:19:22 +0200 Subject: [PATCH 1/5] add circuit breaker support for extension services. Signed-off-by: Clayton Gonsalves --- apis/projectcontour/v1alpha1/contourconfig.go | 8 +- .../v1alpha1/extensionservice.go | 6 + .../v1alpha1/zz_generated.deepcopy.go | 37 +-- .../6539-clayton-gonsalves-minor.md | 3 + cmd/contour/serve.go | 10 +- cmd/contour/serve_test.go | 2 +- cmd/contour/servecontext_test.go | 4 +- examples/contour/01-crds.yaml | 46 ++++ examples/render/contour-deployment.yaml | 46 ++++ .../render/contour-gateway-provisioner.yaml | 46 ++++ examples/render/contour-gateway.yaml | 46 ++++ examples/render/contour.yaml | 46 ++++ internal/dag/accessors.go | 16 +- internal/dag/builder_test.go | 12 +- internal/dag/dag.go | 47 ++-- internal/dag/extension_processor.go | 23 ++ internal/dag/gatewayapi_processor.go | 2 +- internal/dag/httpproxy_processor.go | 2 +- internal/dag/ingress_processor.go | 2 +- internal/dag/policy.go | 18 +- internal/dag/policy_test.go | 48 ++-- internal/envoy/v3/cluster.go | 28 ++- internal/envoy/v3/cluster_test.go | 20 +- internal/featuretests/v3/cluster_test.go | 8 +- .../featuretests/v3/extensionservice_test.go | 218 +++++++++++++++++- pkg/config/parameters.go | 2 +- .../docs/main/config/api-reference.html | 192 +++++++++------ 27 files changed, 759 insertions(+), 179 deletions(-) create mode 100644 changelogs/unreleased/6539-clayton-gonsalves-minor.md diff --git a/apis/projectcontour/v1alpha1/contourconfig.go b/apis/projectcontour/v1alpha1/contourconfig.go index 08fd76a23fc..913e91554a1 100644 --- a/apis/projectcontour/v1alpha1/contourconfig.go +++ b/apis/projectcontour/v1alpha1/contourconfig.go @@ -107,7 +107,7 @@ const ( EnvoyServerType XDSServerType = "envoy" ) -type GlobalCircuitBreakerDefaults struct { +type CircuitBreaker struct { // The maximum number of connections that a single Envoy instance allows to the Kubernetes Service; defaults to 1024. // +optional MaxConnections uint32 `json:"maxConnections,omitempty" yaml:"max-connections,omitempty"` @@ -120,6 +120,10 @@ type GlobalCircuitBreakerDefaults struct { // The maximum number of parallel retries a single Envoy instance allows to the Kubernetes Service; defaults to 3. // +optional MaxRetries uint32 `json:"maxRetries,omitempty" yaml:"max-retries,omitempty"` + + // PerHostMaxConnections is the maximum number of connections + // that Envoy will allow to each individual host in a cluster. + PerHostMaxConnections uint32 `json:"perHostMaxConnections,omitempty" yaml:"per-host-max-connections,omitempty"` } // XDSServerConfig holds the config for the Contour xDS server. @@ -704,7 +708,7 @@ type ClusterParameters struct { // If defined, this will be used as the default for all services. // // +optional - GlobalCircuitBreakerDefaults *GlobalCircuitBreakerDefaults `json:"circuitBreakers,omitempty"` + GlobalCircuitBreakerDefaults *CircuitBreaker `json:"circuitBreakers,omitempty"` // UpstreamTLS contains the TLS policy parameters for upstream connections // diff --git a/apis/projectcontour/v1alpha1/extensionservice.go b/apis/projectcontour/v1alpha1/extensionservice.go index 20c53058b54..8ebea4a5a0e 100644 --- a/apis/projectcontour/v1alpha1/extensionservice.go +++ b/apis/projectcontour/v1alpha1/extensionservice.go @@ -104,6 +104,12 @@ type ExtensionServiceSpec struct { // +optional // +kubebuilder:validation:Enum=v3 ProtocolVersion ExtensionProtocolVersion `json:"protocolVersion,omitempty"` + + // CircuitBreaker specifies the circuit breaker budget across the extension service. + // This overrides the global circuite breaker budget if defined. + // If defined this overrides the global circuit breaker budget. + // +optional + CircuitBreakerPolicy *CircuitBreaker `json:"circuitBreakerPolicy,omitempty"` } // ExtensionServiceStatus defines the observed state of an diff --git a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go index eaa1576bfce..26bea21da48 100644 --- a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go +++ b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go @@ -47,6 +47,21 @@ func (in AccessLogJSONFields) DeepCopy() AccessLogJSONFields { return *out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CircuitBreaker) DeepCopyInto(out *CircuitBreaker) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CircuitBreaker. +func (in *CircuitBreaker) DeepCopy() *CircuitBreaker { + if in == nil { + return nil + } + out := new(CircuitBreaker) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterParameters) DeepCopyInto(out *ClusterParameters) { *out = *in @@ -62,7 +77,7 @@ func (in *ClusterParameters) DeepCopyInto(out *ClusterParameters) { } if in.GlobalCircuitBreakerDefaults != nil { in, out := &in.GlobalCircuitBreakerDefaults, &out.GlobalCircuitBreakerDefaults - *out = new(GlobalCircuitBreakerDefaults) + *out = new(CircuitBreaker) **out = **in } if in.UpstreamTLS != nil { @@ -813,6 +828,11 @@ func (in *ExtensionServiceSpec) DeepCopyInto(out *ExtensionServiceSpec) { *out = new(v1.TimeoutPolicy) **out = **in } + if in.CircuitBreakerPolicy != nil { + in, out := &in.CircuitBreakerPolicy, &out.CircuitBreakerPolicy + *out = new(CircuitBreaker) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExtensionServiceSpec. @@ -897,21 +917,6 @@ func (in *GatewayConfig) DeepCopy() *GatewayConfig { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *GlobalCircuitBreakerDefaults) DeepCopyInto(out *GlobalCircuitBreakerDefaults) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GlobalCircuitBreakerDefaults. -func (in *GlobalCircuitBreakerDefaults) DeepCopy() *GlobalCircuitBreakerDefaults { - if in == nil { - return nil - } - out := new(GlobalCircuitBreakerDefaults) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HTTPProxyConfig) DeepCopyInto(out *HTTPProxyConfig) { *out = *in diff --git a/changelogs/unreleased/6539-clayton-gonsalves-minor.md b/changelogs/unreleased/6539-clayton-gonsalves-minor.md new file mode 100644 index 00000000000..0f9dc955be9 --- /dev/null +++ b/changelogs/unreleased/6539-clayton-gonsalves-minor.md @@ -0,0 +1,3 @@ +## Add Circuit Breaker support for Extension Services + +This change enabled the user to configure the Circuit breakers for extension services either via the global Contour config or on the Extension Service CRD itself on a per Extension Service itself. diff --git a/cmd/contour/serve.go b/cmd/contour/serve.go index e3cdd0adeeb..d8abf8cd7a1 100644 --- a/cmd/contour/serve.go +++ b/cmd/contour/serve.go @@ -1064,7 +1064,7 @@ type dagBuilderConfig struct { maxRequestsPerConnection *uint32 perConnectionBufferLimitBytes *uint32 globalRateLimitService *contour_v1alpha1.RateLimitServiceConfig - globalCircuitBreakerDefaults *contour_v1alpha1.GlobalCircuitBreakerDefaults + globalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker upstreamTLS *dag.UpstreamTLS } @@ -1141,10 +1141,10 @@ func (s *Server) getDAGBuilder(dbc dagBuilderConfig) *dag.Builder { &dag.ExtensionServiceProcessor{ // Note that ExtensionService does not support ExternalName, if it does get added, // need to bring EnableExternalNameService in here too. - FieldLogger: s.log.WithField("context", "ExtensionServiceProcessor"), - ClientCertificate: dbc.clientCert, - ConnectTimeout: dbc.connectTimeout, - UpstreamTLS: dbc.upstreamTLS, + FieldLogger: s.log.WithField("context", "ExtensionServiceProcessor"), + ClientCertificate: dbc.clientCert, + ConnectTimeout: dbc.connectTimeout, + GlobalCircuitBreakerDefaults: dbc.globalCircuitBreakerDefaults, }, &dag.HTTPProxyProcessor{ EnableExternalNameService: dbc.enableExternalNameService, diff --git a/cmd/contour/serve_test.go b/cmd/contour/serve_test.go index 248e9bc17c4..add26319dd2 100644 --- a/cmd/contour/serve_test.go +++ b/cmd/contour/serve_test.go @@ -125,7 +125,7 @@ func TestGetDAGBuilder(t *testing.T) { }) t.Run("GlobalCircuitBreakerDefaults specified for all processors", func(t *testing.T) { - g := contour_v1alpha1.GlobalCircuitBreakerDefaults{ + g := contour_v1alpha1.CircuitBreaker{ MaxConnections: 100, } diff --git a/cmd/contour/servecontext_test.go b/cmd/contour/servecontext_test.go index 766a2f0af83..ce8e2ad53a7 100644 --- a/cmd/contour/servecontext_test.go +++ b/cmd/contour/servecontext_test.go @@ -767,7 +767,7 @@ func TestConvertServeContext(t *testing.T) { }, "global circuit breaker defaults": { getServeContext: func(ctx *serveContext) *serveContext { - ctx.Config.Cluster.GlobalCircuitBreakerDefaults = &contour_v1alpha1.GlobalCircuitBreakerDefaults{ + ctx.Config.Cluster.GlobalCircuitBreakerDefaults = &contour_v1alpha1.CircuitBreaker{ MaxConnections: 4, MaxPendingRequests: 5, MaxRequests: 6, @@ -776,7 +776,7 @@ func TestConvertServeContext(t *testing.T) { return ctx }, getContourConfiguration: func(cfg contour_v1alpha1.ContourConfigurationSpec) contour_v1alpha1.ContourConfigurationSpec { - cfg.Envoy.Cluster.GlobalCircuitBreakerDefaults = &contour_v1alpha1.GlobalCircuitBreakerDefaults{ + cfg.Envoy.Cluster.GlobalCircuitBreakerDefaults = &contour_v1alpha1.CircuitBreaker{ MaxConnections: 4, MaxPendingRequests: 5, MaxRequests: 6, diff --git a/examples/contour/01-crds.yaml b/examples/contour/01-crds.yaml index 00aebca5e95..4ee0e8c9982 100644 --- a/examples/contour/01-crds.yaml +++ b/examples/contour/01-crds.yaml @@ -120,6 +120,12 @@ spec: defaults to 3. format: int32 type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer type: object dnsLookupFamily: description: |- @@ -3894,6 +3900,12 @@ spec: Service; defaults to 3. format: int32 type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer type: object dnsLookupFamily: description: |- @@ -5057,6 +5069,40 @@ spec: description: ExtensionServiceSpec defines the desired state of an ExtensionService resource. properties: + circuitBreakerPolicy: + description: |- + CircuitBreaker specifies the circuit breaker budget across the extension service. + This overrides the global circuite breaker budget if defined. + If defined this overrides the global circuit breaker budget. + properties: + maxConnections: + description: The maximum number of connections that a single Envoy + instance allows to the Kubernetes Service; defaults to 1024. + format: int32 + type: integer + maxPendingRequests: + description: The maximum number of pending requests that a single + Envoy instance allows to the Kubernetes Service; defaults to + 1024. + format: int32 + type: integer + maxRequests: + description: The maximum parallel requests a single Envoy instance + allows to the Kubernetes Service; defaults to 1024 + format: int32 + type: integer + maxRetries: + description: The maximum number of parallel retries a single Envoy + instance allows to the Kubernetes Service; defaults to 3. + format: int32 + type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer + type: object loadBalancerPolicy: description: |- The policy for load balancing GRPC service requests. Note that the diff --git a/examples/render/contour-deployment.yaml b/examples/render/contour-deployment.yaml index b7663b38be1..5514bb75028 100644 --- a/examples/render/contour-deployment.yaml +++ b/examples/render/contour-deployment.yaml @@ -340,6 +340,12 @@ spec: defaults to 3. format: int32 type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer type: object dnsLookupFamily: description: |- @@ -4114,6 +4120,12 @@ spec: Service; defaults to 3. format: int32 type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer type: object dnsLookupFamily: description: |- @@ -5277,6 +5289,40 @@ spec: description: ExtensionServiceSpec defines the desired state of an ExtensionService resource. properties: + circuitBreakerPolicy: + description: |- + CircuitBreaker specifies the circuit breaker budget across the extension service. + This overrides the global circuite breaker budget if defined. + If defined this overrides the global circuit breaker budget. + properties: + maxConnections: + description: The maximum number of connections that a single Envoy + instance allows to the Kubernetes Service; defaults to 1024. + format: int32 + type: integer + maxPendingRequests: + description: The maximum number of pending requests that a single + Envoy instance allows to the Kubernetes Service; defaults to + 1024. + format: int32 + type: integer + maxRequests: + description: The maximum parallel requests a single Envoy instance + allows to the Kubernetes Service; defaults to 1024 + format: int32 + type: integer + maxRetries: + description: The maximum number of parallel retries a single Envoy + instance allows to the Kubernetes Service; defaults to 3. + format: int32 + type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer + type: object loadBalancerPolicy: description: |- The policy for load balancing GRPC service requests. Note that the diff --git a/examples/render/contour-gateway-provisioner.yaml b/examples/render/contour-gateway-provisioner.yaml index 91a8ac8a800..a291e4b389a 100644 --- a/examples/render/contour-gateway-provisioner.yaml +++ b/examples/render/contour-gateway-provisioner.yaml @@ -131,6 +131,12 @@ spec: defaults to 3. format: int32 type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer type: object dnsLookupFamily: description: |- @@ -3905,6 +3911,12 @@ spec: Service; defaults to 3. format: int32 type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer type: object dnsLookupFamily: description: |- @@ -5068,6 +5080,40 @@ spec: description: ExtensionServiceSpec defines the desired state of an ExtensionService resource. properties: + circuitBreakerPolicy: + description: |- + CircuitBreaker specifies the circuit breaker budget across the extension service. + This overrides the global circuite breaker budget if defined. + If defined this overrides the global circuit breaker budget. + properties: + maxConnections: + description: The maximum number of connections that a single Envoy + instance allows to the Kubernetes Service; defaults to 1024. + format: int32 + type: integer + maxPendingRequests: + description: The maximum number of pending requests that a single + Envoy instance allows to the Kubernetes Service; defaults to + 1024. + format: int32 + type: integer + maxRequests: + description: The maximum parallel requests a single Envoy instance + allows to the Kubernetes Service; defaults to 1024 + format: int32 + type: integer + maxRetries: + description: The maximum number of parallel retries a single Envoy + instance allows to the Kubernetes Service; defaults to 3. + format: int32 + type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer + type: object loadBalancerPolicy: description: |- The policy for load balancing GRPC service requests. Note that the diff --git a/examples/render/contour-gateway.yaml b/examples/render/contour-gateway.yaml index 0d70b6c4c4f..3b8bae4c2fd 100644 --- a/examples/render/contour-gateway.yaml +++ b/examples/render/contour-gateway.yaml @@ -156,6 +156,12 @@ spec: defaults to 3. format: int32 type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer type: object dnsLookupFamily: description: |- @@ -3930,6 +3936,12 @@ spec: Service; defaults to 3. format: int32 type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer type: object dnsLookupFamily: description: |- @@ -5093,6 +5105,40 @@ spec: description: ExtensionServiceSpec defines the desired state of an ExtensionService resource. properties: + circuitBreakerPolicy: + description: |- + CircuitBreaker specifies the circuit breaker budget across the extension service. + This overrides the global circuite breaker budget if defined. + If defined this overrides the global circuit breaker budget. + properties: + maxConnections: + description: The maximum number of connections that a single Envoy + instance allows to the Kubernetes Service; defaults to 1024. + format: int32 + type: integer + maxPendingRequests: + description: The maximum number of pending requests that a single + Envoy instance allows to the Kubernetes Service; defaults to + 1024. + format: int32 + type: integer + maxRequests: + description: The maximum parallel requests a single Envoy instance + allows to the Kubernetes Service; defaults to 1024 + format: int32 + type: integer + maxRetries: + description: The maximum number of parallel retries a single Envoy + instance allows to the Kubernetes Service; defaults to 3. + format: int32 + type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer + type: object loadBalancerPolicy: description: |- The policy for load balancing GRPC service requests. Note that the diff --git a/examples/render/contour.yaml b/examples/render/contour.yaml index b44c26538bc..b4b57c86a50 100644 --- a/examples/render/contour.yaml +++ b/examples/render/contour.yaml @@ -340,6 +340,12 @@ spec: defaults to 3. format: int32 type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer type: object dnsLookupFamily: description: |- @@ -4114,6 +4120,12 @@ spec: Service; defaults to 3. format: int32 type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer type: object dnsLookupFamily: description: |- @@ -5277,6 +5289,40 @@ spec: description: ExtensionServiceSpec defines the desired state of an ExtensionService resource. properties: + circuitBreakerPolicy: + description: |- + CircuitBreaker specifies the circuit breaker budget across the extension service. + This overrides the global circuite breaker budget if defined. + If defined this overrides the global circuit breaker budget. + properties: + maxConnections: + description: The maximum number of connections that a single Envoy + instance allows to the Kubernetes Service; defaults to 1024. + format: int32 + type: integer + maxPendingRequests: + description: The maximum number of pending requests that a single + Envoy instance allows to the Kubernetes Service; defaults to + 1024. + format: int32 + type: integer + maxRequests: + description: The maximum parallel requests a single Envoy instance + allows to the Kubernetes Service; defaults to 1024 + format: int32 + type: integer + maxRetries: + description: The maximum number of parallel retries a single Envoy + instance allows to the Kubernetes Service; defaults to 3. + format: int32 + type: integer + perHostMaxConnections: + description: |- + PerHostMaxConnections is the maximum number of connections + that Envoy will allow to each individual host in a cluster. + format: int32 + type: integer + type: object loadBalancerPolicy: description: |- The policy for load balancing GRPC service requests. Note that the diff --git a/internal/dag/accessors.go b/internal/dag/accessors.go index 20a990b3433..5ae27baa56c 100644 --- a/internal/dag/accessors.go +++ b/internal/dag/accessors.go @@ -63,13 +63,15 @@ func (d *DAG) EnsureService(meta types.NamespacedName, port, healthPort int, cac HealthPort: healthSvcPort, Weight: 1, }, - Protocol: upstreamProtocol(svc, svcPort), - MaxConnections: annotation.MaxConnections(svc), - MaxPendingRequests: annotation.MaxPendingRequests(svc), - MaxRequests: annotation.MaxRequests(svc), - MaxRetries: annotation.MaxRetries(svc), - PerHostMaxConnections: annotation.PerHostMaxConnections(svc), - ExternalName: externalName(svc), + Protocol: upstreamProtocol(svc, svcPort), + CircuitBreakersSettings: CircuitBreakersSettings{ + MaxConnections: annotation.MaxConnections(svc), + MaxPendingRequests: annotation.MaxPendingRequests(svc), + MaxRequests: annotation.MaxRequests(svc), + PerHostMaxConnections: annotation.PerHostMaxConnections(svc), + MaxRetries: annotation.MaxRetries(svc), + }, + ExternalName: externalName(svc), }, nil } diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go index 98f4525b890..1dcd55d4673 100644 --- a/internal/dag/builder_test.go +++ b/internal/dag/builder_test.go @@ -10640,11 +10640,13 @@ func TestDAGInsert(t *testing.T) { ServicePort: s1b.Spec.Ports[0], HealthPort: s1b.Spec.Ports[0], }, - MaxConnections: 9000, - MaxPendingRequests: 4096, - MaxRequests: 404, - MaxRetries: 7, - PerHostMaxConnections: 45, + CircuitBreakersSettings: CircuitBreakersSettings{ + MaxConnections: 9000, + MaxPendingRequests: 4096, + MaxRequests: 404, + MaxRetries: 7, + PerHostMaxConnections: 45, + }, }), ), ), diff --git a/internal/dag/dag.go b/internal/dag/dag.go index 9c67318e0c8..5942b66c70b 100644 --- a/internal/dag/dag.go +++ b/internal/dag/dag.go @@ -971,26 +971,7 @@ type Service struct { Protocol string // Circuit breaking limits - - // Max connections is maximum number of connections - // that Envoy will make to the upstream cluster. - MaxConnections uint32 - - // MaxPendingRequests is maximum number of pending - // requests that Envoy will allow to the upstream cluster. - MaxPendingRequests uint32 - - // MaxRequests is the maximum number of parallel requests that - // Envoy will make to the upstream cluster. - MaxRequests uint32 - - // MaxRetries is the maximum number of parallel retries that - // Envoy will allow to the upstream cluster. - MaxRetries uint32 - - // PerHostMaxConnections is the maximum number of connections - // that Envoy will allow to each individual host in a cluster. - PerHostMaxConnections uint32 + CircuitBreakersSettings CircuitBreakersSettings // ExternalName is an optional field referencing a dns entry for Service type "ExternalName" ExternalName string @@ -1261,6 +1242,9 @@ type ExtensionCluster struct { // UpstreamTLS contains the TLS version and cipher suite configurations for upstream connections UpstreamTLS *UpstreamTLS + + // Circuit breaking limits + CircuitBreakersSettings CircuitBreakersSettings } const singleDNSLabelWildcardRegex = "^[a-z0-9]([-a-z0-9]*[a-z0-9])?" @@ -1299,3 +1283,26 @@ type UpstreamTLS struct { MaximumProtocolVersion string CipherSuites []string } + +// CircuitBreakersSettings holds configuration for circuit breakers. +type CircuitBreakersSettings struct { + // Max connections is maximum number of connections + // that Envoy will make to the upstream cluster. + MaxConnections uint32 + + // MaxPendingRequests is maximum number of pending + // requests that Envoy will allow to the upstream cluster. + MaxPendingRequests uint32 + + // MaxRequests is the maximum number of parallel requests that + // Envoy will make to the upstream cluster. + MaxRequests uint32 + + // MaxRetries is the maximum number of parallel retries that + // Envoy will allow to the upstream cluster. + MaxRetries uint32 + + // PerHostMaxConnections is the maximum number of connections + // that Envoy will allow to each individual host in a cluster. + PerHostMaxConnections uint32 +} diff --git a/internal/dag/extension_processor.go b/internal/dag/extension_processor.go index d954f89d300..5c41bd19537 100644 --- a/internal/dag/extension_processor.go +++ b/internal/dag/extension_processor.go @@ -44,6 +44,9 @@ type ExtensionServiceProcessor struct { // UpstreamTLS defines the TLS settings like min/max version // and cipher suites for upstream connections. UpstreamTLS *UpstreamTLS + + // GlobalCircuitBreakerDefaults defines global circuit breaker defaults. + GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker } var _ Processor = &ExtensionServiceProcessor{} @@ -122,6 +125,26 @@ func (p *ExtensionServiceProcessor) buildExtensionService( UpstreamTLS: p.UpstreamTLS, } + if p.GlobalCircuitBreakerDefaults != nil { + extension.CircuitBreakersSettings = CircuitBreakersSettings{ + MaxConnections: p.GlobalCircuitBreakerDefaults.MaxConnections, + MaxPendingRequests: p.GlobalCircuitBreakerDefaults.MaxPendingRequests, + MaxRequests: p.GlobalCircuitBreakerDefaults.MaxRequests, + MaxRetries: p.GlobalCircuitBreakerDefaults.MaxRetries, + PerHostMaxConnections: p.GlobalCircuitBreakerDefaults.PerHostMaxConnections, + } + } + + if ext.Spec.CircuitBreakerPolicy != nil { + extension.CircuitBreakersSettings = CircuitBreakersSettings{ + MaxConnections: ext.Spec.CircuitBreakerPolicy.MaxConnections, + MaxPendingRequests: ext.Spec.CircuitBreakerPolicy.MaxPendingRequests, + MaxRequests: ext.Spec.CircuitBreakerPolicy.MaxRequests, + MaxRetries: ext.Spec.CircuitBreakerPolicy.MaxRetries, + PerHostMaxConnections: ext.Spec.CircuitBreakerPolicy.PerHostMaxConnections, + } + } + lbPolicy := loadBalancerPolicy(ext.Spec.LoadBalancerPolicy) switch lbPolicy { case LoadBalancerPolicyCookie, LoadBalancerPolicyRequestHash: diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index 638ded0a939..431dc1b8388 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -76,7 +76,7 @@ type GatewayAPIProcessor struct { SetSourceMetadataOnRoutes bool // GlobalCircuitBreakerDefaults defines global circuit breaker defaults. - GlobalCircuitBreakerDefaults *contour_v1alpha1.GlobalCircuitBreakerDefaults + GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker // UpstreamTLS defines the TLS settings like min/max version // and cipher suites for upstream connections. diff --git a/internal/dag/httpproxy_processor.go b/internal/dag/httpproxy_processor.go index 7dac1510315..bcac9dfaed0 100644 --- a/internal/dag/httpproxy_processor.go +++ b/internal/dag/httpproxy_processor.go @@ -114,7 +114,7 @@ type HTTPProxyProcessor struct { SetSourceMetadataOnRoutes bool // GlobalCircuitBreakerDefaults defines global circuit breaker defaults. - GlobalCircuitBreakerDefaults *contour_v1alpha1.GlobalCircuitBreakerDefaults + GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker // UpstreamTLS defines the TLS settings like min/max version // and cipher suites for upstream connections. diff --git a/internal/dag/ingress_processor.go b/internal/dag/ingress_processor.go index d5dd05e5bed..fd2c3f761f7 100644 --- a/internal/dag/ingress_processor.go +++ b/internal/dag/ingress_processor.go @@ -68,7 +68,7 @@ type IngressProcessor struct { SetSourceMetadataOnRoutes bool // GlobalCircuitBreakerDefaults defines global circuit breaker defaults. - GlobalCircuitBreakerDefaults *contour_v1alpha1.GlobalCircuitBreakerDefaults + GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker // UpstreamTLS defines the TLS settings like min/max version // and cipher suites for upstream connections. diff --git a/internal/dag/policy.go b/internal/dag/policy.go index 9360d641c1e..bb6b0511fdf 100644 --- a/internal/dag/policy.go +++ b/internal/dag/policy.go @@ -808,25 +808,25 @@ func loadBalancerRequestHashPolicies(lbp *contour_v1.LoadBalancerPolicy, validCo } } -func serviceCircuitBreakerPolicy(s *Service, cb *contour_v1alpha1.GlobalCircuitBreakerDefaults) *Service { +func serviceCircuitBreakerPolicy(s *Service, cb *contour_v1alpha1.CircuitBreaker) *Service { if s == nil { return nil } - if s.MaxConnections == 0 && cb != nil { - s.MaxConnections = cb.MaxConnections + if s.CircuitBreakersSettings.MaxConnections == 0 && cb != nil { + s.CircuitBreakersSettings.MaxConnections = cb.MaxConnections } - if s.MaxPendingRequests == 0 && cb != nil { - s.MaxPendingRequests = cb.MaxPendingRequests + if s.CircuitBreakersSettings.MaxPendingRequests == 0 && cb != nil { + s.CircuitBreakersSettings.MaxPendingRequests = cb.MaxPendingRequests } - if s.MaxRequests == 0 && cb != nil { - s.MaxRequests = cb.MaxRequests + if s.CircuitBreakersSettings.MaxRequests == 0 && cb != nil { + s.CircuitBreakersSettings.MaxRequests = cb.MaxRequests } - if s.MaxRetries == 0 && cb != nil { - s.MaxRetries = cb.MaxRetries + if s.CircuitBreakersSettings.MaxRetries == 0 && cb != nil { + s.CircuitBreakersSettings.MaxRetries = cb.MaxRetries } return s diff --git a/internal/dag/policy_test.go b/internal/dag/policy_test.go index 62301bbb1d6..258e97d1fc2 100644 --- a/internal/dag/policy_test.go +++ b/internal/dag/policy_test.go @@ -1276,7 +1276,7 @@ func TestValidateHeaderAlteration(t *testing.T) { func TestServiceCircuitBreakerPolicy(t *testing.T) { tests := map[string]struct { in *Service - globalDefault *contour_v1alpha1.GlobalCircuitBreakerDefaults + globalDefault *contour_v1alpha1.CircuitBreaker want *Service }{ "service is nil and globalDefault is nil": { @@ -1286,51 +1286,59 @@ func TestServiceCircuitBreakerPolicy(t *testing.T) { }, "service is nil and globalDefault is not nil": { in: nil, - globalDefault: &contour_v1alpha1.GlobalCircuitBreakerDefaults{}, + globalDefault: &contour_v1alpha1.CircuitBreaker{}, want: nil, }, "service is not nil and globalDefault is nil": { in: &Service{ - MaxConnections: 42, - MaxPendingRequests: 73, - MaxRequests: 89, - MaxRetries: 13, + CircuitBreakersSettings: CircuitBreakersSettings{ + MaxConnections: 42, + MaxPendingRequests: 73, + MaxRequests: 89, + MaxRetries: 13, + }, }, globalDefault: nil, want: &Service{ - MaxConnections: 42, - MaxPendingRequests: 73, - MaxRequests: 89, - MaxRetries: 13, + CircuitBreakersSettings: CircuitBreakersSettings{ + MaxConnections: 42, + MaxPendingRequests: 73, + MaxRequests: 89, + MaxRetries: 13, + }, }, }, "service is not set but global is": { in: &Service{}, - globalDefault: &contour_v1alpha1.GlobalCircuitBreakerDefaults{ + globalDefault: &contour_v1alpha1.CircuitBreaker{ MaxConnections: 42, MaxPendingRequests: 73, MaxRequests: 89, MaxRetries: 13, }, want: &Service{ - MaxConnections: 42, - MaxPendingRequests: 73, - MaxRequests: 89, - MaxRetries: 13, + CircuitBreakersSettings: CircuitBreakersSettings{ + MaxConnections: 42, + MaxPendingRequests: 73, + MaxRequests: 89, + MaxRetries: 13, + }, }, }, "service is not set but global is partial": { in: &Service{}, - globalDefault: &contour_v1alpha1.GlobalCircuitBreakerDefaults{ + globalDefault: &contour_v1alpha1.CircuitBreaker{ MaxConnections: 42, MaxPendingRequests: 73, MaxRequests: 89, }, want: &Service{ - MaxConnections: 42, - MaxPendingRequests: 73, - MaxRequests: 89, - MaxRetries: 0, + CircuitBreakersSettings: CircuitBreakersSettings{ + MaxConnections: 42, + MaxPendingRequests: 73, + MaxRequests: 89, + MaxRetries: 0, + }, }, }, } diff --git a/internal/envoy/v3/cluster.go b/internal/envoy/v3/cluster.go index 40fe15afd52..6739224ce6e 100644 --- a/internal/envoy/v3/cluster.go +++ b/internal/envoy/v3/cluster.go @@ -79,17 +79,17 @@ func Cluster(c *dag.Cluster) *envoy_config_cluster_v3.Cluster { cluster.IgnoreHealthOnHostRemoval = true } - if envoy.AnyPositive(service.MaxConnections, service.MaxPendingRequests, service.MaxRequests, service.MaxRetries, service.PerHostMaxConnections) { + if envoy.AnyPositive(service.CircuitBreakersSettings.MaxConnections, service.CircuitBreakersSettings.MaxPendingRequests, service.CircuitBreakersSettings.MaxRequests, service.CircuitBreakersSettings.MaxRetries, service.CircuitBreakersSettings.PerHostMaxConnections) { cluster.CircuitBreakers = &envoy_config_cluster_v3.CircuitBreakers{ Thresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ - MaxConnections: protobuf.UInt32OrNil(service.MaxConnections), - MaxPendingRequests: protobuf.UInt32OrNil(service.MaxPendingRequests), - MaxRequests: protobuf.UInt32OrNil(service.MaxRequests), - MaxRetries: protobuf.UInt32OrNil(service.MaxRetries), + MaxConnections: protobuf.UInt32OrNil(service.CircuitBreakersSettings.MaxConnections), + MaxPendingRequests: protobuf.UInt32OrNil(service.CircuitBreakersSettings.MaxPendingRequests), + MaxRequests: protobuf.UInt32OrNil(service.CircuitBreakersSettings.MaxRequests), + MaxRetries: protobuf.UInt32OrNil(service.CircuitBreakersSettings.MaxRetries), TrackRemaining: true, }}, PerHostThresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ - MaxConnections: protobuf.UInt32OrNil(service.PerHostMaxConnections), + MaxConnections: protobuf.UInt32OrNil(service.CircuitBreakersSettings.PerHostMaxConnections), TrackRemaining: true, }}, } @@ -198,6 +198,22 @@ func ExtensionCluster(ext *dag.ExtensionCluster) *envoy_config_cluster_v3.Cluste } cluster.TypedExtensionProtocolOptions = protocolOptions(http2Version, ext.ClusterTimeoutPolicy.IdleConnectionTimeout, nil) + if envoy.AnyPositive(ext.CircuitBreakersSettings.MaxConnections, ext.CircuitBreakersSettings.MaxPendingRequests, ext.CircuitBreakersSettings.MaxRequests, ext.CircuitBreakersSettings.MaxRetries, ext.CircuitBreakersSettings.PerHostMaxConnections) { + cluster.CircuitBreakers = &envoy_config_cluster_v3.CircuitBreakers{ + Thresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: protobuf.UInt32OrNil(ext.CircuitBreakersSettings.MaxConnections), + MaxPendingRequests: protobuf.UInt32OrNil(ext.CircuitBreakersSettings.MaxPendingRequests), + MaxRequests: protobuf.UInt32OrNil(ext.CircuitBreakersSettings.MaxRequests), + MaxRetries: protobuf.UInt32OrNil(ext.CircuitBreakersSettings.MaxRetries), + TrackRemaining: true, + }}, + PerHostThresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: protobuf.UInt32OrNil(ext.CircuitBreakersSettings.PerHostMaxConnections), + TrackRemaining: true, + }}, + } + } + return cluster } diff --git a/internal/envoy/v3/cluster_test.go b/internal/envoy/v3/cluster_test.go index ff6bf79a012..82ba2b6758d 100644 --- a/internal/envoy/v3/cluster_test.go +++ b/internal/envoy/v3/cluster_test.go @@ -406,7 +406,9 @@ func TestCluster(t *testing.T) { "projectcontour.io/max-connections": { cluster: &dag.Cluster{ Upstream: &dag.Service{ - MaxConnections: 9000, + CircuitBreakersSettings: dag.CircuitBreakersSettings{ + MaxConnections: 9000, + }, Weighted: dag.WeightedService{ Weight: 1, ServiceName: s1.Name, @@ -438,7 +440,9 @@ func TestCluster(t *testing.T) { "projectcontour.io/max-pending-requests": { cluster: &dag.Cluster{ Upstream: &dag.Service{ - MaxPendingRequests: 4096, + CircuitBreakersSettings: dag.CircuitBreakersSettings{ + MaxPendingRequests: 4096, + }, Weighted: dag.WeightedService{ Weight: 1, ServiceName: s1.Name, @@ -470,7 +474,9 @@ func TestCluster(t *testing.T) { "projectcontour.io/max-requests": { cluster: &dag.Cluster{ Upstream: &dag.Service{ - MaxRequests: 404, + CircuitBreakersSettings: dag.CircuitBreakersSettings{ + MaxRequests: 404, + }, Weighted: dag.WeightedService{ Weight: 1, ServiceName: s1.Name, @@ -502,7 +508,9 @@ func TestCluster(t *testing.T) { "projectcontour.io/max-retries": { cluster: &dag.Cluster{ Upstream: &dag.Service{ - MaxRetries: 7, + CircuitBreakersSettings: dag.CircuitBreakersSettings{ + MaxRetries: 7, + }, Weighted: dag.WeightedService{ Weight: 1, ServiceName: s1.Name, @@ -534,7 +542,9 @@ func TestCluster(t *testing.T) { "projectcontour.io/per-host-max-connections": { cluster: &dag.Cluster{ Upstream: &dag.Service{ - PerHostMaxConnections: 45, + CircuitBreakersSettings: dag.CircuitBreakersSettings{ + PerHostMaxConnections: 45, + }, Weighted: dag.WeightedService{ Weight: 1, ServiceName: s1.Name, diff --git a/internal/featuretests/v3/cluster_test.go b/internal/featuretests/v3/cluster_test.go index ff4eb77c07d..c1272b5bbc7 100644 --- a/internal/featuretests/v3/cluster_test.go +++ b/internal/featuretests/v3/cluster_test.go @@ -391,7 +391,7 @@ func TestCDSResourceFiltering(t *testing.T) { }) } -func circuitBreakerGlobalOpt(t *testing.T, g *contour_v1alpha1.GlobalCircuitBreakerDefaults) func(*dag.Builder) { +func circuitBreakerGlobalOpt(t *testing.T, g *contour_v1alpha1.CircuitBreaker) func(*dag.Builder) { return func(b *dag.Builder) { log := fixture.NewTestLogger(t) log.SetLevel(logrus.DebugLevel) @@ -414,7 +414,7 @@ func circuitBreakerGlobalOpt(t *testing.T, g *contour_v1alpha1.GlobalCircuitBrea } func TestClusterCircuitbreakerAnnotationsIngress(t *testing.T) { - g := &contour_v1alpha1.GlobalCircuitBreakerDefaults{ + g := &contour_v1alpha1.CircuitBreaker{ MaxConnections: 13, MaxPendingRequests: 14, MaxRequests: 15, @@ -549,7 +549,7 @@ func TestClusterCircuitbreakerAnnotationsIngress(t *testing.T) { } func TestClusterCircuitbreakerAnnotationsHTTPProxy(t *testing.T) { - g := &contour_v1alpha1.GlobalCircuitBreakerDefaults{ + g := &contour_v1alpha1.CircuitBreaker{ MaxConnections: 13, MaxPendingRequests: 14, MaxRequests: 15, @@ -692,7 +692,7 @@ func TestClusterCircuitbreakerAnnotationsHTTPProxy(t *testing.T) { } func TestClusterCircuitbreakerAnnotationsGateway(t *testing.T) { - g := &contour_v1alpha1.GlobalCircuitBreakerDefaults{ + g := &contour_v1alpha1.CircuitBreaker{ MaxConnections: 13, MaxPendingRequests: 14, MaxRequests: 15, diff --git a/internal/featuretests/v3/extensionservice_test.go b/internal/featuretests/v3/extensionservice_test.go index 93314db9505..060439aee7e 100644 --- a/internal/featuretests/v3/extensionservice_test.go +++ b/internal/featuretests/v3/extensionservice_test.go @@ -23,11 +23,13 @@ import ( envoy_transport_socket_tls_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" envoy_service_discovery_v3 "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3" envoy_matcher_v3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" + "google.golang.org/protobuf/types/known/wrapperspb" core_v1 "k8s.io/api/core/v1" "k8s.io/utils/ptr" contour_v1 "github.com/projectcontour/contour/apis/projectcontour/v1" contour_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" + "github.com/projectcontour/contour/internal/dag" envoy_v3 "github.com/projectcontour/contour/internal/envoy/v3" "github.com/projectcontour/contour/internal/featuretests" "github.com/projectcontour/contour/internal/fixture" @@ -402,6 +404,192 @@ func extInvalidLoadBalancerPolicy(t *testing.T, rh ResourceEventHandlerWrapper, }) } +func extCircuitBreakers(t *testing.T, rh ResourceEventHandlerWrapper, c *Contour) { + ext := &contour_v1alpha1.ExtensionService{ + ObjectMeta: fixture.ObjectMeta("ns/ext"), + Spec: contour_v1alpha1.ExtensionServiceSpec{ + Services: []contour_v1alpha1.ExtensionServiceTarget{ + {Name: "svc1", Port: 8081}, + {Name: "svc2", Port: 8082}, + }, + LoadBalancerPolicy: &contour_v1.LoadBalancerPolicy{ + Strategy: "Cookie", + }, + CircuitBreakerPolicy: &contour_v1alpha1.CircuitBreaker{ + MaxConnections: 10000, + MaxPendingRequests: 1048, + MaxRequests: 494, + MaxRetries: 10, + PerHostMaxConnections: 1, + }, + }, + } + + rh.OnAdd(ext) + + c.Request(clusterType).Equals(&envoy_service_discovery_v3.DiscoveryResponse{ + TypeUrl: clusterType, + Resources: resources(t, + DefaultCluster( + // Default load balancer policy should be set as we were passed + // an invalid value, we can assert we get a basic cluster. + h2cCluster(cluster("extension/ns/ext", "extension/ns/ext", "extension_ns_ext")), + &envoy_config_cluster_v3.Cluster{ + TransportSocket: envoy_v3.UpstreamTLSTransportSocket( + &envoy_transport_socket_tls_v3.UpstreamTlsContext{ + CommonTlsContext: &envoy_transport_socket_tls_v3.CommonTlsContext{ + AlpnProtocols: []string{"h2"}, + }, + }, + ), + CircuitBreakers: &envoy_config_cluster_v3.CircuitBreakers{ + Thresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: wrapperspb.UInt32(10000), + MaxPendingRequests: wrapperspb.UInt32(1048), + MaxRequests: wrapperspb.UInt32(494), + MaxRetries: wrapperspb.UInt32(10), + TrackRemaining: true, + }}, + PerHostThresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: wrapperspb.UInt32(1), + TrackRemaining: true, + }}, + }, + }, + ), + ), + }) + + rh.OnUpdate(ext, &contour_v1alpha1.ExtensionService{ + ObjectMeta: fixture.ObjectMeta("ns/ext"), + Spec: contour_v1alpha1.ExtensionServiceSpec{ + Services: []contour_v1alpha1.ExtensionServiceTarget{ + {Name: "svc1", Port: 8081}, + {Name: "svc2", Port: 8082}, + }, + LoadBalancerPolicy: &contour_v1.LoadBalancerPolicy{ + Strategy: "RequestHash", + }, + }, + }) + + c.Request(clusterType).Equals(&envoy_service_discovery_v3.DiscoveryResponse{ + TypeUrl: clusterType, + Resources: resources(t, + DefaultCluster( + // Default load balancer policy should be set as we were passed + // an invalid value, we can assert we get a basic cluster. + h2cCluster(cluster("extension/ns/ext", "extension/ns/ext", "extension_ns_ext")), + &envoy_config_cluster_v3.Cluster{ + TransportSocket: envoy_v3.UpstreamTLSTransportSocket( + &envoy_transport_socket_tls_v3.UpstreamTlsContext{ + CommonTlsContext: &envoy_transport_socket_tls_v3.CommonTlsContext{ + AlpnProtocols: []string{"h2"}, + }, + }, + ), + }, + ), + ), + }) +} + +func extGlobalCircuitBreakers(t *testing.T, rh ResourceEventHandlerWrapper, c *Contour) { + ext := &contour_v1alpha1.ExtensionService{ + ObjectMeta: fixture.ObjectMeta("ns/ext"), + Spec: contour_v1alpha1.ExtensionServiceSpec{ + Services: []contour_v1alpha1.ExtensionServiceTarget{ + {Name: "svc1", Port: 8081}, + {Name: "svc2", Port: 8082}, + }, + LoadBalancerPolicy: &contour_v1.LoadBalancerPolicy{ + Strategy: "Cookie", + }, + }, + } + + rh.OnAdd(ext) + + c.Request(clusterType).Equals(&envoy_service_discovery_v3.DiscoveryResponse{ + TypeUrl: clusterType, + Resources: resources(t, + DefaultCluster( + // Default load balancer policy should be set as we were passed + // an invalid value, we can assert we get a basic cluster. + h2cCluster(cluster("extension/ns/ext", "extension/ns/ext", "extension_ns_ext")), + &envoy_config_cluster_v3.Cluster{ + TransportSocket: envoy_v3.UpstreamTLSTransportSocket( + &envoy_transport_socket_tls_v3.UpstreamTlsContext{ + CommonTlsContext: &envoy_transport_socket_tls_v3.CommonTlsContext{ + AlpnProtocols: []string{"h2"}, + }, + }, + ), + CircuitBreakers: &envoy_config_cluster_v3.CircuitBreakers{ + Thresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: wrapperspb.UInt32(20000), + MaxPendingRequests: wrapperspb.UInt32(2048), + MaxRequests: wrapperspb.UInt32(294), + MaxRetries: wrapperspb.UInt32(20), + TrackRemaining: true, + }}, + PerHostThresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: wrapperspb.UInt32(10), + TrackRemaining: true, + }}, + }, + }, + ), + ), + }) + + rh.OnUpdate(ext, &contour_v1alpha1.ExtensionService{ + ObjectMeta: fixture.ObjectMeta("ns/ext"), + Spec: contour_v1alpha1.ExtensionServiceSpec{ + Services: []contour_v1alpha1.ExtensionServiceTarget{ + {Name: "svc1", Port: 8081}, + {Name: "svc2", Port: 8082}, + }, + LoadBalancerPolicy: &contour_v1.LoadBalancerPolicy{ + Strategy: "RequestHash", + }, + }, + }) + + c.Request(clusterType).Equals(&envoy_service_discovery_v3.DiscoveryResponse{ + TypeUrl: clusterType, + Resources: resources(t, + DefaultCluster( + // Default load balancer policy should be set as we were passed + // an invalid value, we can assert we get a basic cluster. + h2cCluster(cluster("extension/ns/ext", "extension/ns/ext", "extension_ns_ext")), + &envoy_config_cluster_v3.Cluster{ + TransportSocket: envoy_v3.UpstreamTLSTransportSocket( + &envoy_transport_socket_tls_v3.UpstreamTlsContext{ + CommonTlsContext: &envoy_transport_socket_tls_v3.CommonTlsContext{ + AlpnProtocols: []string{"h2"}, + }, + }, + ), + CircuitBreakers: &envoy_config_cluster_v3.CircuitBreakers{ + Thresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: wrapperspb.UInt32(20000), + MaxPendingRequests: wrapperspb.UInt32(2048), + MaxRequests: wrapperspb.UInt32(294), + MaxRetries: wrapperspb.UInt32(20), + TrackRemaining: true, + }}, + PerHostThresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: wrapperspb.UInt32(10), + TrackRemaining: true, + }}, + }, + }, + ), + ), + }) +} + func TestExtensionService(t *testing.T) { subtests := map[string]func(*testing.T, ResourceEventHandlerWrapper, *Contour){ "Basic": extBasic, @@ -413,12 +601,40 @@ func TestExtensionService(t *testing.T) { "InconsistentProto": extInconsistentProto, "InvalidTimeout": extInvalidTimeout, "InvalidLoadBalancerPolicy": extInvalidLoadBalancerPolicy, + "CircuitBreakers": extCircuitBreakers, + "GlobalCircuitBreakers": extGlobalCircuitBreakers, } for n, f := range subtests { f := f t.Run(n, func(t *testing.T) { - rh, c, done := setup(t) + var ( + rh ResourceEventHandlerWrapper + c *Contour + done func() + ) + + switch n { + case "GlobalCircuitBreakers": + rh, c, done = setup(t, + func(b *dag.Builder) { + for _, processor := range b.Processors { + if extensionProcessor, ok := processor.(*dag.ExtensionServiceProcessor); ok { + extensionProcessor.GlobalCircuitBreakerDefaults = &contour_v1alpha1.CircuitBreaker{ + MaxConnections: 20000, + MaxPendingRequests: 2048, + MaxRequests: 294, + MaxRetries: 20, + PerHostMaxConnections: 10, + } + } + } + }) + + default: + rh, c, done = setup(t) + } + defer done() // Add common test fixtures. diff --git a/pkg/config/parameters.go b/pkg/config/parameters.go index b4c7aaa3869..6aa74be5b62 100644 --- a/pkg/config/parameters.go +++ b/pkg/config/parameters.go @@ -441,7 +441,7 @@ type ClusterParameters struct { // GlobalCircuitBreakerDefaults holds configurable global defaults for the circuit breakers. // // +optional - GlobalCircuitBreakerDefaults *contour_v1alpha1.GlobalCircuitBreakerDefaults `yaml:"circuit-breakers,omitempty"` + GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker `yaml:"circuit-breakers,omitempty"` // UpstreamTLS contains the TLS policy parameters for upstream connections UpstreamTLS ProtocolParameters `yaml:"upstream-tls,omitempty"` diff --git a/site/content/docs/main/config/api-reference.html b/site/content/docs/main/config/api-reference.html index b9ba8f7400d..be2b6683154 100644 --- a/site/content/docs/main/config/api-reference.html +++ b/site/content/docs/main/config/api-reference.html @@ -5522,6 +5522,23 @@

ExtensionService protocol options will be available in future.

+ + +circuitBreakerPolicy +
+ + +CircuitBreaker + + + + +(Optional) +

CircuitBreaker specifies the circuit breaker budget across the extension service. +This overrides the global circuite breaker budget if defined. +If defined this overrides the global circuit breaker budget.

+ + @@ -5610,6 +5627,90 @@

AccessLogType +

CircuitBreaker +

+

+(Appears on: +ClusterParameters, +ExtensionServiceSpec) +

+

+

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+maxConnections +
+ +uint32 + +		 +(Optional) +

The maximum number of connections that a single Envoy instance allows to the Kubernetes Service; defaults to 1024.

+
+maxPendingRequests +
+ +uint32 + +		 +(Optional) +

The maximum number of pending requests that a single Envoy instance allows to the Kubernetes Service; defaults to 1024.

+
+maxRequests +
+ +uint32 + +		 +(Optional) +

The maximum parallel requests a single Envoy instance allows to the Kubernetes Service; defaults to 1024

+
+maxRetries +
+ +uint32 + +		 +(Optional) +

The maximum number of parallel retries a single Envoy instance allows to the Kubernetes Service; defaults to 3.

+
+perHostMaxConnections +
+ +uint32 + +		 +

PerHostMaxConnections is the maximum number of connections +that Envoy will allow to each individual host in a cluster.

+

ClusterDNSFamilyType (string alias)

@@ -5723,8 +5824,8 @@

ClusterParameters circuitBreakers
- -GlobalCircuitBreakerDefaults + +CircuitBreaker @@ -7529,6 +7630,23 @@

ExtensionServiceSpec protocol options will be available in future.

+ + +circuitBreakerPolicy +
+ + +CircuitBreaker + + + + +(Optional) +

CircuitBreaker specifies the circuit breaker budget across the extension service. +This overrides the global circuite breaker budget if defined. +If defined this overrides the global circuit breaker budget.

+ +

ExtensionServiceStatus @@ -7671,76 +7789,6 @@

GatewayConfig -

GlobalCircuitBreakerDefaults -

-

-(Appears on: -ClusterParameters) -

-

-

- - - - - - - - - - - - - - - - - - - - - - - - - -
FieldDescription
-maxConnections -
- -uint32 - -		 -(Optional) -

The maximum number of connections that a single Envoy instance allows to the Kubernetes Service; defaults to 1024.

-
-maxPendingRequests -
- -uint32 - -		 -(Optional) -

The maximum number of pending requests that a single Envoy instance allows to the Kubernetes Service; defaults to 1024.

-
-maxRequests -
- -uint32 - -		 -(Optional) -

The maximum parallel requests a single Envoy instance allows to the Kubernetes Service; defaults to 1024

-
-maxRetries -
- -uint32 - -		 -(Optional) -

The maximum number of parallel retries a single Envoy instance allows to the Kubernetes Service; defaults to 3.

-

HTTPProxyConfig

From 4790f07b4d94965415cb8ef2ac8004949da334e1 Mon Sep 17 00:00:00 2001 From: Clayton Gonsalves Date: Thu, 18 Jul 2024 15:56:07 +0200 Subject: [PATCH 2/5] address review comments Signed-off-by: Clayton Gonsalves --- internal/envoy/v3/cluster.go | 36 ++--- .../featuretests/v3/extensionservice_test.go | 128 ++++++++++++++++-- 2 files changed, 129 insertions(+), 35 deletions(-) diff --git a/internal/envoy/v3/cluster.go b/internal/envoy/v3/cluster.go index 6739224ce6e..7162707354c 100644 --- a/internal/envoy/v3/cluster.go +++ b/internal/envoy/v3/cluster.go @@ -79,21 +79,7 @@ func Cluster(c *dag.Cluster) *envoy_config_cluster_v3.Cluster { cluster.IgnoreHealthOnHostRemoval = true } - if envoy.AnyPositive(service.CircuitBreakersSettings.MaxConnections, service.CircuitBreakersSettings.MaxPendingRequests, service.CircuitBreakersSettings.MaxRequests, service.CircuitBreakersSettings.MaxRetries, service.CircuitBreakersSettings.PerHostMaxConnections) { - cluster.CircuitBreakers = &envoy_config_cluster_v3.CircuitBreakers{ - Thresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ - MaxConnections: protobuf.UInt32OrNil(service.CircuitBreakersSettings.MaxConnections), - MaxPendingRequests: protobuf.UInt32OrNil(service.CircuitBreakersSettings.MaxPendingRequests), - MaxRequests: protobuf.UInt32OrNil(service.CircuitBreakersSettings.MaxRequests), - MaxRetries: protobuf.UInt32OrNil(service.CircuitBreakersSettings.MaxRetries), - TrackRemaining: true, - }}, - PerHostThresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ - MaxConnections: protobuf.UInt32OrNil(service.CircuitBreakersSettings.PerHostMaxConnections), - TrackRemaining: true, - }}, - } - } + applyCircuitBreakerSettings(cluster, &service.CircuitBreakersSettings) httpVersion := HTTPVersionAuto switch c.Protocol { @@ -198,23 +184,27 @@ func ExtensionCluster(ext *dag.ExtensionCluster) *envoy_config_cluster_v3.Cluste } cluster.TypedExtensionProtocolOptions = protocolOptions(http2Version, ext.ClusterTimeoutPolicy.IdleConnectionTimeout, nil) - if envoy.AnyPositive(ext.CircuitBreakersSettings.MaxConnections, ext.CircuitBreakersSettings.MaxPendingRequests, ext.CircuitBreakersSettings.MaxRequests, ext.CircuitBreakersSettings.MaxRetries, ext.CircuitBreakersSettings.PerHostMaxConnections) { + applyCircuitBreakerSettings(cluster, &ext.CircuitBreakersSettings) + + return cluster +} + +func applyCircuitBreakerSettings(cluster *envoy_config_cluster_v3.Cluster, settings *dag.CircuitBreakersSettings) { + if envoy.AnyPositive(settings.MaxConnections, settings.MaxPendingRequests, settings.MaxRequests, settings.MaxRetries, settings.PerHostMaxConnections) { cluster.CircuitBreakers = &envoy_config_cluster_v3.CircuitBreakers{ Thresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ - MaxConnections: protobuf.UInt32OrNil(ext.CircuitBreakersSettings.MaxConnections), - MaxPendingRequests: protobuf.UInt32OrNil(ext.CircuitBreakersSettings.MaxPendingRequests), - MaxRequests: protobuf.UInt32OrNil(ext.CircuitBreakersSettings.MaxRequests), - MaxRetries: protobuf.UInt32OrNil(ext.CircuitBreakersSettings.MaxRetries), + MaxConnections: protobuf.UInt32OrNil(settings.MaxConnections), + MaxPendingRequests: protobuf.UInt32OrNil(settings.MaxPendingRequests), + MaxRequests: protobuf.UInt32OrNil(settings.MaxRequests), + MaxRetries: protobuf.UInt32OrNil(settings.MaxRetries), TrackRemaining: true, }}, PerHostThresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ - MaxConnections: protobuf.UInt32OrNil(ext.CircuitBreakersSettings.PerHostMaxConnections), + MaxConnections: protobuf.UInt32OrNil(settings.PerHostMaxConnections), TrackRemaining: true, }}, } } - - return cluster } // DNSNameCluster builds a envoy_config_cluster_v3.Cluster for the given *dag.DNSNameCluster. diff --git a/internal/featuretests/v3/extensionservice_test.go b/internal/featuretests/v3/extensionservice_test.go index 060439aee7e..7ff7537fa71 100644 --- a/internal/featuretests/v3/extensionservice_test.go +++ b/internal/featuretests/v3/extensionservice_test.go @@ -590,19 +590,123 @@ func extGlobalCircuitBreakers(t *testing.T, rh ResourceEventHandlerWrapper, c *C }) } +func overrideExtGlobalCircuitBreakers(t *testing.T, rh ResourceEventHandlerWrapper, c *Contour) { + ext := &contour_v1alpha1.ExtensionService{ + ObjectMeta: fixture.ObjectMeta("ns/ext"), + Spec: contour_v1alpha1.ExtensionServiceSpec{ + Services: []contour_v1alpha1.ExtensionServiceTarget{ + {Name: "svc1", Port: 8081}, + {Name: "svc2", Port: 8082}, + }, + LoadBalancerPolicy: &contour_v1.LoadBalancerPolicy{ + Strategy: "Cookie", + }, + CircuitBreakerPolicy: &contour_v1alpha1.CircuitBreaker{ + MaxConnections: 30000, + MaxPendingRequests: 3048, + MaxRequests: 394, + MaxRetries: 30, + PerHostMaxConnections: 30, + }, + }, + } + + rh.OnAdd(ext) + + c.Request(clusterType).Equals(&envoy_service_discovery_v3.DiscoveryResponse{ + TypeUrl: clusterType, + Resources: resources(t, + DefaultCluster( + // Default load balancer policy should be set as we were passed + // an invalid value, we can assert we get a basic cluster. + h2cCluster(cluster("extension/ns/ext", "extension/ns/ext", "extension_ns_ext")), + &envoy_config_cluster_v3.Cluster{ + TransportSocket: envoy_v3.UpstreamTLSTransportSocket( + &envoy_transport_socket_tls_v3.UpstreamTlsContext{ + CommonTlsContext: &envoy_transport_socket_tls_v3.CommonTlsContext{ + AlpnProtocols: []string{"h2"}, + }, + }, + ), + CircuitBreakers: &envoy_config_cluster_v3.CircuitBreakers{ + Thresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: wrapperspb.UInt32(30000), + MaxPendingRequests: wrapperspb.UInt32(3048), + MaxRequests: wrapperspb.UInt32(394), + MaxRetries: wrapperspb.UInt32(30), + TrackRemaining: true, + }}, + PerHostThresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: wrapperspb.UInt32(30), + TrackRemaining: true, + }}, + }, + }, + ), + ), + }) + + rh.OnUpdate(ext, &contour_v1alpha1.ExtensionService{ + ObjectMeta: fixture.ObjectMeta("ns/ext"), + Spec: contour_v1alpha1.ExtensionServiceSpec{ + Services: []contour_v1alpha1.ExtensionServiceTarget{ + {Name: "svc1", Port: 8081}, + {Name: "svc2", Port: 8082}, + }, + LoadBalancerPolicy: &contour_v1.LoadBalancerPolicy{ + Strategy: "RequestHash", + }, + }, + }) + + c.Request(clusterType).Equals(&envoy_service_discovery_v3.DiscoveryResponse{ + TypeUrl: clusterType, + Resources: resources(t, + DefaultCluster( + // Default load balancer policy should be set as we were passed + // an invalid value, we can assert we get a basic cluster. + h2cCluster(cluster("extension/ns/ext", "extension/ns/ext", "extension_ns_ext")), + &envoy_config_cluster_v3.Cluster{ + TransportSocket: envoy_v3.UpstreamTLSTransportSocket( + &envoy_transport_socket_tls_v3.UpstreamTlsContext{ + CommonTlsContext: &envoy_transport_socket_tls_v3.CommonTlsContext{ + AlpnProtocols: []string{"h2"}, + }, + }, + ), + CircuitBreakers: &envoy_config_cluster_v3.CircuitBreakers{ + Thresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: wrapperspb.UInt32(20000), + MaxPendingRequests: wrapperspb.UInt32(2048), + MaxRequests: wrapperspb.UInt32(294), + MaxRetries: wrapperspb.UInt32(20), + TrackRemaining: true, + }}, + PerHostThresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ + MaxConnections: wrapperspb.UInt32(10), + TrackRemaining: true, + }}, + }, + }, + ), + ), + }) +} + func TestExtensionService(t *testing.T) { subtests := map[string]func(*testing.T, ResourceEventHandlerWrapper, *Contour){ - "Basic": extBasic, - "Cleartext": extCleartext, - "UpstreamValidation": extUpstreamValidation, - "ExternalName": extExternalName, - "IdleConnectionTimeout": extIdleConnectionTimeout, - "MissingService": extMissingService, - "InconsistentProto": extInconsistentProto, - "InvalidTimeout": extInvalidTimeout, - "InvalidLoadBalancerPolicy": extInvalidLoadBalancerPolicy, - "CircuitBreakers": extCircuitBreakers, - "GlobalCircuitBreakers": extGlobalCircuitBreakers, + "Basic": extBasic, + "Cleartext": extCleartext, + "UpstreamValidation": extUpstreamValidation, + "ExternalName": extExternalName, + "IdleConnectionTimeout": extIdleConnectionTimeout, + "MissingService": extMissingService, + "InconsistentProto": extInconsistentProto, + "InvalidTimeout": extInvalidTimeout, + "InvalidLoadBalancerPolicy": extInvalidLoadBalancerPolicy, + "CircuitBreakers": extCircuitBreakers, + "GlobalCircuitBreakers": extGlobalCircuitBreakers, + "OverrideGlobalCircuitBreakers": overrideExtGlobalCircuitBreakers, } for n, f := range subtests { @@ -615,7 +719,7 @@ func TestExtensionService(t *testing.T) { ) switch n { - case "GlobalCircuitBreakers": + case "GlobalCircuitBreakers", "OverrideGlobalCircuitBreakers": rh, c, done = setup(t, func(b *dag.Builder) { for _, processor := range b.Processors { From 6173e058f779acac77bc71fec06d04b1c28e5a61 Mon Sep 17 00:00:00 2001 From: Clayton Gonsalves Date: Mon, 22 Jul 2024 12:47:29 +0200 Subject: [PATCH 3/5] address review comments pt2 Signed-off-by: Clayton Gonsalves --- changelogs/unreleased/6539-clayton-gonsalves-minor.md | 3 +++ internal/envoy/v3/cluster.go | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/changelogs/unreleased/6539-clayton-gonsalves-minor.md b/changelogs/unreleased/6539-clayton-gonsalves-minor.md index 0f9dc955be9..77fa121f92f 100644 --- a/changelogs/unreleased/6539-clayton-gonsalves-minor.md +++ b/changelogs/unreleased/6539-clayton-gonsalves-minor.md @@ -1,3 +1,6 @@ ## Add Circuit Breaker support for Extension Services This change enabled the user to configure the Circuit breakers for extension services either via the global Contour config or on the Extension Service CRD itself on a per Extension Service itself. + +**NOTE**: The `PerHostMaxConnections` is now also configurable via the global settings. + diff --git a/internal/envoy/v3/cluster.go b/internal/envoy/v3/cluster.go index 7162707354c..f4482d744b9 100644 --- a/internal/envoy/v3/cluster.go +++ b/internal/envoy/v3/cluster.go @@ -79,7 +79,7 @@ func Cluster(c *dag.Cluster) *envoy_config_cluster_v3.Cluster { cluster.IgnoreHealthOnHostRemoval = true } - applyCircuitBreakerSettings(cluster, &service.CircuitBreakersSettings) + applyCircuitBreakerSettings(cluster, service.CircuitBreakersSettings) httpVersion := HTTPVersionAuto switch c.Protocol { @@ -184,12 +184,12 @@ func ExtensionCluster(ext *dag.ExtensionCluster) *envoy_config_cluster_v3.Cluste } cluster.TypedExtensionProtocolOptions = protocolOptions(http2Version, ext.ClusterTimeoutPolicy.IdleConnectionTimeout, nil) - applyCircuitBreakerSettings(cluster, &ext.CircuitBreakersSettings) + applyCircuitBreakerSettings(cluster, ext.CircuitBreakersSettings) return cluster } -func applyCircuitBreakerSettings(cluster *envoy_config_cluster_v3.Cluster, settings *dag.CircuitBreakersSettings) { +func applyCircuitBreakerSettings(cluster *envoy_config_cluster_v3.Cluster, settings dag.CircuitBreakersSettings) { if envoy.AnyPositive(settings.MaxConnections, settings.MaxPendingRequests, settings.MaxRequests, settings.MaxRetries, settings.PerHostMaxConnections) { cluster.CircuitBreakers = &envoy_config_cluster_v3.CircuitBreakers{ Thresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ From e389d74974ab1b164182fda19a7b2ce7731d237a Mon Sep 17 00:00:00 2001 From: Clayton Gonsalves Date: Wed, 24 Jul 2024 12:56:24 +0200 Subject: [PATCH 4/5] address review comments Signed-off-by: Clayton Gonsalves --- apis/projectcontour/v1alpha1/contourconfig.go | 4 ++-- .../v1alpha1/extensionservice.go | 5 ++--- .../v1alpha1/zz_generated.deepcopy.go | 12 +++++------ .../6539-clayton-gonsalves-minor.md | 2 +- cmd/contour/serve.go | 2 +- cmd/contour/serve_test.go | 2 +- cmd/contour/servecontext_test.go | 4 ++-- examples/contour/01-crds.yaml | 3 +-- examples/render/contour-deployment.yaml | 3 +-- .../render/contour-gateway-provisioner.yaml | 3 +-- examples/render/contour-gateway.yaml | 3 +-- examples/render/contour.yaml | 3 +-- internal/dag/accessors.go | 2 +- internal/dag/builder_test.go | 2 +- internal/dag/dag.go | 8 ++++---- internal/dag/extension_processor.go | 6 +++--- internal/dag/gatewayapi_processor.go | 2 +- internal/dag/httpproxy_processor.go | 2 +- internal/dag/ingress_processor.go | 2 +- internal/dag/policy.go | 18 ++++++++--------- internal/dag/policy_test.go | 16 +++++++-------- internal/envoy/v3/cluster.go | 6 +++--- internal/envoy/v3/cluster_test.go | 10 +++++----- internal/featuretests/v3/cluster_test.go | 8 ++++---- .../featuretests/v3/extensionservice_test.go | 6 +++--- internal/xdscache/v3/cluster_test.go | 2 +- pkg/config/parameters.go | 2 +- .../docs/main/config/api-reference.html | 20 +++++++++---------- 28 files changed, 75 insertions(+), 83 deletions(-) diff --git a/apis/projectcontour/v1alpha1/contourconfig.go b/apis/projectcontour/v1alpha1/contourconfig.go index 913e91554a1..d13b58970ab 100644 --- a/apis/projectcontour/v1alpha1/contourconfig.go +++ b/apis/projectcontour/v1alpha1/contourconfig.go @@ -107,7 +107,7 @@ const ( EnvoyServerType XDSServerType = "envoy" ) -type CircuitBreaker struct { +type CircuitBreakers struct { // The maximum number of connections that a single Envoy instance allows to the Kubernetes Service; defaults to 1024. // +optional MaxConnections uint32 `json:"maxConnections,omitempty" yaml:"max-connections,omitempty"` @@ -708,7 +708,7 @@ type ClusterParameters struct { // If defined, this will be used as the default for all services. // // +optional - GlobalCircuitBreakerDefaults *CircuitBreaker `json:"circuitBreakers,omitempty"` + GlobalCircuitBreakerDefaults *CircuitBreakers `json:"circuitBreakers,omitempty"` // UpstreamTLS contains the TLS policy parameters for upstream connections // diff --git a/apis/projectcontour/v1alpha1/extensionservice.go b/apis/projectcontour/v1alpha1/extensionservice.go index 8ebea4a5a0e..c074de546e7 100644 --- a/apis/projectcontour/v1alpha1/extensionservice.go +++ b/apis/projectcontour/v1alpha1/extensionservice.go @@ -105,11 +105,10 @@ type ExtensionServiceSpec struct { // +kubebuilder:validation:Enum=v3 ProtocolVersion ExtensionProtocolVersion `json:"protocolVersion,omitempty"` - // CircuitBreaker specifies the circuit breaker budget across the extension service. - // This overrides the global circuite breaker budget if defined. + // CircuitBreakerPolicy specifies the circuit breaker budget across the extension service. // If defined this overrides the global circuit breaker budget. // +optional - CircuitBreakerPolicy *CircuitBreaker `json:"circuitBreakerPolicy,omitempty"` + CircuitBreakerPolicy *CircuitBreakers `json:"circuitBreakerPolicy,omitempty"` } // ExtensionServiceStatus defines the observed state of an diff --git a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go index 26bea21da48..394c22dff6e 100644 --- a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go +++ b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go @@ -48,16 +48,16 @@ func (in AccessLogJSONFields) DeepCopy() AccessLogJSONFields { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *CircuitBreaker) DeepCopyInto(out *CircuitBreaker) { +func (in *CircuitBreakers) DeepCopyInto(out *CircuitBreakers) { *out = *in } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CircuitBreaker. -func (in *CircuitBreaker) DeepCopy() *CircuitBreaker { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CircuitBreakers. +func (in *CircuitBreakers) DeepCopy() *CircuitBreakers { if in == nil { return nil } - out := new(CircuitBreaker) + out := new(CircuitBreakers) in.DeepCopyInto(out) return out } @@ -77,7 +77,7 @@ func (in *ClusterParameters) DeepCopyInto(out *ClusterParameters) { } if in.GlobalCircuitBreakerDefaults != nil { in, out := &in.GlobalCircuitBreakerDefaults, &out.GlobalCircuitBreakerDefaults - *out = new(CircuitBreaker) + *out = new(CircuitBreakers) **out = **in } if in.UpstreamTLS != nil { @@ -830,7 +830,7 @@ func (in *ExtensionServiceSpec) DeepCopyInto(out *ExtensionServiceSpec) { } if in.CircuitBreakerPolicy != nil { in, out := &in.CircuitBreakerPolicy, &out.CircuitBreakerPolicy - *out = new(CircuitBreaker) + *out = new(CircuitBreakers) **out = **in } } diff --git a/changelogs/unreleased/6539-clayton-gonsalves-minor.md b/changelogs/unreleased/6539-clayton-gonsalves-minor.md index 77fa121f92f..6601ec2203d 100644 --- a/changelogs/unreleased/6539-clayton-gonsalves-minor.md +++ b/changelogs/unreleased/6539-clayton-gonsalves-minor.md @@ -1,6 +1,6 @@ ## Add Circuit Breaker support for Extension Services -This change enabled the user to configure the Circuit breakers for extension services either via the global Contour config or on the Extension Service CRD itself on a per Extension Service itself. +This change enables the user to configure the Circuit breakers for extension services either via the global Contour config or on an individual Extension Service. **NOTE**: The `PerHostMaxConnections` is now also configurable via the global settings. diff --git a/cmd/contour/serve.go b/cmd/contour/serve.go index d8abf8cd7a1..37695e8c018 100644 --- a/cmd/contour/serve.go +++ b/cmd/contour/serve.go @@ -1064,7 +1064,7 @@ type dagBuilderConfig struct { maxRequestsPerConnection *uint32 perConnectionBufferLimitBytes *uint32 globalRateLimitService *contour_v1alpha1.RateLimitServiceConfig - globalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker + globalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreakers upstreamTLS *dag.UpstreamTLS } diff --git a/cmd/contour/serve_test.go b/cmd/contour/serve_test.go index add26319dd2..f7846e5dffb 100644 --- a/cmd/contour/serve_test.go +++ b/cmd/contour/serve_test.go @@ -125,7 +125,7 @@ func TestGetDAGBuilder(t *testing.T) { }) t.Run("GlobalCircuitBreakerDefaults specified for all processors", func(t *testing.T) { - g := contour_v1alpha1.CircuitBreaker{ + g := contour_v1alpha1.CircuitBreakers{ MaxConnections: 100, } diff --git a/cmd/contour/servecontext_test.go b/cmd/contour/servecontext_test.go index ce8e2ad53a7..cf205d81a46 100644 --- a/cmd/contour/servecontext_test.go +++ b/cmd/contour/servecontext_test.go @@ -767,7 +767,7 @@ func TestConvertServeContext(t *testing.T) { }, "global circuit breaker defaults": { getServeContext: func(ctx *serveContext) *serveContext { - ctx.Config.Cluster.GlobalCircuitBreakerDefaults = &contour_v1alpha1.CircuitBreaker{ + ctx.Config.Cluster.GlobalCircuitBreakerDefaults = &contour_v1alpha1.CircuitBreakers{ MaxConnections: 4, MaxPendingRequests: 5, MaxRequests: 6, @@ -776,7 +776,7 @@ func TestConvertServeContext(t *testing.T) { return ctx }, getContourConfiguration: func(cfg contour_v1alpha1.ContourConfigurationSpec) contour_v1alpha1.ContourConfigurationSpec { - cfg.Envoy.Cluster.GlobalCircuitBreakerDefaults = &contour_v1alpha1.CircuitBreaker{ + cfg.Envoy.Cluster.GlobalCircuitBreakerDefaults = &contour_v1alpha1.CircuitBreakers{ MaxConnections: 4, MaxPendingRequests: 5, MaxRequests: 6, diff --git a/examples/contour/01-crds.yaml b/examples/contour/01-crds.yaml index 4ee0e8c9982..ffd1de96495 100644 --- a/examples/contour/01-crds.yaml +++ b/examples/contour/01-crds.yaml @@ -5071,8 +5071,7 @@ spec: properties: circuitBreakerPolicy: description: |- - CircuitBreaker specifies the circuit breaker budget across the extension service. - This overrides the global circuite breaker budget if defined. + CircuitBreakerPolicy specifies the circuit breaker budget across the extension service. If defined this overrides the global circuit breaker budget. properties: maxConnections: diff --git a/examples/render/contour-deployment.yaml b/examples/render/contour-deployment.yaml index 5514bb75028..d09e1fe3e2e 100644 --- a/examples/render/contour-deployment.yaml +++ b/examples/render/contour-deployment.yaml @@ -5291,8 +5291,7 @@ spec: properties: circuitBreakerPolicy: description: |- - CircuitBreaker specifies the circuit breaker budget across the extension service. - This overrides the global circuite breaker budget if defined. + CircuitBreakerPolicy specifies the circuit breaker budget across the extension service. If defined this overrides the global circuit breaker budget. properties: maxConnections: diff --git a/examples/render/contour-gateway-provisioner.yaml b/examples/render/contour-gateway-provisioner.yaml index a291e4b389a..e7ddd83d9b7 100644 --- a/examples/render/contour-gateway-provisioner.yaml +++ b/examples/render/contour-gateway-provisioner.yaml @@ -5082,8 +5082,7 @@ spec: properties: circuitBreakerPolicy: description: |- - CircuitBreaker specifies the circuit breaker budget across the extension service. - This overrides the global circuite breaker budget if defined. + CircuitBreakerPolicy specifies the circuit breaker budget across the extension service. If defined this overrides the global circuit breaker budget. properties: maxConnections: diff --git a/examples/render/contour-gateway.yaml b/examples/render/contour-gateway.yaml index 3b8bae4c2fd..7648078faba 100644 --- a/examples/render/contour-gateway.yaml +++ b/examples/render/contour-gateway.yaml @@ -5107,8 +5107,7 @@ spec: properties: circuitBreakerPolicy: description: |- - CircuitBreaker specifies the circuit breaker budget across the extension service. - This overrides the global circuite breaker budget if defined. + CircuitBreakerPolicy specifies the circuit breaker budget across the extension service. If defined this overrides the global circuit breaker budget. properties: maxConnections: diff --git a/examples/render/contour.yaml b/examples/render/contour.yaml index b4b57c86a50..816a9db0f4e 100644 --- a/examples/render/contour.yaml +++ b/examples/render/contour.yaml @@ -5291,8 +5291,7 @@ spec: properties: circuitBreakerPolicy: description: |- - CircuitBreaker specifies the circuit breaker budget across the extension service. - This overrides the global circuite breaker budget if defined. + CircuitBreakerPolicy specifies the circuit breaker budget across the extension service. If defined this overrides the global circuit breaker budget. properties: maxConnections: diff --git a/internal/dag/accessors.go b/internal/dag/accessors.go index 5ae27baa56c..c039d28ccba 100644 --- a/internal/dag/accessors.go +++ b/internal/dag/accessors.go @@ -64,7 +64,7 @@ func (d *DAG) EnsureService(meta types.NamespacedName, port, healthPort int, cac Weight: 1, }, Protocol: upstreamProtocol(svc, svcPort), - CircuitBreakersSettings: CircuitBreakersSettings{ + CircuitBreakers: CircuitBreakers{ MaxConnections: annotation.MaxConnections(svc), MaxPendingRequests: annotation.MaxPendingRequests(svc), MaxRequests: annotation.MaxRequests(svc), diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go index 1dcd55d4673..70f9ee3ed0f 100644 --- a/internal/dag/builder_test.go +++ b/internal/dag/builder_test.go @@ -10640,7 +10640,7 @@ func TestDAGInsert(t *testing.T) { ServicePort: s1b.Spec.Ports[0], HealthPort: s1b.Spec.Ports[0], }, - CircuitBreakersSettings: CircuitBreakersSettings{ + CircuitBreakers: CircuitBreakers{ MaxConnections: 9000, MaxPendingRequests: 4096, MaxRequests: 404, diff --git a/internal/dag/dag.go b/internal/dag/dag.go index 5942b66c70b..8ec08fea717 100644 --- a/internal/dag/dag.go +++ b/internal/dag/dag.go @@ -971,7 +971,7 @@ type Service struct { Protocol string // Circuit breaking limits - CircuitBreakersSettings CircuitBreakersSettings + CircuitBreakers CircuitBreakers // ExternalName is an optional field referencing a dns entry for Service type "ExternalName" ExternalName string @@ -1244,7 +1244,7 @@ type ExtensionCluster struct { UpstreamTLS *UpstreamTLS // Circuit breaking limits - CircuitBreakersSettings CircuitBreakersSettings + CircuitBreakers CircuitBreakers } const singleDNSLabelWildcardRegex = "^[a-z0-9]([-a-z0-9]*[a-z0-9])?" @@ -1284,8 +1284,8 @@ type UpstreamTLS struct { CipherSuites []string } -// CircuitBreakersSettings holds configuration for circuit breakers. -type CircuitBreakersSettings struct { +// CircuitBreakers holds configuration for circuit breakers. +type CircuitBreakers struct { // Max connections is maximum number of connections // that Envoy will make to the upstream cluster. MaxConnections uint32 diff --git a/internal/dag/extension_processor.go b/internal/dag/extension_processor.go index 5c41bd19537..8d03f60a360 100644 --- a/internal/dag/extension_processor.go +++ b/internal/dag/extension_processor.go @@ -46,7 +46,7 @@ type ExtensionServiceProcessor struct { UpstreamTLS *UpstreamTLS // GlobalCircuitBreakerDefaults defines global circuit breaker defaults. - GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker + GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreakers } var _ Processor = &ExtensionServiceProcessor{} @@ -126,7 +126,7 @@ func (p *ExtensionServiceProcessor) buildExtensionService( } if p.GlobalCircuitBreakerDefaults != nil { - extension.CircuitBreakersSettings = CircuitBreakersSettings{ + extension.CircuitBreakers = CircuitBreakers{ MaxConnections: p.GlobalCircuitBreakerDefaults.MaxConnections, MaxPendingRequests: p.GlobalCircuitBreakerDefaults.MaxPendingRequests, MaxRequests: p.GlobalCircuitBreakerDefaults.MaxRequests, @@ -136,7 +136,7 @@ func (p *ExtensionServiceProcessor) buildExtensionService( } if ext.Spec.CircuitBreakerPolicy != nil { - extension.CircuitBreakersSettings = CircuitBreakersSettings{ + extension.CircuitBreakers = CircuitBreakers{ MaxConnections: ext.Spec.CircuitBreakerPolicy.MaxConnections, MaxPendingRequests: ext.Spec.CircuitBreakerPolicy.MaxPendingRequests, MaxRequests: ext.Spec.CircuitBreakerPolicy.MaxRequests, diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index 431dc1b8388..5dd9c1deb6e 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -76,7 +76,7 @@ type GatewayAPIProcessor struct { SetSourceMetadataOnRoutes bool // GlobalCircuitBreakerDefaults defines global circuit breaker defaults. - GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker + GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreakers // UpstreamTLS defines the TLS settings like min/max version // and cipher suites for upstream connections. diff --git a/internal/dag/httpproxy_processor.go b/internal/dag/httpproxy_processor.go index bcac9dfaed0..083fd741353 100644 --- a/internal/dag/httpproxy_processor.go +++ b/internal/dag/httpproxy_processor.go @@ -114,7 +114,7 @@ type HTTPProxyProcessor struct { SetSourceMetadataOnRoutes bool // GlobalCircuitBreakerDefaults defines global circuit breaker defaults. - GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker + GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreakers // UpstreamTLS defines the TLS settings like min/max version // and cipher suites for upstream connections. diff --git a/internal/dag/ingress_processor.go b/internal/dag/ingress_processor.go index fd2c3f761f7..63639e22712 100644 --- a/internal/dag/ingress_processor.go +++ b/internal/dag/ingress_processor.go @@ -68,7 +68,7 @@ type IngressProcessor struct { SetSourceMetadataOnRoutes bool // GlobalCircuitBreakerDefaults defines global circuit breaker defaults. - GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker + GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreakers // UpstreamTLS defines the TLS settings like min/max version // and cipher suites for upstream connections. diff --git a/internal/dag/policy.go b/internal/dag/policy.go index bb6b0511fdf..46a90bb22a0 100644 --- a/internal/dag/policy.go +++ b/internal/dag/policy.go @@ -808,25 +808,25 @@ func loadBalancerRequestHashPolicies(lbp *contour_v1.LoadBalancerPolicy, validCo } } -func serviceCircuitBreakerPolicy(s *Service, cb *contour_v1alpha1.CircuitBreaker) *Service { +func serviceCircuitBreakerPolicy(s *Service, cb *contour_v1alpha1.CircuitBreakers) *Service { if s == nil { return nil } - if s.CircuitBreakersSettings.MaxConnections == 0 && cb != nil { - s.CircuitBreakersSettings.MaxConnections = cb.MaxConnections + if s.CircuitBreakers.MaxConnections == 0 && cb != nil { + s.CircuitBreakers.MaxConnections = cb.MaxConnections } - if s.CircuitBreakersSettings.MaxPendingRequests == 0 && cb != nil { - s.CircuitBreakersSettings.MaxPendingRequests = cb.MaxPendingRequests + if s.CircuitBreakers.MaxPendingRequests == 0 && cb != nil { + s.CircuitBreakers.MaxPendingRequests = cb.MaxPendingRequests } - if s.CircuitBreakersSettings.MaxRequests == 0 && cb != nil { - s.CircuitBreakersSettings.MaxRequests = cb.MaxRequests + if s.CircuitBreakers.MaxRequests == 0 && cb != nil { + s.CircuitBreakers.MaxRequests = cb.MaxRequests } - if s.CircuitBreakersSettings.MaxRetries == 0 && cb != nil { - s.CircuitBreakersSettings.MaxRetries = cb.MaxRetries + if s.CircuitBreakers.MaxRetries == 0 && cb != nil { + s.CircuitBreakers.MaxRetries = cb.MaxRetries } return s diff --git a/internal/dag/policy_test.go b/internal/dag/policy_test.go index 258e97d1fc2..b59a018af6d 100644 --- a/internal/dag/policy_test.go +++ b/internal/dag/policy_test.go @@ -1276,7 +1276,7 @@ func TestValidateHeaderAlteration(t *testing.T) { func TestServiceCircuitBreakerPolicy(t *testing.T) { tests := map[string]struct { in *Service - globalDefault *contour_v1alpha1.CircuitBreaker + globalDefault *contour_v1alpha1.CircuitBreakers want *Service }{ "service is nil and globalDefault is nil": { @@ -1286,12 +1286,12 @@ func TestServiceCircuitBreakerPolicy(t *testing.T) { }, "service is nil and globalDefault is not nil": { in: nil, - globalDefault: &contour_v1alpha1.CircuitBreaker{}, + globalDefault: &contour_v1alpha1.CircuitBreakers{}, want: nil, }, "service is not nil and globalDefault is nil": { in: &Service{ - CircuitBreakersSettings: CircuitBreakersSettings{ + CircuitBreakers: CircuitBreakers{ MaxConnections: 42, MaxPendingRequests: 73, MaxRequests: 89, @@ -1300,7 +1300,7 @@ func TestServiceCircuitBreakerPolicy(t *testing.T) { }, globalDefault: nil, want: &Service{ - CircuitBreakersSettings: CircuitBreakersSettings{ + CircuitBreakers: CircuitBreakers{ MaxConnections: 42, MaxPendingRequests: 73, MaxRequests: 89, @@ -1310,14 +1310,14 @@ func TestServiceCircuitBreakerPolicy(t *testing.T) { }, "service is not set but global is": { in: &Service{}, - globalDefault: &contour_v1alpha1.CircuitBreaker{ + globalDefault: &contour_v1alpha1.CircuitBreakers{ MaxConnections: 42, MaxPendingRequests: 73, MaxRequests: 89, MaxRetries: 13, }, want: &Service{ - CircuitBreakersSettings: CircuitBreakersSettings{ + CircuitBreakers: CircuitBreakers{ MaxConnections: 42, MaxPendingRequests: 73, MaxRequests: 89, @@ -1327,13 +1327,13 @@ func TestServiceCircuitBreakerPolicy(t *testing.T) { }, "service is not set but global is partial": { in: &Service{}, - globalDefault: &contour_v1alpha1.CircuitBreaker{ + globalDefault: &contour_v1alpha1.CircuitBreakers{ MaxConnections: 42, MaxPendingRequests: 73, MaxRequests: 89, }, want: &Service{ - CircuitBreakersSettings: CircuitBreakersSettings{ + CircuitBreakers: CircuitBreakers{ MaxConnections: 42, MaxPendingRequests: 73, MaxRequests: 89, diff --git a/internal/envoy/v3/cluster.go b/internal/envoy/v3/cluster.go index f4482d744b9..24346c48421 100644 --- a/internal/envoy/v3/cluster.go +++ b/internal/envoy/v3/cluster.go @@ -79,7 +79,7 @@ func Cluster(c *dag.Cluster) *envoy_config_cluster_v3.Cluster { cluster.IgnoreHealthOnHostRemoval = true } - applyCircuitBreakerSettings(cluster, service.CircuitBreakersSettings) + applyCircuitBreakers(cluster, service.CircuitBreakers) httpVersion := HTTPVersionAuto switch c.Protocol { @@ -184,12 +184,12 @@ func ExtensionCluster(ext *dag.ExtensionCluster) *envoy_config_cluster_v3.Cluste } cluster.TypedExtensionProtocolOptions = protocolOptions(http2Version, ext.ClusterTimeoutPolicy.IdleConnectionTimeout, nil) - applyCircuitBreakerSettings(cluster, ext.CircuitBreakersSettings) + applyCircuitBreakers(cluster, ext.CircuitBreakers) return cluster } -func applyCircuitBreakerSettings(cluster *envoy_config_cluster_v3.Cluster, settings dag.CircuitBreakersSettings) { +func applyCircuitBreakers(cluster *envoy_config_cluster_v3.Cluster, settings dag.CircuitBreakers) { if envoy.AnyPositive(settings.MaxConnections, settings.MaxPendingRequests, settings.MaxRequests, settings.MaxRetries, settings.PerHostMaxConnections) { cluster.CircuitBreakers = &envoy_config_cluster_v3.CircuitBreakers{ Thresholds: []*envoy_config_cluster_v3.CircuitBreakers_Thresholds{{ diff --git a/internal/envoy/v3/cluster_test.go b/internal/envoy/v3/cluster_test.go index 82ba2b6758d..cc9f64bcec5 100644 --- a/internal/envoy/v3/cluster_test.go +++ b/internal/envoy/v3/cluster_test.go @@ -406,7 +406,7 @@ func TestCluster(t *testing.T) { "projectcontour.io/max-connections": { cluster: &dag.Cluster{ Upstream: &dag.Service{ - CircuitBreakersSettings: dag.CircuitBreakersSettings{ + CircuitBreakers: dag.CircuitBreakers{ MaxConnections: 9000, }, Weighted: dag.WeightedService{ @@ -440,7 +440,7 @@ func TestCluster(t *testing.T) { "projectcontour.io/max-pending-requests": { cluster: &dag.Cluster{ Upstream: &dag.Service{ - CircuitBreakersSettings: dag.CircuitBreakersSettings{ + CircuitBreakers: dag.CircuitBreakers{ MaxPendingRequests: 4096, }, Weighted: dag.WeightedService{ @@ -474,7 +474,7 @@ func TestCluster(t *testing.T) { "projectcontour.io/max-requests": { cluster: &dag.Cluster{ Upstream: &dag.Service{ - CircuitBreakersSettings: dag.CircuitBreakersSettings{ + CircuitBreakers: dag.CircuitBreakers{ MaxRequests: 404, }, Weighted: dag.WeightedService{ @@ -508,7 +508,7 @@ func TestCluster(t *testing.T) { "projectcontour.io/max-retries": { cluster: &dag.Cluster{ Upstream: &dag.Service{ - CircuitBreakersSettings: dag.CircuitBreakersSettings{ + CircuitBreakers: dag.CircuitBreakers{ MaxRetries: 7, }, Weighted: dag.WeightedService{ @@ -542,7 +542,7 @@ func TestCluster(t *testing.T) { "projectcontour.io/per-host-max-connections": { cluster: &dag.Cluster{ Upstream: &dag.Service{ - CircuitBreakersSettings: dag.CircuitBreakersSettings{ + CircuitBreakers: dag.CircuitBreakers{ PerHostMaxConnections: 45, }, Weighted: dag.WeightedService{ diff --git a/internal/featuretests/v3/cluster_test.go b/internal/featuretests/v3/cluster_test.go index c1272b5bbc7..2680585ba4b 100644 --- a/internal/featuretests/v3/cluster_test.go +++ b/internal/featuretests/v3/cluster_test.go @@ -391,7 +391,7 @@ func TestCDSResourceFiltering(t *testing.T) { }) } -func circuitBreakerGlobalOpt(t *testing.T, g *contour_v1alpha1.CircuitBreaker) func(*dag.Builder) { +func circuitBreakerGlobalOpt(t *testing.T, g *contour_v1alpha1.CircuitBreakers) func(*dag.Builder) { return func(b *dag.Builder) { log := fixture.NewTestLogger(t) log.SetLevel(logrus.DebugLevel) @@ -414,7 +414,7 @@ func circuitBreakerGlobalOpt(t *testing.T, g *contour_v1alpha1.CircuitBreaker) f } func TestClusterCircuitbreakerAnnotationsIngress(t *testing.T) { - g := &contour_v1alpha1.CircuitBreaker{ + g := &contour_v1alpha1.CircuitBreakers{ MaxConnections: 13, MaxPendingRequests: 14, MaxRequests: 15, @@ -549,7 +549,7 @@ func TestClusterCircuitbreakerAnnotationsIngress(t *testing.T) { } func TestClusterCircuitbreakerAnnotationsHTTPProxy(t *testing.T) { - g := &contour_v1alpha1.CircuitBreaker{ + g := &contour_v1alpha1.CircuitBreakers{ MaxConnections: 13, MaxPendingRequests: 14, MaxRequests: 15, @@ -692,7 +692,7 @@ func TestClusterCircuitbreakerAnnotationsHTTPProxy(t *testing.T) { } func TestClusterCircuitbreakerAnnotationsGateway(t *testing.T) { - g := &contour_v1alpha1.CircuitBreaker{ + g := &contour_v1alpha1.CircuitBreakers{ MaxConnections: 13, MaxPendingRequests: 14, MaxRequests: 15, diff --git a/internal/featuretests/v3/extensionservice_test.go b/internal/featuretests/v3/extensionservice_test.go index 7ff7537fa71..b341882e4d7 100644 --- a/internal/featuretests/v3/extensionservice_test.go +++ b/internal/featuretests/v3/extensionservice_test.go @@ -415,7 +415,7 @@ func extCircuitBreakers(t *testing.T, rh ResourceEventHandlerWrapper, c *Contour LoadBalancerPolicy: &contour_v1.LoadBalancerPolicy{ Strategy: "Cookie", }, - CircuitBreakerPolicy: &contour_v1alpha1.CircuitBreaker{ + CircuitBreakerPolicy: &contour_v1alpha1.CircuitBreakers{ MaxConnections: 10000, MaxPendingRequests: 1048, MaxRequests: 494, @@ -601,7 +601,7 @@ func overrideExtGlobalCircuitBreakers(t *testing.T, rh ResourceEventHandlerWrapp LoadBalancerPolicy: &contour_v1.LoadBalancerPolicy{ Strategy: "Cookie", }, - CircuitBreakerPolicy: &contour_v1alpha1.CircuitBreaker{ + CircuitBreakerPolicy: &contour_v1alpha1.CircuitBreakers{ MaxConnections: 30000, MaxPendingRequests: 3048, MaxRequests: 394, @@ -724,7 +724,7 @@ func TestExtensionService(t *testing.T) { func(b *dag.Builder) { for _, processor := range b.Processors { if extensionProcessor, ok := processor.(*dag.ExtensionServiceProcessor); ok { - extensionProcessor.GlobalCircuitBreakerDefaults = &contour_v1alpha1.CircuitBreaker{ + extensionProcessor.GlobalCircuitBreakerDefaults = &contour_v1alpha1.CircuitBreakers{ MaxConnections: 20000, MaxPendingRequests: 2048, MaxRequests: 294, diff --git a/internal/xdscache/v3/cluster_test.go b/internal/xdscache/v3/cluster_test.go index 585c3bfea57..b370948c6e8 100644 --- a/internal/xdscache/v3/cluster_test.go +++ b/internal/xdscache/v3/cluster_test.go @@ -724,7 +724,7 @@ func TestClusterVisit(t *testing.T) { }, ), }, - "circuitbreaker annotations": { + "CircuitBreakers annotations": { objs: []any{ &networking_v1.Ingress{ ObjectMeta: meta_v1.ObjectMeta{ diff --git a/pkg/config/parameters.go b/pkg/config/parameters.go index 6aa74be5b62..6c0262d50c4 100644 --- a/pkg/config/parameters.go +++ b/pkg/config/parameters.go @@ -441,7 +441,7 @@ type ClusterParameters struct { // GlobalCircuitBreakerDefaults holds configurable global defaults for the circuit breakers. // // +optional - GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreaker `yaml:"circuit-breakers,omitempty"` + GlobalCircuitBreakerDefaults *contour_v1alpha1.CircuitBreakers `yaml:"circuit-breakers,omitempty"` // UpstreamTLS contains the TLS policy parameters for upstream connections UpstreamTLS ProtocolParameters `yaml:"upstream-tls,omitempty"` diff --git a/site/content/docs/main/config/api-reference.html b/site/content/docs/main/config/api-reference.html index be2b6683154..d9584edd341 100644 --- a/site/content/docs/main/config/api-reference.html +++ b/site/content/docs/main/config/api-reference.html @@ -5527,15 +5527,14 @@

ExtensionService circuitBreakerPolicy
- -CircuitBreaker + +CircuitBreakers (Optional) -

CircuitBreaker specifies the circuit breaker budget across the extension service. -This overrides the global circuite breaker budget if defined. +

CircuitBreakerPolicy specifies the circuit breaker budget across the extension service. If defined this overrides the global circuit breaker budget.

@@ -5627,7 +5626,7 @@

AccessLogType -

CircuitBreaker +

CircuitBreakers

(Appears on: @@ -5824,8 +5823,8 @@

ClusterParameters circuitBreakers
- -CircuitBreaker + +CircuitBreakers @@ -7635,15 +7634,14 @@

ExtensionServiceSpec circuitBreakerPolicy
- -CircuitBreaker + +CircuitBreakers (Optional) -

CircuitBreaker specifies the circuit breaker budget across the extension service. -This overrides the global circuite breaker budget if defined. +

CircuitBreakerPolicy specifies the circuit breaker budget across the extension service. If defined this overrides the global circuit breaker budget.

From 61e56a58fba0d2fd68c94ec6fdf70756441df18a Mon Sep 17 00:00:00 2001 From: Clayton Gonsalves Date: Thu, 25 Jul 2024 11:55:58 +0200 Subject: [PATCH 5/5] address review comments Signed-off-by: Clayton Gonsalves --- internal/dag/policy.go | 4 +++ internal/dag/policy_test.go | 52 +++++++++++++++++++++---------------- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/internal/dag/policy.go b/internal/dag/policy.go index 46a90bb22a0..720d959e534 100644 --- a/internal/dag/policy.go +++ b/internal/dag/policy.go @@ -829,5 +829,9 @@ func serviceCircuitBreakerPolicy(s *Service, cb *contour_v1alpha1.CircuitBreaker s.CircuitBreakers.MaxRetries = cb.MaxRetries } + if s.CircuitBreakers.PerHostMaxConnections == 0 && cb != nil { + s.CircuitBreakers.PerHostMaxConnections = cb.PerHostMaxConnections + } + return s } diff --git a/internal/dag/policy_test.go b/internal/dag/policy_test.go index b59a018af6d..ab006db9b57 100644 --- a/internal/dag/policy_test.go +++ b/internal/dag/policy_test.go @@ -1292,52 +1292,58 @@ func TestServiceCircuitBreakerPolicy(t *testing.T) { "service is not nil and globalDefault is nil": { in: &Service{ CircuitBreakers: CircuitBreakers{ - MaxConnections: 42, - MaxPendingRequests: 73, - MaxRequests: 89, - MaxRetries: 13, + MaxConnections: 42, + MaxPendingRequests: 73, + MaxRequests: 89, + MaxRetries: 13, + PerHostMaxConnections: 23, }, }, globalDefault: nil, want: &Service{ CircuitBreakers: CircuitBreakers{ - MaxConnections: 42, - MaxPendingRequests: 73, - MaxRequests: 89, - MaxRetries: 13, + MaxConnections: 42, + MaxPendingRequests: 73, + MaxRequests: 89, + MaxRetries: 13, + PerHostMaxConnections: 23, }, }, }, "service is not set but global is": { in: &Service{}, globalDefault: &contour_v1alpha1.CircuitBreakers{ - MaxConnections: 42, - MaxPendingRequests: 73, - MaxRequests: 89, - MaxRetries: 13, + MaxConnections: 42, + MaxPendingRequests: 73, + MaxRequests: 89, + MaxRetries: 13, + PerHostMaxConnections: 23, }, want: &Service{ CircuitBreakers: CircuitBreakers{ - MaxConnections: 42, - MaxPendingRequests: 73, - MaxRequests: 89, - MaxRetries: 13, + MaxConnections: 42, + MaxPendingRequests: 73, + MaxRequests: 89, + MaxRetries: 13, + PerHostMaxConnections: 23, }, }, }, "service is not set but global is partial": { in: &Service{}, globalDefault: &contour_v1alpha1.CircuitBreakers{ - MaxConnections: 42, - MaxPendingRequests: 73, - MaxRequests: 89, + MaxConnections: 42, + MaxPendingRequests: 73, + MaxRequests: 89, + PerHostMaxConnections: 23, }, want: &Service{ CircuitBreakers: CircuitBreakers{ - MaxConnections: 42, - MaxPendingRequests: 73, - MaxRequests: 89, - MaxRetries: 0, + MaxConnections: 42, + MaxPendingRequests: 73, + MaxRequests: 89, + MaxRetries: 0, + PerHostMaxConnections: 23, }, }, },