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

Timeout pulling large images #105

Closed
afbjorklund opened this issue Aug 14, 2022 · 2 comments · Fixed by #108
Closed

Timeout pulling large images #105

afbjorklund opened this issue Aug 14, 2022 · 2 comments · Fixed by #108

Comments

@afbjorklund
Copy link
Contributor

afbjorklund commented Aug 14, 2022

Looks like a timeout got mangled (from "long" to "short"), during the transfer from Kubernetes to Mirantis ?

Pulling a big image (600M) seems to be timing out after 1 minute, even though it is still making good progress.

Related code:

 	}
 	opts.RegistryAuth = base64Auth
-	ctx, cancel := d.getCancelableContext()
+	ctx, cancel := context.WithTimeout(context.Background(), d.timeout)
 	defer cancel()
 	resp, err := d.client.ImagePull(ctx, image, opts)
 	if err != nil {

For some reason, the readable functions were lost.

// getCancelableContext returns a new cancelable context. For long running requests without timeout, we use cancelable
// context to avoid potential resource leak, although the current implementation shouldn't leak resource.
func (d *kubeDockerClient) getCancelableContext() (context.Context, context.CancelFunc) {
        return context.WithCancel(context.Background())
}

// getTimeoutContext returns a new context with default request timeout
func (d *kubeDockerClient) getTimeoutContext() (context.Context, context.CancelFunc) {
        return context.WithTimeout(context.Background(), d.timeout)
}

Another strange thing is that default is 2 minutes ?

defaultTimeout = 2*time.Minute - 1*time.Second

Reported in:


docker@minikube:~$ cri-dockerd --version
cri-dockerd 0.2.1 (HEAD)

Built from: 0737013 (v0.2.2)

@evol262
Copy link
Contributor

evol262 commented Aug 17, 2022

Definitely my fault. Partly, I have an aversion to "method which wraps a one-line call". Mostly, context is very commonly understood by Go developers these days, and the "nicety" function forces anyone reading the code to jump back to the "helper" every time they look at the code just to see if it's doing anything special. Using plain context.With* is immediately obvious.

@afbjorklund
Copy link
Contributor Author

I documented the "docker pull" workaround (or "docker load"), in case cri-dockerd 0.2.6 doesn't make it out before k8s 1.25.0

Naturally this would affect users of both 1.24 and 1.25, but every new release of Kubernetes does see additional new installs...

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

Successfully merging a pull request may close this issue.

2 participants