From bea302e061eab7e7694c95857d3bbc7c5da1a027 Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Mon, 26 Nov 2018 13:39:35 +0100 Subject: [PATCH] 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 --- discovery/marathon/marathon.go | 13 +++++++++++-- discovery/marathon/marathon_test.go | 30 ++++++++++++++++++----------- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/discovery/marathon/marathon.go b/discovery/marathon/marathon.go index 32b9824b2..a5ce1aff5 100644 --- a/discovery/marathon/marathon.go +++ b/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. - ports[i] = portMappings[i].ServicePort + // Otherwise, connect to the allocated host port for the container. + // 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 } } diff --git a/discovery/marathon/marathon_test.go b/discovery/marathon/marathon_test.go index f8e95da7b..0307efdb6 100644 --- a/discovery/marathon/marathon_test.go +++ b/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: make(map[string]string), ServicePort: 32000}, + {Labels: labels, HostPort: 31000}, + {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: make(map[string]string), ServicePort: 32000}, + {Labels: labels, HostPort: 0}, + {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: make(map[string]string), ServicePort: 32000}, + {Labels: labels, HostPort: 31000}, + {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: make(map[string]string), ContainerPort: 1234, ServicePort: 32000}, + {Labels: labels, ContainerPort: 8080, HostPort: 31000}, + {Labels: make(map[string]string), ContainerPort: 1234, HostPort: 32000}, } container = Container{ Docker: docker,