Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http client timeout config #131

Merged
merged 4 commits into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ FEATURES:
* config: Add `cpu_hard_limit` and `cpu_cfs_period` options [[GH-149](https://github.com/hashicorp/nomad-driver-podman/pull/149)]
* config: Allow mounting rootfs as read-only. [[GH-133](https://github.com/hashicorp/nomad-driver-podman/pull/133)]
* config: Allow setting `ulimit` configuration. [[GH-166](https://github.com/hashicorp/nomad-driver-podman/pull/166)]
* config: Allow setting `image_pull_timeout` and `client_http_timeout ` [[GH-131](https://github.com/hashicorp/nomad-driver-podman/pull/131)]
* runtime: Add support for host and CSI volumes and using podman tasks as CSI plugins [[GH-169](https://github.com/hashicorp/nomad-driver-podman/pull/169)][[GH-152](https://github.com/hashicorp/nomad-driver-podman/pull/152)]

BUG FIXES:
Expand Down
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,14 @@ plugin "nomad-driver-podman" {
}
```

* client_http_timeout (string) Defaults to `60s` default timeout used by http.Client requests
```
plugin "nomad-driver-podman" {
config {
client_http_timeout = "60s"
}
```

## Task Configuration

* **image** - The image to run. Accepted transports are `docker` (default if missing), `oci-archive` and `docker-archive`. Images reference as [short-names](https://github.com/containers/image/blob/master/docs/containers-registries.conf.5.md#short-name-aliasing) will be treated according to user-configured preferences.
Expand Down Expand Up @@ -434,6 +442,12 @@ config {
nproc = "4242"
nofile = "2048:4096"
}
```

* **image_pull_timeout** - (Optional) time duration for your pull timeout (default to 5m), cannot be longer than the client_http_timeout
```
config {
image_pull_timeout = "5m"
}
```

Expand Down
13 changes: 10 additions & 3 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ var (
// disable_log_collection indicates whether nomad should collect logs of podman
// task containers. If true, logs are not forwarded to nomad.
"disable_log_collection": hclspec.NewAttr("disable_log_collection", "bool", false),
"client_http_timeout": hclspec.NewAttr("client_http_timeout", "string", false),
})

// taskConfigSpec is the hcl specification for the driver config section of
Expand All @@ -57,9 +58,13 @@ var (
"working_dir": hclspec.NewAttr("working_dir", "string", false),
"hostname": hclspec.NewAttr("hostname", "string", false),
"image": hclspec.NewAttr("image", "string", true),
"init": hclspec.NewAttr("init", "bool", false),
"init_path": hclspec.NewAttr("init_path", "string", false),
"labels": hclspec.NewAttr("labels", "list(map(string))", false),
"image_pull_timeout": hclspec.NewDefault(
hclspec.NewAttr("image_pull_timeout", "string", false),
hclspec.NewLiteral(`"5m"`),
),
"init": hclspec.NewAttr("init", "bool", false),
"init_path": hclspec.NewAttr("init_path", "string", false),
"labels": hclspec.NewAttr("labels", "list(map(string))", false),
"logging": hclspec.NewBlock("logging", false, hclspec.NewObject(map[string]*hclspec.Spec{
"driver": hclspec.NewAttr("driver", "string", false),
"options": hclspec.NewAttr("options", "list(map(string))", false),
Expand Down Expand Up @@ -111,6 +116,7 @@ type PluginConfig struct {
RecoverStopped bool `codec:"recover_stopped"`
DisableLogCollection bool `codec:"disable_log_collection"`
SocketPath string `codec:"socket_path"`
ClientHttpTimeout string `codec:"client_http_timeout"`
}

// TaskConfig is the driver configuration of a task within a job
Expand All @@ -128,6 +134,7 @@ type TaskConfig struct {
WorkingDir string `codec:"working_dir"`
Hostname string `codec:"hostname"`
Image string `codec:"image"`
ImagePullTimeout string `codec:"image_pull_timeout"`
InitPath string `codec:"init_path"`
Logging LoggingConfig `codec:"logging"`
Labels hclutils.MapStrStr `codec:"labels"`
Expand Down
15 changes: 15 additions & 0 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,18 @@ func TestConfig_CPUHardLimit(t *testing.T) {
require.EqualValues(t, true, tc.CPUHardLimit)
require.EqualValues(t, 200000, tc.CPUCFSPeriod)
}

func TestConfig_ImagePullTimeout(t *testing.T) {
parser := hclutils.NewConfigParser(taskConfigSpec)

validHCL := `
config {
image = "docker://redis"
image_pull_timeout = "10m"
}
`

var tc *TaskConfig
parser.ParseHCL(t, validHCL, &tc)
require.EqualValues(t, "10m", tc.ImagePullTimeout)
}
23 changes: 20 additions & 3 deletions driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,14 @@ func (d *Driver) SetConfig(cfg *base.Config) error {
clientConfig.SocketPath = pluginConfig.SocketPath
}

if pluginConfig.ClientHttpTimeout != "" {
t, err := time.ParseDuration(pluginConfig.ClientHttpTimeout)
if err != nil {
return err
}
clientConfig.HttpTimeout = t
}

d.podman = api.NewClient(d.logger, clientConfig)
return nil
}
Expand Down Expand Up @@ -545,7 +553,12 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
}

if !recoverRunningContainer {
imageID, err := d.createImage(createOpts.Image, &driverConfig.Auth, driverConfig.ForcePull, cfg)
imagePullTimeout, err := time.ParseDuration(driverConfig.ImagePullTimeout)
if err != nil {
return nil, nil, fmt.Errorf("failed to parse image_pull_timeout: %w", err)
}

imageID, err := d.createImage(createOpts.Image, &driverConfig.Auth, driverConfig.ForcePull, imagePullTimeout, cfg)
if err != nil {
return nil, nil, fmt.Errorf("failed to create image: %s: %w", createOpts.Image, err)
}
Expand Down Expand Up @@ -746,7 +759,7 @@ func sliceMergeUlimit(ulimitsRaw map[string]string) ([]spec.POSIXRlimit, error)

// Creates the requested image if missing from storage
// returns the 64-byte image ID as an unique image identifier
func (d *Driver) createImage(image string, auth *AuthConfig, forcePull bool, cfg *drivers.TaskConfig) (string, error) {
func (d *Driver) createImage(image string, auth *AuthConfig, forcePull bool, imagePullTimeout time.Duration, cfg *drivers.TaskConfig) (string, error) {
var imageID string
imageName := image
// If it is a shortname, we should not have to worry
Expand Down Expand Up @@ -805,7 +818,11 @@ func (d *Driver) createImage(image string, auth *AuthConfig, forcePull bool, cfg
Username: auth.Username,
Password: auth.Password,
}
if imageID, err = d.podman.ImagePull(d.ctx, imageName, imageAuth); err != nil {

ctx, cancel := context.WithTimeout(context.Background(), imagePullTimeout)
defer cancel()

if imageID, err = d.podman.ImagePull(ctx, imageName, imageAuth); err != nil {
return imageID, fmt.Errorf("failed to start task, unable to pull image %s : %w", imageName, err)
}
d.logger.Debug("Pulled image ID", "imageID", imageID)
Expand Down
9 changes: 5 additions & 4 deletions driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1801,7 +1801,7 @@ func startDestroyInspectImage(t *testing.T, image string, taskName string) {
AllocID: uuid.Generate(),
Resources: createBasicResources(),
}
imageID, err := getPodmanDriver(t, d).createImage(image, &AuthConfig{}, false, task)
imageID, err := getPodmanDriver(t, d).createImage(image, &AuthConfig{}, false, 5*time.Minute, task)
require.NoError(t, err)
require.Equal(t, imageID, inspectData.Image)
}
Expand Down Expand Up @@ -1862,7 +1862,7 @@ func createInspectImage(t *testing.T, image, reference string) {
AllocID: uuid.Generate(),
Resources: createBasicResources(),
}
idTest, err := getPodmanDriver(t, d).createImage(image, &AuthConfig{}, false, task)
idTest, err := getPodmanDriver(t, d).createImage(image, &AuthConfig{}, false, 5*time.Minute, task)
require.NoError(t, err)

idRef, err := getPodmanDriver(t, d).podman.ImageInspectID(context.Background(), reference)
Expand Down Expand Up @@ -2094,8 +2094,9 @@ func newTaskConfig(image string, command []string) TaskConfig {
return TaskConfig{
Image: image,
// LoadImage: loadImage,
Command: command[0],
Args: command[1:],
Command: command[0],
Args: command[1:],
ImagePullTimeout: "5m",
}
}

Expand Down