From 7ff2f9c1bc236b9337985658b244fa32702b5c27 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 19 Jan 2021 09:46:44 -0600 Subject: [PATCH 1/4] consul/connect: Enable running multiple ingress gateways per Nomad agent Connect ingress gateway services were being registered into Consul without an explicit deterministic service ID. Consul would generate one automatically, but then Nomad would have no way to register a second gateway on the same agent as it would not supply 'proxy-id' during envoy bootstrap. Set the ServiceID for gateways, and supply 'proxy-id' when doing envoy bootstrap. Fixes #9834 --- CHANGELOG.md | 1 + .../taskrunner/envoy_bootstrap_hook.go | 18 +- .../taskrunner/envoy_bootstrap_hook_test.go | 4 +- e2e/connect/acls.go | 14 -- e2e/connect/connect.go | 28 +++ e2e/connect/input/multi-ingress.nomad | 171 ++++++++++++++++++ 6 files changed, 218 insertions(+), 18 deletions(-) create mode 100644 e2e/connect/input/multi-ingress.nomad diff --git a/CHANGELOG.md b/CHANGELOG.md index b4fd16acc8ea..9fa107aee170 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ BUG FIXES: * consul/connect: Fixed a bug where gateway proxy connection default timeout not set [[GH-9851](https://github.com/hashicorp/nomad/pull/9851)] + * consul/connect: Fixed a bug prevening more than one connect gateway per Nomad client [[GH-9849](https://github.com/hashicorp/nomad/pull/9849)] * scheduler: Fixed a bug where shared ports were not persisted during inplace updates for service jobs. [[GH-9830](https://github.com/hashicorp/nomad/issues/9830)] ## 1.0.2 (January 14, 2020) diff --git a/client/allocrunner/taskrunner/envoy_bootstrap_hook.go b/client/allocrunner/taskrunner/envoy_bootstrap_hook.go index 0186e2ea77f9..4eefbb67f1a1 100644 --- a/client/allocrunner/taskrunner/envoy_bootstrap_hook.go +++ b/client/allocrunner/taskrunner/envoy_bootstrap_hook.go @@ -336,28 +336,34 @@ func (h *envoyBootstrapHook) grpcAddress(env map[string]string) string { } } +func (h *envoyBootstrapHook) proxyServiceID(group string, service *structs.Service) string { + return agentconsul.MakeAllocServiceID(h.alloc.ID, "group-"+group, service) +} + func (h *envoyBootstrapHook) newEnvoyBootstrapArgs( - tgName string, - service *structs.Service, + group string, service *structs.Service, grpcAddr, envoyAdminBind, siToken, filepath string, ) envoyBootstrapArgs { var ( sidecarForID string // sidecar only gateway string // gateway only + proxyID string // gateway only ) if service.Connect.HasSidecar() { - sidecarForID = agentconsul.MakeAllocServiceID(h.alloc.ID, "group-"+tgName, service) + sidecarForID = h.proxyServiceID(group, service) } if service.Connect.IsGateway() { gateway = "ingress" // more types in the future + proxyID = h.proxyServiceID(group, service) } h.logger.Debug("bootstrapping envoy", "sidecar_for", service.Name, "bootstrap_file", filepath, "sidecar_for_id", sidecarForID, "grpc_addr", grpcAddr, "admin_bind", envoyAdminBind, "gateway", gateway, + "proxy_id", proxyID, ) return envoyBootstrapArgs{ @@ -367,6 +373,7 @@ func (h *envoyBootstrapHook) newEnvoyBootstrapArgs( envoyAdminBind: envoyAdminBind, siToken: siToken, gateway: gateway, + proxyID: proxyID, } } @@ -380,6 +387,7 @@ type envoyBootstrapArgs struct { envoyAdminBind string siToken string gateway string // gateways only + proxyID string // gateways only } // args returns the CLI arguments consul needs in the correct order, with the @@ -402,6 +410,10 @@ func (e envoyBootstrapArgs) args() []string { arguments = append(arguments, "-gateway", e.gateway) } + if v := e.proxyID; v != "" { + arguments = append(arguments, "-proxy-id", e.proxyID) + } + 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 2075f561ae41..afe0d49f6f2d 100644 --- a/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go +++ b/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go @@ -173,6 +173,7 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { grpcAddr: "1.1.1.1", envoyAdminBind: "localhost:3333", 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", @@ -181,6 +182,7 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { "-admin-bind", "localhost:3333", "-bootstrap", "-gateway", "my-ingress-gateway", + "-proxy-id", "_nomad-task-803cb569-881c-b0d8-9222-360bcc33157e-group-ig-ig-8080", }, result) }) } @@ -516,7 +518,7 @@ func TestTaskRunner_EnvoyBootstrapHook_gateway_ok(t *testing.T) { // the only interesting thing on bootstrap is the presence of the cluster, // everything is configured at runtime through xDS - require.Equal(t, "my-ingress-service", out.Node.Cluster) + require.Equal(t, "ingress-gateway", out.Node.Cluster) } // TestTaskRunner_EnvoyBootstrapHook_Noop asserts that the Envoy bootstrap hook diff --git a/e2e/connect/acls.go b/e2e/connect/acls.go index 4033e9ff343e..3441c7066c63 100644 --- a/e2e/connect/acls.go +++ b/e2e/connect/acls.go @@ -16,20 +16,6 @@ import ( "github.com/stretchr/testify/require" ) -const ( - // envConsulToken is the consul http token environment variable - envConsulToken = "CONSUL_HTTP_TOKEN" - - // demoConnectJob is the example connect enabled job useful for testing - demoConnectJob = "connect/input/demo.nomad" - - // demoConnectNativeJob is the example connect native enabled job useful for testing - demoConnectNativeJob = "connect/input/native-demo.nomad" - - // demoConnectIngressGateway is the example ingress gateway job useful for testing - demoConnectIngressGateway = "connect/input/ingress-gateway.nomad" -) - type ConnectACLsE2ETest struct { framework.TC diff --git a/e2e/connect/connect.go b/e2e/connect/connect.go index 66d582dc48f8..6cdc663f0fc1 100644 --- a/e2e/connect/connect.go +++ b/e2e/connect/connect.go @@ -8,6 +8,23 @@ import ( "github.com/hashicorp/nomad/helper/uuid" ) +const ( + // envConsulToken is the consul http token environment variable + envConsulToken = "CONSUL_HTTP_TOKEN" + + // demoConnectJob is the example connect enabled job useful for testing + demoConnectJob = "connect/input/demo.nomad" + + // demoConnectNativeJob is the example connect native enabled job useful for testing + demoConnectNativeJob = "connect/input/native-demo.nomad" + + // demoConnectIngressGateway is the example ingress gateway job useful for testing + demoConnectIngressGateway = "connect/input/ingress-gateway.nomad" + + // demoConnectMultiIngressGateway is the example multi ingress gateway job useful for testing + demoConnectMultiIngressGateway = "connect/input/multi-ingress.nomad" +) + type ConnectE2ETest struct { framework.TC jobIds []string @@ -92,3 +109,14 @@ func (tc *ConnectE2ETest) TestConnectIngressGatewayDemo(f *framework.F) { allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocs) e2eutil.WaitForAllocsRunning(t, tc.Nomad(), allocIDs) } + +func (tc *ConnectE2ETest) TestConnectMultiIngressGatewayDemo(f *framework.F) { + t := f.T() + + jobID := connectJobID() + tc.jobIds = append(tc.jobIds, jobID) + + allocs := e2eutil.RegisterAndWaitForAllocs(t, tc.Nomad(), demoConnectMultiIngressGateway, jobID, "") + allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocs) + e2eutil.WaitForAllocsRunning(t, tc.Nomad(), allocIDs) +} diff --git a/e2e/connect/input/multi-ingress.nomad b/e2e/connect/input/multi-ingress.nomad new file mode 100644 index 000000000000..f7acd4cfbda4 --- /dev/null +++ b/e2e/connect/input/multi-ingress.nomad @@ -0,0 +1,171 @@ +job "multi-ingress" { + + datacenters = ["dc1"] + + constraint { + attribute = "${attr.kernel.name}" + value = "linux" + } + + group "gateways" { + network { + mode = "bridge" + port "inbound1" { + static = 8081 + to = 8081 + } + port "inbound2" { + static = 8082 + to = 8082 + } + port "inbound3" { + static = 8083 + to = 8083 + } + } + + service { + name = "ig1" + port = "8081" + connect { + gateway { + ingress { + listener { + port = 8081 + protocol = "tcp" + service { + name = "api1" + } + } + } + } + } + } + + service { + name = "ig2" + port = "8082" + connect { + gateway { + ingress { + listener { + port = 8082 + protocol = "tcp" + service { + name = "api2" + } + } + } + } + } + } + + service { + name = "ig3" + port = "8083" + connect { + gateway { + ingress { + listener { + port = 8083 + protocol = "tcp" + service { + name = "api3" + } + } + } + } + } + } + } + + group "api1" { + network { + mode = "host" + port "api" {} + } + + service { + name = "api1" + port = "api" + + connect { + native = true + } + } + + task "api1" { + driver = "docker" + + config { + image = "hashicorpnomad/uuid-api:v5" + network_mode = "host" + } + + env { + BIND = "0.0.0.0" + PORT = "${NOMAD_PORT_api}" + } + } + } + + group "api2" { + network { + mode = "host" + port "api" {} + } + + service { + name = "api2" + port = "api" + + connect { + native = true + } + } + + task "api2" { + driver = "docker" + + config { + image = "hashicorpnomad/uuid-api:v5" + network_mode = "host" + } + + env { + BIND = "0.0.0.0" + PORT = "${NOMAD_PORT_api}" + } + } + } + + group "api3" { + network { + mode = "host" + port "api" {} + } + + service { + name = "api3" + port = "api" + + connect { + native = true + } + } + + task "api3" { + driver = "docker" + + config { + image = "hashicorpnomad/uuid-api:v5" + network_mode = "host" + } + + env { + BIND = "0.0.0.0" + PORT = "${NOMAD_PORT_api}" + } + } + } +} From 071060f19d8df2ca0749900e7399344b8cab5397 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Wed, 20 Jan 2021 09:20:50 -0600 Subject: [PATCH 2/4] client: use closed variable in append --- client/allocrunner/taskrunner/envoy_bootstrap_hook.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/allocrunner/taskrunner/envoy_bootstrap_hook.go b/client/allocrunner/taskrunner/envoy_bootstrap_hook.go index 4eefbb67f1a1..1ae3e19659aa 100644 --- a/client/allocrunner/taskrunner/envoy_bootstrap_hook.go +++ b/client/allocrunner/taskrunner/envoy_bootstrap_hook.go @@ -403,15 +403,15 @@ func (e envoyBootstrapArgs) args() []string { } if v := e.sidecarFor; v != "" { - arguments = append(arguments, "-sidecar-for", e.sidecarFor) + arguments = append(arguments, "-sidecar-for", v) } if v := e.gateway; v != "" { - arguments = append(arguments, "-gateway", e.gateway) + arguments = append(arguments, "-gateway", v) } if v := e.proxyID; v != "" { - arguments = append(arguments, "-proxy-id", e.proxyID) + arguments = append(arguments, "-proxy-id", v) } if v := e.siToken; v != "" { From 0019f2cfedba5a355352903d852a96bfc6dff439 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Wed, 20 Jan 2021 09:48:12 -0600 Subject: [PATCH 3/4] consul/connect: ensure proxyID in test case --- .../allocrunner/taskrunner/envoy_bootstrap_hook_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go b/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go index afe0d49f6f2d..cb638e88e8fa 100644 --- a/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go +++ b/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go @@ -7,6 +7,7 @@ package taskrunner import ( "context" "encoding/json" + "fmt" "io/ioutil" "os" "path/filepath" @@ -231,6 +232,7 @@ type envoyConfig struct { } `json:"admin"` Node struct { Cluster string `json:"cluster"` + ID string `json:"id"` Metadata struct { Namespace string `json:"namespace"` Version string `json:"envoy_version"` @@ -516,8 +518,11 @@ func TestTaskRunner_EnvoyBootstrapHook_gateway_ok(t *testing.T) { var out envoyConfig require.NoError(t, json.NewDecoder(f).Decode(&out)) - // the only interesting thing on bootstrap is the presence of the cluster, - // everything is configured at runtime through xDS + // The only interesting thing on bootstrap is the presence of the cluster, + // and its associated ID that Nomad sets. Everything is configured at runtime + // through xDS. + expID := fmt.Sprintf("_nomad-task-%s-group-web-my-ingress-service-9999", alloc.ID) + require.Equal(t, expID, out.Node.ID) require.Equal(t, "ingress-gateway", out.Node.Cluster) } From 1b8d8b1ec37a77f57bcec57263bc97c5c920bf25 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Wed, 20 Jan 2021 09:50:59 -0600 Subject: [PATCH 4/4] docs: fix typo in changelog Co-authored-by: Tim Gross --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fa107aee170..8ae8f9864395 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ BUG FIXES: * consul/connect: Fixed a bug where gateway proxy connection default timeout not set [[GH-9851](https://github.com/hashicorp/nomad/pull/9851)] - * consul/connect: Fixed a bug prevening more than one connect gateway per Nomad client [[GH-9849](https://github.com/hashicorp/nomad/pull/9849)] + * consul/connect: Fixed a bug preventing more than one connect gateway per Nomad client [[GH-9849](https://github.com/hashicorp/nomad/pull/9849)] * scheduler: Fixed a bug where shared ports were not persisted during inplace updates for service jobs. [[GH-9830](https://github.com/hashicorp/nomad/issues/9830)] ## 1.0.2 (January 14, 2020)