From d1d8945d2e0a163c76ef51ff680815678154499b Mon Sep 17 00:00:00 2001 From: Allison Larson Date: Mon, 24 Mar 2025 13:03:14 -0700 Subject: [PATCH] Add docker plugin config option image_pull_timeout value for default timeout (#25489) * Add docker plugin config image_pull_timeout value for default timeout * Add image_pull_timeout docker plugin config to docs * Add changelog --- .changelog/25489.txt | 3 + drivers/docker/config.go | 64 +++++---- drivers/docker/config_test.go | 55 ++++---- drivers/docker/driver.go | 2 + drivers/docker/utils.go | 8 ++ drivers/docker/utils_test.go | 21 +++ helper/pluginutils/hclutils/util_test.go | 169 +++++++++++------------ website/content/docs/drivers/docker.mdx | 4 + 8 files changed, 177 insertions(+), 149 deletions(-) create mode 100644 .changelog/25489.txt diff --git a/.changelog/25489.txt b/.changelog/25489.txt new file mode 100644 index 000000000..ab95368e1 --- /dev/null +++ b/.changelog/25489.txt @@ -0,0 +1,3 @@ +```release-note:improvement +drivers/docker: adds image_pull_timeout to plugin config options +``` diff --git a/drivers/docker/config.go b/drivers/docker/config.go index 6d5b824af..8dc2e7a85 100644 --- a/drivers/docker/config.go +++ b/drivers/docker/config.go @@ -286,6 +286,11 @@ var ( hclspec.NewAttr("infra_image_pull_timeout", "string", false), hclspec.NewLiteral(`"5m"`), ), + // default timeout to use when pulling images. + "image_pull_timeout": hclspec.NewDefault( + hclspec.NewAttr("image_pull_timeout", "string", false), + hclspec.NewLiteral(`"5m"`), + ), // number of attempts to try to purge an existing container if it already exists "container_exists_attempts": hclspec.NewDefault( hclspec.NewAttr("container_exists_attempts", "number", false), @@ -404,33 +409,30 @@ var ( // mount and mounts are effectively aliases, but `mounts` is meant for pre-1.0 // assignment syntax `mounts = [{type="..." ..."}]` while // `mount` is 1.0 repeated block syntax `mount { type = "..." }` - "mount": hclspec.NewBlockList("mount", mountBodySpec), - "mounts": hclspec.NewBlockList("mounts", mountBodySpec), - "network_aliases": hclspec.NewAttr("network_aliases", "list(string)", false), - "network_mode": hclspec.NewAttr("network_mode", "string", false), - "oom_score_adj": hclspec.NewAttr("oom_score_adj", "number", false), - "runtime": hclspec.NewAttr("runtime", "string", false), - "pids_limit": hclspec.NewAttr("pids_limit", "number", false), - "pid_mode": hclspec.NewAttr("pid_mode", "string", false), - "ports": hclspec.NewAttr("ports", "list(string)", false), - "port_map": hclspec.NewAttr("port_map", "list(map(number))", false), - "privileged": hclspec.NewAttr("privileged", "bool", false), - "image_pull_timeout": hclspec.NewDefault( - hclspec.NewAttr("image_pull_timeout", "string", false), - hclspec.NewLiteral(`"5m"`), - ), - "readonly_rootfs": hclspec.NewAttr("readonly_rootfs", "bool", false), - "security_opt": hclspec.NewAttr("security_opt", "list(string)", false), - "shm_size": hclspec.NewAttr("shm_size", "number", false), - "storage_opt": hclspec.NewBlockAttrs("storage_opt", "string", false), - "sysctl": hclspec.NewAttr("sysctl", "list(map(string))", false), - "tty": hclspec.NewAttr("tty", "bool", false), - "ulimit": hclspec.NewAttr("ulimit", "list(map(string))", false), - "uts_mode": hclspec.NewAttr("uts_mode", "string", false), - "userns_mode": hclspec.NewAttr("userns_mode", "string", false), - "volumes": hclspec.NewAttr("volumes", "list(string)", false), - "volume_driver": hclspec.NewAttr("volume_driver", "string", false), - "work_dir": hclspec.NewAttr("work_dir", "string", false), + "mount": hclspec.NewBlockList("mount", mountBodySpec), + "mounts": hclspec.NewBlockList("mounts", mountBodySpec), + "network_aliases": hclspec.NewAttr("network_aliases", "list(string)", false), + "network_mode": hclspec.NewAttr("network_mode", "string", false), + "oom_score_adj": hclspec.NewAttr("oom_score_adj", "number", false), + "runtime": hclspec.NewAttr("runtime", "string", false), + "pids_limit": hclspec.NewAttr("pids_limit", "number", false), + "pid_mode": hclspec.NewAttr("pid_mode", "string", false), + "ports": hclspec.NewAttr("ports", "list(string)", false), + "port_map": hclspec.NewAttr("port_map", "list(map(number))", false), + "privileged": hclspec.NewAttr("privileged", "bool", false), + "image_pull_timeout": hclspec.NewAttr("image_pull_timeout", "string", false), + "readonly_rootfs": hclspec.NewAttr("readonly_rootfs", "bool", false), + "security_opt": hclspec.NewAttr("security_opt", "list(string)", false), + "shm_size": hclspec.NewAttr("shm_size", "number", false), + "storage_opt": hclspec.NewBlockAttrs("storage_opt", "string", false), + "sysctl": hclspec.NewAttr("sysctl", "list(map(string))", false), + "tty": hclspec.NewAttr("tty", "bool", false), + "ulimit": hclspec.NewAttr("ulimit", "list(map(string))", false), + "uts_mode": hclspec.NewAttr("uts_mode", "string", false), + "userns_mode": hclspec.NewAttr("userns_mode", "string", false), + "volumes": hclspec.NewAttr("volumes", "list(string)", false), + "volume_driver": hclspec.NewAttr("volume_driver", "string", false), + "work_dir": hclspec.NewAttr("work_dir", "string", false), }) // driverCapabilities represents the RPC response for what features are @@ -673,6 +675,7 @@ type DriverConfig struct { InfraImage string `codec:"infra_image"` InfraImagePullTimeout string `codec:"infra_image_pull_timeout"` infraImagePullTimeoutDuration time.Duration `codec:"-"` + ImagePullTimeout string `codec:"image_pull_timeout"` ContainerExistsAttempts uint64 `codec:"container_exists_attempts"` DisableLogCollection bool `codec:"disable_log_collection"` PullActivityTimeout string `codec:"pull_activity_timeout"` @@ -790,6 +793,13 @@ func (d *Driver) SetConfig(c *base.Config) error { d.config.infraImagePullTimeoutDuration = dur } + if d.config.ImagePullTimeout != "" { + _, err := time.ParseDuration(d.config.ImagePullTimeout) + if err != nil { + return fmt.Errorf("failed to parse 'image_pull_timeout' duration: %v", err) + } + } + d.config.allowRuntimes = make(map[string]struct{}, len(d.config.AllowRuntimesList)) for _, r := range d.config.AllowRuntimesList { d.config.allowRuntimes[r] = struct{}{} diff --git a/drivers/docker/config_test.go b/drivers/docker/config_test.go index d30e7cb54..f9b1d2a90 100644 --- a/drivers/docker/config_test.go +++ b/drivers/docker/config_test.go @@ -28,12 +28,11 @@ func TestConfig_ParseHCL(t *testing.T) { image = "redis:7" }`, &TaskConfig{ - Image: "redis:7", - Devices: []DockerDevice{}, - Mounts: []DockerMount{}, - MountsList: []DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "redis:7", + Devices: []DockerDevice{}, + Mounts: []DockerMount{}, + MountsList: []DockerMount{}, + CPUCFSPeriod: 100000, }, }, } @@ -64,48 +63,44 @@ func TestConfig_ParseJSON(t *testing.T) { name: "nil values for blocks are safe", input: `{"Config": {"image": "bash:3", "mounts": null}}`, expected: TaskConfig{ - Image: "bash:3", - Mounts: []DockerMount{}, - MountsList: []DockerMount{}, - Devices: []DockerDevice{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "bash:3", + Mounts: []DockerMount{}, + MountsList: []DockerMount{}, + Devices: []DockerDevice{}, + CPUCFSPeriod: 100000, }, }, { name: "nil values for 'volumes' field are safe", input: `{"Config": {"image": "bash:3", "volumes": null}}`, expected: TaskConfig{ - Image: "bash:3", - Mounts: []DockerMount{}, - MountsList: []DockerMount{}, - Devices: []DockerDevice{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "bash:3", + Mounts: []DockerMount{}, + MountsList: []DockerMount{}, + Devices: []DockerDevice{}, + CPUCFSPeriod: 100000, }, }, { name: "nil values for 'args' field are safe", input: `{"Config": {"image": "bash:3", "args": null}}`, expected: TaskConfig{ - Image: "bash:3", - Mounts: []DockerMount{}, - MountsList: []DockerMount{}, - Devices: []DockerDevice{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "bash:3", + Mounts: []DockerMount{}, + MountsList: []DockerMount{}, + Devices: []DockerDevice{}, + CPUCFSPeriod: 100000, }, }, { name: "nil values for string fields are safe", input: `{"Config": {"image": "bash:3", "command": null}}`, expected: TaskConfig{ - Image: "bash:3", - Mounts: []DockerMount{}, - MountsList: []DockerMount{}, - Devices: []DockerDevice{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "bash:3", + Mounts: []DockerMount{}, + MountsList: []DockerMount{}, + Devices: []DockerDevice{}, + CPUCFSPeriod: 100000, }, }, } diff --git a/drivers/docker/driver.go b/drivers/docker/driver.go index ae4e8382a..6f912df32 100644 --- a/drivers/docker/driver.go +++ b/drivers/docker/driver.go @@ -343,6 +343,8 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive driverConfig.Image = strings.TrimPrefix(driverConfig.Image, "https://") + driverConfig.ImagePullTimeout = getValue(driverConfig.ImagePullTimeout, d.config.ImagePullTimeout) + handle := drivers.NewTaskHandle(taskHandleVersion) handle.Config = cfg diff --git a/drivers/docker/utils.go b/drivers/docker/utils.go index e1997d4a3..1846524f5 100644 --- a/drivers/docker/utils.go +++ b/drivers/docker/utils.go @@ -350,3 +350,11 @@ func registryResolveAuthConfig(authConfigs map[string]types.AuthConfig, index *r // When all else fails, return an empty auth config return types.AuthConfig{} } + +// getValue returns the val if provided, or returns the defaultVal as a fallback +func getValue(val, defaultVal string) string { + if val == "" { + return defaultVal + } + return val +} diff --git a/drivers/docker/utils_test.go b/drivers/docker/utils_test.go index 09fe6d412..1927a5ab1 100644 --- a/drivers/docker/utils_test.go +++ b/drivers/docker/utils_test.go @@ -114,3 +114,24 @@ func TestParseDockerImage(t *testing.T) { }) } } + +func TestGetValue(t *testing.T) { + ci.Parallel(t) + + tests := []struct { + value string + defaultValue string + expected string + }{ + {value: "value", defaultValue: "default", expected: "value"}, + {value: "", defaultValue: "default", expected: "default"}, + {value: "value", defaultValue: "", expected: "value"}, + {value: "", defaultValue: "", expected: ""}, + } + + for _, test := range tests { + t.Run(test.expected, func(t *testing.T) { + must.Eq(t, test.expected, getValue(test.value, test.defaultValue)) + }) + } +} diff --git a/helper/pluginutils/hclutils/util_test.go b/helper/pluginutils/hclutils/util_test.go index 746355ce5..4d1e74b5d 100644 --- a/helper/pluginutils/hclutils/util_test.go +++ b/helper/pluginutils/hclutils/util_test.go @@ -45,12 +45,11 @@ func TestParseHclInterface_Hcl(t *testing.T) { }`), spec: dockerDecSpec, expected: &docker.TaskConfig{ - Image: "redis:7", - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "redis:7", + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -64,12 +63,11 @@ func TestParseHclInterface_Hcl(t *testing.T) { }`), spec: dockerDecSpec, expected: &docker.TaskConfig{ - Image: "redis:7", - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "redis:7", + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -82,13 +80,12 @@ func TestParseHclInterface_Hcl(t *testing.T) { }`), spec: dockerDecSpec, expected: &docker.TaskConfig{ - Image: "redis:7", - PidsLimit: 2, - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "redis:7", + PidsLimit: 2, + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -103,13 +100,12 @@ func TestParseHclInterface_Hcl(t *testing.T) { }`), spec: dockerDecSpec, expected: &docker.TaskConfig{ - Image: "redis:7", - PidsLimit: 2, - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "redis:7", + PidsLimit: 2, + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -122,13 +118,12 @@ func TestParseHclInterface_Hcl(t *testing.T) { }`), spec: dockerDecSpec, expected: &docker.TaskConfig{ - Image: "redis:7", - PidsLimit: 4, - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "redis:7", + PidsLimit: 4, + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -143,13 +138,12 @@ func TestParseHclInterface_Hcl(t *testing.T) { }`), spec: dockerDecSpec, expected: &docker.TaskConfig{ - Image: "redis:7", - PidsLimit: 4, - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "redis:7", + PidsLimit: 4, + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -162,13 +156,12 @@ func TestParseHclInterface_Hcl(t *testing.T) { }`), spec: dockerDecSpec, expected: &docker.TaskConfig{ - Image: "redis:7", - Args: []string{"foo", "bar"}, - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "redis:7", + Args: []string{"foo", "bar"}, + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -183,13 +176,12 @@ func TestParseHclInterface_Hcl(t *testing.T) { }`), spec: dockerDecSpec, expected: &docker.TaskConfig{ - Image: "redis:7", - Args: []string{"foo", "bar"}, - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "redis:7", + Args: []string{"foo", "bar"}, + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -204,14 +196,13 @@ func TestParseHclInterface_Hcl(t *testing.T) { spec: dockerDecSpec, vars: vars, expected: &docker.TaskConfig{ - Image: "redis:7", - Args: []string{"world", "2"}, - PidsLimit: 4, - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "redis:7", + Args: []string{"world", "2"}, + PidsLimit: 4, + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -226,13 +217,12 @@ func TestParseHclInterface_Hcl(t *testing.T) { }`), spec: dockerDecSpec, expected: &docker.TaskConfig{ - Image: "redis:7", - Args: []string{"foo", "bar"}, - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Image: "redis:7", + Args: []string{"foo", "bar"}, + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -253,11 +243,10 @@ func TestParseHclInterface_Hcl(t *testing.T) { "foo": 1234, "bar": 5678, }, - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -280,11 +269,10 @@ func TestParseHclInterface_Hcl(t *testing.T) { "foo": 1234, "bar": 5678, }, - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -319,10 +307,9 @@ func TestParseHclInterface_Hcl(t *testing.T) { ContainerPath: "/dev/xvdd", }, }, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -351,11 +338,10 @@ func TestParseHclInterface_Hcl(t *testing.T) { "tag": "driver-test", }, }, - Devices: []docker.DockerDevice{}, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Devices: []docker.DockerDevice{}, + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, @@ -392,10 +378,9 @@ func TestParseHclInterface_Hcl(t *testing.T) { ContainerPath: "/dev/xvdd", }, }, - Mounts: []docker.DockerMount{}, - MountsList: []docker.DockerMount{}, - CPUCFSPeriod: 100000, - ImagePullTimeout: "5m", + Mounts: []docker.DockerMount{}, + MountsList: []docker.DockerMount{}, + CPUCFSPeriod: 100000, }, expectedType: &docker.TaskConfig{}, }, diff --git a/website/content/docs/drivers/docker.mdx b/website/content/docs/drivers/docker.mdx index 8505ac12b..dbd535502 100644 --- a/website/content/docs/drivers/docker.mdx +++ b/website/content/docs/drivers/docker.mdx @@ -1055,6 +1055,10 @@ host system. wait before cancelling an in-progress pull of the Docker image as specified in `infra_image`. Defaults to `"5m"`. +- `image_pull_timeout` - (Optional) A default time duration that controls how long Nomad + waits before cancelling an in-progress pull of the Docker image as specified + in `image` across all tasks. Defaults to `"5m"`. + - `windows_allow_insecure_container_admin` - Indicates that on windows, docker checks the `task.user` field or, if unset, the container image manifest after pulling the container, to see if it's running as `ContainerAdmin`. If so, exits