Enable govmomi context timeouts

[dougm]

We are currently using a single Context.Background for all govmomi operations. We should look at using the context to provide a default timeout for vsphere oprations and more fine grained timeouts as needed.

https://godoc.org/golang.org/x/net/context

https://blog.golang.org/context

[dougm]

We can start with the vsphere/session package

[cgtexmex]

To provide a generic timeout I'm thinking of injecting one in the /vim25/soap.RoundTrip function. I believe this is the common entry point from the caller (govmomi) to the server. I would check to see if the passed context has a time out -- or a timeout that doesn't exceed a certain threshold. If the provided context doesn't then I would create a child context with a specified default timeout and pass that to the /vim25/soap.do function.

Then we would enforce a basic timeout, but easily allow users to specify their own. Thoughts? Am I missing something?

cc:/ @caglar10ur

[dougm]

Don't think we want to inject a default timeout into the vim25/soap code, can't we do that at the vsphere/session package level?

[cgtexmex]

The injection at the soap layer would enforce a timeout for every call to VC or ESX. If the timeout is exceeded then the soap layer would cancel the request on VC / ESX and return an error. This would only inject if a timeout wasn't provided.

At the session package level are you talking about the /session/keep_alive.RoundTrip ? So do something similar there? i.e. inject a default if one isn't provided? Or am I missing something?

[dougm]

By vsphere/session level, I mean https://github.com/vmware/vic/tree/master/pkg/vsphere/session

[cgtexmex]

Ahhhhh....I was completely focused of implementing the timeouts in govmomi proper, so that all projects utilizing would benefit. I'll look at the vic session stuff and implement there.

Why wouldn't we want sane timeouts in the core govmomi code?

[dougm]

Leave it to the caller to decide. We should have a timeout option in govc with a reasonable default, but that'd be another story.

[cgtexmex]

Is this what you're thinking for injecting a timeout? Look at the create method:

https://github.com/cgtexmex/vic/blob/context/pkg/vsphere/session/session.go

This will create a new child context with a timeout if no deadline exist. If this is what you're thinking then I'll wrap it in a function that will take the parent context as a param.

[dougm]

Looks like the context package will be included with Go 1.7: golang/go#14660

[mdubya66]

low for now.

[dougm]

We are using Context timeouts/deadlines now where appropriate.