Merge pull request #6236 from hashicorp/b-ignore-connect-services

consul: ignore connect services when syncing
This commit is contained in:
Michael Schurter 2019-08-30 13:11:09 -07:00 committed by GitHub
commit 4bd53deba9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 84 additions and 26 deletions

View File

@ -499,6 +499,11 @@ func (c *ServiceClient) sync() error {
continue
}
// Ignore if this is a service for a Nomad managed sidecar proxy.
if isNomadSidecar(id, c.services) {
continue
}
// Unknown Nomad managed service; kill
if err := c.client.ServiceDeregister(id); err != nil {
if isOldNomadService(id) {
@ -556,6 +561,11 @@ func (c *ServiceClient) sync() error {
continue
}
// Ignore if this is a check for a Nomad managed sidecar proxy.
if isNomadSidecar(check.ServiceID, c.services) {
continue
}
// Unknown Nomad managed check; remove
if err := c.client.CheckDeregister(id); err != nil {
if isOldNomadService(check.ServiceID) {
@ -1370,6 +1380,28 @@ func isOldNomadService(id string) bool {
return strings.HasPrefix(id, prefix)
}
// isNomadSidecar returns true if the ID matches a sidecar proxy for a Nomad
// managed service.
//
// For example if you have a Connect enabled service with the ID:
//
// _nomad-task-5229c7f8-376b-3ccc-edd9-981e238f7033-cache-redis-cache-db
//
// Consul will create a service for the sidecar proxy with the ID:
//
// _nomad-task-5229c7f8-376b-3ccc-edd9-981e238f7033-cache-redis-cache-db-sidecar-proxy
//
func isNomadSidecar(id string, services map[string]*api.AgentServiceRegistration) bool {
const suffix = "-sidecar-proxy"
if !strings.HasSuffix(id, suffix) {
return false
}
// Make sure the Nomad managed service for this proxy still exists.
_, ok := services[id[:len(id)-len(suffix)]]
return ok
}
// getAddress returns the IP and port to use for a service or check. If no port
// label is specified (an empty value), zero values are returned because no
// address could be resolved.

View File

@ -5,6 +5,7 @@ import (
"testing"
"time"
"github.com/hashicorp/consul/api"
consulapi "github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/testutil"
"github.com/hashicorp/nomad/helper/testlog"
@ -32,7 +33,17 @@ func TestConsul_Connect(t *testing.T) {
consulClient, err := consulapi.NewClient(consulConfig)
require.NoError(t, err)
serviceClient := NewServiceClient(consulClient.Agent(), testlog.HCLogger(t), true)
// Lower periodicInterval to ensure periodic syncing doesn't improperly
// remove Connect services.
const interval = 50 * time.Millisecond
serviceClient.periodicInterval = interval
// Disable deregistration probation to test syncing
serviceClient.deregisterProbationExpiry = time.Time{}
go serviceClient.Run()
defer serviceClient.Shutdown()
alloc := mock.Alloc()
alloc.AllocatedResources.Shared.Networks = []*structs.NetworkResource{
@ -59,6 +70,11 @@ func TestConsul_Connect(t *testing.T) {
},
}
// required by isNomadSidecar assertion below
serviceRegMap := map[string]*api.AgentServiceRegistration{
MakeTaskServiceID(alloc.ID, "group-"+alloc.TaskGroup, tg.Services[0], false): nil,
}
require.NoError(t, serviceClient.RegisterGroup(alloc))
require.Eventually(t, func() bool {
@ -67,33 +83,43 @@ func TestConsul_Connect(t *testing.T) {
return len(services) == 2
}, 3*time.Second, 100*time.Millisecond)
services, err := consulClient.Agent().Services()
require.NoError(t, err)
require.Len(t, services, 2)
// Test a few times to ensure Nomad doesn't improperly deregister
// Connect services.
for i := 10; i > 0; i-- {
services, err := consulClient.Agent().Services()
require.NoError(t, err)
require.Len(t, services, 2)
serviceID := MakeTaskServiceID(alloc.ID, "group-"+alloc.TaskGroup, tg.Services[0], false)
connectID := serviceID + "-sidecar-proxy"
serviceID := MakeTaskServiceID(alloc.ID, "group-"+alloc.TaskGroup, tg.Services[0], false)
connectID := serviceID + "-sidecar-proxy"
require.Contains(t, services, serviceID)
agentService := services[serviceID]
require.Equal(t, agentService.Service, "testconnect")
require.Equal(t, agentService.Address, "10.0.0.1")
require.Equal(t, agentService.Port, 9999)
require.Nil(t, agentService.Connect)
require.Nil(t, agentService.Proxy)
require.Contains(t, services, serviceID)
require.True(t, isNomadService(serviceID))
require.False(t, isNomadSidecar(serviceID, serviceRegMap))
agentService := services[serviceID]
require.Equal(t, agentService.Service, "testconnect")
require.Equal(t, agentService.Address, "10.0.0.1")
require.Equal(t, agentService.Port, 9999)
require.Nil(t, agentService.Connect)
require.Nil(t, agentService.Proxy)
require.Contains(t, services, connectID)
connectService := services[connectID]
require.Equal(t, connectService.Service, "testconnect-sidecar-proxy")
require.Equal(t, connectService.Address, "10.0.0.1")
require.Equal(t, connectService.Port, 9999)
require.Nil(t, connectService.Connect)
require.Equal(t, connectService.Proxy.DestinationServiceName, "testconnect")
require.Equal(t, connectService.Proxy.DestinationServiceID, serviceID)
require.Equal(t, connectService.Proxy.LocalServiceAddress, "127.0.0.1")
require.Equal(t, connectService.Proxy.LocalServicePort, 9999)
require.Equal(t, connectService.Proxy.Config, map[string]interface{}{
"bind_address": "0.0.0.0",
"bind_port": float64(9998),
})
require.Contains(t, services, connectID)
require.True(t, isNomadService(connectID))
require.True(t, isNomadSidecar(connectID, serviceRegMap))
connectService := services[connectID]
require.Equal(t, connectService.Service, "testconnect-sidecar-proxy")
require.Equal(t, connectService.Address, "10.0.0.1")
require.Equal(t, connectService.Port, 9999)
require.Nil(t, connectService.Connect)
require.Equal(t, connectService.Proxy.DestinationServiceName, "testconnect")
require.Equal(t, connectService.Proxy.DestinationServiceID, serviceID)
require.Equal(t, connectService.Proxy.LocalServiceAddress, "127.0.0.1")
require.Equal(t, connectService.Proxy.LocalServicePort, 9999)
require.Equal(t, connectService.Proxy.Config, map[string]interface{}{
"bind_address": "0.0.0.0",
"bind_port": float64(9998),
})
time.Sleep(interval >> 2)
}
}