Merge pull request #17 from allegro/destringify

Create alias types for Id

.travis.yml

@@ -7,10 +7,6 @@ before_install:
   - go get github.com/axw/gocov/gocov
   - go get github.com/mattn/goveralls
   - if ! go get github.com/golang/tools/cmd/cover; then go get golang.org/x/tools/cmd/cover; fi
-after_success:
-  - $HOME/gopath/bin/goveralls -service=travis-ci
-  - go get github.com/tcnksm/ghr
-  - make xcompile
-  - ghr -t $GITHUB_TOKEN -u allegro --replace `shell awk -F\" '/^const Version/ { print $$2 }' main.go` dist/
-
-script: make test
+script:
+  - make deps
+  - $HOME/gopath/bin/goveralls -package ./... -service=travis-ci

apps/app.go

@@ -26,7 +26,7 @@ type AppsResponse struct {
 type App struct {
 	Labels       map[string]string `json:"labels"`
 	HealthChecks []HealthCheck     `json:"healthChecks"`
-	ID           string            `json:"id"`
+	ID           tasks.AppId       `json:"id"`
 	Tasks        []tasks.Task      `json:"tasks"`
 }
 

consul/consul.go

@@ -3,13 +3,14 @@ package consul
 import (
 	log "github.com/Sirupsen/logrus"
 	"github.com/allegro/marathon-consul/metrics"
+	"github.com/allegro/marathon-consul/tasks"
 	consulapi "github.com/hashicorp/consul/api"
 )
 
 type ConsulServices interface {
 	GetAllServices() ([]*consulapi.CatalogService, error)
 	Register(service *consulapi.AgentServiceRegistration) error
-	Deregister(serviceId string, agentAddress string) error
+	Deregister(serviceId tasks.Id, agentAddress string) error
 }
 
 type Consul struct {
@@ -96,7 +97,7 @@ func (c *Consul) register(service *consulapi.AgentServiceRegistration) error {
 	return err
 }
 
-func (c *Consul) Deregister(serviceId string, agentAddress string) error {
+func (c *Consul) Deregister(serviceId tasks.Id, agentAddress string) error {
 	var err error
 	metrics.Time("consul.deregister", func() { err = c.deregister(serviceId, agentAddress) })
 	if err != nil {
@@ -107,15 +108,15 @@ func (c *Consul) Deregister(serviceId string, agentAddress string) error {
 	return err
 }
 
-func (c *Consul) deregister(serviceId string, agentAddress string) error {
+func (c *Consul) deregister(serviceId tasks.Id, agentAddress string) error {
 	agent, err := c.agents.GetAgent(agentAddress)
 	if err != nil {
 		return err
 	}
 
 	log.WithField("Id", serviceId).WithField("Address", agentAddress).Info("Deregistering")
 
-	err = agent.Agent().ServiceDeregister(serviceId)
+	err = agent.Agent().ServiceDeregister(serviceId.String())
 	if err != nil {
 		log.WithError(err).WithField("Id", serviceId).WithField("Address", agentAddress).Error("Unable to deregister")
 	}

consul/consul_stub.go

@@ -1,18 +1,19 @@
 package consul
 
 import (
+	"github.com/allegro/marathon-consul/tasks"
 	consulapi "github.com/hashicorp/consul/api"
 )
 
 type ConsulStub struct {
-	services      map[string]*consulapi.AgentServiceRegistration
-	ErrorServices map[string]error
+	services      map[tasks.Id]*consulapi.AgentServiceRegistration
+	ErrorServices map[tasks.Id]error
 }
 
 func NewConsulStub() *ConsulStub {
 	return &ConsulStub{
-		services:      make(map[string]*consulapi.AgentServiceRegistration),
-		ErrorServices: make(map[string]error),
+		services:      make(map[tasks.Id]*consulapi.AgentServiceRegistration),
+		ErrorServices: make(map[tasks.Id]error),
 	}
 }
 
@@ -32,15 +33,16 @@ func (c ConsulStub) GetAllServices() ([]*consulapi.CatalogService, error) {
 }
 
 func (c *ConsulStub) Register(service *consulapi.AgentServiceRegistration) error {
-	if err, ok := c.ErrorServices[service.ID]; ok {
+	taskId := tasks.Id(service.ID)
+	if err, ok := c.ErrorServices[taskId]; ok {
 		return err
 	} else {
-		c.services[service.ID] = service
+		c.services[taskId] = service
 		return nil
 	}
 }
 
-func (c *ConsulStub) Deregister(serviceId string, agent string) error {
+func (c *ConsulStub) Deregister(serviceId tasks.Id, agent string) error {
 	if err, ok := c.ErrorServices[serviceId]; ok {
 		return err
 	} else {

consul/services.go

@@ -8,13 +8,12 @@ import (
 	"github.com/allegro/marathon-consul/tasks"
 	"net/url"
 	"strconv"
-	"strings"
 )
 
 func MarathonTaskToConsulService(task tasks.Task, healthChecks []apps.HealthCheck, labels map[string]string) *consulapi.AgentServiceRegistration {
 	return &consulapi.AgentServiceRegistration{
-		ID:      task.ID,
-		Name:    appIdToServiceName(task.AppID),
+		ID:      task.ID.String(),
+		Name:    task.AppID.ConsulServiceName(),
 		Port:    task.Ports[0],
 		Address: task.Host,
 		Tags:    marathonLabelsToConsulTags(labels),
@@ -63,8 +62,3 @@ func marathonLabelsToConsulTags(labels map[string]string) []string {
 	}
 	return tags
 }
-
-func appIdToServiceName(appId string) (serviceId string) {
-	serviceId = strings.Replace(strings.Trim(appId, "/"), "/", ".", -1)
-	return serviceId
-}

events/events.go

@@ -2,6 +2,7 @@ package events
 
 import (
 	"github.com/allegro/marathon-consul/apps"
+	"github.com/allegro/marathon-consul/tasks"
 )
 
 type Event interface {
@@ -48,9 +49,9 @@ func (event DeploymentInfoEvent) GetType() string {
 }
 
 type AppTerminatedEvent struct {
-	Type      string `json:"eventType"`
-	AppID     string `json:"appId"`
-	Timestamp string `json:"timestamp"`
+	Type      string      `json:"eventType"`
+	AppID     tasks.AppId `json:"appId"`
+	Timestamp string      `json:"timestamp"`
 }
 
 func (event AppTerminatedEvent) Apps() []*apps.App {

events/task_health_change.go

@@ -2,15 +2,16 @@ package events
 
 import (
 	"encoding/json"
+	"github.com/allegro/marathon-consul/tasks"
 )
 
 type TaskHealthChange struct {
-	Timestamp  string `json:"timestamp"`
-	ID         string `json:"id"`
-	TaskStatus string `json:"taskStatus"`
-	AppID      string `json:"appId"`
-	Version    string `json:"version"`
-	Alive      bool   `json:"alive"`
+	Timestamp  string      `json:"timestamp"`
+	ID         tasks.Id    `json:"id"`
+	TaskStatus string      `json:"taskStatus"`
+	AppID      tasks.AppId `json:"appId"`
+	Version    string      `json:"version"`
+	Alive      bool        `json:"alive"`
 }
 
 func ParseTaskHealthChange(event []byte) (*TaskHealthChange, error) {

marathon/marathon.go

@@ -16,8 +16,8 @@ import (
 
 type Marathoner interface {
 	Apps() ([]*apps.App, error)
-	App(string) (*apps.App, error)
-	Tasks(string) ([]*tasks.Task, error)
+	App(tasks.AppId) (*apps.App, error)
+	Tasks(tasks.AppId) ([]*tasks.Task, error)
 }
 
 type Marathon struct {
@@ -47,10 +47,10 @@ func New(config Config) (*Marathon, error) {
 	}, nil
 }
 
-func (m Marathon) App(appId string) (*apps.App, error) {
+func (m Marathon) App(appId tasks.AppId) (*apps.App, error) {
 	log.WithField("Location", m.Location).Debug("Asking Marathon for " + appId)
 
-	body, err := m.get(m.urlWithQuery("/v2/apps/"+appId, "embed=apps.tasks"))
+	body, err := m.get(m.urlWithQuery(fmt.Sprintf("/v2/apps/%s", appId), "embed=apps.tasks"))
 	if err != nil {
 		return nil, err
 	}
@@ -68,14 +68,14 @@ func (m Marathon) Apps() ([]*apps.App, error) {
 	return apps.ParseApps(body)
 }
 
-func (m Marathon) Tasks(app string) ([]*tasks.Task, error) {
+func (m Marathon) Tasks(app tasks.AppId) ([]*tasks.Task, error) {
 	log.WithFields(log.Fields{
 		"Location": m.Location,
 		"Id":       app,
 	}).Debug("asking Marathon for tasks")
 
-	app = strings.Trim(app, "/")
-	body, err := m.get(m.url(fmt.Sprintf("/v2/apps/%s/tasks", app)))
+	trimmedAppId := strings.Trim(app.String(), "/")
+	body, err := m.get(m.url(fmt.Sprintf("/v2/apps/%s/tasks", trimmedAppId)))
 	if err != nil {
 		return nil, err
 	}

marathon/marathon_stub.go

@@ -8,23 +8,23 @@ import (
 
 type MarathonerStub struct {
 	AppsStub  []*apps.App
-	AppStub   map[string]*apps.App
-	TasksStub map[string][]*tasks.Task
+	AppStub   map[tasks.AppId]*apps.App
+	TasksStub map[tasks.AppId][]*tasks.Task
 }
 
 func (m MarathonerStub) Apps() ([]*apps.App, error) {
 	return m.AppsStub, nil
 }
 
-func (m MarathonerStub) App(id string) (*apps.App, error) {
+func (m MarathonerStub) App(id tasks.AppId) (*apps.App, error) {
 	if app, ok := m.AppStub[id]; ok {
 		return app, nil
 	} else {
 		return nil, fmt.Errorf("app not found")
 	}
 }
 
-func (m MarathonerStub) Tasks(appId string) ([]*tasks.Task, error) {
+func (m MarathonerStub) Tasks(appId tasks.AppId) ([]*tasks.Task, error) {
 	if app, ok := m.TasksStub[appId]; ok {
 		return app, nil
 	} else {
@@ -33,8 +33,8 @@ func (m MarathonerStub) Tasks(appId string) ([]*tasks.Task, error) {
 }
 
 func MarathonerStubForApps(args ...*apps.App) *MarathonerStub {
-	appsMap := make(map[string]*apps.App)
-	tasksMap := make(map[string][]*tasks.Task)
+	appsMap := make(map[tasks.AppId]*apps.App)
+	tasksMap := make(map[tasks.AppId][]*tasks.Task)
 
 	for _, app := range args {
 		appsMap[app.ID] = app

sync/sync.go

@@ -6,6 +6,7 @@ import (
 	service "github.com/allegro/marathon-consul/consul"
 	"github.com/allegro/marathon-consul/marathon"
 	"github.com/allegro/marathon-consul/metrics"
+	"github.com/allegro/marathon-consul/tasks"
 	consul "github.com/hashicorp/consul/api"
 	"time"
 )
@@ -89,19 +90,20 @@ func (s *Sync) registerMarathonApps(apps []*apps.App) {
 }
 
 func (s Sync) deregisterConsulServicesThatAreNotInMarathonApps(apps []*apps.App, services []*consul.CatalogService) {
-	marathonTasksIdSet := make(map[string]struct{})
+	marathonTasksIdSet := make(map[tasks.Id]struct{})
 	var exist struct{}
 	for _, app := range apps {
 		for _, task := range app.Tasks {
 			marathonTasksIdSet[task.ID] = exist
 		}
 	}
 	for _, instance := range services {
-		if _, ok := marathonTasksIdSet[instance.ServiceID]; !ok {
-			err := s.service.Deregister(instance.ServiceID, instance.Address)
+		instanceId := tasks.Id(instance.ServiceID)
+		if _, ok := marathonTasksIdSet[instanceId]; !ok {
+			err := s.service.Deregister(instanceId, instance.Address)
 			if err != nil {
 				log.WithError(err).WithFields(log.Fields{
-					"Id":      instance.ServiceID,
+					"Id":      instanceId,
 					"Address": instance.Address,
 				}).Error("Can't deregister service")
 			}

sync/sync_bench_test.go

@@ -65,8 +65,8 @@ func consulInstances(appsCount, instancesCount int) []*consulapi.CatalogService
 				ServiceAddress: task.Host,
 				ServicePort:    task.Ports[0],
 				ServiceTags:    []string{"marathon"},
-				ServiceID:      task.ID,
-				ServiceName:    app.ID,
+				ServiceID:      task.ID.String(),
+				ServiceName:    app.ID.String(),
 			}
 		}
 	}

sync/sync_test.go

@@ -29,7 +29,7 @@ func TestSyncJob(t *testing.T) {
 	select {
 	case <-time.After(15 * time.Millisecond):
 		ticker.Stop()
-		assert.Equal(t, 2, services.RegistrationsCount(app.Tasks[0].ID))
+		assert.Equal(t, 2, services.RegistrationsCount(app.Tasks[0].ID.String()))
 	}
 }
 
@@ -56,7 +56,7 @@ func (c *ConsulServicesMock) RegistrationsCount(instanceId string) int {
 	return c.registrations[instanceId]
 }
 
-func (c *ConsulServicesMock) Deregister(serviceId string, agent string) error {
+func (c *ConsulServicesMock) Deregister(serviceId tasks.Id, agent string) error {
 	return nil
 }
 
@@ -186,11 +186,11 @@ func (m errorMarathon) Apps() ([]*apps.App, error) {
 	return nil, fmt.Errorf("Error")
 }
 
-func (m errorMarathon) App(id string) (*apps.App, error) {
+func (m errorMarathon) App(id tasks.AppId) (*apps.App, error) {
 	return nil, fmt.Errorf("Error")
 }
 
-func (m errorMarathon) Tasks(appId string) ([]*tasks.Task, error) {
+func (m errorMarathon) Tasks(appId tasks.AppId) ([]*tasks.Task, error) {
 	return nil, fmt.Errorf("Error")
 }
 
@@ -204,6 +204,6 @@ func (c errorConsul) Register(service *consulapi.AgentServiceRegistration) error
 	return fmt.Errorf("Error occured")
 
 }
-func (c errorConsul) Deregister(serviceId string, agent string) error {
+func (c errorConsul) Deregister(serviceId tasks.Id, agent string) error {
 	return fmt.Errorf("Error occured")
 }

tasks/id.go

@@ -0,0 +1,24 @@
+package tasks
+
+import "strings"
+
+// Marathon Task Id
+// Usually in the form of AppId.uuid with '/' replaced with '_'
+type Id string
+
+func (id Id) String() string {
+	return string(id)
+}
+
+// Marathon Application Id (aka PathId)
+// Usually in the form of /rootGroup/subGroup/subSubGroup/name
+// allowed characters: lowercase letters, digits, hyphens, slash
+type AppId string
+
+func (id AppId) String() string {
+	return string(id)
+}
+
+func (id AppId) ConsulServiceName() string {
+	return strings.Replace(strings.Trim(id.String(), "/"), "/", ".", -1)
+}

tasks/id_test.go

@@ -0,0 +1,28 @@
+package tasks
+
+import (
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestId_String(t *testing.T) {
+	t.Parallel()
+	assert.Equal(t, "id", Id("id").String())
+}
+
+func TestAppId_String(t *testing.T) {
+	t.Parallel()
+	assert.Equal(t, "appId", AppId("appId").String())
+}
+
+func TestAppId_ConsulServiceName(t *testing.T) {
+	t.Parallel()
+	// given
+	id := AppId("/rootGroup/subGroup/subSubGroup/name")
+
+	// when
+	serviceName := id.ConsulServiceName()
+
+	// then
+	assert.Equal(t, "rootGroup.subGroup.subSubGroup.name", serviceName)
+}