Skip to content

Commit

Permalink
fix: use interpolated address when performing health checks (#18584)
Browse files Browse the repository at this point in the history
* fix: use interpolated address when performing health checks

* Fix tests, add changelog

* Update .changelog/18584.txt

Co-authored-by: Seth Hoenig <shoenig@duck.com>

---------

Co-authored-by: Seth Hoenig <shoenig@duck.com>
  • Loading branch information
MattJustMatt and shoenig committed Oct 4, 2023
1 parent fb7582d commit aa9ff3a
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 5 deletions.
3 changes: 3 additions & 0 deletions .changelog/18584.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
services: use interpolated address when performing nomad service health checks
```
4 changes: 2 additions & 2 deletions client/allocrunner/alloc_runner_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (ar *allocRunner) initRunnerHooks(config *clientconfig.Config) error {
}

// Create a taskenv.TaskEnv which is used for read only purposes by the
// newNetworkHook.
// newNetworkHook and newChecksHook.
builtTaskEnv := newEnvBuilder().Build()

// Create the alloc directory hook. This is run first to ensure the
Expand Down Expand Up @@ -139,7 +139,7 @@ func (ar *allocRunner) initRunnerHooks(config *clientconfig.Config) error {
newConsulGRPCSocketHook(hookLogger, alloc, ar.allocDir, config.ConsulConfig, config.Node.Attributes),
newConsulHTTPSocketHook(hookLogger, alloc, ar.allocDir, config.ConsulConfig),
newCSIHook(alloc, hookLogger, ar.csiManager, ar.rpcClient, ar, ar.hookResources, ar.clientConfig.Node.SecretID),
newChecksHook(hookLogger, alloc, ar.checkStore, ar),
newChecksHook(hookLogger, alloc, ar.checkStore, ar, builtTaskEnv),
}
if config.ExtraAllocHooks != nil {
ar.runnerHooks = append(ar.runnerHooks, config.ExtraAllocHooks...)
Expand Down
8 changes: 7 additions & 1 deletion client/allocrunner/checks_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/hashicorp/nomad/client/allocrunner/interfaces"
"github.com/hashicorp/nomad/client/serviceregistration/checks"
"github.com/hashicorp/nomad/client/serviceregistration/checks/checkstore"
"github.com/hashicorp/nomad/client/taskenv"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/nomad/structs"
)
Expand Down Expand Up @@ -82,6 +83,7 @@ type checksHook struct {
shim checkstore.Shim
checker checks.Checker
allocID string
taskEnv *taskenv.TaskEnv

// fields that get re-initialized on allocation update
lock sync.RWMutex
Expand All @@ -96,6 +98,7 @@ func newChecksHook(
alloc *structs.Allocation,
shim checkstore.Shim,
network structs.NetworkStatus,
taskEnv *taskenv.TaskEnv,
) *checksHook {
h := &checksHook{
logger: logger.Named(checksHookName),
Expand All @@ -104,6 +107,7 @@ func newChecksHook(
shim: shim,
network: network,
checker: checks.New(logger),
taskEnv: taskEnv,
}
h.initialize(alloc)
return h
Expand Down Expand Up @@ -208,8 +212,10 @@ func (h *checksHook) Prerun() error {
return nil
}

interpolatedServices := taskenv.InterpolateServices(h.taskEnv, group.NomadServices())

// create and start observers of nomad service checks in alloc
h.observe(h.alloc, group.NomadServices())
h.observe(h.alloc, interpolatedServices)

return nil
}
Expand Down
9 changes: 7 additions & 2 deletions client/allocrunner/checks_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/hashicorp/nomad/client/allocrunner/interfaces"
"github.com/hashicorp/nomad/client/serviceregistration/checks/checkstore"
"github.com/hashicorp/nomad/client/state"
"github.com/hashicorp/nomad/client/taskenv"
"github.com/hashicorp/nomad/helper/testlog"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
Expand Down Expand Up @@ -169,7 +170,9 @@ func TestCheckHook_Checks_ResultsSet(t *testing.T) {

alloc := allocWithNomadChecks(addr, port, tc.onGroup)

h := newChecksHook(logger, alloc, checkStore, network)
envBuilder := taskenv.NewBuilder(mock.Node(), alloc, nil, alloc.Job.Region)

h := newChecksHook(logger, alloc, checkStore, network, envBuilder.Build())

// initialize is called; observers are created but not started yet
must.MapEmpty(t, h.observers)
Expand Down Expand Up @@ -234,7 +237,9 @@ func TestCheckHook_Checks_UpdateSet(t *testing.T) {

alloc := allocWithNomadChecks(addr, port, true)

h := newChecksHook(logger, alloc, shim, network)
envBuilder := taskenv.NewBuilder(mock.Node(), alloc, nil, alloc.Job.Region)

h := newChecksHook(logger, alloc, shim, network, envBuilder.Build())

// calling pre-run starts the observers
err := h.Prerun()
Expand Down

1 comment on commit aa9ff3a

@vercel
Copy link

@vercel vercel bot commented on aa9ff3a Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.