consul: Include port-label in service registration

It is possible to provide multiple identically named services with
different port assignments in a Nomad configuration.

We introduced a regression when migrating to stable service identifiers where
multiple services with the same name would conflict, and the last definition
would take precedence.

This commit includes the port label in the stable service identifier to
allow the previous behaviour where this was supported, for example
providing:

```hcl
service {
  name = "redis-cache"
  tags = ["global", "cache"]
  port = "db"
  check {
    name     = "alive"
    type     = "tcp"
    interval = "10s"
    timeout  = "2s"
  }
}

service {
  name = "redis-cache"
  tags = ["global", "foo"]
  port = "foo"

  check {
    name     = "alive"
    type     = "tcp"
    port     = "db"
    interval = "10s"
    timeout  = "2s"
  }
}

service {
  name = "redis-cache"
  tags = ["global", "bar"]
  port = "bar"

  check {
    name     = "alive"
    type     = "tcp"
    port     = "db"
    interval = "10s"
    timeout  = "2s"
  }
}
```

in a nomad task definition is now completely valid. Each service
definition with the same name must still have a unique port label however.

command/agent/consul/client.go

@@ -1107,12 +1107,11 @@ func makeAgentServiceID(role string, service *structs.Service) string {
 }
 
 // makeTaskServiceID creates a unique ID for identifying a task service in
-// Consul. All structs.Service fields are included in the ID's hash except
-// Checks. This allows updates to merely compare IDs.
+// Consul.
 //
-//	Example Service ID: _nomad-task-b4e61df9-b095-d64e-f241-23860da1375f-redis-http
+//	Example Service ID: _nomad-task-b4e61df9-b095-d64e-f241-23860da1375f-redis-http-http
 func makeTaskServiceID(allocID, taskName string, service *structs.Service, canary bool) string {
-	return fmt.Sprintf("%s%s-%s-%s", nomadTaskPrefix, allocID, taskName, service.Name)
+	return fmt.Sprintf("%s%s-%s-%s-%s", nomadTaskPrefix, allocID, taskName, service.Name, service.PortLabel)
 }
 
 // makeCheckID creates a unique ID for a check.

command/agent/consul/unit_test.go

@@ -205,7 +205,7 @@ func TestConsul_ChangePorts(t *testing.T) {
 		require.Equal(xPort, v.Port)
 	}
 
-	require.Equal(3, len(ctx.FakeConsul.checks))
+	require.Len(ctx.FakeConsul.checks, 3)
 
 	origTCPKey := ""
 	origScriptKey := ""
@@ -279,12 +279,12 @@ func TestConsul_ChangePorts(t *testing.T) {
 	for k, v := range ctx.FakeConsul.checks {
 		switch v.Name {
 		case "c1":
-			// C1 is not changed
-			require.Equal(origTCPKey, k)
+			// C1 is changed because the service was re-registered
+			require.NotEqual(origTCPKey, k)
 			require.Equal(fmt.Sprintf(":%d", xPort), v.TCP)
 		case "c2":
-			// C2 is not changed and should not have been re-registered
-			require.Equal(origScriptKey, k)
+			// C2 is changed because the service was re-registered
+			require.NotEqual(origScriptKey, k)
 		case "c3":
 			require.NotEqual(origHTTPKey, k)
 			require.Equal(fmt.Sprintf("http://:%d/", yPort), v.HTTP)
@@ -1615,3 +1615,64 @@ func TestGetAddress(t *testing.T) {
 		})
 	}
 }
+
+func TestConsul_ServiceName_Duplicates(t *testing.T) {
+	t.Parallel()
+	ctx := setupFake(t)
+	require := require.New(t)
+
+	ctx.Task.Services = []*structs.Service{
+		{
+			Name:      "best-service",
+			PortLabel: "x",
+			Tags: []string{
+				"foo",
+			},
+			Checks: []*structs.ServiceCheck{
+				{
+					Name:     "check-a",
+					Type:     "tcp",
+					Interval: time.Second,
+					Timeout:  time.Second,
+				},
+			},
+		},
+		{
+			Name:      "best-service",
+			PortLabel: "y",
+			Tags: []string{
+				"bar",
+			},
+			Checks: []*structs.ServiceCheck{
+				{
+					Name:     "checky-mccheckface",
+					Type:     "tcp",
+					Interval: time.Second,
+					Timeout:  time.Second,
+				},
+			},
+		},
+		{
+			Name:      "worst-service",
+			PortLabel: "y",
+		},
+	}
+
+	require.NoError(ctx.ServiceClient.RegisterTask(ctx.Task))
+
+	require.NoError(ctx.syncOnce())
+
+	require.Len(ctx.FakeConsul.services, 3)
+
+	for _, v := range ctx.FakeConsul.services {
+		if v.Name == ctx.Task.Services[0].Name && v.Port == xPort {
+			require.ElementsMatch(v.Tags, ctx.Task.Services[0].Tags)
+			require.Len(v.Checks, 1)
+		} else if v.Name == ctx.Task.Services[1].Name && v.Port == yPort {
+			require.ElementsMatch(v.Tags, ctx.Task.Services[1].Tags)
+			require.Len(v.Checks, 1)
+		} else if v.Name == ctx.Task.Services[2].Name {
+			require.Len(v.Checks, 0)
+		}
+	}
+}