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
hashicorp · Feb 18, 2022 · efee15f · efee15f
1 parent dd4a3a9
commit efee15f
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 36 deletions.
diff --git a/.changelog/12011.txt 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
@@ -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
@@ -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",