Merge pull request #300 from allegro/multiple-names-under-one-port

Handle same port multiple service registration

Author: ojagodzinski

.goxc.json

@@ -8,7 +8,7 @@
 		"deb-source"
 	],
 	"BuildConstraints": "linux,!arm darwin",
-	"PackageVersion": "1.5.2",
+	"PackageVersion": "1.5.3",
 	"TaskSettings": {
 		"bintray": {
 			"downloadspage": "bintray.md",

.travis.yml

@@ -3,7 +3,7 @@ dist: trusty
 sudo: false
 
 go:
-  - 1.x
+  - 1.13
 before_install:
   - gem install fpm
   - go get github.com/mattn/goveralls

Makefile

@@ -52,9 +52,9 @@ $(TEST_TARGETS):
 
 check-deps: deps
 	@which golangci-lint > /dev/null || \
-		(go get -u github.com/golangci/golangci-lint/cmd/golangci-lint)
+		(GO111MODULE=on go get -v github.com/golangci/golangci-lint/cmd/golangci-lint@v1.25.0)
 
-check: check-deps $(SOURCES)
+check: check-deps $(SOURCES) test
 	golangci-lint run --config=golangcilinter.yaml ./...
 
 format:

apps/app.go

@@ -10,6 +10,7 @@ import (
 
 // Only Marathon apps with this label will be registered in Consul
 const MarathonConsulLabel = "consul"
+const MarathonConsulTagValue = "tag"
 
 type HealthCheck struct {
 	Path                   string `json:"path"`
@@ -147,7 +148,7 @@ func marathonAppNameToServiceName(name string, nameSeparator string) string {
 func labelsToTags(labels map[string]string, tagPlaceholderMapping map[string]string) []string {
 	tags := make([]string, 0, len(labels))
 	for key, value := range labels {
-		if value == "tag" {
+		if value == MarathonConsulTagValue {
 			tags = append(tags, resolvePlaceholders(key, tagPlaceholderMapping))
 		}
 	}
@@ -206,14 +207,45 @@ func (app App) extractIndexedPortDefinitions() []indexedPortDefinition {
 func (app App) filterConsulDefinitions(all []indexedPortDefinition) []indexedPortDefinition {
 	var consulDefinitions []indexedPortDefinition
 	for _, d := range all {
-		if _, ok := d.Labels[MarathonConsulLabel]; ok {
-			consulDefinitions = append(consulDefinitions, d)
+		if labelName, ok := d.Labels[MarathonConsulLabel]; ok {
+			multipleDefinitions := strings.Split(labelName, ",")
+
+			for _, name := range multipleDefinitions {
+				labels := extractLabelsForService(name, d.Labels)
+				consulDefinitions = append(consulDefinitions, indexedPortDefinition{
+					Index:  d.Index,
+					Labels: labels,
+					Name:   name,
+				})
+			}
 		}
 	}
 
 	return consulDefinitions
 }
 
+func extractLabelsForService(serviceName string, labels map[string]string) map[string]string {
+	newLabels := make(map[string]string)
+
+	for key, value := range labels {
+		valueAndSelector := strings.Split(value, ":")
+		if len(valueAndSelector) > 1 {
+			extractedValue := valueAndSelector[0]
+			serviceSelector := valueAndSelector[1]
+
+			if extractedValue == MarathonConsulTagValue && serviceSelector == serviceName {
+				newLabels[key] = extractedValue
+			}
+		} else if key == MarathonConsulLabel {
+			newLabels[key] = serviceName
+		} else {
+			newLabels[key] = value
+		}
+	}
+
+	return newLabels
+}
+
 // Deprecated: Allows for backward compatibility with Marathons' network API
 // PortDefinitions are deprecated in favor of Marathons' new PortMappings
 // see https://github.com/mesosphere/marathon/pull/5391

consul/consul_test.go

@@ -483,6 +483,48 @@ func TestRegisterServices_CustomServiceName(t *testing.T) {
 	assert.Equal(t, "myCustomServiceName", services[0].Name)
 }
 
+func TestRegisterServices_MultipleRegistrationsWithSamePort(t *testing.T) {
+	t.Parallel()
+	server := CreateTestServer(t)
+	defer server.Stop()
+
+	consul := ClientAtServer(server)
+	consul.config.Tag = "marathon"
+
+	// given
+	app := utils.ConsulApp("serviceA", 1)
+	app.PortDefinitions = []apps.PortDefinition{
+		{
+			Labels: map[string]string{"consul": "first-name,second-name", "first-tag": "tag:first-name", "second-tag": "tag:second-name"},
+		},
+	}
+	app.Tasks[0].Host = server.Config.Bind
+	app.Tasks[0].Ports = []int{8080}
+	app.Labels["common-tag"] = "tag"
+
+	// when
+	err := consul.Register(&app.Tasks[0], app)
+
+	// then
+	assert.NoError(t, err)
+
+	// when
+	services, _ := consul.GetAllServices()
+
+	// then
+	assert.Len(t, services, 2)
+
+	first, found := findServiceByName("first-name", services)
+	assert.True(t, found, "first-name not found in services")
+	second, found := findServiceByName("second-name", services)
+	assert.True(t, found, "second-name not found in services")
+
+	assert.Equal(t, "first-name", first.Name)
+	assert.Equal(t, []string{"marathon", "first-tag", "common-tag", "marathon-task:serviceA.0"}, first.Tags)
+	assert.Equal(t, "second-name", second.Name)
+	assert.Equal(t, []string{"marathon", "second-tag", "common-tag", "marathon-task:serviceA.0"}, second.Tags)
+}
+
 func TestRegisterServices_MultipleRegistrations(t *testing.T) {
 	t.Parallel()
 	server := CreateTestServer(t)