From 03a90f497aad75c8a505be2661e3f39daa1db6fc Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Wed, 29 Mar 2023 16:20:37 -0400
Subject: [PATCH] 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.
---
 .changelog/16713.txt     |  3 +++
 drivers/docker/config.go |  4 +++-
 drivers/docker/driver.go | 14 +++++++-------
 3 files changed, 13 insertions(+), 8 deletions(-)
 create mode 100644 .changelog/16713.txt

diff --git a/.changelog/16713.txt b/.changelog/16713.txt
new file mode 100644
index 000000000000..14d8c14c83a8
--- /dev/null
+++ b/.changelog/16713.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+docker: Fixed a bug where plugin config values were ignored
+```
diff --git a/drivers/docker/config.go b/drivers/docker/config.go
index 7dd3828b2067..0aefc96b19fe 100644
--- a/drivers/docker/config.go
+++ b/drivers/docker/config.go
@@ -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"
@@ -773,6 +773,8 @@ func (d *Driver) SetConfig(c *base.Config) error {
 
 	d.cpusetFixer = newCpusetFixer(d)
 
+	go d.recoverPauseContainers(d.ctx)
+
 	return nil
 }
 
diff --git a/drivers/docker/driver.go b/drivers/docker/driver.go
index fcf4241f0439..d20db05e17c3 100644
--- a/drivers/docker/driver.go
+++ b/drivers/docker/driver.go
@@ -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
 	}