From cfb9c8c4653215daa74cb6737cc73669b513d857 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Mon, 30 Nov 2020 09:46:40 -0600 Subject: [PATCH 1/3] docs: note manual jobspec parsing generally no longer required --- contributing/checklist-jobspec.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/contributing/checklist-jobspec.md b/contributing/checklist-jobspec.md index 5d6215124b4..315b00f4320 100644 --- a/contributing/checklist-jobspec.md +++ b/contributing/checklist-jobspec.md @@ -4,9 +4,6 @@ * [ ] Consider similar features in Consul, Kubernetes, and other tools. Is there prior art we should match? Terminology, structure, etc? -* [ ] Parse in `jobspec/parse.go` -* [ ] Test in `jobspec/parse_test.go` (preferably with a - `jobspec/text-fixtures/.hcl` test file) * [ ] Add structs/fields to `api/` package * structs usually have Canonicalize, Copy, and Merge methods * New fields should be added to existing Canonicalize, Copy, and Merge @@ -21,6 +18,16 @@ * Note that fields must be listed in alphabetical order in `FieldDiff` slices in `nomad/structs/diff_test.go` * [ ] Test conversion +## HCL1 (deprecated) + +New jobspec entries should only be added to `jobspec2`. It makes use of HCL2 +and the `api` package for automatic parsing. Before, additional parsing was +required in the original `jobspec` package. + +* [ ] ~~Parse in `jobspec/parse.go`~~ (HCL1 only) +* [ ] ~~Test in `jobspec/parse_test.go` (preferably with a + `jobspec/text-fixtures/.hcl` test file)~~ (HCL1 only) + ## Docs * [ ] Changelog From ce32e311950a27fd594b1cc6bfc7ca11c5c59f3c Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Mon, 30 Nov 2020 09:57:29 -0600 Subject: [PATCH 2/3] consul/connect: enable setting datacenter in connect upstream Before, upstreams could only be defined using the default datacenter. Now, the `datacenter` field can be set in a connect upstream definition, informing consul of the desire for an instance of the upstream service in the specified datacenter. The field is optional and continues to default to the local datacenter. Closes #8964 --- CHANGELOG.md | 1 + api/services.go | 1 + api/services_test.go | 4 +++- command/agent/consul/connect.go | 1 + command/agent/consul/connect_test.go | 2 ++ command/agent/job_endpoint.go | 1 + command/agent/job_endpoint_test.go | 2 ++ nomad/structs/diff_test.go | 7 +++++++ nomad/structs/services.go | 5 +++++ vendor/github.com/hashicorp/nomad/api/services.go | 1 + website/pages/docs/job-specification/upstreams.mdx | 3 +++ 11 files changed, 27 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 555b8eb7404..c8ce59ac82e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ IMPROVEMENTS: * consul: Support advertising CNI and multi-host network addresses to consul [[GH-8801](https://github.com/hashicorp/nomad/issues/8801)] * consul: Support Consul namespace (Consul Enterprise) in client configuration. [[GH-8849](https://github.com/hashicorp/nomad/pull/8849)] * consul/connect: Dynamically select envoy sidecar at runtime [[GH-8945](https://github.com/hashicorp/nomad/pull/8945)] + * consul/connect: Enable setting `datacenter` field on connect upstreams [[GH-8964](https://github.com/hashicorp/nomad/issues/8964)] * csi: Support `nomad volume detach` with previously garbage-collected nodes. [[GH-9057](https://github.com/hashicorp/nomad/issues/9057)] * csi: Relaxed validation requirements when checking volume capabilities with controller plugins, to accommodate existing plugin behaviors. [[GH-9049](https://github.com/hashicorp/nomad/issues/9049)] * driver/docker: Upgrade pause container and detect architecture [[GH-8957](https://github.com/hashicorp/nomad/pull/8957)] diff --git a/api/services.go b/api/services.go index fb9220de490..59e92f6794e 100644 --- a/api/services.go +++ b/api/services.go @@ -270,6 +270,7 @@ func (cp *ConsulProxy) Canonicalize() { type ConsulUpstream struct { DestinationName string `mapstructure:"destination_name" hcl:"destination_name,optional"` LocalBindPort int `mapstructure:"local_bind_port" hcl:"local_bind_port,optional"` + Datacenter string `mapstructure:"datacenter" hcl:"datacenter,optional"` } type ConsulExposeConfig struct { diff --git a/api/services_test.go b/api/services_test.go index 2e533685577..6bfeaed4e8b 100644 --- a/api/services_test.go +++ b/api/services_test.go @@ -195,6 +195,7 @@ func TestService_Connect_proxy_settings(t *testing.T) { { DestinationName: "upstream", LocalBindPort: 80, + Datacenter: "dc2", }, }, LocalServicePort: 8000, @@ -205,8 +206,9 @@ func TestService_Connect_proxy_settings(t *testing.T) { service.Canonicalize(task, tg, job) proxy := service.Connect.SidecarService.Proxy - require.Equal(t, proxy.Upstreams[0].LocalBindPort, 80) require.Equal(t, proxy.Upstreams[0].DestinationName, "upstream") + require.Equal(t, proxy.Upstreams[0].LocalBindPort, 80) + require.Equal(t, proxy.Upstreams[0].Datacenter, "dc2") require.Equal(t, proxy.LocalServicePort, 8000) } diff --git a/command/agent/consul/connect.go b/command/agent/consul/connect.go index c2cdc4ef153..62049241bcf 100644 --- a/command/agent/consul/connect.go +++ b/command/agent/consul/connect.go @@ -165,6 +165,7 @@ func connectUpstreams(in []structs.ConsulUpstream) []api.Upstream { upstreams[i] = api.Upstream{ DestinationName: upstream.DestinationName, LocalBindPort: upstream.LocalBindPort, + Datacenter: upstream.Datacenter, } } return upstreams diff --git a/command/agent/consul/connect_test.go b/command/agent/consul/connect_test.go index d5628b91e48..ea1728bda38 100644 --- a/command/agent/consul/connect_test.go +++ b/command/agent/consul/connect_test.go @@ -288,6 +288,7 @@ func TestConnect_connectUpstreams(t *testing.T) { }, { DestinationName: "bar", LocalBindPort: 9000, + Datacenter: "dc2", }}, connectUpstreams([]structs.ConsulUpstream{{ DestinationName: "foo", @@ -295,6 +296,7 @@ func TestConnect_connectUpstreams(t *testing.T) { }, { DestinationName: "bar", LocalBindPort: 9000, + Datacenter: "dc2", }}), ) }) diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 3d956ca0941..396916627dd 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1460,6 +1460,7 @@ func apiUpstreamsToStructs(in []*api.ConsulUpstream) []structs.ConsulUpstream { upstreams[i] = structs.ConsulUpstream{ DestinationName: upstream.DestinationName, LocalBindPort: upstream.LocalBindPort, + Datacenter: upstream.Datacenter, } } return upstreams diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 8c6f92dd4a4..166c995ba95 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -3006,9 +3006,11 @@ func TestConversion_apiUpstreamsToStructs(t *testing.T) { require.Equal(t, []structs.ConsulUpstream{{ DestinationName: "upstream", LocalBindPort: 8000, + Datacenter: "dc2", }}, apiUpstreamsToStructs([]*api.ConsulUpstream{{ DestinationName: "upstream", LocalBindPort: 8000, + Datacenter: "dc2", }})) } diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index e89537693e9..be83b64917d 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -2687,6 +2687,7 @@ func TestTaskGroupDiff(t *testing.T) { { DestinationName: "foo", LocalBindPort: 8000, + Datacenter: "dc2", }, }, Config: map[string]interface{}{ @@ -2941,6 +2942,12 @@ func TestTaskGroupDiff(t *testing.T) { Type: DiffTypeAdded, Name: "ConsulUpstreams", Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Datacenter", + Old: "", + New: "dc2", + }, { Type: DiffTypeAdded, Name: "DestinationName", diff --git a/nomad/structs/services.go b/nomad/structs/services.go index af26354dffe..9d7d9152c3e 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -577,6 +577,7 @@ func hashConnect(h hash.Hash, connect *ConsulConnect) { for _, upstream := range p.Upstreams { hashString(h, upstream.DestinationName) hashString(h, strconv.Itoa(upstream.LocalBindPort)) + hashStringIfNonEmpty(h, upstream.Datacenter) } } } @@ -1125,6 +1126,9 @@ type ConsulUpstream struct { // LocalBindPort is the port the proxy will receive connections for the // upstream on. LocalBindPort int + + // Datacenter is the datacenter in which to issue the discovery query to. + Datacenter string } func upstreamsEquals(a, b []ConsulUpstream) bool { @@ -1153,6 +1157,7 @@ func (u *ConsulUpstream) Copy() *ConsulUpstream { return &ConsulUpstream{ DestinationName: u.DestinationName, LocalBindPort: u.LocalBindPort, + Datacenter: u.Datacenter, } } diff --git a/vendor/github.com/hashicorp/nomad/api/services.go b/vendor/github.com/hashicorp/nomad/api/services.go index fb9220de490..59e92f6794e 100644 --- a/vendor/github.com/hashicorp/nomad/api/services.go +++ b/vendor/github.com/hashicorp/nomad/api/services.go @@ -270,6 +270,7 @@ func (cp *ConsulProxy) Canonicalize() { type ConsulUpstream struct { DestinationName string `mapstructure:"destination_name" hcl:"destination_name,optional"` LocalBindPort int `mapstructure:"local_bind_port" hcl:"local_bind_port,optional"` + Datacenter string `mapstructure:"datacenter" hcl:"datacenter,optional"` } type ConsulExposeConfig struct { diff --git a/website/pages/docs/job-specification/upstreams.mdx b/website/pages/docs/job-specification/upstreams.mdx index b48c3f10a86..6a0bbd0029c 100644 --- a/website/pages/docs/job-specification/upstreams.mdx +++ b/website/pages/docs/job-specification/upstreams.mdx @@ -53,6 +53,7 @@ job "countdash" { upstreams { destination_name = "count-api" local_bind_port = 8080 + datacenter = "dc1" } } } @@ -80,6 +81,8 @@ job "countdash" { - `destination_name` `(string: )` - Name of the upstream service. - `local_bind_port` - `(int: )` - The port the proxy will receive connections for the upstream on. +- `datacenter` `(string: )` - The datacenter in which to issue the + discovery query. Defaults to the local datacenter. The `NOMAD_UPSTREAM_ADDR_` environment variables may be used to interpolate the upstream's `host:port` address. From 8faf997065b26c87e35a10f0b37f0f571c04cbb7 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Mon, 30 Nov 2020 12:28:08 -0600 Subject: [PATCH 3/3] docs: better clarify connect upstream datacenter --- website/pages/docs/job-specification/upstreams.mdx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/website/pages/docs/job-specification/upstreams.mdx b/website/pages/docs/job-specification/upstreams.mdx index 6a0bbd0029c..ec0ac20208d 100644 --- a/website/pages/docs/job-specification/upstreams.mdx +++ b/website/pages/docs/job-specification/upstreams.mdx @@ -81,8 +81,9 @@ job "countdash" { - `destination_name` `(string: )` - Name of the upstream service. - `local_bind_port` - `(int: )` - The port the proxy will receive connections for the upstream on. -- `datacenter` `(string: )` - The datacenter in which to issue the - discovery query. Defaults to the local datacenter. +- `datacenter` `(string: "")` - The Consul datacenter in which to issue the + discovery query. Defaults to the empty string, which Consul interprets as the + local Consul datacenter. The `NOMAD_UPSTREAM_ADDR_` environment variables may be used to interpolate the upstream's `host:port` address.