luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

docker: move pause container recovery to after `SetConfig` (#16713) 

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.
This commit is contained in:
Tim Gross 2023-03-29 16:20:37 -04:00 committed by GitHub
parent afa9608475
commit 76284a09a0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 13 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/16713.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
docker: Fixed a bug where plugin config values were ignored
```

2
drivers/docker/config.go
View File

@ -773,6 +773,8 @@ func (d *Driver) SetConfig(c *base.Config) error {
d.cpusetFixer = newCpusetFixer(d)
go d.recoverPauseContainers(d.ctx)
return nil
}

14
drivers/docker/driver.go
View File

@ -170,7 +170,7 @@ func NewDockerDriver(ctx context.Context, logger hclog.Logger) drivers.DriverPlu
ctx: ctx,
logger: logger,
}
go driver.recoverPauseContainers(ctx)
return driver
}
@ -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)
@ -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
}