Skip to content

Commit

Permalink
services: ensure task group is set on service hook
Browse files Browse the repository at this point in the history
This PR fixes a bug where the task group information was not being set
on the serviceHook.AllocInfo struct, which is needed later on for calculating
the CheckID of a nomad service check. The CheckID is calculated independently
from multiple callsites, and the information being passed in must be consistent,
including the group name.

The workload.AllocInfo.Group was not set at this callsite, due to the bug fixed in this PR.
 https://github.com/hashicorp/nomad/blob/main/client/serviceregistration/nsd/nsd.go#L114
  • Loading branch information
shoenig committed Feb 22, 2023
1 parent 2a0dde3 commit 96532e0
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 6 deletions.
16 changes: 10 additions & 6 deletions client/allocrunner/taskrunner/script_check_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"time"

"github.com/hashicorp/consul/api"
hclog "github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/nomad/ci"
"github.com/hashicorp/nomad/client/allocrunner/taskrunner/interfaces"
"github.com/hashicorp/nomad/client/serviceregistration"
Expand All @@ -19,6 +19,7 @@ import (
"github.com/hashicorp/nomad/helper/testlog"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/shoenig/test/must"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -271,14 +272,17 @@ func TestScript_TaskEnvInterpolation(t *testing.T) {
scHook.taskEnv = env
scHook.driverExec = exec

expectedSvc := svcHook.getWorkloadServices().Services[0]
workload := svcHook.getWorkloadServices()
must.Eq(t, "web", workload.AllocInfo.Group)

expectedSvc := workload.Services[0]
expected := agentconsul.MakeCheckID(serviceregistration.MakeAllocServiceID(
alloc.ID, task.Name, expectedSvc), expectedSvc.Checks[0])

actual := scHook.newScriptChecks()
check, ok := actual[expected]
require.True(t, ok)
require.Equal(t, "my-job-frontend-check", check.check.Name)
must.True(t, ok)
must.Eq(t, "my-job-frontend-check", check.check.Name)

// emulate an update
env = taskenv.NewBuilder(mock.Node(), alloc, task, "global").SetHookEnv(
Expand All @@ -293,8 +297,8 @@ func TestScript_TaskEnvInterpolation(t *testing.T) {

actual = scHook.newScriptChecks()
check, ok = actual[expected]
require.True(t, ok)
require.Equal(t, "my-job-backend-check", check.check.Name)
must.True(t, ok)
must.Eq(t, "my-job-backend-check", check.check.Name)
}

func TestScript_associated(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions client/allocrunner/taskrunner/service_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ func (h *serviceHook) getWorkloadServices() *serviceregistration.WorkloadService
info := structs.AllocInfo{
AllocID: h.allocID,
JobID: h.jobID,
Group: h.groupName,
Task: h.taskName,
Namespace: h.namespace,
}
Expand Down

0 comments on commit 96532e0

Please sign in to comment.