marathon-sd - use 'hostPort' member of portMapping to construct target endpoints (#4887)

Fixes #4855 - ServicePort was wrongly used to construct an address to endpoints
defined in portMappings. This was changed to HostPort. Support for obtaining
auto-generated host ports was also added.

Signed-off-by: Timo Beckers <timo@incline.eu>

discovery/marathon/marathon.go

@@ -290,6 +290,7 @@ type IPAddress struct {
 type PortMapping struct {
 	Labels        map[string]string `json:"labels"`
 	ContainerPort uint32            `json:"containerPort"`
+	HostPort      uint32            `json:"hostPort"`
 	ServicePort   uint32            `json:"servicePort"`
 }
 
@@ -467,6 +468,12 @@ func targetsForApp(app *App) []model.LabelSet {
 		// Iterate over the ports we gathered using one of the methods above.
 		for i, port := range ports {
 
+			// A zero port can appear in a portMapping, which means it is auto-generated.
+			// The allocated port appears at the corresponding index in the task's 'ports' array.
+			if port == 0 && len(t.Ports) == len(ports) {
+				port = t.Ports[i]
+			}
+
 			// Each port represents a possible Prometheus target.
 			targetAddress := targetEndpoint(&t, port, app.isContainerNet())
 			target := model.LabelSet{
@@ -520,8 +527,10 @@ func extractPortMapping(portMappings []PortMapping, containerNet bool) ([]uint32
 			// If the app is in a container network, connect directly to the container port.
 			ports[i] = portMappings[i].ContainerPort
 		} else {
-			// Otherwise, connect to the randomly-generated service port.
+			// Otherwise, connect to the allocated host port for the container.
-			ports[i] = portMappings[i].ServicePort
+			// Note that this host port is likely set to 0 in the app definition, which means it is
+			// automatically generated and needs to be extracted from the task's 'ports' array at a later stage.
+			ports[i] = portMappings[i].HostPort
 		}
 	}
 

discovery/marathon/marathon_test.go

@@ -84,7 +84,7 @@ func marathonTestAppList(labels map[string]string, runningTasks int) *AppList {
 			Image: "repo/image:tag",
 		}
 		portMappings = []PortMapping{
-			{Labels: labels, ServicePort: 31000},
+			{Labels: labels, HostPort: 31000},
 		}
 		container = Container{Docker: docker, PortMappings: portMappings}
 		app       = App{
@@ -208,8 +208,8 @@ func marathonTestAppListWithMultiplePorts(labels map[string]string, runningTasks
 			Image: "repo/image:tag",
 		}
 		portMappings = []PortMapping{
-			{Labels: labels, ServicePort: 31000},
+			{Labels: labels, HostPort: 31000},
-			{Labels: make(map[string]string), ServicePort: 32000},
+			{Labels: make(map[string]string), HostPort: 32000},
 		}
 		container = Container{Docker: docker, PortMappings: portMappings}
 		app       = App{
@@ -484,6 +484,10 @@ func marathonTestAppListWithContainerPortMappings(labels map[string]string, runn
 		task = Task{
 			ID:   "test-task-1",
 			Host: "mesos-slave1",
+			Ports: []uint32{
+				12345, // 'Automatically-generated' port
+				32000,
+			},
 		}
 		docker = DockerContainer{
 			Image: "repo/image:tag",
@@ -491,8 +495,8 @@ func marathonTestAppListWithContainerPortMappings(labels map[string]string, runn
 		container = Container{
 			Docker: docker,
 			PortMappings: []PortMapping{
-				{Labels: labels, ServicePort: 31000},
+				{Labels: labels, HostPort: 0},
-				{Labels: make(map[string]string), ServicePort: 32000},
+				{Labels: make(map[string]string), HostPort: 32000},
 			},
 		}
 		app = App{
@@ -529,7 +533,7 @@ func TestMarathonSDSendGroupWithContainerPortMappings(t *testing.T) {
 			t.Fatalf("Wrong number of targets: %v", tg.Targets)
 		}
 		tgt := tg.Targets[0]
-		if tgt[model.AddressLabel] != "mesos-slave1:31000" {
+		if tgt[model.AddressLabel] != "mesos-slave1:12345" {
 			t.Fatalf("Wrong target address: %s", tgt[model.AddressLabel])
 		}
 		if tgt[model.LabelName(portMappingLabelPrefix+"prometheus")] != "yes" {
@@ -558,12 +562,16 @@ func marathonTestAppListWithDockerContainerPortMappings(labels map[string]string
 		task = Task{
 			ID:   "test-task-1",
 			Host: "mesos-slave1",
+			Ports: []uint32{
+				31000,
+				12345, // 'Automatically-generated' port
+			},
 		}
 		docker = DockerContainer{
 			Image: "repo/image:tag",
 			PortMappings: []PortMapping{
-				{Labels: labels, ServicePort: 31000},
+				{Labels: labels, HostPort: 31000},
-				{Labels: make(map[string]string), ServicePort: 32000},
+				{Labels: make(map[string]string), HostPort: 0},
 			},
 		}
 		container = Container{
@@ -613,7 +621,7 @@ func TestMarathonSDSendGroupWithDockerContainerPortMappings(t *testing.T) {
 			t.Fatalf("Wrong first portDefinitions label from the first port: %s", tgt[model.AddressLabel])
 		}
 		tgt = tg.Targets[1]
-		if tgt[model.AddressLabel] != "mesos-slave1:32000" {
+		if tgt[model.AddressLabel] != "mesos-slave1:12345" {
 			t.Fatalf("Wrong target address: %s", tgt[model.AddressLabel])
 		}
 		if tgt[model.LabelName(portMappingLabelPrefix+"prometheus")] != "" {
@@ -640,8 +648,8 @@ func marathonTestAppListWithContainerNetworkAndPortMappings(labels map[string]st
 			Image: "repo/image:tag",
 		}
 		portMappings = []PortMapping{
-			{Labels: labels, ContainerPort: 8080, ServicePort: 31000},
+			{Labels: labels, ContainerPort: 8080, HostPort: 31000},
-			{Labels: make(map[string]string), ContainerPort: 1234, ServicePort: 32000},
+			{Labels: make(map[string]string), ContainerPort: 1234, HostPort: 32000},
 		}
 		container = Container{
 			Docker:       docker,