ar: fix group services hook tests

hashicorp · Nov 8, 2019 · 02a9b54 · 02a9b54
1 parent 022a016
commit 02a9b54
Show file tree

Hide file tree

Showing 14 changed files with 164 additions and 141 deletions.
diff --git a/client/allochealth/tracker.go b/client/allochealth/tracker.go
@@ -419,7 +419,7 @@ OUTER:
 type taskHealthState struct {
 	task              *structs.Task
 	state             *structs.TaskState
-	taskRegistrations *consul.TaskRegistration
+	taskRegistrations *consul.ServiceRegistrations
 }
 
 // event takes the deadline time for the allocation to be healthy and the update

diff --git a/client/allocrunner/alloc_runner_test.go b/client/allocrunner/alloc_runner_test.go
@@ -528,7 +528,7 @@ func TestAllocRunner_DeploymentHealth_Unhealthy_Checks(t *testing.T) {
 	consulClient := conf.Consul.(*cconsul.MockConsulServiceClient)
 	consulClient.AllocRegistrationsFn = func(allocID string) (*consul.AllocRegistration, error) {
 		return &consul.AllocRegistration{
-			Tasks: map[string]*consul.TaskRegistration{
+			Tasks: map[string]*consul.ServiceRegistrations{
 				task.Name: {
 					Services: map[string]*consul.ServiceRegistration{
 						"123": {
@@ -847,7 +847,7 @@ func TestAllocRunner_TaskFailed_KillTG(t *testing.T) {
 	consulClient := conf.Consul.(*cconsul.MockConsulServiceClient)
 	consulClient.AllocRegistrationsFn = func(allocID string) (*consul.AllocRegistration, error) {
 		return &consul.AllocRegistration{
-			Tasks: map[string]*consul.TaskRegistration{
+			Tasks: map[string]*consul.ServiceRegistrations{
 				task.Name: {
 					Services: map[string]*consul.ServiceRegistration{
 						"123": {

diff --git a/client/allocrunner/alloc_runner_unix_test.go b/client/allocrunner/alloc_runner_unix_test.go
@@ -33,6 +33,12 @@ func TestAllocRunner_Restore_RunningTerminal(t *testing.T) {
 	// 5. Assert task and logmon are cleaned up
 
 	alloc := mock.Alloc()
+	alloc.Job.TaskGroups[0].Services = []*structs.Service{
+		{
+			Name:      "foo",
+			PortLabel: "8888",
+		},
+	}
 	task := alloc.Job.TaskGroups[0].Tasks[0]
 	task.Driver = "mock_driver"
 	task.Config = map[string]interface{}{
@@ -117,13 +123,12 @@ func TestAllocRunner_Restore_RunningTerminal(t *testing.T) {
 	//   2 removals (canary+noncanary) during prekill
 	//   2 removals (canary+noncanary) during exited
 	//   2 removals (canary+noncanary) during stop
-	//   1 remove group during stop
+	//   2 removals (canary+noncanary) group during stop
 	consulOps := conf2.Consul.(*consul.MockConsulServiceClient).GetOps()
-	require.Len(t, consulOps, 7)
-	for _, op := range consulOps[:6] {
+	require.Len(t, consulOps, 8)
+	for _, op := range consulOps {
 		require.Equal(t, "remove", op.Op)
 	}
-	require.Equal(t, "remove_group", consulOps[6].Op)
 
 	// Assert terminated task event was emitted
 	events := ar2.AllocState().TaskStates[task.Name].Events

diff --git a/client/allocrunner/groupservice_hook_test.go b/client/allocrunner/groupservice_hook_test.go
@@ -1,7 +1,6 @@
 package allocrunner
 
 import (
-	"context"
 	"testing"
 
 	"github.com/hashicorp/nomad/client/allocrunner/interfaces"
@@ -18,41 +17,39 @@ var _ interfaces.RunnerPrerunHook = (*groupServiceHook)(nil)
 var _ interfaces.RunnerUpdateHook = (*groupServiceHook)(nil)
 var _ interfaces.RunnerPostrunHook = (*groupServiceHook)(nil)
 
-type testRestartCounter int
-
-func (t *testRestartCounter) Restart(ctx context.Context, event *structs.TaskEvent, failure bool) error {
-	*t++
-	return nil
-}
-
 // TestGroupServiceHook_NoGroupServices asserts calling group service hooks
 // without group services does not error.
 func TestGroupServiceHook_NoGroupServices(t *testing.T) {
 	t.Parallel()
 
 	alloc := mock.Alloc()
+	alloc.Job.TaskGroups[0].Services = []*structs.Service{{
+		Name:      "foo",
+		PortLabel: "9999",
+	}}
+	node := mock.Node()
 	logger := testlog.HCLogger(t)
 	consulClient := consul.NewMockConsulServiceClient(t, logger)
 
 	h := newGroupServiceHook(groupServiceHookConfig{
 		alloc:     alloc,
 		consul:    consulClient,
-		restarter: new(testRestartCounter),
+		restarter: agentconsul.NoopRestarter(),
 		taskEnv:   taskenv.NewBuilder(mock.Node(), alloc, nil, alloc.Job.Region).Build(),
 		logger:    logger,
 	})
 	require.NoError(t, h.Prerun())
 
-	req := &interfaces.RunnerUpdateRequest{Alloc: alloc}
+	req := &interfaces.RunnerUpdateRequest{Alloc: alloc, Node: node}
 	require.NoError(t, h.Update(req))
 
 	require.NoError(t, h.Postrun())
 
 	ops := consulClient.GetOps()
-	require.Len(t, ops, 3)
-	require.Equal(t, "add_group", ops[0].Op)
-	require.Equal(t, "update_group", ops[1].Op)
-	require.Equal(t, "remove_group", ops[2].Op)
+	require.Len(t, ops, 4)
+	require.Equal(t, "add", ops[0].Op)
+	require.Equal(t, "update", ops[1].Op)
+	require.Equal(t, "remove", ops[2].Op)
 }
 
 // TestGroupServiceHook_GroupServices asserts group service hooks with group
@@ -61,36 +58,39 @@ func TestGroupServiceHook_GroupServices(t *testing.T) {
 	t.Parallel()
 
 	alloc := mock.ConnectAlloc()
+	node := mock.Node()
 	logger := testlog.HCLogger(t)
 	consulClient := consul.NewMockConsulServiceClient(t, logger)
 
 	h := newGroupServiceHook(groupServiceHookConfig{
 		alloc:     alloc,
 		consul:    consulClient,
-		restarter: new(testRestartCounter),
+		restarter: agentconsul.NoopRestarter(),
 		taskEnv:   taskenv.NewBuilder(mock.Node(), alloc, nil, alloc.Job.Region).Build(),
 		logger:    logger,
 	})
 	require.NoError(t, h.Prerun())
 
-	req := &interfaces.RunnerUpdateRequest{Alloc: alloc}
+	req := &interfaces.RunnerUpdateRequest{Alloc: alloc, Node: node}
 	require.NoError(t, h.Update(req))
 
 	require.NoError(t, h.Postrun())
 
 	ops := consulClient.GetOps()
-	require.Len(t, ops, 3)
-	require.Equal(t, "add_group", ops[0].Op)
-	require.Equal(t, "update_group", ops[1].Op)
-	require.Equal(t, "remove_group", ops[2].Op)
+	require.Len(t, ops, 4)
+	require.Equal(t, "add", ops[0].Op)
+	require.Equal(t, "update", ops[1].Op)
+	require.Equal(t, "remove", ops[2].Op)
 }
 
 // TestGroupServiceHook_Error asserts group service hooks with group
 // services but no group network returns an error.
-func TestGroupServiceHook_Error(t *testing.T) {
+func TestGroupServiceHook_NoNetwork(t *testing.T) {
 	t.Parallel()
 
 	alloc := mock.Alloc()
+	alloc.Job.TaskGroups[0].Networks = []*structs.NetworkResource{}
+	node := mock.Node()
 	tg := alloc.Job.LookupTaskGroup(alloc.TaskGroup)
 	tg.Services = []*structs.Service{
 		{
@@ -103,21 +103,56 @@ func TestGroupServiceHook_Error(t *testing.T) {
 	}
 	logger := testlog.HCLogger(t)
 
-	// No need to set Consul client or call Run. This hould fail before
-	// attempting to register.
-	consulClient := agentconsul.NewServiceClient(nil, logger, false)
+	consulClient := consul.NewMockConsulServiceClient(t, logger)
 
 	h := newGroupServiceHook(groupServiceHookConfig{
 		alloc:     alloc,
 		consul:    consulClient,
-		restarter: new(testRestartCounter),
+		restarter: agentconsul.NoopRestarter(),
 		taskEnv:   taskenv.NewBuilder(mock.Node(), alloc, nil, alloc.Job.Region).Build(),
 		logger:    logger,
 	})
-	require.Error(t, h.Prerun())
+	require.NoError(t, h.Prerun())
+
+	req := &interfaces.RunnerUpdateRequest{Alloc: alloc, Node: node}
+	require.NoError(t, h.Update(req))
+
+	require.NoError(t, h.Postrun())
 
-	req := &interfaces.RunnerUpdateRequest{Alloc: alloc}
-	require.Error(t, h.Update(req))
+	ops := consulClient.GetOps()
+	require.Len(t, ops, 4)
+	require.Equal(t, "add", ops[0].Op)
+	require.Equal(t, "update", ops[1].Op)
+	require.Equal(t, "remove", ops[2].Op)
+}
+
+func TestGroupServiceHook_getWorkloadServices(t *testing.T) {
+	t.Parallel()
+
+	alloc := mock.Alloc()
+	alloc.Job.TaskGroups[0].Networks = []*structs.NetworkResource{}
+	tg := alloc.Job.LookupTaskGroup(alloc.TaskGroup)
+	tg.Services = []*structs.Service{
+		{
+			Name:      "testconnect",
+			PortLabel: "9999",
+			Connect: &structs.ConsulConnect{
+				SidecarService: &structs.ConsulSidecarService{},
+			},
+		},
+	}
+	logger := testlog.HCLogger(t)
+
+	consulClient := consul.NewMockConsulServiceClient(t, logger)
+
+	h := newGroupServiceHook(groupServiceHookConfig{
+		alloc:     alloc,
+		consul:    consulClient,
+		restarter: agentconsul.NoopRestarter(),
+		taskEnv:   taskenv.NewBuilder(mock.Node(), alloc, nil, alloc.Job.Region).Build(),
+		logger:    logger,
+	})
 
-	require.Error(t, h.Postrun())
+	services := h.getWorkloadServices()
+	require.Len(t, services.Services, 1)
 }
diff --git a/client/allocrunner/health_hook_test.go b/client/allocrunner/health_hook_test.go
@@ -243,7 +243,7 @@ func TestHealthHook_SetHealth(t *testing.T) {
 		Name:   task.Services[0].Checks[0].Name,
 		Status: consulapi.HealthPassing,
 	}
-	taskRegs := map[string]*agentconsul.TaskRegistration{
+	taskRegs := map[string]*agentconsul.ServiceRegistrations{
 		task.Name: {
 			Services: map[string]*agentconsul.ServiceRegistration{
 				task.Services[0].Name: {

diff --git a/client/allocrunner/taskrunner/envoybootstrap_hook.go b/client/allocrunner/taskrunner/envoybootstrap_hook.go
@@ -80,7 +80,7 @@ func (h *envoyBootstrapHook) Prestart(ctx context.Context, req *interfaces.TaskP
 	// it to the secrets directory like Vault tokens.
 	fn := filepath.Join(req.TaskDir.SecretsDir, "envoy_bootstrap.json")
 
-	id := agentconsul.MakeTaskServiceID(h.alloc.ID, "group-"+tg.Name, service)
+	id := agentconsul.MakeAllocServiceID(h.alloc.ID, "group-"+tg.Name, service)
 	h.logger.Debug("bootstrapping envoy", "sidecar_for", service.Name, "boostrap_file", fn, "sidecar_for_id", id, "grpc_addr", grpcAddr)
 
 	// Since Consul services are registered asynchronously with this task

diff --git a/client/allocrunner/taskrunner/script_check_hook.go b/client/allocrunner/taskrunner/script_check_hook.go
@@ -179,7 +179,7 @@ func (h *scriptCheckHook) newScriptChecks() map[string]*scriptCheck {
 			if check.Type != structs.ServiceCheckScript {
 				continue
 			}
-			serviceID := agentconsul.MakeTaskServiceID(
+			serviceID := agentconsul.MakeAllocServiceID(
 				h.alloc.ID, h.task.Name, service)
 			sc := newScriptCheck(&scriptCheckConfig{
 				allocID:    h.alloc.ID,
@@ -213,7 +213,7 @@ func (h *scriptCheckHook) newScriptChecks() map[string]*scriptCheck {
 				continue
 			}
 			groupTaskName := "group-" + tg.Name
-			serviceID := agentconsul.MakeTaskServiceID(
+			serviceID := agentconsul.MakeAllocServiceID(
 				h.alloc.ID, groupTaskName, service)
 			sc := newScriptCheck(&scriptCheckConfig{
 				allocID:    h.alloc.ID,

diff --git a/client/taskenv/env_test.go b/client/taskenv/env_test.go
@@ -8,7 +8,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/davecgh/go-spew/spew"
 	"github.com/hashicorp/hcl2/gohcl"
 	"github.com/hashicorp/hcl2/hcl"
 	"github.com/hashicorp/hcl2/hcl/hclsyntax"
@@ -845,5 +844,4 @@ func TestEnvironment_TasklessBuilder(t *testing.T) {
 		taskEnv = NewBuilder(node, alloc, nil, "global").SetAllocDir("/tmp/alloc").Build()
 	})
 
-	spew.Dump(taskEnv.All())
 }
diff --git a/command/agent/bindata_assetfs.go b/command/agent/bindata_assetfs.go