Add support for configurable envoy proxy concurrency (#1277)

* Adds support for configuring the envoy proxy concurrency, aka worker threads using annotations and helm stanzas.

CHANGELOG.md

@@ -6,6 +6,9 @@ IMPROVEMENTS:
   * Enable configuring the pod topologySpreadConstraints for mesh, terminating, and ingress gateways. [[GH-1257](https://github.com/hashicorp/consul-k8s/pull/1257)]
   * Present Consul server CA chain when using Vault secrets backend. [[GH-1251](https://github.com/hashicorp/consul-k8s/pull/1251)]
   * API Gateway: Enable configuring of the new High Availability feature (requires Consul API Gateway v0.3.0+). [[GH-1261](https://github.com/hashicorp/consul-k8s/pull/1261)]
+  * Enable the configuration of Envoy proxy concurrency via `connectInject.sidecarProxy.concurrency` which can
+    be overridden at the pod level via the annotation `consul.hashicorp.com/consul-envoy-proxy-concurrency`.
+    This PR also sets the default concurrency for envoy proxies to `2`. [[GH-1277](https://github.com/hashicorp/consul-k8s/pull/1277)]
   * Update Mesh CRD with Mesh HTTP Config. [[GH-1282](https://github.com/hashicorp/consul-k8s/pull/1282)]
 * Control Plane
   * Bump Dockerfile base image for RedHat UBI `consul-k8s-control-plane` image to `ubi-minimal:8.6`. [[GH-1244](https://github.com/hashicorp/consul-k8s/pull/1244)]

charts/consul/templates/connect-inject-deployment.yaml

@@ -212,6 +212,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 }}

charts/consul/test/unit/connect-inject-deployment.bats

@@ -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
 

charts/consul/values.yaml

@@ -2070,6 +2070,18 @@ connectInject:
     secretKey: null
 
   sidecarProxy:
+    # The number of worker threads to be used by the Envoy 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.
+    # For additional information see also: https://blog.envoyproxy.io/envoy-threading-model-a8d44b922310
+    #
+    # This setting can be overridden on a per-pod basis via this annotation:
+    # - `consul.hashicorp.com/consul-envoy-proxy-concurrency`
+    # @type: string
+    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:

control-plane/connect-inject/annotations.go

@@ -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.

control-plane/connect-inject/envoy_sidecar.go

@@ -96,6 +96,22 @@ func (w *MeshWebhook) getContainerSidecarCommand(pod corev1.Pod, multiPortSvcNam
 		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] != "" {
+		val, err := strconv.ParseInt(pod.Annotations[annotationEnvoyProxyConcurrency], 10, 64)
+		if err != nil {
+			return nil, fmt.Errorf("unable to parse annotation: %s", annotationEnvoyProxyConcurrency)
+		}
+		if val < 0 {
+			return nil, fmt.Errorf("invalid envoy concurrency, must be >= 0: %s", pod.Annotations[annotationEnvoyProxyConcurrency])
+		} else {
+			cmd = append(cmd, "--concurrency", pod.Annotations[annotationEnvoyProxyConcurrency])
+		}
+	} else {
+		// Use the default concurrency.
+		cmd = append(cmd, "--concurrency", fmt.Sprintf("%d", w.DefaultEnvoyProxyConcurrency))
+	}
+
 	extraArgs, annotationSet := pod.Annotations[annotationEnvoyExtraArgs]
 
 	if annotationSet || w.EnvoyExtraArgs != "" {

control-plane/connect-inject/envoy_sidecar_test.go

@@ -12,35 +12,70 @@ import (
 
 func TestHandlerEnvoySidecar(t *testing.T) {
 	require := require.New(t)
-	w := MeshWebhook{}
-	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", "0",
+			},
 		},
-
-		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 := w.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 negative number": {
+			annotations: map[string]string{
+				annotationService:               "foo",
+				annotationEnvoyProxyConcurrency: "-42",
+			},
+			expErr: "invalid envoy concurrency, must be >= 0: -42",
 		},
-	})
+	}
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			h := MeshWebhook{}
+			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) {
@@ -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", "0"},
+		1: {"envoy", "--config-path", "/consul/connect-inject/envoy-bootstrap-web-admin.yaml", "--base-id", "1", "--concurrency", "0"},
 	}
 	for i := 0; i < 2; i++ {
 		container, err := w.envoySidecar(testNS, pod, multiPortInfos[i])
@@ -280,6 +315,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
 			expectedContainerCommand: []string{
 				"envoy",
 				"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
+				"--concurrency", "0",
 			},
 		},
 		{
@@ -289,6 +325,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
 			expectedContainerCommand: []string{
 				"envoy",
 				"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
+				"--concurrency", "0",
 				"--log-level", "debug",
 			},
 		},
@@ -299,6 +336,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
 			expectedContainerCommand: []string{
 				"envoy",
 				"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
+				"--concurrency", "0",
 				"--log-level", "debug",
 				"--admin-address-path", "\"/tmp/consul/foo bar\"",
 			},
@@ -316,6 +354,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
 			expectedContainerCommand: []string{
 				"envoy",
 				"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
+				"--concurrency", "0",
 				"--log-level", "debug",
 				"--admin-address-path", "\"/tmp/consul/foo bar\"",
 			},
@@ -333,6 +372,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
 			expectedContainerCommand: []string{
 				"envoy",
 				"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
+				"--concurrency", "0",
 				"--log-level", "debug",
 				"--admin-address-path", "\"/tmp/consul/foo bar\"",
 			},

control-plane/connect-inject/mesh_webhook.go

@@ -115,6 +115,9 @@ type MeshWebhook 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 int
+
 	// 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 meshWebhook uses this to configure prometheus
 	// annotations and the merged metrics server.

control-plane/subcommand/inject-connect/command.go

@@ -73,6 +73,7 @@ type Command struct {
 	flagDefaultSidecarProxyCPURequest    string
 	flagDefaultSidecarProxyMemoryLimit   string
 	flagDefaultSidecarProxyMemoryRequest string
+	flagDefaultEnvoyProxyConcurrency     int
 
 	// Metrics settings.
 	flagDefaultEnableMetrics        bool
@@ -213,6 +214,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.IntVar(&c.flagDefaultEnvoyProxyConcurrency, "default-envoy-proxy-concurrency", 2, "Default Envoy proxy concurrency.")
 
 	c.http = &flags.HTTPFlags{}
 
@@ -476,6 +478,7 @@ func (c *Command) Run(args []string) int {
 			DefaultProxyCPULimit:          sidecarProxyCPULimit,
 			DefaultProxyMemoryRequest:     sidecarProxyMemoryRequest,
 			DefaultProxyMemoryLimit:       sidecarProxyMemoryLimit,
+			DefaultEnvoyProxyConcurrency:  c.flagDefaultEnvoyProxyConcurrency,
 			MetricsConfig:                 metricsConfig,
 			InitContainerResources:        initResources,
 			DefaultConsulSidecarResources: consulSidecarResources,
@@ -553,6 +556,10 @@ func (c *Command) validateFlags() error {
 		return errors.New("-enable-partitions must be set to 'true' if -partition-name is set")
 	}
 
+	if c.flagDefaultEnvoyProxyConcurrency < 0 {
+		return errors.New("-default-envoy-proxy-concurrency must be >= 0 if set")
+	}
+
 	if c.http.ConsulAPITimeout() <= 0 {
 		return errors.New("-consul-api-timeout must be set to a value greater than 0")
 	}

control-plane/subcommand/inject-connect/command_test.go

@@ -177,6 +177,12 @@ func TestRun_FlagValidation(t *testing.T) {
 				"-listen", ":foobar"},
 			expErr: "unable to parse port string: strconv.Atoi: parsing \"foobar\": invalid syntax",
 		},
+		{
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-consul-api-timeout", "5s", "-default-envoy-proxy-concurrency=-42",
+			},
+			expErr: "-default-envoy-proxy-concurrency must be >= 0 if set",
+		},
 	}
 
 	for _, c := range cases {