sanity: optional gRPC keepalive

The gRPC defaults for a client do not work for a gRPC server that also
uses the defaults (grpc/grpc-go#3171) which
caused connection drops when a CSI driver is slow to respond and the
client starts sending keepalive pings too quickly
(#238).

The sanity package no longer uses gRPC keepalive. Instead, custom test
suites can enable that via some new configuration options if needed
and when it is certain that the CSI driver under test can handle it.

Author: pohly

driver/mock.go

@@ -75,7 +75,7 @@ func (m *MockCSIDriver) Nexus() (*grpc.ClientConn, error) {
 	}
 
 	// Create a client connection
-	m.conn, err = utils.Connect(m.Address())
+	m.conn, err = utils.Connect(m.Address(), grpc.WithInsecure())
 	if err != nil {
 		return nil, err
 	}

pkg/sanity/sanity.go

@@ -71,10 +71,19 @@ type TestConfig struct {
 	// is empty, it must provide both the controller and node service.
 	Address string
 
+	// DialOptions specifies the options that are to be used
+	// when connecting to Address. The default is grpc.WithInsecure().
+	// A dialer will be added for Unix Domain Sockets.
+	DialOptions []grpc.DialOption
+
 	// ControllerAddress optionally provides the gRPC endpoint of
 	// the controller service.
 	ControllerAddress string
 
+	// ControllerDialOptions specifies the options that are to be used
+	// for ControllerAddress.
+	ControllerDialOptions []grpc.DialOption
+
 	// SecretsFile is the filename of a .yaml file which is used
 	// to populate CSISecrets which are then used for calls to the
 	// CSI driver.
@@ -175,6 +184,9 @@ func NewTestConfig() TestConfig {
 		RemovePathCmdTimeout: 10 * time.Second,
 		TestVolumeSize:       10 * 1024 * 1024 * 1024, // 10 GiB
 		IDGen:                &DefaultIDGenerator{},
+
+		DialOptions:           []grpc.DialOption{grpc.WithInsecure()},
+		ControllerDialOptions: []grpc.DialOption{grpc.WithInsecure()},
 	}
 }
 
@@ -240,7 +252,7 @@ func (sc *TestContext) Setup() {
 			sc.Conn.Close()
 		}
 		By("connecting to CSI driver")
-		sc.Conn, err = utils.Connect(sc.Config.Address)
+		sc.Conn, err = utils.Connect(sc.Config.Address, sc.Config.DialOptions...)
 		Expect(err).NotTo(HaveOccurred())
 		sc.connAddress = sc.Config.Address
 	} else {
@@ -253,7 +265,7 @@ func (sc *TestContext) Setup() {
 			sc.ControllerConn = sc.Conn
 			sc.controllerConnAddress = sc.Config.Address
 		} else {
-			sc.ControllerConn, err = utils.Connect(sc.Config.ControllerAddress)
+			sc.ControllerConn, err = utils.Connect(sc.Config.ControllerAddress, sc.Config.ControllerDialOptions...)
 			Expect(err).NotTo(HaveOccurred())
 			sc.controllerConnAddress = sc.Config.ControllerAddress
 		}

test/driver_test.go

@@ -106,7 +106,7 @@ func TestSimpleDriver(t *testing.T) {
 	defer s.Stop()
 
 	// Setup a connection to the driver
-	conn, err := utils.Connect(s.Address())
+	conn, err := utils.Connect(s.Address(), grpc.WithInsecure())
 	if err != nil {
 		t.Errorf("Error: %s", err.Error())
 	}

utils/grpcutil.go

@@ -25,14 +25,10 @@ import (
 
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/connectivity"
-	"google.golang.org/grpc/keepalive"
 )
 
 // Connect address by grpc
-func Connect(address string) (*grpc.ClientConn, error) {
-	dialOptions := []grpc.DialOption{
-		grpc.WithInsecure(),
-	}
+func Connect(address string, dialOptions ...grpc.DialOption) (*grpc.ClientConn, error) {
 	u, err := url.Parse(address)
 	if err == nil && (!u.IsAbs() || u.Scheme == "unix") {
 		dialOptions = append(dialOptions,
@@ -41,11 +37,6 @@ func Connect(address string) (*grpc.ClientConn, error) {
 					return net.DialTimeout("unix", u.Path, timeout)
 				}))
 	}
-	// This is necessary when connecting via TCP and does not hurt
-	// when using Unix domain sockets. It ensures that gRPC detects a dead connection
-	// in a timely manner.
-	dialOptions = append(dialOptions,
-		grpc.WithKeepaliveParams(keepalive.ClientParameters{PermitWithoutStream: true}))
 
 	conn, err := grpc.Dial(address, dialOptions...)
 	if err != nil {