From 64f4e9e69190128cdefb8af2cfcfce93f0afdd70 Mon Sep 17 00:00:00 2001 From: Nick Ethier Date: Tue, 12 Nov 2019 22:27:54 -0500 Subject: [PATCH 1/4] consul: add support for canary meta --- CHANGELOG.md | 1 + api/services.go | 1 + command/agent/consul/catalog_testing.go | 2 + command/agent/consul/client.go | 17 +++-- command/agent/consul/unit_test.go | 68 +++++++++++++++++++ command/agent/job_endpoint.go | 2 + jobspec/parse_service.go | 16 +++++ jobspec/parse_test.go | 8 ++- jobspec/test-fixtures/basic.hcl | 8 +++ nomad/structs/services.go | 9 +++ .../docs/job-specification/service.html.md | 9 ++- 11 files changed, 135 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38d94ca3d1a..0765c39d6c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ IMPROVEMENTS: * api: Added JSON representation of rules to policy endpoint response [[GH-6017](https://github.com/hashicorp/nomad/pull/6017)] * api: Update policy endpoint to permit anonymous access [[GH-6021](https://github.com/hashicorp/nomad/issues/6021)] * build: Updated to Go 1.12.13 [[GH-6606](https://github.com/hashicorp/nomad/issues/6606)] + * consul: Add support for service `canary_meta` * cli: Show full ID in node and alloc individual status views [[GH-6425](https://github.com/hashicorp/nomad/issues/6425)] * client: Enable setting tags on Consul Connect sidecar service [[GH-6448](https://github.com/hashicorp/nomad/issues/6448)] * client: Added support for downloading artifacts from Google Cloud Storage [[GH-6692](https://github.com/hashicorp/nomad/pull/6692)] diff --git a/api/services.go b/api/services.go index 8630126ceb4..6b1220ea367 100644 --- a/api/services.go +++ b/api/services.go @@ -107,6 +107,7 @@ type Service struct { CheckRestart *CheckRestart `mapstructure:"check_restart"` Connect *ConsulConnect Meta map[string]string + CanaryMeta map[string]string } // Canonicalize the Service by ensuring its name and address mode are set. Task diff --git a/command/agent/consul/catalog_testing.go b/command/agent/consul/catalog_testing.go index 621d4143992..235f419ce62 100644 --- a/command/agent/consul/catalog_testing.go +++ b/command/agent/consul/catalog_testing.go @@ -5,6 +5,7 @@ import ( "sync" log "github.com/hashicorp/go-hclog" + "github.com/hashicorp/nomad/helper" "github.com/hashicorp/consul/api" ) @@ -111,6 +112,7 @@ func (c *MockAgent) Services() (map[string]*api.AgentService, error) { ID: v.ID, Service: v.Name, Tags: make([]string, len(v.Tags)), + Meta: helper.CopyMapStringString(v.Meta), Port: v.Port, Address: v.Address, EnableTagOverride: v.EnableTagOverride, diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go index e7480a8b758..88965c6f927 100644 --- a/command/agent/consul/client.go +++ b/command/agent/consul/client.go @@ -99,7 +99,8 @@ func agentServiceUpdateRequired(reg *api.AgentServiceRegistration, svc *api.Agen reg.Port == svc.Port && reg.Address == svc.Address && reg.Name == svc.Service && - reflect.DeepEqual(reg.Tags, svc.Tags)) + reflect.DeepEqual(reg.Tags, svc.Tags) && + reflect.DeepEqual(reg.Meta, svc.Meta)) } // operations are submitted to the main loop via commit() for synchronizing @@ -713,9 +714,17 @@ func (c *ServiceClient) serviceRegs(ops *operations, service *structs.Service, w return nil, fmt.Errorf("invalid Consul Connect configuration for service %q: %v", service.Name, err) } - meta := make(map[string]string, len(service.Meta)) - for k, v := range service.Meta { - meta[k] = v + var meta map[string]string + if task.Canary && len(service.CanaryMeta) > 0 { + meta = make(map[string]string, len(service.CanaryMeta)+1) + for k, v := range service.CanaryMeta { + meta[k] = v + } + } else { + meta = make(map[string]string, len(service.Meta)+1) + for k, v := range service.Meta { + meta[k] = v + } } // This enables the consul UI to show that Nomad registered this service diff --git a/command/agent/consul/unit_test.go b/command/agent/consul/unit_test.go index 3cc702f003d..65ab5aa6631 100644 --- a/command/agent/consul/unit_test.go +++ b/command/agent/consul/unit_test.go @@ -35,6 +35,7 @@ func testWorkload() *WorkloadServices { Name: "taskname-service", PortLabel: "x", Tags: []string{"tag1", "tag2"}, + Meta: map[string]string{"meta1": "foo"}, }, }, Networks: []*structs.NetworkResource{ @@ -1077,6 +1078,73 @@ func TestConsul_CanaryTags_NoTags(t *testing.T) { require.Len(ctx.FakeConsul.services, 0) } +// TestConsul_CanaryMeta asserts CanaryMeta are used when Canary=true +func TestConsul_CanaryMeta(t *testing.T) { + t.Parallel() + require := require.New(t) + ctx := setupFake(t) + + canaryMeta := map[string]string{"meta1": "canary"} + canaryMeta["external-source"] = "nomad" + ctx.Task.Canary = true + ctx.Task.Services[0].CanaryMeta = canaryMeta + + require.NoError(ctx.ServiceClient.RegisterTask(ctx.Task)) + require.NoError(ctx.syncOnce()) + require.Len(ctx.FakeConsul.services, 1) + for _, service := range ctx.FakeConsul.services { + require.Equal(canaryMeta, service.Meta) + } + + // Disable canary and assert meta are not the canary meta + origTask := ctx.Task.Copy() + ctx.Task.Canary = false + require.NoError(ctx.ServiceClient.UpdateTask(origTask, ctx.Task)) + require.NoError(ctx.syncOnce()) + require.Len(ctx.FakeConsul.services, 1) + for _, service := range ctx.FakeConsul.services { + require.NotEqual(canaryMeta, service.Meta) + } + + ctx.ServiceClient.RemoveTask(ctx.Task) + require.NoError(ctx.syncOnce()) + require.Len(ctx.FakeConsul.services, 0) +} + +// TestConsul_CanaryMeta_NoMeta asserts Meta are used when Canary=true and there +// are no specified canary meta +func TestConsul_CanaryMeta_NoMeta(t *testing.T) { + t.Parallel() + require := require.New(t) + ctx := setupFake(t) + + meta := map[string]string{"meta1": "foo"} + meta["external-source"] = "nomad" + ctx.Task.Canary = true + ctx.Task.Services[0].Meta = meta + + require.NoError(ctx.ServiceClient.RegisterTask(ctx.Task)) + require.NoError(ctx.syncOnce()) + require.Len(ctx.FakeConsul.services, 1) + for _, service := range ctx.FakeConsul.services { + require.Equal(meta, service.Meta) + } + + // Disable canary and assert meta dont change + origTask := ctx.Task.Copy() + ctx.Task.Canary = false + require.NoError(ctx.ServiceClient.UpdateTask(origTask, ctx.Task)) + require.NoError(ctx.syncOnce()) + require.Len(ctx.FakeConsul.services, 1) + for _, service := range ctx.FakeConsul.services { + require.Equal(meta, service.Meta) + } + + ctx.ServiceClient.RemoveTask(ctx.Task) + require.NoError(ctx.syncOnce()) + require.Len(ctx.FakeConsul.services, 0) +} + // TestConsul_PeriodicSync asserts that Nomad periodically reconciles with // Consul. func TestConsul_PeriodicSync(t *testing.T) { diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 808e3ff1497..1ce796eb24f 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -834,6 +834,7 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) { CanaryTags: service.CanaryTags, AddressMode: service.AddressMode, Meta: helper.CopyMapStringString(service.Meta), + CanaryMeta: helper.CopyMapStringString(service.CanaryMeta), } if l := len(service.Checks); l != 0 { @@ -1012,6 +1013,7 @@ func ApiServicesToStructs(in []*api.Service) []*structs.Service { CanaryTags: s.CanaryTags, AddressMode: s.AddressMode, Meta: helper.CopyMapStringString(s.Meta), + CanaryMeta: helper.CopyMapStringString(s.CanaryMeta), } if l := len(s.Checks); l != 0 { diff --git a/jobspec/parse_service.go b/jobspec/parse_service.go index 783b006bca3..a974527f009 100644 --- a/jobspec/parse_service.go +++ b/jobspec/parse_service.go @@ -47,6 +47,7 @@ func parseService(o *ast.ObjectItem) (*api.Service, error) { "check_restart", "connect", "meta", + "canary_meta", } if err := helper.CheckHCLKeys(o.Val, valid); err != nil { return nil, err @@ -62,6 +63,7 @@ func parseService(o *ast.ObjectItem) (*api.Service, error) { delete(m, "check_restart") delete(m, "connect") delete(m, "meta") + delete(m, "canary_meta") if err := mapstructure.WeakDecode(m, &service); err != nil { return nil, err @@ -122,6 +124,20 @@ func parseService(o *ast.ObjectItem) (*api.Service, error) { } } + // Parse out canary_meta fields. These are in HCL as a list so we need + // to iterate over them and merge them. + if metaO := listVal.Filter("canary_meta"); len(metaO.Items) > 0 { + for _, o := range metaO.Elem().Items { + var m map[string]interface{} + if err := hcl.DecodeObject(&m, o.Val); err != nil { + return nil, err + } + if err := mapstructure.WeakDecode(m, &service.CanaryMeta); err != nil { + return nil, err + } + } + } + return &service, nil } diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 9bac757df61..d4cea880330 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -218,7 +218,13 @@ func TestParse(t *testing.T) { { Tags: []string{"foo", "bar"}, CanaryTags: []string{"canary", "bam"}, - PortLabel: "http", + Meta: map[string]string{ + "abc": "123", + }, + CanaryMeta: map[string]string{ + "canary": "boom", + }, + PortLabel: "http", Checks: []api.ServiceCheck{ { Name: "check-name", diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index 19b8a596487..57aeba4180d 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -169,6 +169,14 @@ job "binstore-storagelocker" { } service { + meta { + abc = "123" + } + + canary_meta { + canary = "boom" + } + tags = ["foo", "bar"] canary_tags = ["canary", "bam"] port = "http" diff --git a/nomad/structs/services.go b/nomad/structs/services.go index abc79256eb1..e3b40c27933 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -331,6 +331,7 @@ type Service struct { Checks []*ServiceCheck // List of checks associated with the service Connect *ConsulConnect // Consul Connect configuration Meta map[string]string // Consul service meta + CanaryMeta map[string]string // Consul service meta when it is a canary } // Copy the stanza recursively. Returns nil if nil. @@ -354,6 +355,7 @@ func (s *Service) Copy() *Service { ns.Connect = s.Connect.Copy() ns.Meta = helper.CopyMapStringString(s.Meta) + ns.CanaryMeta = helper.CopyMapStringString(s.CanaryMeta) return ns } @@ -466,6 +468,9 @@ func (s *Service) Hash(allocID, taskName string, canary bool) string { if len(s.Meta) > 0 { fmt.Fprintf(h, "%v", s.Meta) } + if len(s.CanaryMeta) > 0 { + fmt.Fprintf(h, "%v", s.CanaryMeta) + } // Vary ID on whether or not CanaryTags will be used if canary { @@ -526,6 +531,10 @@ OUTER: return false } + if !reflect.DeepEqual(s.CanaryMeta, o.CanaryMeta) { + return false + } + if !helper.CompareSliceSetString(s.Tags, o.Tags) { return false } diff --git a/website/source/docs/job-specification/service.html.md b/website/source/docs/job-specification/service.html.md index 4b2a59c01fe..7e616777dce 100644 --- a/website/source/docs/job-specification/service.html.md +++ b/website/source/docs/job-specification/service.html.md @@ -128,7 +128,7 @@ Connect][connect] integration. this service when the service is part of an allocation that is currently a canary. Once the canary is promoted, the registered tags will be updated to those specified in the `tags` parameter. If this is not supplied, the - registered tags will be equal to that of the `tags parameter. + registered tags will be equal to that of the `tags` parameter. - `address_mode` `(string: "auto")` - Specifies what address (host or driver-specific) this service should advertise. This setting is supported in @@ -151,6 +151,13 @@ Connect][connect] integration. - `meta` ([Meta][]: nil) - Specifies a key-value map that annotates the Consul service with user-defined metadata. +- `canary_meta` ([Meta][]: nil) - Specifies a key-value map that + annotates the Consul service with user-defined metadata when the service is + part of an allocation that is currently a canary. Once the canary is + promoted, the registered meta will be updated to those specified in the + `meta` parameter. If this is not supploed, the registered meta will be set to + that of the `meta` parameter. + ### `check` Parameters Note that health checks run inside the task. If your task is a Docker container, From 018f0717e76b487023c32e999243b801165ee2ce Mon Sep 17 00:00:00 2001 From: Nick Ethier Date: Wed, 8 Jan 2020 10:18:07 -0500 Subject: [PATCH 2/4] website: add canary meta to api docs --- website/source/api/json-jobs.html.md | 6 ++++++ website/source/docs/job-specification/service.html.md | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/website/source/api/json-jobs.html.md b/website/source/api/json-jobs.html.md index 1ed91c1f997..0397fe07b56 100644 --- a/website/source/api/json-jobs.html.md +++ b/website/source/api/json-jobs.html.md @@ -412,6 +412,12 @@ The `Task` object supports the following keys: updated to the set defined in the `Tags` field. String interpolation is supported in tags. + - `CanaryMeta`: A key-value map that annotates this Service while it + is a canary. Once the canary is promoted, the registered meta will be + updated to the set defined in the `Meta` field or removed if the `Meta` + field is not set. String interpolation is supported in meta keys and + values. + - `PortLabel`: `PortLabel` is an optional string and is used to associate a port with the service. If specified, the port label must match one defined in the resources block. This could be a label of either a diff --git a/website/source/docs/job-specification/service.html.md b/website/source/docs/job-specification/service.html.md index 7e616777dce..4988958c686 100644 --- a/website/source/docs/job-specification/service.html.md +++ b/website/source/docs/job-specification/service.html.md @@ -155,7 +155,7 @@ Connect][connect] integration. annotates the Consul service with user-defined metadata when the service is part of an allocation that is currently a canary. Once the canary is promoted, the registered meta will be updated to those specified in the - `meta` parameter. If this is not supploed, the registered meta will be set to + `meta` parameter. If this is not supplied, the registered meta will be set to that of the `meta` parameter. ### `check` Parameters From 330d24cb8080228820e9929547800e8362271a2c Mon Sep 17 00:00:00 2001 From: Nick Ethier Date: Mon, 27 Jan 2020 12:55:52 -0500 Subject: [PATCH 3/4] consul: fix var name from rebase --- command/agent/consul/client.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go index 88965c6f927..b97f01e0719 100644 --- a/command/agent/consul/client.go +++ b/command/agent/consul/client.go @@ -714,8 +714,9 @@ func (c *ServiceClient) serviceRegs(ops *operations, service *structs.Service, w return nil, fmt.Errorf("invalid Consul Connect configuration for service %q: %v", service.Name, err) } + // Determine whether to use meta or canary_meta var meta map[string]string - if task.Canary && len(service.CanaryMeta) > 0 { + if workload.Canary && len(service.CanaryMeta) > 0 { meta = make(map[string]string, len(service.CanaryMeta)+1) for k, v := range service.CanaryMeta { meta[k] = v From d6dd9b61ef0a88f44da7bee2daef44d0941ad50f Mon Sep 17 00:00:00 2001 From: Nick Ethier Date: Mon, 27 Jan 2020 14:00:19 -0500 Subject: [PATCH 4/4] consul: fix var name from rebase --- command/agent/consul/unit_test.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/command/agent/consul/unit_test.go b/command/agent/consul/unit_test.go index 65ab5aa6631..a7c8f5d8162 100644 --- a/command/agent/consul/unit_test.go +++ b/command/agent/consul/unit_test.go @@ -1086,10 +1086,10 @@ func TestConsul_CanaryMeta(t *testing.T) { canaryMeta := map[string]string{"meta1": "canary"} canaryMeta["external-source"] = "nomad" - ctx.Task.Canary = true - ctx.Task.Services[0].CanaryMeta = canaryMeta + ctx.Workload.Canary = true + ctx.Workload.Services[0].CanaryMeta = canaryMeta - require.NoError(ctx.ServiceClient.RegisterTask(ctx.Task)) + require.NoError(ctx.ServiceClient.RegisterWorkload(ctx.Workload)) require.NoError(ctx.syncOnce()) require.Len(ctx.FakeConsul.services, 1) for _, service := range ctx.FakeConsul.services { @@ -1097,16 +1097,16 @@ func TestConsul_CanaryMeta(t *testing.T) { } // Disable canary and assert meta are not the canary meta - origTask := ctx.Task.Copy() - ctx.Task.Canary = false - require.NoError(ctx.ServiceClient.UpdateTask(origTask, ctx.Task)) + origWorkload := ctx.Workload.Copy() + ctx.Workload.Canary = false + require.NoError(ctx.ServiceClient.UpdateWorkload(origWorkload, ctx.Workload)) require.NoError(ctx.syncOnce()) require.Len(ctx.FakeConsul.services, 1) for _, service := range ctx.FakeConsul.services { require.NotEqual(canaryMeta, service.Meta) } - ctx.ServiceClient.RemoveTask(ctx.Task) + ctx.ServiceClient.RemoveWorkload(ctx.Workload) require.NoError(ctx.syncOnce()) require.Len(ctx.FakeConsul.services, 0) } @@ -1120,10 +1120,10 @@ func TestConsul_CanaryMeta_NoMeta(t *testing.T) { meta := map[string]string{"meta1": "foo"} meta["external-source"] = "nomad" - ctx.Task.Canary = true - ctx.Task.Services[0].Meta = meta + ctx.Workload.Canary = true + ctx.Workload.Services[0].Meta = meta - require.NoError(ctx.ServiceClient.RegisterTask(ctx.Task)) + require.NoError(ctx.ServiceClient.RegisterWorkload(ctx.Workload)) require.NoError(ctx.syncOnce()) require.Len(ctx.FakeConsul.services, 1) for _, service := range ctx.FakeConsul.services { @@ -1131,16 +1131,16 @@ func TestConsul_CanaryMeta_NoMeta(t *testing.T) { } // Disable canary and assert meta dont change - origTask := ctx.Task.Copy() - ctx.Task.Canary = false - require.NoError(ctx.ServiceClient.UpdateTask(origTask, ctx.Task)) + origWorkload := ctx.Workload.Copy() + ctx.Workload.Canary = false + require.NoError(ctx.ServiceClient.UpdateWorkload(origWorkload, ctx.Workload)) require.NoError(ctx.syncOnce()) require.Len(ctx.FakeConsul.services, 1) for _, service := range ctx.FakeConsul.services { require.Equal(meta, service.Meta) } - ctx.ServiceClient.RemoveTask(ctx.Task) + ctx.ServiceClient.RemoveWorkload(ctx.Workload) require.NoError(ctx.syncOnce()) require.Len(ctx.FakeConsul.services, 0) }