Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for configurable envoy proxy concurrency #1277

Merged
merged 7 commits into from
Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions charts/consul/templates/connect-inject-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ spec:
{{- if not (kindIs "invalid" $resources.requests.cpu) }}
-default-sidecar-proxy-cpu-request={{ $resources.requests.cpu }} \
{{- end }}
-default-envoy-proxy-concurrency={{ .Values.connectInject.sidecarProxy.concurrency }} \

{{- if .Values.connectInject.initContainer }}
{{- $initResources := .Values.connectInject.initContainer.resources }}
Expand Down
30 changes: 30 additions & 0 deletions charts/consul/test/unit/connect-inject-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1636,6 +1636,36 @@ EOF
[ "${actual}" = "true" ]
}

#--------------------------------------------------------------------
# sidecarProxy.concurrency

@test "connectInject/Deployment: by default envoy concurrency is set to 2" {
cd `chart_dir`
local cmd=$(helm template \
-s templates/connect-inject-deployment.yaml \
--set 'connectInject.enabled=true' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command' | tee /dev/stderr)

local actual=$(echo "$cmd" |
yq 'any(contains("-default-envoy-proxy-concurrency=2"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "connectInject/Deployment: envoy concurrency can bet set" {
cd `chart_dir`
local cmd=$(helm template \
-s templates/connect-inject-deployment.yaml \
--set 'connectInject.enabled=true' \
--set 'connectInject.sidecarProxy.concurrency=4' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command' | tee /dev/stderr)

local actual=$(echo "$cmd" |
yq 'any(contains("-default-envoy-proxy-concurrency=4"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

#--------------------------------------------------------------------
# priorityClassName

Expand Down
10 changes: 10 additions & 0 deletions charts/consul/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2063,6 +2063,16 @@ connectInject:
secretKey: null

sidecarProxy:
# The number of threads to be used by the Envoy proxy to manage worker threads. idecar proxy.
# By default the threading model of Envoy will use one thread per CPU core per envoy proxy. This
# leads to unnecessary thread and memory usage and leaves unnecessary idle connections open. It is
# advised to keep this number low for sidecars and high for edge proxies.
# This will control the `--concurrency` flag to Envoy.
# This setting can be overridden on a per-pod basis via this annotation:
# - `consul.hashicorp.com/consul-envoy-proxy-concurrency`
# See: also https://blog.envoyproxy.io/envoy-threading-model-a8d44b922310
# @type: string
kschoche marked this conversation as resolved.
Show resolved Hide resolved
concurrency: 2
# Set default resources for sidecar proxy. If null, that resource won't
# be set.
# These settings can be overridden on a per-pod basis via these annotations:
Expand Down
3 changes: 3 additions & 0 deletions control-plane/connect-inject/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ const (
annotationConsulSidecarMemoryLimit = "consul.hashicorp.com/consul-sidecar-memory-limit"
annotationConsulSidecarMemoryRequest = "consul.hashicorp.com/consul-sidecar-memory-request"

// annotations for sidecar concurrency.
annotationEnvoyProxyConcurrency = "consul.hashicorp.com/consul-envoy-proxy-concurrency"

// annotations for metrics to configure where Prometheus scrapes
// metrics from, whether to run a merged metrics endpoint on the consul
// sidecar, and configure the connect service metrics.
Expand Down
14 changes: 14 additions & 0 deletions control-plane/connect-inject/envoy_sidecar.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,20 @@ func (h *Handler) getContainerSidecarCommand(pod corev1.Pod, multiPortSvcName st
cmd = append(cmd, "--base-id", fmt.Sprintf("%d", multiPortSvcIdx))
}

// Check to see if the user has overriden concurrency via an annotation.
if pod.Annotations[annotationEnvoyProxyConcurrency] != "" {
_, err := strconv.ParseInt(pod.Annotations[annotationEnvoyProxyConcurrency], 10, 64)
if err != nil {
//h.Log.Error(err, "unable to parse annotation ", annotationEnvoyProxyConcurrency) //, pod.Annotations[annotationEnvoyProxyConcurrency])
kschoche marked this conversation as resolved.
Show resolved Hide resolved
return nil, fmt.Errorf("unable to parse annotation: %s", annotationEnvoyProxyConcurrency)
} else {
cmd = append(cmd, "--concurrency", pod.Annotations[annotationEnvoyProxyConcurrency])
}
} else {
// Use the default concurrency.
cmd = append(cmd, "--concurrency", h.DefaultEnvoyProxyConcurrency)
}

extraArgs, annotationSet := pod.Annotations[annotationEnvoyExtraArgs]

if annotationSet || h.EnvoyExtraArgs != "" {
Expand Down
99 changes: 70 additions & 29 deletions control-plane/connect-inject/envoy_sidecar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,40 +12,75 @@ import (

func TestHandlerEnvoySidecar(t *testing.T) {
require := require.New(t)
h := Handler{}
pod := corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
cases := map[string]struct {
annotations map[string]string
expCommand []string
expErr string
}{
"default settings, no annotations": {
annotations: map[string]string{
annotationService: "foo",
},
expCommand: []string{
"envoy",
"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
"--concurrency", "2",
},
},

Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "web",
},
"default settings, annotation override": {
annotations: map[string]string{
annotationService: "foo",
annotationEnvoyProxyConcurrency: "42",
},
expCommand: []string{
"envoy",
"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
"--concurrency", "42",
},
},
}
container, err := h.envoySidecar(testNS, pod, multiPortInfo{})
require.NoError(err)
require.Equal(container.Command, []string{
"envoy",
"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
})

require.Equal(container.VolumeMounts, []corev1.VolumeMount{
{
Name: volumeName,
MountPath: "/consul/connect-inject",
"default settings, invalid concurrency annotation": {
annotations: map[string]string{
annotationService: "foo",
annotationEnvoyProxyConcurrency: "abcd",
},
expErr: "unable to parse annotation: consul.hashicorp.com/consul-envoy-proxy-concurrency",
},
})
}
for name, c := range cases {
t.Run(name, func(t *testing.T) {
h := Handler{DefaultEnvoyProxyConcurrency: "2"}
pod := corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: c.annotations,
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "web",
},
},
},
}
container, err := h.envoySidecar(testNS, pod, multiPortInfo{})
if c.expErr != "" {
require.Contains(err.Error(), c.expErr)
} else {
require.NoError(err)
require.Equal(c.expCommand, container.Command)
require.Equal(container.VolumeMounts, []corev1.VolumeMount{
{
Name: volumeName,
MountPath: "/consul/connect-inject",
},
})
}
})
}
}

func TestHandlerEnvoySidecar_Multiport(t *testing.T) {
require := require.New(t)
h := Handler{}
h := Handler{DefaultEnvoyProxyConcurrency: "2"}
pod := corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
Expand Down Expand Up @@ -75,8 +110,8 @@ func TestHandlerEnvoySidecar_Multiport(t *testing.T) {
},
}
expCommand := map[int][]string{
0: {"envoy", "--config-path", "/consul/connect-inject/envoy-bootstrap-web.yaml", "--base-id", "0"},
1: {"envoy", "--config-path", "/consul/connect-inject/envoy-bootstrap-web-admin.yaml", "--base-id", "1"},
0: {"envoy", "--config-path", "/consul/connect-inject/envoy-bootstrap-web.yaml", "--base-id", "0", "--concurrency", "2"},
1: {"envoy", "--config-path", "/consul/connect-inject/envoy-bootstrap-web-admin.yaml", "--base-id", "1", "--concurrency", "2"},
}
for i := 0; i < 2; i++ {
container, err := h.envoySidecar(testNS, pod, multiPortInfos[i])
Expand Down Expand Up @@ -280,6 +315,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
expectedContainerCommand: []string{
"envoy",
"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
"--concurrency", "2",
},
},
{
Expand All @@ -289,6 +325,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
expectedContainerCommand: []string{
"envoy",
"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
"--concurrency", "2",
"--log-level", "debug",
},
},
Expand All @@ -299,6 +336,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
expectedContainerCommand: []string{
"envoy",
"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
"--concurrency", "2",
"--log-level", "debug",
"--admin-address-path", "\"/tmp/consul/foo bar\"",
},
Expand All @@ -316,6 +354,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
expectedContainerCommand: []string{
"envoy",
"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
"--concurrency", "2",
"--log-level", "debug",
"--admin-address-path", "\"/tmp/consul/foo bar\"",
},
Expand All @@ -333,6 +372,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
expectedContainerCommand: []string{
"envoy",
"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
"--concurrency", "2",
"--log-level", "debug",
"--admin-address-path", "\"/tmp/consul/foo bar\"",
},
Expand All @@ -342,9 +382,10 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
h := Handler{
ImageConsul: "hashicorp/consul:latest",
ImageEnvoy: "hashicorp/consul-k8s:latest",
EnvoyExtraArgs: tc.envoyExtraArgs,
ImageConsul: "hashicorp/consul:latest",
ImageEnvoy: "hashicorp/consul-k8s:latest",
EnvoyExtraArgs: tc.envoyExtraArgs,
DefaultEnvoyProxyConcurrency: "2",
}

c, err := h.envoySidecar(testNS, *tc.pod, multiPortInfo{})
Expand Down
3 changes: 3 additions & 0 deletions control-plane/connect-inject/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ type Handler struct {
DefaultProxyMemoryRequest resource.Quantity
DefaultProxyMemoryLimit resource.Quantity

// Default Envoy concurrency flag, this is the number of worker threads to be used by the proxy.
DefaultEnvoyProxyConcurrency string

// MetricsConfig contains metrics configuration from the inject-connect command and has methods to determine whether
// configuration should come from the default flags or annotations. The handler uses this to configure prometheus
// annotations and the merged metrics server.
Expand Down
3 changes: 3 additions & 0 deletions control-plane/subcommand/inject-connect/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ type Command struct {
flagDefaultSidecarProxyCPURequest string
flagDefaultSidecarProxyMemoryLimit string
flagDefaultSidecarProxyMemoryRequest string
flagDefaultEnvoyProxyConcurrency string

// Metrics settings.
flagDefaultEnableMetrics bool
Expand Down Expand Up @@ -208,6 +209,7 @@ func (c *Command) init() {
c.flagSet.StringVar(&c.flagDefaultConsulSidecarCPULimit, "default-consul-sidecar-cpu-limit", "20m", "Default consul sidecar CPU limit.")
c.flagSet.StringVar(&c.flagDefaultConsulSidecarMemoryRequest, "default-consul-sidecar-memory-request", "25Mi", "Default consul sidecar memory request.")
c.flagSet.StringVar(&c.flagDefaultConsulSidecarMemoryLimit, "default-consul-sidecar-memory-limit", "50Mi", "Default consul sidecar memory limit.")
c.flagSet.StringVar(&c.flagDefaultEnvoyProxyConcurrency, "default-envoy-proxy-concurrency", "2", "Default Envoy proxy concurrency.")

c.http = &flags.HTTPFlags{}

Expand Down Expand Up @@ -448,6 +450,7 @@ func (c *Command) Run(args []string) int {
DefaultProxyCPULimit: sidecarProxyCPULimit,
DefaultProxyMemoryRequest: sidecarProxyMemoryRequest,
DefaultProxyMemoryLimit: sidecarProxyMemoryLimit,
DefaultEnvoyProxyConcurrency: c.flagDefaultEnvoyProxyConcurrency,
MetricsConfig: metricsConfig,
InitContainerResources: initResources,
DefaultConsulSidecarResources: consulSidecarResources,
Expand Down