Support Context with remote.Image and remote.Get

[jpreese]

I am not seeing a way to pass in a context when using remote.Get(). Under the hood, it looks like it's just using the standard net/http package so I don't think it'd be too difficult of an addition if this proposal makes sense.

..Context.. 🥁 being in the event of a tcp timeout (e.g. unresposive registry), its just using the default timeout which is quite lengthy for my case. I'm defining a parent context and would like to cancel the request sooner.

[jonjohnsonjr]

I'm in favor of it. This was definitely an oversight in the API -- a lot of the people involved hadn't written much go code yet 😄

But I'm not sure how to add this, seems like we have two approaches:

1. Add GetContext and ImageContext (or similar name) functions, as recommended by https://blog.golang.org/module-compatibility#TOC_2.
2. Add a remote.WithContext option and keep everything the same. I think I've read some guidance against putting contexts in structs, but this issue seems to indicate maybe it's okay: context: relax recommendation against putting Contexts in structs golang/go#22602

If we ever do a v2 version of this, we'd just add context to the original functions, but until then I'd like to retain some compatibility.

cc @imjasonh do you have opinions?

[imjasonh]

remote.WithContext sgtm. My understanding of the guidance not to put contexts into structs is mostly about retaining them longer than you need to, especially across multiple requests of the same long-lived object. Stuffing context in an HTTP handler struct runs the risk of confusing two contexts which might have different creds, for instance, and so don't do that.

remote.ImageContext would also have to stuff the context in a struct to be able to lazily fetch layer data for instance, unless we're also talking about img.LayerByDigestContext(ctx, h) etc.

[jonjohnsonjr]

My understanding of the guidance not to put contexts into structs is mostly about retaining them longer than you need to, especially across multiple requests of the same long-lived object.

This seems kind of like what we'd want to do... the initial requests to get the manfiest should obviously use the given context, but what about subsequent http requests to e.g. download config or layers? Should we use that context still? Or just on the initial volley of requests?

[imjasonh]

I think* the difference is that once you give the struct a context, you'll never give it another one, so you always know which one to use. Contrast with an http.Handler that does s.currentCtx = req.Context() (don't do this!).

It seems fine to me* to have a v1.Image impl that stores a context and uses it to fetch layers. Most users will either a.) never fetch layers, or b.) tend to fetch them pretty quickly after they fetch the manifest, so sharing the timeout seems like what the user would expect, to me*.

*I'm not an expert, don't listen to me.

[jonjohnsonjr]

I added a big comment to hopefully make this behavior evident.