From 0e86e82421f8d0985b71585e9d9dcc4ce4447478 Mon Sep 17 00:00:00 2001 From: Zhouyang Zhang Date: Mon, 24 Oct 2022 10:44:24 +0800 Subject: [PATCH 1/3] Add support for LocalConnectTimeoutMs and LocalRequestTimeoutMs on the Service Defaults CRD --- .../consul/templates/crd-servicedefaults.yaml | 9 +++++++++ .../api/v1alpha1/servicedefaults_types.go | 17 +++++++++++++++++ .../api/v1alpha1/servicedefaults_types_test.go | 4 ++++ .../consul.hashicorp.com_servicedefaults.yaml | 9 +++++++++ .../controller/configentry_controller_test.go | 4 ++++ 5 files changed, 43 insertions(+) diff --git a/charts/consul/templates/crd-servicedefaults.yaml b/charts/consul/templates/crd-servicedefaults.yaml index 636e8ce51f..b8b71e9ac2 100644 --- a/charts/consul/templates/crd-servicedefaults.yaml +++ b/charts/consul/templates/crd-servicedefaults.yaml @@ -118,6 +118,15 @@ spec: inbound connections to each service instance. Defaults to 0 (using consul's default) if not set. type: integer + localConnectTimeoutMs: + description: The number of milliseconds allowed to make connections to + the local application instance before timing out. Defaults to 5000. + type: integer + localRequestTimeoutMs: + description: In milliseconds, the timeout for HTTP requests to the local + application instance. Applies to HTTP-based protocols only. If not specified, + inherits the Envoy default for route timeouts (15s). + type: integer meshGateway: description: MeshGateway controls the default mesh gateway configuration for this service. diff --git a/control-plane/api/v1alpha1/servicedefaults_types.go b/control-plane/api/v1alpha1/servicedefaults_types.go index ae0cbb039c..0a4020e010 100644 --- a/control-plane/api/v1alpha1/servicedefaults_types.go +++ b/control-plane/api/v1alpha1/servicedefaults_types.go @@ -88,6 +88,13 @@ type ServiceDefaultsSpec struct { // MaxInboundConnections is the maximum number of concurrent inbound connections to // each service instance. Defaults to 0 (using consul's default) if not set. MaxInboundConnections int `json:"maxInboundConnections,omitempty"` + // The number of milliseconds allowed to make connections to the local application + // instance before timing out. Defaults to 5000. + LocalConnectTimeoutMs int `json:"localConnectTimeoutMs,omitempty"` + // In milliseconds, the timeout for HTTP requests to the local application instance. + // Applies to HTTP-based protocols only. If not specified, inherits the Envoy default for + // route timeouts (15s). + LocalRequestTimeoutMs int `json:"localRequestTimeoutMs,omitempty"` } type Upstreams struct { @@ -264,6 +271,8 @@ func (in *ServiceDefaults) ToConsul(datacenter string) capi.ConfigEntry { Destination: in.Spec.Destination.toConsul(), Meta: meta(datacenter), MaxInboundConnections: in.Spec.MaxInboundConnections, + LocalConnectTimeoutMs: in.Spec.LocalConnectTimeoutMs, + LocalRequestTimeoutMs: in.Spec.LocalRequestTimeoutMs, } } @@ -294,6 +303,14 @@ func (in *ServiceDefaults) Validate(consulMeta common.ConsulMeta) error { allErrs = append(allErrs, field.Invalid(path.Child("maxinboundconnections"), in.Spec.MaxInboundConnections, "MaxInboundConnections must be > 0")) } + if in.Spec.LocalConnectTimeoutMs < 0 { + allErrs = append(allErrs, field.Invalid(path.Child("localConnectTimeoutMs"), in.Spec.LocalConnectTimeoutMs, "LocalConnectTimeoutMs must be > 0")) + } + + if in.Spec.LocalRequestTimeoutMs < 0 { + allErrs = append(allErrs, field.Invalid(path.Child("localRequestTimeoutMs"), in.Spec.LocalRequestTimeoutMs, "LocalRequestTimeoutMs must be > 0")) + } + allErrs = append(allErrs, in.Spec.UpstreamConfig.validate(path.Child("upstreamConfig"), consulMeta.PartitionsEnabled)...) allErrs = append(allErrs, in.Spec.Expose.validate(path.Child("expose"))...) diff --git a/control-plane/api/v1alpha1/servicedefaults_types_test.go b/control-plane/api/v1alpha1/servicedefaults_types_test.go index c70c8a6408..e7fdae2575 100644 --- a/control-plane/api/v1alpha1/servicedefaults_types_test.go +++ b/control-plane/api/v1alpha1/servicedefaults_types_test.go @@ -146,6 +146,8 @@ func TestServiceDefaults_ToConsul(t *testing.T) { Port: 443, }, MaxInboundConnections: 20, + LocalConnectTimeoutMs: 5000, + LocalRequestTimeoutMs: 15000, }, }, &capi.ServiceConfigEntry{ @@ -252,6 +254,8 @@ func TestServiceDefaults_ToConsul(t *testing.T) { Port: 443, }, MaxInboundConnections: 20, + LocalConnectTimeoutMs: 5000, + LocalRequestTimeoutMs: 15000, Meta: map[string]string{ common.SourceKey: common.SourceValue, common.DatacenterKey: "datacenter", diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml index ae815fce47..60a7a58e24 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml @@ -111,6 +111,15 @@ spec: inbound connections to each service instance. Defaults to 0 (using consul's default) if not set. type: integer + localConnectTimeoutMs: + description: The number of milliseconds allowed to make connections to + the local application instance before timing out. Defaults to 5000. + type: integer + localRequestTimeoutMs: + description: In milliseconds, the timeout for HTTP requests to the local + application instance. Applies to HTTP-based protocols only. If not specified, + inherits the Envoy default for route timeouts (15s). + type: integer meshGateway: description: MeshGateway controls the default mesh gateway configuration for this service. diff --git a/control-plane/controller/configentry_controller_test.go b/control-plane/controller/configentry_controller_test.go index 41e832642d..bdf64d2290 100644 --- a/control-plane/controller/configentry_controller_test.go +++ b/control-plane/controller/configentry_controller_test.go @@ -55,6 +55,8 @@ func TestConfigEntryControllers_createsConfigEntry(t *testing.T) { Spec: v1alpha1.ServiceDefaultsSpec{ Protocol: "http", MaxInboundConnections: 100, + LocalConnectTimeoutMs: 5000, + LocalRequestTimeoutMs: 15000, }, }, reconciler: func(client client.Client, cfg *consul.Config, watcher *discovery.Watcher, logger logr.Logger) testReconciler { @@ -73,6 +75,8 @@ func TestConfigEntryControllers_createsConfigEntry(t *testing.T) { require.True(t, ok, "cast error") require.Equal(t, "http", svcDefault.Protocol) require.Equal(t, 100, svcDefault.MaxInboundConnections) + require.Equal(t, 5000, svcDefault.LocalConnectTimeoutMs) + require.Equal(t, 15000, svcDefault.LocalRequestTimeoutMs) }, }, { From 001c0cd7c670b4ec530fd5d82827c3815bc4a5be Mon Sep 17 00:00:00 2001 From: Zhouyang Zhang Date: Mon, 24 Oct 2022 10:44:41 +0800 Subject: [PATCH 2/3] auto gen code --- .../templates/crd-peeringacceptors.yaml | 2 +- .../consul/templates/crd-peeringdialers.yaml | 2 +- .../consul/templates/crd-servicedefaults.yaml | 20 ++++++++++--------- .../api/v1alpha1/zz_generated.deepcopy.go | 7 ++++++- .../consul.hashicorp.com_servicedefaults.yaml | 20 ++++++++++--------- 5 files changed, 30 insertions(+), 21 deletions(-) diff --git a/charts/consul/templates/crd-peeringacceptors.yaml b/charts/consul/templates/crd-peeringacceptors.yaml index e06e830f04..b29b1cd790 100644 --- a/charts/consul/templates/crd-peeringacceptors.yaml +++ b/charts/consul/templates/crd-peeringacceptors.yaml @@ -1,4 +1,4 @@ -{{- if and .Values.connectInject.enabled .Values.global.peering.enabled }} +{{- if .Values.controller.enabled }} --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition diff --git a/charts/consul/templates/crd-peeringdialers.yaml b/charts/consul/templates/crd-peeringdialers.yaml index e24401e761..e876daa15e 100644 --- a/charts/consul/templates/crd-peeringdialers.yaml +++ b/charts/consul/templates/crd-peeringdialers.yaml @@ -1,4 +1,4 @@ -{{- if and .Values.connectInject.enabled .Values.global.peering.enabled }} +{{- if .Values.controller.enabled }} --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition diff --git a/charts/consul/templates/crd-servicedefaults.yaml b/charts/consul/templates/crd-servicedefaults.yaml index b8b71e9ac2..1cf42673a8 100644 --- a/charts/consul/templates/crd-servicedefaults.yaml +++ b/charts/consul/templates/crd-servicedefaults.yaml @@ -113,20 +113,22 @@ spec: TLS SNI value to be changed to a non-connect value when federating with an external system. type: string + localConnectTimeoutMs: + description: The number of milliseconds allowed to make connections + to the local application instance before timing out. Defaults to + 5000. + type: integer + localRequestTimeoutMs: + description: In milliseconds, the timeout for HTTP requests to the + local application instance. Applies to HTTP-based protocols only. + If not specified, inherits the Envoy default for route timeouts + (15s). + type: integer maxInboundConnections: description: MaxInboundConnections is the maximum number of concurrent inbound connections to each service instance. Defaults to 0 (using consul's default) if not set. type: integer - localConnectTimeoutMs: - description: The number of milliseconds allowed to make connections to - the local application instance before timing out. Defaults to 5000. - type: integer - localRequestTimeoutMs: - description: In milliseconds, the timeout for HTTP requests to the local - application instance. Applies to HTTP-based protocols only. If not specified, - inherits the Envoy default for route timeouts (15s). - type: integer meshGateway: description: MeshGateway controls the default mesh gateway configuration for this service. diff --git a/control-plane/api/v1alpha1/zz_generated.deepcopy.go b/control-plane/api/v1alpha1/zz_generated.deepcopy.go index 63ef426790..8e96bdf0c2 100644 --- a/control-plane/api/v1alpha1/zz_generated.deepcopy.go +++ b/control-plane/api/v1alpha1/zz_generated.deepcopy.go @@ -807,6 +807,11 @@ func (in *MeshTLSConfig) DeepCopy() *MeshTLSConfig { func (in *PassiveHealthCheck) DeepCopyInto(out *PassiveHealthCheck) { *out = *in out.Interval = in.Interval + if in.EnforcingConsecutive5xx != nil { + in, out := &in.EnforcingConsecutive5xx, &out.EnforcingConsecutive5xx + *out = new(uint32) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PassiveHealthCheck. @@ -2190,7 +2195,7 @@ func (in *Upstream) DeepCopyInto(out *Upstream) { if in.PassiveHealthCheck != nil { in, out := &in.PassiveHealthCheck, &out.PassiveHealthCheck *out = new(PassiveHealthCheck) - **out = **in + (*in).DeepCopyInto(*out) } out.MeshGateway = in.MeshGateway } diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml index 60a7a58e24..944f494f98 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml @@ -106,20 +106,22 @@ spec: TLS SNI value to be changed to a non-connect value when federating with an external system. type: string + localConnectTimeoutMs: + description: The number of milliseconds allowed to make connections + to the local application instance before timing out. Defaults to + 5000. + type: integer + localRequestTimeoutMs: + description: In milliseconds, the timeout for HTTP requests to the + local application instance. Applies to HTTP-based protocols only. + If not specified, inherits the Envoy default for route timeouts + (15s). + type: integer maxInboundConnections: description: MaxInboundConnections is the maximum number of concurrent inbound connections to each service instance. Defaults to 0 (using consul's default) if not set. type: integer - localConnectTimeoutMs: - description: The number of milliseconds allowed to make connections to - the local application instance before timing out. Defaults to 5000. - type: integer - localRequestTimeoutMs: - description: In milliseconds, the timeout for HTTP requests to the local - application instance. Applies to HTTP-based protocols only. If not specified, - inherits the Envoy default for route timeouts (15s). - type: integer meshGateway: description: MeshGateway controls the default mesh gateway configuration for this service. From 5aaa140fdaa33d69c98a6d722b2b99ea347d3dc4 Mon Sep 17 00:00:00 2001 From: Zhouyang Zhang Date: Mon, 7 Nov 2022 11:09:52 +0800 Subject: [PATCH 3/3] revert the change from make ctrl-generate ctrl-manifests --- charts/consul/templates/crd-peeringacceptors.yaml | 2 +- charts/consul/templates/crd-peeringdialers.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/charts/consul/templates/crd-peeringacceptors.yaml b/charts/consul/templates/crd-peeringacceptors.yaml index b29b1cd790..e06e830f04 100644 --- a/charts/consul/templates/crd-peeringacceptors.yaml +++ b/charts/consul/templates/crd-peeringacceptors.yaml @@ -1,4 +1,4 @@ -{{- if .Values.controller.enabled }} +{{- if and .Values.connectInject.enabled .Values.global.peering.enabled }} --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition diff --git a/charts/consul/templates/crd-peeringdialers.yaml b/charts/consul/templates/crd-peeringdialers.yaml index e876daa15e..e24401e761 100644 --- a/charts/consul/templates/crd-peeringdialers.yaml +++ b/charts/consul/templates/crd-peeringdialers.yaml @@ -1,4 +1,4 @@ -{{- if .Values.controller.enabled }} +{{- if and .Values.connectInject.enabled .Values.global.peering.enabled }} --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition