Merge pull request #6653 from hashicorp/b-6646

nomad: fix bug that didn't allow for multiple connect services in same tg
hashicorp · Nov 8, 2019 · 1f6dbea · 1f6dbea
2 parents e715e14 + fa4ea8a
commit 1f6dbea
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,8 @@ IMPROVEMENTS:
 BUG FIXES:
 
  * cli: Make scoring column orders consistent `nomad alloc status` [[GH-6609](https://github.com/hashicorp/nomad/issues/6609)]
+ * nomad: Multiple connect enabled services in the same taskgroup failed to
+   register [[GH-6646](https://github.com/hashicorp/nomad/issues/6646)]
  * scheduler: Changes to devices in resource stanza should cause rescheduling [[GH-6644](https://github.com/hashicorp/nomad/issues/6644)]
 
 ## 0.10.1 (November 4, 2019)

diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go
@@ -142,7 +142,6 @@ func groupConnectHook(g *structs.TaskGroup) error {
 			if !found {
 				g.Networks[0].DynamicPorts = append(g.Networks[0].DynamicPorts, port)
 			}
-			return nil
 		}
 	}
 	return nil

diff --git a/nomad/job_endpoint_hook_connect_test.go b/nomad/job_endpoint_hook_connect_test.go
@@ -63,18 +63,30 @@ func Test_groupConnectHook(t *testing.T) {
 					SidecarService: &structs.ConsulSidecarService{},
 				},
 			},
+			{
+				Name:      "admin",
+				PortLabel: "9090",
+				Connect: &structs.ConsulConnect{
+					SidecarService: &structs.ConsulSidecarService{},
+				},
+			},
 		},
 	}
 
 	tgOut := tgIn.Copy()
 	tgOut.Tasks = []*structs.Task{
 		newConnectTask(tgOut.Services[0]),
+		newConnectTask(tgOut.Services[1]),
 	}
 	tgOut.Networks[0].DynamicPorts = []structs.Port{
 		{
 			Label: fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, "backend"),
 			To:    -1,
 		},
+		{
+			Label: fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, "admin"),
+			To:    -1,
+		},
 	}
 
 	require.NoError(t, groupConnectHook(tgIn))

diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
@@ -5013,13 +5013,16 @@ func (tg *TaskGroup) validateNetworks() error {
 				}
 			}
 
-			if port.To != 0 {
+			if port.To > 0 {
 				if other, ok := mappedPorts[port.To]; ok {
 					err := fmt.Errorf("Port mapped to %d already in use by %s", port.To, other)
 					mErr.Errors = append(mErr.Errors, err)
 				} else {
 					mappedPorts[port.To] = fmt.Sprintf("taskgroup network:%s", port.Label)
 				}
+			} else if port.To < -1 {
+				err := fmt.Errorf("Port %q cannot be mapped to negative value %d", port.Label, port.To)
+				mErr.Errors = append(mErr.Errors, err)
 			}
 		}
 	}