Merge pull request #14 from allegro/hostname_to_address_refactoring

Hostname to Agent-address refactoring
allegro · Dec 15, 2015 · 353f33f · 353f33f
2 parents 9ffa782 + 7dd5379
commit 353f33f
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 25 deletions.
diff --git a/consul/consul.go b/consul/consul.go
@@ -9,7 +9,7 @@ import (
 type ConsulServices interface {
 	GetAllServices() ([]*consulapi.CatalogService, error)
 	Register(service *consulapi.AgentServiceRegistration) error
-	Deregister(serviceId string, agent string) error
+	Deregister(serviceId string, agentAddress string) error
 }
 
 type Consul struct {
@@ -81,11 +81,11 @@ func (c *Consul) register(service *consulapi.AgentServiceRegistration) error {
 		return err
 	}
 	fields := log.Fields{
-		"Name": service.Name,
-		"Id":   service.ID,
-		"Tags": service.Tags,
-		"Host": service.Address,
-		"Port": service.Port,
+		"Name":    service.Name,
+		"Id":      service.ID,
+		"Tags":    service.Tags,
+		"Address": service.Address,
+		"Port":    service.Port,
 	}
 	log.WithFields(fields).Info("Registering")
 
@@ -96,9 +96,9 @@ func (c *Consul) register(service *consulapi.AgentServiceRegistration) error {
 	return err
 }
 
-func (c *Consul) Deregister(serviceId string, agentHost string) error {
+func (c *Consul) Deregister(serviceId string, agentAddress string) error {
 	var err error
-	metrics.Time("consul.deregister", func() { err = c.deregister(serviceId, agentHost) })
+	metrics.Time("consul.deregister", func() { err = c.deregister(serviceId, agentAddress) })
 	if err != nil {
 		metrics.Mark("consul.deregister.error")
 	} else {
@@ -107,17 +107,17 @@ func (c *Consul) Deregister(serviceId string, agentHost string) error {
 	return err
 }
 
-func (c *Consul) deregister(serviceId string, agentHost string) error {
-	agent, err := c.agents.GetAgent(agentHost)
+func (c *Consul) deregister(serviceId string, agentAddress string) error {
+	agent, err := c.agents.GetAgent(agentAddress)
 	if err != nil {
 		return err
 	}
 
-	log.WithField("Id", serviceId).WithField("Host", agentHost).Info("Deregistering")
+	log.WithField("Id", serviceId).WithField("Address", agentAddress).Info("Deregistering")
 
 	err = agent.Agent().ServiceDeregister(serviceId)
 	if err != nil {
-		log.WithError(err).WithField("Id", serviceId).WithField("Host", agentHost).Error("Unable to deregister")
+		log.WithError(err).WithField("Id", serviceId).WithField("Address", agentAddress).Error("Unable to deregister")
 	}
 	return err
 }
diff --git a/consul/consul_test.go b/consul/consul_test.go
@@ -131,7 +131,7 @@ func TestRegisterServices_shouldReturnErrorOnFailure(t *testing.T) {
 
 func TestDeregisterServices(t *testing.T) {
 	t.Parallel()
-	server := CreateNamedConsulTestServer("localhost", "dc1", t)
+	server := CreateConsulTestServer("dc1", t)
 	defer server.Stop()
 
 	consul := ConsulClientAtServer(server)
@@ -143,7 +143,7 @@ func TestDeregisterServices(t *testing.T) {
 	assert.Len(t, services, 2)
 
 	// when
-	consul.Deregister("serviceA", server.Config.NodeName)
+	consul.Deregister("serviceA", server.Config.Bind)
 
 	// then
 	services, _ = consul.GetAllServices()
@@ -153,7 +153,7 @@ func TestDeregisterServices(t *testing.T) {
 
 func TestDeregisterServices_shouldReturnErrorOnFailure(t *testing.T) {
 	t.Parallel()
-	server := CreateNamedConsulTestServer("localhost", "dc1", t)
+	server := CreateConsulTestServer("dc1", t)
 	defer server.Stop()
 
 	consul := ConsulClientAtServer(server)
@@ -163,7 +163,7 @@ func TestDeregisterServices_shouldReturnErrorOnFailure(t *testing.T) {
 
 	// when
 	server.Stop()
-	err := consul.Deregister("serviceA", server.Config.NodeName)
+	err := consul.Deregister("serviceA", server.Config.Bind)
 
 	// then
 	assert.Error(t, err)

diff --git a/consul/consul_test_server.go b/consul/consul_test_server.go
@@ -12,13 +12,6 @@ func CreateConsulTestServer(dc string, t *testing.T) *testutil.TestServer {
 	})
 }
 
-func CreateNamedConsulTestServer(hostname string, dc string, t *testing.T) *testutil.TestServer {
-	return testutil.NewTestServerConfig(t, func(c *testutil.TestServerConfig) {
-		c.Datacenter = dc
-		c.NodeName = hostname
-	})
-}
-
 func ConsulClientAtServer(server *testutil.TestServer) *Consul {
 	return consulClientAtAddress(server.Config.Bind, server.Config.Ports.HTTP)
 }

diff --git a/sync/sync.go b/sync/sync.go
@@ -98,9 +98,12 @@ func (s Sync) deregisterConsulServicesThatAreNotInMarathonApps(apps []*apps.App,
 			}
 		}
 		if !found {
-			err := s.service.Deregister(instance.ServiceID, instance.Node)
+			err := s.service.Deregister(instance.ServiceID, instance.Address)
 			if err != nil {
-				log.WithError(err).WithField("Id", instance.ServiceID).Error("Can't deregister service")
+				log.WithError(err).WithFields(log.Fields{
+					"Id":      instance.ServiceID,
+					"Address": instance.Address,
+				}).Error("Can't deregister service")
 			}
 		}
 	}