connection: add tests

[pohly]

This verifies that both unix:/// and absolute paths work and that the
timing behavior is as expected (wait for server, react promptly once
it appears).

[pohly]

There's one more test that might be useful: ensuring that gRPC does not attempt to reconnect after loosing the connection. I'll have a look at that now.

[jsafrane]

I am not sure what this test tries to do. If server is gone, gRPC should return codes.Unavailable quickly. That's correct. But it will try to reconnect if you re-start the server. So No reconnection either comment is quite confusing.

[pohly]

Jan Šafránek <notifications@github.com> writes:

jsafrane commented on this pull request. + // No reconnection either. I am not sure what this test tries to do. If server is gone, gRPC should return `codes.Unavailable` quickly. That's correct. But it will try to reconnect if you re-start the server. So `No reconnection either` comment is quite confusing.

Is was trying to rule out that a second method call somehow causes gRPC to do something - not sure what exactly, though. You are right, a much better test is to restart the server. We don't want to reconnect to the server under any condition, do we? If gRPC does it under the hood, then we won't notice that cached information became stale. The test above does not cover that because the client gets a "Unavailable" once. A better test is to have no call pending, restart the server and then do a method call. This should still result in an "Unavailable" error.

[jsafrane]

We need to be very careful here. Is Unavailable returned in any case other than driver restart? We should not restart provisioner unless it's absolutely necessary, because it may leak volumes in case user deleted corresponding PVC.

Digging into code, there are few places when Unavailable is returned for not closed connection.

csi-lib-utils/vendor/google.golang.org/grpc/internal/transport/http_util.go

Lines 90 to 91 in 179d6f9

	// 429 Too Many Requests - UNAVAILABLE.
	http.StatusTooManyRequests: codes.Unavailable,

csi-lib-utils/vendor/google.golang.org/grpc/internal/transport/http_util.go

Lines 90 to 97 in 179d6f9

	// 429 Too Many Requests - UNAVAILABLE.
	http.StatusTooManyRequests: codes.Unavailable,
	// 502 Bad Gateway - UNAVAILABLE.
	http.StatusBadGateway: codes.Unavailable,
	// 503 Service Unavailable - UNAVAILABLE.
	http.StatusServiceUnavailable: codes.Unavailable,
	// 504 Gateway timeout - UNAVAILABLE.
	http.StatusGatewayTimeout: codes.Unavailable,

csi-lib-utils/vendor/google.golang.org/grpc/internal/transport/transport.go

Lines 664 to 673 in 179d6f9

	// errStreamDrain indicates that the stream is rejected because the
	// connection is draining. This could be caused by goaway or balancer
	// removing the address.
	errStreamDrain = status.Error(codes.Unavailable, "the connection is draining")
	// errStreamDone is returned from write at the client side to indiacte application
	// layer of an error.
	errStreamDone = errors.New("the stream is done")
	// StatusGoAway indicates that the server sent a GOAWAY that included this
	// stream's ID in unprocessed RPCs.
	statusGoAway = status.New(codes.Unavailable, "the stream is rejected because server is draining the connection")

[pohly]

@jsafrane I've implemented the approach with an explicit "connection lost" callback in this PR. Shall we merge that into your branch and from there into the upstream repo or shall I create a new PR?