Config for Retry and RetryDelay for pulling images #1865
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@dustymabe @baude @mtrmac @vrothberg @giuseppe @containers/podman-maintainers PTAL |
|
What would the corresponding settings for pushing look like? |
rhatdan
force-pushed
the
retry
branch
2 times, most recently
from
1cdeff9
to
609dfd8
Compare
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
|
LGTM |
| @@ -354,6 +354,7 @@ func defaultEngineConfig() (*EngineConfig, error) { | |||
| c.PodmanshTimeout = uint(30) | |||
| c.ExitCommandDelay = uint(5 * 60) | |||
| c.Remote = isRemote() | |||
| c.Retry = 3 | |||
There was a problem hiding this comment.
does having a value set here change current default behavior?
There was a problem hiding this comment.
The only place we have a default behaviour currently is in Buildah and it is hard coded to 3.
The retry_delay is more complicated.
| // Number of times to retry pulling/pushing images in case of failure | ||
| Retry uint `toml:"retry,omitempty"` | ||
|
|
||
| // Delay between retries in case pulling/pushing image fails | ||
| // If set, container engines will retry at the set interval, | ||
| // otherwise they delay 2 seconds and then exponentially back off. | ||
| RetryDelay string `toml:"retry_delay,omitempty"` |
There was a problem hiding this comment.
Since these are new values it's worth asking: Retry is a pretty global term. Would there ever be a case where we'd want to have another setting for retrying something else in a different part of the stack? If so we'd want to make these values more specific maybe.
There was a problem hiding this comment.
Do you think we should change it to image_retry_delay and image_retry?
There was a problem hiding this comment.
I think retry is safe since these are the options in buildah build, podman pull and podman build.
There was a problem hiding this comment.
+1 - Just wanted to provoke the thought :)
|
Thanks Dan for working on this. Just a few questions from an outsider. Nothing that should block this merging. |
| @@ -764,6 +764,14 @@ Pull image before running or creating a container. The default is **missing**. | |||
| Indicates whether the application should be running in remote mode. This flag modifies the | |||
| --remote option on container engines. Setting the flag to true will default `podman --remote=true` for access to the remote Podman service. | |||
|
|
|||
| **retry** = 3 | |||
|
|
|||
| Number of times to retry pulling/pushing images in case of failure. | |||
There was a problem hiding this comment.
Does this actually retry pushes or just pulls?
There was a problem hiding this comment.
Buildah uses this for both push and pull, so I added it as well. We will need to get this into Podman and Buildah, once we merge this.
No description provided.