Skip to content

Commit

Permalink
template: protect use of template manager with a lock
Browse files Browse the repository at this point in the history
This PR protects access to `templateHook.templateManager` with its lock. So
far we have not been able to reproduce the panic - but it seems either Poststart
is running without a Prestart being run first (should be impossible), or the
Update hook is running concurrently with Poststart, nil-ing out the templateManager
in a race with Poststart.

Fixes #15189
  • Loading branch information
shoenig committed Nov 10, 2022
1 parent 72d58fc commit 97c6f30
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 9 deletions.
3 changes: 3 additions & 0 deletions .changelog/15192.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
template: Fixed a bug where template could cause agent panic on startup
```
25 changes: 16 additions & 9 deletions client/allocrunner/taskrunner/template_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,18 @@ func (h *templateHook) Prestart(ctx context.Context, req *interfaces.TaskPrestar
}

func (h *templateHook) Poststart(ctx context.Context, req *interfaces.TaskPoststartRequest, resp *interfaces.TaskPoststartResponse) error {
h.managerLock.Lock()
defer h.managerLock.Unlock()

if h.templateManager == nil {
return nil
}

if req.DriverExec != nil {
h.templateManager.SetDriverHandle(req.DriverExec)
} else {
for _, template := range h.config.templates {
if template.ChangeMode == structs.TemplateChangeModeScript {
for _, tmpl := range h.config.templates {
if tmpl.ChangeMode == structs.TemplateChangeModeScript {
return fmt.Errorf("template has change mode set to 'script' but the driver it uses does not provide exec capability")
}
}
Expand Down Expand Up @@ -166,33 +173,33 @@ func (h *templateHook) Stop(ctx context.Context, req *interfaces.TaskStopRequest
return nil
}

// Handle new Vault token
// Update is used to handle updates to vault and/or nomad tokens.
func (h *templateHook) Update(ctx context.Context, req *interfaces.TaskUpdateRequest, resp *interfaces.TaskUpdateResponse) error {
h.managerLock.Lock()
defer h.managerLock.Unlock()

// Nothing to do
// no template manager to manage
if h.templateManager == nil {
return nil
}

// Check if either the Nomad or Vault tokens have changed
// neither vault or nomad token has been updated, nothing to do
if req.VaultToken == h.vaultToken && req.NomadToken == h.nomadToken {
return nil
} else {
h.vaultToken = req.VaultToken
h.nomadToken = req.NomadToken
}

// Shutdown the old template
// shutdown the old template
h.templateManager.Stop()
h.templateManager = nil

// Create the new template
// create the new template
if _, err := h.newManager(); err != nil {
err := fmt.Errorf("failed to build template manager: %v", err)
err = fmt.Errorf("failed to build template manager: %v", err)
h.logger.Error("failed to build template manager", "error", err)
h.config.lifecycle.Kill(context.Background(),
_ = h.config.lifecycle.Kill(context.Background(),
structs.NewTaskEvent(structs.TaskKilling).
SetFailsTask().
SetDisplayMessage(fmt.Sprintf("Template update %v", err)))
Expand Down

0 comments on commit 97c6f30

Please sign in to comment.