Description:
There is an early return in ConnPoolImplBase::checkForDrained that prevents closeIdleConnections from being called in the case where there are no drained callbacks registered for the pool:

void ConnPoolImplBase::checkForDrained() {
  if (drained_callbacks_.empty()) {
    return;
  }

  closeIdleConnections();

In TcpTunnelingIntegrationTest.InvalidResponseHeaders inside test/integration/tcp_tunneling_integration_test.cc, the behaviour expected by the test changes based on the presence of a callback.

In this InvalidResponseHeaders test, an invalid response header is sent from the upstream fake server. The test expects that Envoy will send an HTTP/2 reset frame, but if the connection pool has a drained callback, the connection closes at the transport layer instead.

The relevant sequence is as follows:

1. The server sends a non-200 response code
2. Inside TcpProxy::HttpUpstream::DecoderShim::decodeHeaders, TcpProxy::HttpUpstream::resetEncoder is called.
3. After a few more intermediate function calls, Http::CodecClient::deleteRequest is called and the request is removed from the client's active_requests_ list.
4. After this, codec_client_callbacks_->onStreamDestroy() is called, which ends up with a call to Http::Http2::ConnPoolImpl::onStreamDestroy.
5. In onStreamDestroy, ConnPoolImplBase::checkForDrained is called.

In the current code, when checkForDrained is called at step (5), the early exit is hit so no idle connections are affected. The connection is then allowed to send out a HTTP/2 reset frame and the upstream server endpoint closes the connection.

However, if you add a callback to the connection pool, the early exit is skipped and closeIdleConnections is executed. Because the request was removed in step (3) above, the ActiveClient associated with the connection is considered idle and is closed, which in turn closes the connection on the Envoy upstream client side. This prevents the HTTP/2 reset frame from being sent.

I have a few questions:

1. Is it correct that checkForDrained should have different observable behaviour depending on the presence of an idle callback? (I'm guessing 'no')
   ** If we wanted to do this, we could move closeIdleConnections above the early return, but that affects question (2)
2. The Http2::ConnPoolImpl::onStreamDestroy code greedily attempts to reap idle connections (which closes the socket at the transport layer), while the integration test asserts that the HTTP/2 layer should be allowed to sent a reset frame. Which is correct?

This was found while I was testing requested changes for #13061 so if there's a good solution to this problem I might be able to wrap it up in that PR.

Repro steps:
Add a drained callback to the constructor of ConnectionPool::ConnPoolImplBase and run the bazel target //test/integration:tcp_tunneling_integration_test.

Logs:
The test log after the repro steps above (with -l trace) invalid_response_headers.log