Skip to content

Commit

Permalink
docker: move pause container recovery to after SetConfig
Browse files Browse the repository at this point in the history
When we added recovery of pause containers in #16352 we called the recovery
function from the plugin factory function. But in our plugin setup protocol, a
plugin isn't ready for use until we call `SetConfig`. This meant that
recovering pause containers was always done with the default
config. Setting up the Docker client only happens once, so setting the wrong
config in the recovery function also means that all other Docker API calls will
use the default config.

Move the `recoveryPauseContainers` call into the `SetConfig`. Fix the error
handling so that we return any error but also don't log when the context is
canceled, which happens twice during normal startup as we fingerprint the
driver.
  • Loading branch information
tgross committed Mar 29, 2023
1 parent 7f5d19e commit fef719c
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 8 deletions.
3 changes: 3 additions & 0 deletions .changelog/16713.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
docker: Fixed a bug where plugin config values were ignored
```
4 changes: 3 additions & 1 deletion drivers/docker/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ var (
"type": hclspec.NewAttr("type", "string", false),
"config": hclspec.NewBlockAttrs("config", "string", false),
})), hclspec.NewLiteral(`{
type = "json-file"
type = "json-file"
config = {
max-file = "2"
max-size = "2m"
Expand Down Expand Up @@ -773,6 +773,8 @@ func (d *Driver) SetConfig(c *base.Config) error {

d.cpusetFixer = newCpusetFixer(d)

go d.recoverPauseContainers(d.ctx)

return nil
}

Expand Down
14 changes: 7 additions & 7 deletions drivers/docker/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func NewDockerDriver(ctx context.Context, logger hclog.Logger) drivers.DriverPlu
ctx: ctx,
logger: logger,
}
go driver.recoverPauseContainers(ctx)

return driver
}

Expand Down Expand Up @@ -747,11 +747,11 @@ func (d *Driver) containerBinds(task *drivers.TaskConfig, driverConfig *TaskConf
return binds, nil
}

// recoverPauseContainers gets called when we start up the plugin. On client
// restarts we need to rebuild the set of pause containers we are
// tracking. Basically just scan all containers and pull the ID from anything
// that has the Nomad Label and has Name with prefix "/nomad_init_".
func (d *Driver) recoverPauseContainers(ctx context.Context) {
// On Client restart, we must rebuild the set of pause containers
// we are tracking. Basically just scan all containers and pull the ID from
// anything that has the Nomad Label and has Name with prefix "/nomad_init_".

_, dockerClient, err := d.dockerClients()
if err != nil {
d.logger.Error("failed to recover pause containers", "error", err)
Expand All @@ -765,8 +765,8 @@ func (d *Driver) recoverPauseContainers(ctx context.Context) {
"label": {dockerLabelAllocID},
},
})
if listErr != nil {
d.logger.Error("failed to list pause containers", "error", err)
if listErr != nil && listErr != ctx.Err() {
d.logger.Error("failed to list pause containers", "error", listErr)
return
}

Expand Down

0 comments on commit fef719c

Please sign in to comment.