http client timeout config

[ncode]

Simple change to allow set http timeout via driver config.

Fix #100

[jrasell]

Hi @ncode and thanks for taking the time to raise this PR. The code looks good, however, I am wary of the approach as the timeout parameter is set at the plugin level and therefore will impact every HTTP call made.

We had a similar problem on the Docker driver which we solved by adding a config parameter within the job specification driver config itself. This allows overriding this setting per image, rather than on the whole driver. I wonder how you feel about this approach as an addition to the client setting?

The relevant code to this potential approach:

• api.ImagePull which accepts a context
• driver calls ImagePull passing a context

[ncode]

Hi @ncode and thanks for taking the time to raise this PR. The code looks good, however, I am wary of the approach as the timeout parameter is set at the plugin level and therefore will impact every HTTP call made.

We had a similar problem on the Docker driver which we solved by adding a config parameter within the job specification driver config itself. This allows overriding this setting per image, rather than on the whole driver. I wonder how you feel about this approach as an addition to the client setting?

The relevant code to this potential approach:

• api.ImagePull which accepts a context
• driver calls ImagePull passing a context

Hi @jrasell,

I think the timeout set on the task level sounds better and makes the configuration more granular, so I'm happy to change it. The suggestion from #100 points to plugin level so I followed it :). I will update the PR with the proposed changes.

[ncode]

Hi @jrasell,

I've implemented image_pull_timeout, but I also help the client_http_timeout. We need to use both to be able to achieve the timeout from the task. Since the default timeout on the driver for http requests is set to 60s, the context with timeout will drop without the increase. It would be possible to do something similar to https://github.com/hashicorp/nomad/blob/main/api/api.go#L319, but since my previous change helped I decided to keep it.

[sycured]

Any ETA about merge/release?

[hashicorp-cla]

All committers have signed the CLA.

[jdoss]

This feature would be great to have. I am trying to launch a ton of Podman containers across a large Nomad cluster and being able to set the timeout would help out a lot.

[lgfa29]

Sorry for getting so long to get this reviewed. I rebased on main to get the latest code changes and fix the conflicts.

Thank you so much for the contribution!