From 9922c35a23fdf70eba2955ab6cd444014b4f1bc0 Mon Sep 17 00:00:00 2001 From: tommarute <23015421+tommarute@users.noreply.github.com> Date: Tue, 15 Jan 2019 02:20:22 +0900 Subject: [PATCH] marathon-sd - use Tasks.Ports instead of PortDefinitions.Ports if RequirePorts is false (#5022) (#5026) Signed-off-by: tommarute --- discovery/marathon/marathon.go | 12 ++- discovery/marathon/marathon_test.go | 159 ++++++++++++++-------------- 2 files changed, 86 insertions(+), 85 deletions(-) diff --git a/discovery/marathon/marathon.go b/discovery/marathon/marathon.go index b32eb175a..40ce0505e 100644 --- a/discovery/marathon/marathon.go +++ b/discovery/marathon/marathon.go @@ -327,6 +327,7 @@ type App struct { Container Container `json:"container"` PortDefinitions []PortDefinition `json:"portDefinitions"` Networks []Network `json:"networks"` + RequirePorts bool `json:"requirePorts"` } // isContainerNet checks if the app's first network is set to mode 'container'. @@ -435,7 +436,11 @@ func targetsForApp(app *App) []model.LabelSet { for i := 0; i < len(app.PortDefinitions); i++ { labels[i] = app.PortDefinitions[i].Labels - ports[i] = app.PortDefinitions[i].Port + // When requirePorts is false, this port becomes the 'servicePort', not the listen port. + // In this case, the port needs to be taken from the task instead of the app. + if app.RequirePorts { + ports[i] = app.PortDefinitions[i].Port + } } prefix = portDefinitionLabelPrefix @@ -456,8 +461,9 @@ 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. + // A zero port here means that either the portMapping has a zero port defined, + // or there is a portDefinition with requirePorts set to false. This means the port + // is auto-generated by Mesos and needs to be looked up in the task. if port == 0 && len(t.Ports) == len(ports) { port = t.Ports[i] } diff --git a/discovery/marathon/marathon_test.go b/discovery/marathon/marathon_test.go index 8764e947a..30567ca99 100644 --- a/discovery/marathon/marathon_test.go +++ b/discovery/marathon/marathon_test.go @@ -343,6 +343,8 @@ func marathonTestAppListWithPortDefinitions(labels map[string]string, runningTas task = Task{ ID: "test-task-1", Host: "mesos-slave1", + // Auto-generated ports when requirePorts is false + Ports: []uint32{1234, 5678}, } docker = DockerContainer{ Image: "repo/image:tag", @@ -358,6 +360,7 @@ func marathonTestAppListWithPortDefinitions(labels map[string]string, runningTas {Labels: make(map[string]string), Port: 31000}, {Labels: labels, Port: 32000}, }, + RequirePorts: false, // default } ) return &AppList{ @@ -379,6 +382,80 @@ func TestMarathonSDSendGroupWithPortDefinitions(t *testing.T) { case tgs := <-ch: tg := tgs[0] + if tg.Source != "test-service" { + t.Fatalf("Wrong target group name: %s", tg.Source) + } + if len(tg.Targets) != 2 { + t.Fatalf("Wrong number of targets: %v", tg.Targets) + } + tgt := tg.Targets[0] + if tgt[model.AddressLabel] != "mesos-slave1:1234" { + t.Fatalf("Wrong target address: %s", tgt[model.AddressLabel]) + } + if tgt[model.LabelName(portMappingLabelPrefix+"prometheus")] != "" { + t.Fatalf("Wrong first portMappings label from the first port: %s", tgt[model.AddressLabel]) + } + if tgt[model.LabelName(portDefinitionLabelPrefix+"prometheus")] != "" { + t.Fatalf("Wrong first portDefinitions label from the first port: %s", tgt[model.AddressLabel]) + } + tgt = tg.Targets[1] + if tgt[model.AddressLabel] != "mesos-slave1:5678" { + t.Fatalf("Wrong target address: %s", tgt[model.AddressLabel]) + } + if tgt[model.LabelName(portMappingLabelPrefix+"prometheus")] != "" { + t.Fatalf("Wrong portMappings label from the second port: %s", tgt[model.AddressLabel]) + } + if tgt[model.LabelName(portDefinitionLabelPrefix+"prometheus")] != "yes" { + t.Fatalf("Wrong portDefinitions label from the second port: %s", tgt[model.AddressLabel]) + } + default: + t.Fatal("Did not get a target group.") + } +} + +func marathonTestAppListWithPortDefinitionsRequirePorts(labels map[string]string, runningTasks int) *AppList { + var ( + task = Task{ + ID: "test-task-1", + Host: "mesos-slave1", + Ports: []uint32{31000, 32000}, + } + docker = DockerContainer{ + Image: "repo/image:tag", + } + container = Container{Docker: docker} + app = App{ + ID: "test-service", + Tasks: []Task{task}, + RunningTasks: runningTasks, + Labels: labels, + Container: container, + PortDefinitions: []PortDefinition{ + {Labels: make(map[string]string), Port: 31000}, + {Labels: labels, Port: 32000}, + }, + RequirePorts: true, + } + ) + return &AppList{ + Apps: []App{app}, + } +} + +func TestMarathonSDSendGroupWithPortDefinitionsRequirePorts(t *testing.T) { + var ( + ch = make(chan []*targetgroup.Group, 1) + client = func(client *http.Client, url string) (*AppList, error) { + return marathonTestAppListWithPortDefinitionsRequirePorts(marathonValidLabel, 1), nil + } + ) + if err := testUpdateServices(client, ch); err != nil { + t.Fatalf("Got error: %s", err) + } + select { + case tgs := <-ch: + tg := tgs[0] + if tg.Source != "test-service" { t.Fatalf("Wrong target group name: %s", tg.Source) } @@ -716,85 +793,3 @@ func TestMarathonSDSendGroupWithContainerNetworkAndPortMapping(t *testing.T) { t.Fatal("Did not get a target group.") } } - -func marathonTestAppListWithContainerNetworkAndPortDefinition(labels map[string]string, runningTasks int) *AppList { - var ( - task = Task{ - ID: "test-task-1", - Host: "mesos-slave1", - IPAddresses: []IPAddress{ - {Address: "1.2.3.4"}, - }, - } - docker = DockerContainer{ - Image: "repo/image:tag", - } - portDefinitions = []PortDefinition{ - {Labels: labels, Port: 8080}, - {Labels: make(map[string]string), Port: 1234}, - } - container = Container{ - Docker: docker, - } - networks = []Network{ - {Mode: "container", Name: "test-network"}, - } - app = App{ - ID: "test-service", - Tasks: []Task{task}, - RunningTasks: runningTasks, - Labels: labels, - Container: container, - Networks: networks, - PortDefinitions: portDefinitions, - } - ) - return &AppList{ - Apps: []App{app}, - } -} - -func TestMarathonSDSendGroupWithContainerNetworkAndPortDefinition(t *testing.T) { - var ( - ch = make(chan []*targetgroup.Group, 1) - client = func(client *http.Client, url string) (*AppList, error) { - return marathonTestAppListWithContainerNetworkAndPortDefinition(marathonValidLabel, 1), nil - } - ) - if err := testUpdateServices(client, ch); err != nil { - t.Fatalf("Got error: %s", err) - } - select { - case tgs := <-ch: - tg := tgs[0] - - if tg.Source != "test-service" { - t.Fatalf("Wrong target group name: %s", tg.Source) - } - if len(tg.Targets) != 2 { - t.Fatalf("Wrong number of targets: %v", tg.Targets) - } - tgt := tg.Targets[0] - if tgt[model.AddressLabel] != "1.2.3.4:8080" { - t.Fatalf("Wrong target address: %s", tgt[model.AddressLabel]) - } - if tgt[model.LabelName(portMappingLabelPrefix+"prometheus")] != "" { - t.Fatalf("Wrong first portMappings label from the first port: %s", tgt[model.AddressLabel]) - } - if tgt[model.LabelName(portDefinitionLabelPrefix+"prometheus")] != "yes" { - t.Fatalf("Wrong first portDefinitions label from the first port: %s", tgt[model.AddressLabel]) - } - tgt = tg.Targets[1] - if tgt[model.AddressLabel] != "1.2.3.4:1234" { - t.Fatalf("Wrong target address: %s", tgt[model.AddressLabel]) - } - if tgt[model.LabelName(portMappingLabelPrefix+"prometheus")] != "" { - t.Fatalf("Wrong portMappings label from the second port: %s", tgt[model.AddressLabel]) - } - if tgt[model.LabelName(portDefinitionLabelPrefix+"prometheus")] != "" { - t.Fatalf("Wrong portDefinitions label from the second port: %s", tgt[model.AddressLabel]) - } - default: - t.Fatal("Did not get a target group.") - } -}