Allow configuring request timeout per plugin

[CoderSherlock]

Support per-plugin request timeout in NRI while maintaining global timeout as the default fallback.

• Add request_timeout field (in milliseconds) to ConfigureResponse protobuf message.
• Update adaptation layer to store per-plugin request timeout on configuration response and use it across plugin request invocations.
• Add WithRequestTimeout option and SetRequestTimeout method to plugin stub interface to allow plugins to specify a custom timeout.
• Add unit test verifying per-plugin request timeout configuration.

Fixes #287

[CoderSherlock]

/reviewer @samuelkarp @mikebrow

[mikebrow]

wave..see comment #287 (comment)

[mikebrow]

@klihub let's revisit this ..

[mikebrow]

maybe pass the context with cancel timeout values we get over grpc from the kubelet client (etal) to NRI … set timeout values for each call type for each current plugin, based on a plugin config and some defaults.  Let NRI manage (validate) ingress of plugins based on the sum of requests being less than something reasonable in view of the client's timeout

[mikebrow]

maybe create the new context timeout used for each call over the NRI adaptation based on the time remaining on the client context received from the CRI client.. which could make it more dynamic...

[klihub]

@CoderSherlock Thank you for this PR. @CoderSherlock @mikebrow I think I understand the problems this PR is aimed to solve and I think it is a valid problem. The part I'm a bit vary about regarding the approach taken here is that it brings in a (rather big) semantic divergence from how we've treated configuration (or related administrative control) so far.

In the past and still to date, admin-controlled NRI configuration (NRI related settings in the runtime configuration, the build in validator, and pluggable validation) have provided cluster admins a way to set up boundary conditions within which NRI plugins must stay. If some plugin does not stay within these boundaries, the runtime take corrective actions (depending on the situation, rejecting actions request by the plugin (failed validation), or even kicking out the plugin from the runtime (failure respond within a timeout).

These have not been so far advisory restrictions but strictly enforced ones. AFAICT, this PR now changes that, basically diminishing the admin's setting to a mere wish, or an advice (since it is communicated to the plugin during the initial handshake phase). If a plugin configures itself with a different timeout, it does not matter what the cluster admin has configured. The plugins always wins and can override the admin's settings.

I'm not claiming that we definitely should not do this. But if we go ahead and merge this, the above mentioned divergence from the status quo/consequences should come as a conscious decision and not as a side-effect/something which just happened to slip in. So I would like to hear what others (@mikebrow @samuelkarp @kad @askervin) think about this. And if this is considered iffy, what alternatives we might have.

One alternative that comes to mind, which would also be a fairly small change to this PR, is that we could instead treat this as a (worst case) boundary condition reported by the plugin, check it against the one configured by the admin, and prevent/abort registration/startup of the plugin on the runtime side if it exceeds the timeout set by the admin.

Effectively this same behavior would be also possible to implement on the plugin/stub side, since the admin configured timeouts are communicated to the plugin, so the stub could fail with an error if the plugin would like to have a larger timeout that what is allowed by the admin. But doing it on the runtime side would have the advantage of ensuring identical behavior across multiple plugin/stub implementations (golang and rust).

[mikebrow]

One alternative that comes to mind, which would also be a fairly small change to this PR, is that we could instead treat this as a (worst case) boundary condition reported by the plugin, check it against the one configured by the admin, and prevent/abort registration/startup of the plugin on the runtime side if it exceeds the timeout set by the admin.

Effectively this same behavior would be also possible to implement on the plugin/stub side, since the admin configured timeouts are communicated to the plugin, so the stub could fail with an error if the plugin would like to have a larger timeout that what is allowed by the admin. But doing it on the runtime side would have the advantage of ensuring identical behavior across multiple plugin/stub implementations (golang and rust).

this ^ is also what I was thinking in the prior comment #304 (comment) so maybe on load do reject based on the pattern you mentioned then on the adaptation invokes pass in the context timeout remaining to inform the plugin just how much time they have in ctx before the response is cancelled.

Course this needs to be considered in the context of some soln for must include dependencies/ordering.