From 6550c901982865632dd410e13e501e3bb63afc0d Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Fri, 4 Feb 2022 10:49:15 -0600 Subject: [PATCH] connect: bootstrap envoy using -proxy-id This PR modifies the Consul CLI arguments used to bootstrap envoy for Connect sidecars to make use of '-proxy-id' instead of '-sidecar-for'. Nomad registers the sidecar service, so we know what ID it has. The '-sidecar-for' was intended for use when you only know the name of the service for which the sidecar is being created. The improvement here is that using '-proxy-id' does not require an underlying request for listing Consul services. This will make make the interaction between Nomad and Consul more efficient. Closes #10452 --- .changelog/12011.txt | 3 ++ .../taskrunner/envoy_bootstrap_hook.go | 36 ++++++------------- .../taskrunner/envoy_bootstrap_hook_test.go | 20 +++++------ 3 files changed, 23 insertions(+), 36 deletions(-) create mode 100644 .changelog/12011.txt diff --git a/.changelog/12011.txt b/.changelog/12011.txt new file mode 100644 index 000000000..47897aa5c --- /dev/null +++ b/.changelog/12011.txt @@ -0,0 +1,3 @@ +```release-note:improvement +connect: bootstrap envoy sidecars using -proxy-for +``` diff --git a/client/allocrunner/taskrunner/envoy_bootstrap_hook.go b/client/allocrunner/taskrunner/envoy_bootstrap_hook.go index c5aab12ed..1d058ba63 100644 --- a/client/allocrunner/taskrunner/envoy_bootstrap_hook.go +++ b/client/allocrunner/taskrunner/envoy_bootstrap_hook.go @@ -433,6 +433,8 @@ func (h *envoyBootstrapHook) grpcAddress(env map[string]string) string { } func (h *envoyBootstrapHook) proxyServiceID(group string, service *structs.Service) string { + // Note, it is critical the ID here matches what is actually registered in Consul. + // See: WorkloadServices.Name in structs.go return agentconsul.MakeAllocServiceID(h.alloc.ID, "group-"+group, service) } @@ -445,40 +447,30 @@ func (h *envoyBootstrapHook) newEnvoyBootstrapArgs( group string, service *structs.Service, grpcAddr, envoyAdminBind, envoyReadyBind, siToken, filepath string, ) envoyBootstrapArgs { - var ( - sidecarForID string // sidecar only - gateway string // gateway only - proxyID string // gateway only - namespace string - ) - namespace = h.getConsulNamespace() - id := h.proxyServiceID(group, service) + namespace := h.getConsulNamespace() + proxyID := h.proxyServiceID(group, service) + var gateway string switch { case service.Connect.HasSidecar(): - sidecarForID = id + proxyID += "-sidecar-proxy" case service.Connect.IsIngress(): - proxyID = id gateway = "ingress" case service.Connect.IsTerminating(): - proxyID = id gateway = "terminating" case service.Connect.IsMesh(): - proxyID = id gateway = "mesh" } h.logger.Info("bootstrapping envoy", - "sidecar_for", service.Name, "bootstrap_file", filepath, - "sidecar_for_id", sidecarForID, "grpc_addr", grpcAddr, + "namespace", namespace, "proxy_id", proxyID, "service", service.Name, + "gateway", gateway, "bootstrap_file", filepath, "grpc_addr", grpcAddr, "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, @@ -494,13 +486,12 @@ func (h *envoyBootstrapHook) newEnvoyBootstrapArgs( // configuration file for envoy. type envoyBootstrapArgs struct { consulConfig consulTransportConfig - sidecarFor string // sidecars only grpcAddr string envoyAdminBind string envoyReadyBind string siToken string gateway string // gateways only - proxyID string // gateways only + proxyID string // gateways and sidecars namespace string } @@ -514,21 +505,14 @@ func (e envoyBootstrapArgs) args() []string { "-http-addr", e.consulConfig.HTTPAddr, "-admin-bind", e.envoyAdminBind, "-address", e.envoyReadyBind, + "-proxy-id", e.proxyID, "-bootstrap", } - if v := e.sidecarFor; v != "" { - arguments = append(arguments, "-sidecar-for", v) - } - if v := e.gateway; v != "" { arguments = append(arguments, "-gateway", v) } - if v := e.proxyID; v != "" { - arguments = append(arguments, "-proxy-id", v) - } - if v := e.siToken; v != "" { arguments = append(arguments, "-token", v) } diff --git a/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go b/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go index 33e0420f1..a1a9857cb 100644 --- a/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go +++ b/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go @@ -122,7 +122,7 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { t.Run("excluding SI token", func(t *testing.T) { ebArgs := envoyBootstrapArgs{ - sidecarFor: "s1", + proxyID: "s1-sidecar-proxy", grpcAddr: "1.1.1.1", consulConfig: consulPlainConfig, envoyAdminBind: "127.0.0.2:19000", @@ -134,15 +134,15 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { "-http-addr", "2.2.2.2", "-admin-bind", "127.0.0.2:19000", "-address", "127.0.0.1:19100", + "-proxy-id", "s1-sidecar-proxy", "-bootstrap", - "-sidecar-for", "s1", }, result) }) t.Run("including SI token", func(t *testing.T) { token := uuid.Generate() ebArgs := envoyBootstrapArgs{ - sidecarFor: "s1", + proxyID: "s1-sidecar-proxy", grpcAddr: "1.1.1.1", consulConfig: consulPlainConfig, envoyAdminBind: "127.0.0.2:19000", @@ -155,15 +155,15 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { "-http-addr", "2.2.2.2", "-admin-bind", "127.0.0.2:19000", "-address", "127.0.0.1:19100", + "-proxy-id", "s1-sidecar-proxy", "-bootstrap", - "-sidecar-for", "s1", "-token", token, }, result) }) t.Run("including certificates", func(t *testing.T) { ebArgs := envoyBootstrapArgs{ - sidecarFor: "s1", + proxyID: "s1-sidecar-proxy", grpcAddr: "1.1.1.1", consulConfig: consulTLSConfig, envoyAdminBind: "127.0.0.2:19000", @@ -175,8 +175,8 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { "-http-addr", "2.2.2.2", "-admin-bind", "127.0.0.2:19000", "-address", "127.0.0.1:19100", + "-proxy-id", "s1-sidecar-proxy", "-bootstrap", - "-sidecar-for", "s1", "-ca-file", "/etc/tls/ca-file", "-client-cert", "/etc/tls/cert-file", "-client-key", "/etc/tls/key-file", @@ -198,9 +198,9 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { "-http-addr", "2.2.2.2", "-admin-bind", "127.0.0.2:19000", "-address", "127.0.0.1:19100", + "-proxy-id", "_nomad-task-803cb569-881c-b0d8-9222-360bcc33157e-group-ig-ig-8080", "-bootstrap", "-gateway", "my-ingress-gateway", - "-proxy-id", "_nomad-task-803cb569-881c-b0d8-9222-360bcc33157e-group-ig-ig-8080", }, result) }) @@ -219,9 +219,9 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { "-http-addr", "2.2.2.2", "-admin-bind", "127.0.0.2:19000", "-address", "127.0.0.1:19100", + "-proxy-id", "_nomad-task-803cb569-881c-b0d8-9222-360bcc33157e-group-mesh-mesh-8080", "-bootstrap", "-gateway", "my-mesh-gateway", - "-proxy-id", "_nomad-task-803cb569-881c-b0d8-9222-360bcc33157e-group-mesh-mesh-8080", }, result) }) } @@ -235,7 +235,7 @@ func TestEnvoyBootstrapHook_envoyBootstrapEnv(t *testing.T) { require.Equal(t, []string{ "foo=bar", "baz=1", }, envoyBootstrapArgs{ - sidecarFor: "s1", + proxyID: "s1-sidecar-proxy", grpcAddr: "1.1.1.1", consulConfig: consulPlainConfig, envoyAdminBind: "localhost:3333", @@ -249,7 +249,7 @@ func TestEnvoyBootstrapHook_envoyBootstrapEnv(t *testing.T) { "CONSUL_HTTP_SSL=true", "CONSUL_HTTP_SSL_VERIFY=true", }, envoyBootstrapArgs{ - sidecarFor: "s1", + proxyID: "s1-sidecar-proxy", grpcAddr: "1.1.1.1", consulConfig: consulTLSConfig, envoyAdminBind: "localhost:3333",