Skip to content

Commit

Permalink
Merge pull request #10883 from hashicorp/b-multi-ingress
Browse files Browse the repository at this point in the history
consul/connect: fix bug causing high cpu with multiple connect sidecars in group
  • Loading branch information
shoenig committed Jul 12, 2021
2 parents 0ef51f1 + a18d901 commit f453e48
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 33 deletions.
3 changes: 3 additions & 0 deletions .changelog/10883.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
consul/connect: Fixed a bug causing high CPU with multiple connect sidecars in one group
```
61 changes: 43 additions & 18 deletions client/allocrunner/taskrunner/envoy_bootstrap_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import (
"context"
"fmt"
"io/ioutil"
"net"
"os"
"os/exec"
"path/filepath"
"strconv"
"time"

"github.com/hashicorp/go-hclog"
Expand Down Expand Up @@ -92,8 +94,10 @@ func newEnvoyBootstrapHookConfig(alloc *structs.Allocation, consul *config.Consu
}

const (
envoyBaseAdminPort = 19000
envoyBaseAdminPort = 19000 // Consul default (bridge only)
envoyBaseReadyPort = 19100 // Consul default (bridge only)
envoyAdminBindEnvPrefix = "NOMAD_ENVOY_ADMIN_ADDR_"
envoyReadyBindEnvPrefix = "NOMAD_ENVOY_READY_ADDR_"
)

const (
Expand Down Expand Up @@ -240,12 +244,17 @@ func (h *envoyBootstrapHook) Prestart(ctx context.Context, req *ifs.TaskPrestart

h.logger.Debug("bootstrapping Consul "+serviceKind, "task", req.Task.Name, "service", serviceName)

// Envoy runs an administrative API on the loopback interface. There is no
// way to turn this feature off.
// Envoy runs an administrative listener. There is no way to turn this feature off.
// https://github.com/envoyproxy/envoy/issues/1297
envoyAdminBind := buildEnvoyAdminBind(h.alloc, serviceName, req.Task.Name, req.TaskEnv)

// Consul configures a ready listener. There is no way to turn this feature off.
envoyReadyBind := buildEnvoyReadyBind(h.alloc, serviceName, req.Task.Name, req.TaskEnv)

// Set runtime environment variables for the envoy admin and ready listeners.
resp.Env = map[string]string{
helper.CleanEnvVar(envoyAdminBindEnvPrefix+serviceName, '_'): envoyAdminBind,
helper.CleanEnvVar(envoyReadyBindEnvPrefix+serviceName, '_'): envoyReadyBind,
}

// Envoy bootstrap configuration may contain a Consul token, so write
Expand All @@ -259,7 +268,7 @@ func (h *envoyBootstrapHook) Prestart(ctx context.Context, req *ifs.TaskPrestart
}
h.logger.Debug("check for SI token for task", "task", req.Task.Name, "exists", siToken != "")

bootstrap := h.newEnvoyBootstrapArgs(h.alloc.TaskGroup, service, grpcAddr, envoyAdminBind, siToken, bootstrapFilePath)
bootstrap := h.newEnvoyBootstrapArgs(h.alloc.TaskGroup, service, grpcAddr, envoyAdminBind, envoyReadyBind, siToken, bootstrapFilePath)
bootstrapArgs := bootstrap.args()
bootstrapEnv := bootstrap.env(os.Environ())

Expand Down Expand Up @@ -331,37 +340,50 @@ func (h *envoyBootstrapHook) Prestart(ctx context.Context, req *ifs.TaskPrestart
return nil
}

// buildEnvoyAdminBind determines a unique port for use by the envoy admin
// listener.
// buildEnvoyAdminBind determines a unique port for use by the envoy admin listener.
//
// This listener will be bound to 127.0.0.2.
func buildEnvoyAdminBind(alloc *structs.Allocation, service, task string, env *taskenv.TaskEnv) string {
return buildEnvoyBind(alloc, "127.0.0.2", service, task, env, envoyBaseAdminPort)
}

// buildEnvoyAdminBind determines a unique port for use by the envoy ready listener.
//
// This listener will be bound to 127.0.0.1.
func buildEnvoyReadyBind(alloc *structs.Allocation, service, task string, env *taskenv.TaskEnv) string {
return buildEnvoyBind(alloc, "127.0.0.1", service, task, env, envoyBaseReadyPort)
}

// buildEnvoyBind is used to determine a unique port for an envoy listener.
//
// In bridge mode, if multiple sidecars are running, the bind addresses need
// to be unique within the namespace, so we simply start at 19000 and increment
// to be unique within the namespace, so we simply start at basePort and increment
// by the index of the task.
//
// In host mode, use the port provided through the service definition, which can
// be a port chosen by Nomad.
func buildEnvoyAdminBind(alloc *structs.Allocation, serviceName, taskName string, taskEnv *taskenv.TaskEnv) string {
func buildEnvoyBind(alloc *structs.Allocation, ifce, service, task string, taskEnv *taskenv.TaskEnv, basePort int) string {
tg := alloc.Job.LookupTaskGroup(alloc.TaskGroup)
port := envoyBaseAdminPort
port := basePort
switch tg.Networks[0].Mode {
case "host":
interpolatedServices := taskenv.InterpolateServices(taskEnv, tg.Services)
for _, service := range interpolatedServices {
if service.Name == serviceName {
mapping := tg.Networks.Port(service.PortLabel)
for _, svc := range interpolatedServices {
if svc.Name == service {
mapping := tg.Networks.Port(svc.PortLabel)
port = mapping.Value
break
}
}
default:
for idx, task := range tg.Tasks {
if task.Name == taskName {
for idx, tgTask := range tg.Tasks {
if tgTask.Name == task {
port += idx
break
}
}
}
return fmt.Sprintf("localhost:%d", port)
return net.JoinHostPort(ifce, strconv.Itoa(port))
}

func (h *envoyBootstrapHook) writeConfig(filename, config string) error {
Expand Down Expand Up @@ -421,7 +443,7 @@ func (h *envoyBootstrapHook) proxyServiceID(group string, service *structs.Servi
// https://www.consul.io/commands/connect/envoy#consul-connect-envoy
func (h *envoyBootstrapHook) newEnvoyBootstrapArgs(
group string, service *structs.Service,
grpcAddr, envoyAdminBind, siToken, filepath string,
grpcAddr, envoyAdminBind, envoyReadyBind, siToken, filepath string,
) envoyBootstrapArgs {
var (
sidecarForID string // sidecar only
Expand Down Expand Up @@ -450,15 +472,16 @@ func (h *envoyBootstrapHook) newEnvoyBootstrapArgs(
h.logger.Info("bootstrapping envoy",
"sidecar_for", service.Name, "bootstrap_file", filepath,
"sidecar_for_id", sidecarForID, "grpc_addr", grpcAddr,
"admin_bind", envoyAdminBind, "gateway", gateway,
"proxy_id", proxyID, "namespace", namespace,
"admin_bind", envoyAdminBind, "ready_bind", envoyReadyBind,
"gateway", gateway, "proxy_id", proxyID, "namespace", namespace,
)

return envoyBootstrapArgs{
consulConfig: h.consulConfig,
sidecarFor: sidecarForID,
grpcAddr: grpcAddr,
envoyAdminBind: envoyAdminBind,
envoyReadyBind: envoyReadyBind,
siToken: siToken,
gateway: gateway,
proxyID: proxyID,
Expand All @@ -474,6 +497,7 @@ type envoyBootstrapArgs struct {
sidecarFor string // sidecars only
grpcAddr string
envoyAdminBind string
envoyReadyBind string
siToken string
gateway string // gateways only
proxyID string // gateways only
Expand All @@ -489,6 +513,7 @@ func (e envoyBootstrapArgs) args() []string {
"-grpc-addr", e.grpcAddr,
"-http-addr", e.consulConfig.HTTPAddr,
"-admin-bind", e.envoyAdminBind,
"-address", e.envoyReadyBind,
"-bootstrap",
}

Expand Down
32 changes: 21 additions & 11 deletions client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,15 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) {
sidecarFor: "s1",
grpcAddr: "1.1.1.1",
consulConfig: consulPlainConfig,
envoyAdminBind: "localhost:3333",
envoyAdminBind: "127.0.0.2:19000",
envoyReadyBind: "127.0.0.1:19100",
}
result := ebArgs.args()
require.Equal(t, []string{"connect", "envoy",
"-grpc-addr", "1.1.1.1",
"-http-addr", "2.2.2.2",
"-admin-bind", "localhost:3333",
"-admin-bind", "127.0.0.2:19000",
"-address", "127.0.0.1:19100",
"-bootstrap",
"-sidecar-for", "s1",
}, result)
Expand All @@ -141,14 +143,16 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) {
sidecarFor: "s1",
grpcAddr: "1.1.1.1",
consulConfig: consulPlainConfig,
envoyAdminBind: "localhost:3333",
envoyAdminBind: "127.0.0.2:19000",
envoyReadyBind: "127.0.0.1:19100",
siToken: token,
}
result := ebArgs.args()
require.Equal(t, []string{"connect", "envoy",
"-grpc-addr", "1.1.1.1",
"-http-addr", "2.2.2.2",
"-admin-bind", "localhost:3333",
"-admin-bind", "127.0.0.2:19000",
"-address", "127.0.0.1:19100",
"-bootstrap",
"-sidecar-for", "s1",
"-token", token,
Expand All @@ -160,13 +164,15 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) {
sidecarFor: "s1",
grpcAddr: "1.1.1.1",
consulConfig: consulTLSConfig,
envoyAdminBind: "localhost:3333",
envoyAdminBind: "127.0.0.2:19000",
envoyReadyBind: "127.0.0.1:19100",
}
result := ebArgs.args()
require.Equal(t, []string{"connect", "envoy",
"-grpc-addr", "1.1.1.1",
"-http-addr", "2.2.2.2",
"-admin-bind", "localhost:3333",
"-admin-bind", "127.0.0.2:19000",
"-address", "127.0.0.1:19100",
"-bootstrap",
"-sidecar-for", "s1",
"-ca-file", "/etc/tls/ca-file",
Expand All @@ -179,15 +185,17 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) {
ebArgs := envoyBootstrapArgs{
consulConfig: consulPlainConfig,
grpcAddr: "1.1.1.1",
envoyAdminBind: "localhost:3333",
envoyAdminBind: "127.0.0.2:19000",
envoyReadyBind: "127.0.0.1:19100",
gateway: "my-ingress-gateway",
proxyID: "_nomad-task-803cb569-881c-b0d8-9222-360bcc33157e-group-ig-ig-8080",
}
result := ebArgs.args()
require.Equal(t, []string{"connect", "envoy",
"-grpc-addr", "1.1.1.1",
"-http-addr", "2.2.2.2",
"-admin-bind", "localhost:3333",
"-admin-bind", "127.0.0.2:19000",
"-address", "127.0.0.1:19100",
"-bootstrap",
"-gateway", "my-ingress-gateway",
"-proxy-id", "_nomad-task-803cb569-881c-b0d8-9222-360bcc33157e-group-ig-ig-8080",
Expand All @@ -198,15 +206,17 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) {
ebArgs := envoyBootstrapArgs{
consulConfig: consulPlainConfig,
grpcAddr: "1.1.1.1",
envoyAdminBind: "localhost:3333",
envoyAdminBind: "127.0.0.2:19000",
envoyReadyBind: "127.0.0.1:19100",
gateway: "my-mesh-gateway",
proxyID: "_nomad-task-803cb569-881c-b0d8-9222-360bcc33157e-group-mesh-mesh-8080",
}
result := ebArgs.args()
require.Equal(t, []string{"connect", "envoy",
"-grpc-addr", "1.1.1.1",
"-http-addr", "2.2.2.2",
"-admin-bind", "localhost:3333",
"-admin-bind", "127.0.0.2:19000",
"-address", "127.0.0.1:19100",
"-bootstrap",
"-gateway", "my-mesh-gateway",
"-proxy-id", "_nomad-task-803cb569-881c-b0d8-9222-360bcc33157e-group-mesh-mesh-8080",
Expand Down Expand Up @@ -453,7 +463,7 @@ func TestTaskRunner_EnvoyBootstrapHook_sidecar_ok(t *testing.T) {
require.True(t, resp.Done)

require.NotNil(t, resp.Env)
require.Equal(t, "localhost:19001", resp.Env[envoyAdminBindEnvPrefix+"foo"])
require.Equal(t, "127.0.0.2:19001", resp.Env[envoyAdminBindEnvPrefix+"foo"])

// Ensure the default path matches
env := map[string]string{
Expand Down
20 changes: 16 additions & 4 deletions website/content/partials/envvars.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,23 @@
<code>NOMAD_ENVOY_ADMIN_ADDR_&lt;service&gt;</code>
</td>
<td>
Local address <code>localhost:Port</code> for the admin port of the
Local address <code>127.0.0.2:Port</code> for the admin port of the
envoy sidecar for the given <code>service</code> when defined as a
Consul Connect enabled service.
</td>
</tr>
Consul Connect enabled service. Envoy runs inside the group network
namespace unless configured for host networking.
</td>
</tr>
<tr>
<td>
<code>NOMAD_ENVOY_READY_ADDR_&lt;service&gt;</code>
</td>
<td>
Local address <code>127.0.0.1:Port</code> for the ready port of the
envoy sidecar for the given <code>service</code> when defined as a
Consul Connect enabled service. Envoy runs inside the group network
namespace unless configured for host networking.
</td>
</tr>
<tr>
<th colspan="2">Consul-related Variables (only set for connect native tasks)</th>
</tr>
Expand Down

0 comments on commit f453e48

Please sign in to comment.