Skip to content

Commit

Permalink
Add support for LocalConnectTimeoutMs and LocalRequestTimeoutMs on se…
Browse files Browse the repository at this point in the history
…rvice-defaults CRD (#1647)

* Add support for LocalConnectTimeoutMs and LocalRequestTimeoutMs on the Service Defaults CRD
* auto gen code
* revert the change from make ctrl-generate ctrl-manifests
  • Loading branch information
erdanzhang authored and wilkermichael committed Nov 18, 2022
1 parent 6198708 commit 8491aee
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 1 deletion.
11 changes: 11 additions & 0 deletions charts/consul/templates/crd-servicedefaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ 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
Expand Down
17 changes: 17 additions & 0 deletions control-plane/api/v1alpha1/servicedefaults_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,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 {
Expand Down Expand Up @@ -263,6 +270,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,
}
}

Expand Down Expand Up @@ -293,6 +302,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"))...)

Expand Down
4 changes: 4 additions & 0 deletions control-plane/api/v1alpha1/servicedefaults_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ func TestServiceDefaults_ToConsul(t *testing.T) {
Port: 443,
},
MaxInboundConnections: 20,
LocalConnectTimeoutMs: 5000,
LocalRequestTimeoutMs: 15000,
},
},
&capi.ServiceConfigEntry{
Expand Down Expand Up @@ -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",
Expand Down
7 changes: 6 additions & 1 deletion control-plane/api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,17 @@ 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
Expand Down
4 changes: 4 additions & 0 deletions control-plane/controller/configentry_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ func TestConfigEntryControllers_createsConfigEntry(t *testing.T) {
Spec: v1alpha1.ServiceDefaultsSpec{
Protocol: "http",
MaxInboundConnections: 100,
LocalConnectTimeoutMs: 5000,
LocalRequestTimeoutMs: 15000,
},
},
reconciler: func(client client.Client, consulClient *capi.Client, logger logr.Logger) testReconciler {
Expand All @@ -70,6 +72,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)
},
},
{
Expand Down

0 comments on commit 8491aee

Please sign in to comment.