Overall, I'm in favor of it, and I like that it's parameterized, but I'd like to see that pods do eventually come up in a situation where there's more than the concurrency limit, I believe those pods would crash loop?

I think maybe we should have some testing for it, best case scenario: e2e tests that both capture the error and the success.

Tomo also suggested taking a look at the limiters on https://pkg.go.dev/golang.org/x/net/netutil

Overall, I'm in favor of it, and I like that it's parameterized, but I'd like to see that pods do eventually come up in a situation where there's more than the concurrency limit, I believe those pods would crash loop?

I think maybe we should have some testing for it, best case scenario: e2e tests that both capture the error and the success.

These pods would wait as the semaphore blocks, until the shim timeouts the requests. In this case the pod will fail and kubelet try again in the future. Note that the overall CRI timeout is 4 minutes, we could adjust the shims timeout to match that.

So, we should decide if we want to

• fail the HTTP request if concurrency is too high, retry from shim?
• or block while concurrency limit is exceeded ?
• block using semaphore on listener level (LimitListener)?
• or block inside the handler (this PR)?

@dougbtv fixed unit test & rebased. Implementation using the LimitListener you suggested is here: #1356