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 875efd9 commit be63cb9
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 8 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
```
23 changes: 15 additions & 8 deletions client/allocrunner/taskrunner/template_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,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 @@ -161,12 +168,12 @@ 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
}
Expand All @@ -178,15 +185,15 @@ func (h *templateHook) Update(ctx context.Context, req *interfaces.TaskUpdateReq
h.vaultToken = req.VaultToken
}

// 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 be63cb9

Please sign in to comment.