From 796e623ddece8446f7f17b60e4a8e1ae608a4d07 Mon Sep 17 00:00:00 2001
From: Alex Dadgar <alex.dadgar@gmail.com>
Date: Wed, 23 May 2018 13:07:47 -0700
Subject: [PATCH] Use Tags when CanaryTags isn't specified

This PR fixes a bug where we weren't defaulting to `tags` when
`canary_tags` was empty and adds documentation.
---
 command/agent/consul/client.go                |  2 +-
 command/agent/consul/unit_test.go             | 33 +++++++++++++++++++
 website/source/api/json-jobs.html.md          |  5 +++
 .../docs/job-specification/service.html.md    |  6 ++++
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go
index 4c8498128..6a0c789dc 100644
--- a/command/agent/consul/client.go
+++ b/command/agent/consul/client.go
@@ -626,7 +626,7 @@ func (c *ServiceClient) serviceRegs(ops *operations, service *structs.Service, t
 
 	// Determine whether to use tags or canary_tags
 	var tags []string
-	if task.Canary {
+	if task.Canary && len(service.CanaryTags) > 0 {
 		tags = make([]string, len(service.CanaryTags))
 		copy(tags, service.CanaryTags)
 	} else {
diff --git a/command/agent/consul/unit_test.go b/command/agent/consul/unit_test.go
index 9b6ed2f78..dcc5a1dcb 100644
--- a/command/agent/consul/unit_test.go
+++ b/command/agent/consul/unit_test.go
@@ -1369,6 +1369,39 @@ func TestConsul_CanaryTags(t *testing.T) {
 	require.Len(ctx.FakeConsul.services, 0)
 }
 
+// TestConsul_CanaryTags_NoTags asserts Tags are used when Canary=true and there
+// are no specified canary tags
+func TestConsul_CanaryTags_NoTags(t *testing.T) {
+	t.Parallel()
+	require := require.New(t)
+	ctx := setupFake(t)
+
+	tags := []string{"tag1", "foo"}
+	ctx.Task.Canary = true
+	ctx.Task.Services[0].Tags = tags
+
+	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(tags, service.Tags)
+	}
+
+	// Disable canary and assert tags 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(tags, service.Tags)
+	}
+
+	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/website/source/api/json-jobs.html.md b/website/source/api/json-jobs.html.md
index 6059f6e4a..e5f169161 100644
--- a/website/source/api/json-jobs.html.md
+++ b/website/source/api/json-jobs.html.md
@@ -383,6 +383,11 @@ The `Task` object supports the following keys:
      - `Tags`: A list of string tags associated with this Service. String
        interpolation is supported in tags.
 
+     - `CanaryTags`: A list of string tags associated with this Service while it
+       is a canary. Once the canary is promoted, the registered tags will be
+       updated to the set defined in the `Tags` field. String interpolation is
+       supported in tags.
+
      - `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 7ebbe4c5b..6746b3f4e 100644
--- a/website/source/docs/job-specification/service.html.md
+++ b/website/source/docs/job-specification/service.html.md
@@ -112,6 +112,12 @@ does not automatically enable service discovery.
   this service. If this is not supplied, no tags will be assigned to the service
   when it is registered.
 
+- `canary_tags` `(array<string>: [])` - Specifies the list of tags to associate with
+  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.
+
 - `address_mode` `(string: "auto")` - Specifies what address (host or
   driver-specific) this service should advertise.  This setting is supported in
   Docker since Nomad 0.6 and rkt since Nomad 0.7. See [below for