client: refactor common service registration objects from Consul.

This commit performs refactoring to pull out common service
registration objects into a new `client/serviceregistration`
package. This new package will form the base point for all
client specific service registration functionality.

The Consul specific implementation is not moved as it also
includes non-service registration implementations; this reduces
the blast radius of the changes as well.

client/allochealth/tracker.go

@@ -9,9 +9,8 @@ import (
 
 	"github.com/hashicorp/consul/api"
 	hclog "github.com/hashicorp/go-hclog"
-	cconsul "github.com/hashicorp/nomad/client/consul"
+	"github.com/hashicorp/nomad/client/serviceregistration"
 	cstructs "github.com/hashicorp/nomad/client/structs"
-	"github.com/hashicorp/nomad/command/agent/consul"
 	"github.com/hashicorp/nomad/nomad/structs"
 )
 
@@ -56,7 +55,7 @@ type Tracker struct {
 	allocUpdates *cstructs.AllocListener
 
 	// consulClient is used to look up the state of the task's checks
-	consulClient cconsul.ConsulServiceAPI
+	consulClient serviceregistration.Handler
 
 	// healthy is used to signal whether we have determined the allocation to be
 	// healthy or unhealthy
@@ -93,7 +92,7 @@ type Tracker struct {
 // listener and consul API object are given so that the watcher can detect
 // health changes.
 func NewTracker(parentCtx context.Context, logger hclog.Logger, alloc *structs.Allocation,
-	allocUpdates *cstructs.AllocListener, consulClient cconsul.ConsulServiceAPI,
+	allocUpdates *cstructs.AllocListener, consulClient serviceregistration.Handler,
 	minHealthyTime time.Duration, useChecks bool) *Tracker {
 
 	// Do not create a named sub-logger as the hook controlling
@@ -377,7 +376,7 @@ func (t *Tracker) watchConsulEvents() {
 	consulChecksErr := false
 
 	// allocReg are the registered objects in Consul for the allocation
-	var allocReg *consul.AllocRegistration
+	var allocReg *serviceregistration.AllocRegistration
 
 OUTER:
 	for {
@@ -482,7 +481,7 @@ OUTER:
 type taskHealthState struct {
 	task              *structs.Task
 	state             *structs.TaskState
-	taskRegistrations *consul.ServiceRegistrations
+	taskRegistrations *serviceregistration.ServiceRegistrations
 }
 
 // event takes the deadline time for the allocation to be healthy and the update

client/allochealth/tracker_test.go

@@ -8,9 +8,9 @@ import (
 	"time"
 
 	consulapi "github.com/hashicorp/consul/api"
-	"github.com/hashicorp/nomad/client/consul"
+	"github.com/hashicorp/nomad/client/serviceregistration"
+	regMock "github.com/hashicorp/nomad/client/serviceregistration/mock"
 	cstructs "github.com/hashicorp/nomad/client/structs"
-	agentconsul "github.com/hashicorp/nomad/command/agent/consul"
 	"github.com/hashicorp/nomad/helper/testlog"
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
@@ -39,9 +39,9 @@ func TestTracker_Checks_Healthy(t *testing.T) {
 		Name:   task.Services[0].Checks[0].Name,
 		Status: consulapi.HealthPassing,
 	}
-	taskRegs := map[string]*agentconsul.ServiceRegistrations{
+	taskRegs := map[string]*serviceregistration.ServiceRegistrations{
 		task.Name: {
-			Services: map[string]*agentconsul.ServiceRegistration{
+			Services: map[string]*serviceregistration.ServiceRegistration{
 				task.Services[0].Name: {
 					Service: &consulapi.AgentService{
 						ID:      "foo",
@@ -59,13 +59,13 @@ func TestTracker_Checks_Healthy(t *testing.T) {
 
 	// Don't reply on the first call
 	var called uint64
-	consul := consul.NewMockConsulServiceClient(t, logger)
-	consul.AllocRegistrationsFn = func(string) (*agentconsul.AllocRegistration, error) {
+	consul := regMock.NewServiceRegistrationHandler(logger)
+	consul.AllocRegistrationsFn = func(string) (*serviceregistration.AllocRegistration, error) {
 		if atomic.AddUint64(&called, 1) == 1 {
 			return nil, nil
 		}
 
-		reg := &agentconsul.AllocRegistration{
+		reg := &serviceregistration.AllocRegistration{
 			Tasks: taskRegs,
 		}
 
@@ -111,7 +111,7 @@ func TestTracker_Checks_PendingPostStop_Healthy(t *testing.T) {
 	b := cstructs.NewAllocBroadcaster(logger)
 	defer b.Close()
 
-	consul := consul.NewMockConsulServiceClient(t, logger)
+	consul := regMock.NewServiceRegistrationHandler(logger)
 	ctx, cancelFn := context.WithCancel(context.Background())
 	defer cancelFn()
 
@@ -152,7 +152,7 @@ func TestTracker_Succeeded_PostStart_Healthy(t *testing.T) {
 	b := cstructs.NewAllocBroadcaster(logger)
 	defer b.Close()
 
-	consul := consul.NewMockConsulServiceClient(t, logger)
+	consul := regMock.NewServiceRegistrationHandler(logger)
 	ctx, cancelFn := context.WithCancel(context.Background())
 	defer cancelFn()
 
@@ -199,9 +199,9 @@ func TestTracker_Checks_Unhealthy(t *testing.T) {
 		Name:   task.Services[0].Checks[1].Name,
 		Status: consulapi.HealthCritical,
 	}
-	taskRegs := map[string]*agentconsul.ServiceRegistrations{
+	taskRegs := map[string]*serviceregistration.ServiceRegistrations{
 		task.Name: {
-			Services: map[string]*agentconsul.ServiceRegistration{
+			Services: map[string]*serviceregistration.ServiceRegistration{
 				task.Services[0].Name: {
 					Service: &consulapi.AgentService{
 						ID:      "foo",
@@ -219,13 +219,13 @@ func TestTracker_Checks_Unhealthy(t *testing.T) {
 
 	// Don't reply on the first call
 	var called uint64
-	consul := consul.NewMockConsulServiceClient(t, logger)
-	consul.AllocRegistrationsFn = func(string) (*agentconsul.AllocRegistration, error) {
+	consul := regMock.NewServiceRegistrationHandler(logger)
+	consul.AllocRegistrationsFn = func(string) (*serviceregistration.AllocRegistration, error) {
 		if atomic.AddUint64(&called, 1) == 1 {
 			return nil, nil
 		}
 
-		reg := &agentconsul.AllocRegistration{
+		reg := &serviceregistration.AllocRegistration{
 			Tasks: taskRegs,
 		}
 
@@ -341,9 +341,9 @@ func TestTracker_Checks_Healthy_Before_TaskHealth(t *testing.T) {
 		Name:   task.Services[0].Checks[0].Name,
 		Status: consulapi.HealthPassing,
 	}
-	taskRegs := map[string]*agentconsul.ServiceRegistrations{
+	taskRegs := map[string]*serviceregistration.ServiceRegistrations{
 		task.Name: {
-			Services: map[string]*agentconsul.ServiceRegistration{
+			Services: map[string]*serviceregistration.ServiceRegistration{
 				task.Services[0].Name: {
 					Service: &consulapi.AgentService{
 						ID:      "foo",
@@ -361,13 +361,13 @@ func TestTracker_Checks_Healthy_Before_TaskHealth(t *testing.T) {
 
 	// Don't reply on the first call
 	var called uint64
-	consul := consul.NewMockConsulServiceClient(t, logger)
-	consul.AllocRegistrationsFn = func(string) (*agentconsul.AllocRegistration, error) {
+	consul := regMock.NewServiceRegistrationHandler(logger)
+	consul.AllocRegistrationsFn = func(string) (*serviceregistration.AllocRegistration, error) {
 		if atomic.AddUint64(&called, 1) == 1 {
 			return nil, nil
 		}
 
-		reg := &agentconsul.AllocRegistration{
+		reg := &serviceregistration.AllocRegistration{
 			Tasks: taskRegs,
 		}
 
@@ -480,9 +480,9 @@ func TestTracker_Checks_OnUpdate(t *testing.T) {
 				Name:   task.Services[0].Checks[0].Name,
 				Status: tc.consulResp,
 			}
-			taskRegs := map[string]*agentconsul.ServiceRegistrations{
+			taskRegs := map[string]*serviceregistration.ServiceRegistrations{
 				task.Name: {
-					Services: map[string]*agentconsul.ServiceRegistration{
+					Services: map[string]*serviceregistration.ServiceRegistration{
 						task.Services[0].Name: {
 							Service: &consulapi.AgentService{
 								ID:      "foo",
@@ -503,13 +503,13 @@ func TestTracker_Checks_OnUpdate(t *testing.T) {
 
 			// Don't reply on the first call
 			var called uint64
-			consul := consul.NewMockConsulServiceClient(t, logger)
-			consul.AllocRegistrationsFn = func(string) (*agentconsul.AllocRegistration, error) {
+			consul := regMock.NewServiceRegistrationHandler(logger)
+			consul.AllocRegistrationsFn = func(string) (*serviceregistration.AllocRegistration, error) {
 				if atomic.AddUint64(&called, 1) == 1 {
 					return nil, nil
 				}
 
-				reg := &agentconsul.AllocRegistration{
+				reg := &serviceregistration.AllocRegistration{
 					Tasks: taskRegs,
 				}
 

client/allocrunner/alloc_runner.go

@@ -22,6 +22,7 @@ import (
 	cinterfaces "github.com/hashicorp/nomad/client/interfaces"
 	"github.com/hashicorp/nomad/client/pluginmanager/csimanager"
 	"github.com/hashicorp/nomad/client/pluginmanager/drivermanager"
+	"github.com/hashicorp/nomad/client/serviceregistration"
 	cstate "github.com/hashicorp/nomad/client/state"
 	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/client/vaultclient"
@@ -63,7 +64,7 @@ type allocRunner struct {
 
 	// consulClient is the client used by the consul service hook for
 	// registering services and checks
-	consulClient consul.ConsulServiceAPI
+	consulClient serviceregistration.Handler
 
 	// consulProxiesClient is the client used by the envoy version hook for
 	// looking up supported envoy versions of the consul agent.

client/allocrunner/alloc_runner_test.go

@@ -11,9 +11,9 @@ import (
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/nomad/client/allochealth"
 	"github.com/hashicorp/nomad/client/allocwatcher"
-	cconsul "github.com/hashicorp/nomad/client/consul"
+	"github.com/hashicorp/nomad/client/serviceregistration"
+	regMock "github.com/hashicorp/nomad/client/serviceregistration/mock"
 	"github.com/hashicorp/nomad/client/state"
-	"github.com/hashicorp/nomad/command/agent/consul"
 	"github.com/hashicorp/nomad/helper/uuid"
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
@@ -577,9 +577,9 @@ func TestAllocRunner_TaskGroup_ShutdownDelay(t *testing.T) {
 	})
 
 	// Get consul client operations
-	consulClient := conf.Consul.(*cconsul.MockConsulServiceClient)
+	consulClient := conf.Consul.(*regMock.ServiceRegistrationHandler)
 	consulOpts := consulClient.GetOps()
-	var groupRemoveOp cconsul.MockConsulOp
+	var groupRemoveOp regMock.Operation
 	for _, op := range consulOpts {
 		// Grab the first deregistration request
 		if op.Op == "remove" && op.Name == "group-web" {
@@ -1030,12 +1030,12 @@ func TestAllocRunner_DeploymentHealth_Unhealthy_Checks(t *testing.T) {
 	defer cleanup()
 
 	// Only return the check as healthy after a duration
-	consulClient := conf.Consul.(*cconsul.MockConsulServiceClient)
-	consulClient.AllocRegistrationsFn = func(allocID string) (*consul.AllocRegistration, error) {
-		return &consul.AllocRegistration{
-			Tasks: map[string]*consul.ServiceRegistrations{
+	consulClient := conf.Consul.(*regMock.ServiceRegistrationHandler)
+	consulClient.AllocRegistrationsFn = func(allocID string) (*serviceregistration.AllocRegistration, error) {
+		return &serviceregistration.AllocRegistration{
+			Tasks: map[string]*serviceregistration.ServiceRegistrations{
 				task.Name: {
-					Services: map[string]*consul.ServiceRegistration{
+					Services: map[string]*serviceregistration.ServiceRegistration{
 						"123": {
 							Service: &api.AgentService{Service: "fakeservice"},
 							Checks:  []*api.AgentCheck{checkUnhealthy},
@@ -1352,12 +1352,12 @@ func TestAllocRunner_TaskFailed_KillTG(t *testing.T) {
 	conf, cleanup := testAllocRunnerConfig(t, alloc)
 	defer cleanup()
 
-	consulClient := conf.Consul.(*cconsul.MockConsulServiceClient)
-	consulClient.AllocRegistrationsFn = func(allocID string) (*consul.AllocRegistration, error) {
-		return &consul.AllocRegistration{
-			Tasks: map[string]*consul.ServiceRegistrations{
+	consulClient := conf.Consul.(*regMock.ServiceRegistrationHandler)
+	consulClient.AllocRegistrationsFn = func(allocID string) (*serviceregistration.AllocRegistration, error) {
+		return &serviceregistration.AllocRegistration{
+			Tasks: map[string]*serviceregistration.ServiceRegistrations{
 				task.Name: {
-					Services: map[string]*consul.ServiceRegistration{
+					Services: map[string]*serviceregistration.ServiceRegistration{
 						"123": {
 							Service: &api.AgentService{Service: "fakeservice"},
 							Checks:  []*api.AgentCheck{checkHealthy},

client/allocrunner/alloc_runner_unix_test.go

@@ -11,7 +11,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/hashicorp/nomad/client/consul"
+	regMock "github.com/hashicorp/nomad/client/serviceregistration/mock"
 	"github.com/hashicorp/nomad/client/state"
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
@@ -125,7 +125,7 @@ func TestAllocRunner_Restore_RunningTerminal(t *testing.T) {
 	//    - removal during exited is de-duped due to prekill
 	//    - removal during stop is de-duped due to prekill
 	//   1 removal group during stop
-	consulOps := conf2.Consul.(*consul.MockConsulServiceClient).GetOps()
+	consulOps := conf2.Consul.(*regMock.ServiceRegistrationHandler).GetOps()
 	require.Len(t, consulOps, 2)
 	for _, op := range consulOps {
 		require.Equal(t, "remove", op.Op)

client/allocrunner/config.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/nomad/client/lib/cgutil"
 	"github.com/hashicorp/nomad/client/pluginmanager/csimanager"
 	"github.com/hashicorp/nomad/client/pluginmanager/drivermanager"
+	"github.com/hashicorp/nomad/client/serviceregistration"
 	cstate "github.com/hashicorp/nomad/client/state"
 	"github.com/hashicorp/nomad/client/vaultclient"
 	"github.com/hashicorp/nomad/nomad/structs"
@@ -31,7 +32,7 @@ type Config struct {
 	StateDB cstate.StateDB
 
 	// Consul is the Consul client used to register task services and checks
-	Consul consul.ConsulServiceAPI
+	Consul serviceregistration.Handler
 
 	// ConsulProxies is the Consul client used to lookup supported envoy versions
 	// of the Consul agent.

client/allocrunner/groupservice_hook.go

@@ -7,7 +7,7 @@ import (
 
 	log "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/nomad/client/allocrunner/interfaces"
-	"github.com/hashicorp/nomad/client/consul"
+	"github.com/hashicorp/nomad/client/serviceregistration"
 	"github.com/hashicorp/nomad/client/taskenv"
 	agentconsul "github.com/hashicorp/nomad/command/agent/consul"
 	"github.com/hashicorp/nomad/nomad/structs"
@@ -27,7 +27,7 @@ type groupServiceHook struct {
 	allocID             string
 	group               string
 	restarter           agentconsul.WorkloadRestarter
-	consulClient        consul.ConsulServiceAPI
+	consulClient        serviceregistration.Handler
 	consulNamespace     string
 	prerun              bool
 	deregistered        bool
@@ -51,7 +51,7 @@ type groupServiceHook struct {
 
 type groupServiceHookConfig struct {
 	alloc               *structs.Allocation
-	consul              consul.ConsulServiceAPI
+	consul              serviceregistration.Handler
 	consulNamespace     string
 	restarter           agentconsul.WorkloadRestarter
 	taskEnvBuilder      *taskenv.Builder
@@ -217,7 +217,7 @@ func (h *groupServiceHook) deregister() {
 	}
 }
 
-func (h *groupServiceHook) getWorkloadServices() *agentconsul.WorkloadServices {
+func (h *groupServiceHook) getWorkloadServices() *serviceregistration.WorkloadServices {
 	// Interpolate with the task's environment
 	interpolatedServices := taskenv.InterpolateServices(h.taskEnvBuilder.Build(), h.services)
 
@@ -227,15 +227,15 @@ func (h *groupServiceHook) getWorkloadServices() *agentconsul.WorkloadServices {
 	}
 
 	// Create task services struct with request's driver metadata
-	return &agentconsul.WorkloadServices{
-		AllocID:         h.allocID,
-		Group:           h.group,
-		ConsulNamespace: h.consulNamespace,
-		Restarter:       h.restarter,
-		Services:        interpolatedServices,
-		Networks:        h.networks,
-		NetworkStatus:   netStatus,
-		Ports:           h.ports,
-		Canary:          h.canary,
+	return &serviceregistration.WorkloadServices{
+		AllocID:       h.allocID,
+		Group:         h.group,
+		Namespace:     h.consulNamespace,
+		Restarter:     h.restarter,
+		Services:      interpolatedServices,
+		Networks:      h.networks,
+		NetworkStatus: netStatus,
+		Ports:         h.ports,
+		Canary:        h.canary,
 	}
 }