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

Pulling images fails after 1 minute due to Timeout #100

Closed
pabloyoyoista opened this issue Apr 8, 2021 · 6 comments · Fixed by #131
Closed

Pulling images fails after 1 minute due to Timeout #100

pabloyoyoista opened this issue Apr 8, 2021 · 6 comments · Fixed by #131

Comments

@pabloyoyoista
Copy link
Contributor

I realized while having a look at #99, that the current timeout on api.Post for any api operation has a timeout of just one minute. That is definitely enough for most operations, but can easily become an issue for pulls in systems with slow Internet connections.

At the same time, just using a streaming http client like in that PR without setting any other kind of timeouts seems dangerous.
@towe75, any suggestions on how to deal with this?

@towe75
Copy link
Collaborator

towe75 commented Apr 9, 2021

Again pulling the image before starting a container is a very special situation. Later operations, like logging, can AFAIK rely on a second periodically running state poll and a parent context which is attached to the lifetime of the handle. So i don't think that this unbounded request is dangerous.

For your particular problem i would recommend to make that timeout configurable in the driver (not task) options and keep a sensible default of one minute. Running it unbounded is problematic because we do not have any chance to recover from a stalled request. The only thing left here is the nomad fingerprint poll which would deactivate the entire driver in case that we do no longer get podman system info but even this would not catch a blocking pull request. I assume that it would not even tear down this context because the handle is not registered until the container is up.

@pabloyoyoista
Copy link
Contributor Author

Again pulling the image before starting a container is a very special situation. Later operations, like logging, can AFAIK rely on a second periodically running state poll and a parent context which is attached to the lifetime of the handle. So i don't think that this unbounded request is dangerous.

Sorry, I think I expressed myself wrong. I meant that it is dangerous for the pull case.

For your particular problem i would recommend to make that timeout configurable in the driver (not task) options and keep a sensible default of one minute. Running it unbounded is problematic because we do not have any chance to recover from a stalled request. The only thing left here is the nomad fingerprint poll which would deactivate the entire driver in case that we do no longer get podman system info but even this would not catch a blocking pull request. I assume that it would not even tear down this context because the handle is not registered until the container is up.

Adding a driver option seems completely reasonable. I'll open a PR once I have it tested locally.

And thank you for your time and ideas!

@TimothyMDean
Copy link

Does this driver provide any workaround for the issue when pulling images? Is there any way to configure it to pull the image from a known location rather than from an external image repository? Perhaps something like the docker driver's load option?

@towe75
Copy link
Collaborator

towe75 commented Aug 13, 2021

@TimothyMDean you can pull images from any location. This includes local repositories. Alternatively you can load images from a host folder by using a "oci-archive:..." prefix, see #69

@sycured
Copy link

sycured commented Feb 15, 2022

@pabloyoyoista have you a diff or a current Work In Progress one?

It'll be good to be able to increase this timeout

@pabloyoyoista
Copy link
Contributor Author

No, I ended up logging into the relevant system and pulling the images manually when required. But it would be nice to have this implemented!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants