Revert #19676, we decided this was unnecessary. (#20826) (#20833)

changelog/20826.txt

@@ -0,0 +1,3 @@
+```release-note:change
+core: Revert #19676 (VAULT_GRPC_MIN_CONNECT_TIMEOUT env var) as we decided it was unnecessary.
+```

vault/cluster.go

@@ -327,8 +327,7 @@ func (c *Core) startClusterListener(ctx context.Context) error {
 	c.clusterListener.Store(cluster.NewListener(networkLayer,
 		c.clusterCipherSuites,
 		listenerLogger,
-		5*c.clusterHeartbeatInterval,
-		c.grpcMinConnectTimeout))
+		5*c.clusterHeartbeatInterval))
 
 	c.AddLogger(listenerLogger)
 

vault/cluster/cluster.go

@@ -72,10 +72,9 @@ type Listener struct {
 	logger                    log.Logger
 	l                         sync.RWMutex
 	tlsConnectionLoggingLevel log.Level
-	grpcMinConnectTimeout     time.Duration
 }
 
-func NewListener(networkLayer NetworkLayer, cipherSuites []uint16, logger log.Logger, idleTimeout, grpcMinConnectTimeout time.Duration) *Listener {
+func NewListener(networkLayer NetworkLayer, cipherSuites []uint16, logger log.Logger, idleTimeout time.Duration) *Listener {
 	var maxStreams uint32 = math.MaxUint32
 	if override := os.Getenv("VAULT_GRPC_MAX_STREAMS"); override != "" {
 		i, err := strconv.ParseUint(override, 10, 32)
@@ -112,7 +111,6 @@ func NewListener(networkLayer NetworkLayer, cipherSuites []uint16, logger log.Lo
 		cipherSuites:              cipherSuites,
 		logger:                    logger,
 		tlsConnectionLoggingLevel: log.LevelFromString(os.Getenv("VAULT_CLUSTER_TLS_SESSION_LOG_LEVEL")),
-		grpcMinConnectTimeout:     grpcMinConnectTimeout,
 	}
 }
 
@@ -463,21 +461,10 @@ func (cl *Listener) GetDialerFunc(ctx context.Context, alpn string) func(string,
 		}
 
 		tlsConfig.NextProtos = []string{alpn}
-		args := []interface{}{
-			"address", addr,
-			"alpn", alpn,
-			"host", tlsConfig.ServerName,
-			"timeout", fmt.Sprintf("%s", timeout),
-		}
-		if cl.grpcMinConnectTimeout != 0 {
-			args = append(args, "timeout_env_override", fmt.Sprintf("%s", cl.grpcMinConnectTimeout))
-		}
-		cl.logger.Debug("creating rpc dialer", args...)
+		cl.logger.Debug("creating rpc dialer", "address", addr, "alpn", alpn, "host", tlsConfig.ServerName)
 
-		start := time.Now()
 		conn, err := cl.networkLayer.Dial(addr, timeout, tlsConfig)
 		if err != nil {
-			cl.logger.Debug("dial failure", "address", addr, "alpn", alpn, "host", tlsConfig.ServerName, "duration", fmt.Sprintf("%s", time.Since(start)), "error", err)
 			return nil, err
 		}
 		cl.logTLSSessionStart(conn.RemoteAddr().String(), conn.ConnectionState())

vault/core.go

@@ -682,9 +682,6 @@ type Core struct {
 	// contains absolute paths that we intend to forward (and template) when
 	// we're on a secondary cluster.
 	writeForwardedPaths *pathmanager.PathManager
-
-	// if populated, override the default gRPC min connect timeout (currently 20s in grpc 1.51)
-	grpcMinConnectTimeout time.Duration
 }
 
 // c.stateLock needs to be held in read mode before calling this function.
@@ -1268,16 +1265,6 @@ func NewCore(conf *CoreConfig) (*Core, error) {
 		c.events.Start()
 	}
 
-	minConnectTimeoutRaw := os.Getenv("VAULT_GRPC_MIN_CONNECT_TIMEOUT")
-	if minConnectTimeoutRaw != "" {
-		dur, err := time.ParseDuration(minConnectTimeoutRaw)
-		if err != nil {
-			c.logger.Warn("VAULT_GRPC_MIN_CONNECT_TIMEOUT contains non-duration value, ignoring")
-		} else if dur != 0 {
-			c.grpcMinConnectTimeout = dur
-		}
-	}
-
 	return c, nil
 }
 

vault/request_forwarding.go

@@ -22,7 +22,6 @@ import (
 	"github.com/hashicorp/vault/vault/replication"
 	"golang.org/x/net/http2"
 	"google.golang.org/grpc"
-	"google.golang.org/grpc/backoff"
 	"google.golang.org/grpc/keepalive"
 )
 
@@ -276,8 +275,7 @@ func (c *Core) refreshRequestForwardingConnection(ctx context.Context, clusterAd
 	// ALPN header right. It's just "insecure" because GRPC isn't managing
 	// the TLS state.
 	dctx, cancelFunc := context.WithCancel(ctx)
-
-	opts := []grpc.DialOption{
+	c.rpcClientConn, err = grpc.DialContext(dctx, clusterURL.Host,
 		grpc.WithDialer(clusterListener.GetDialerFunc(ctx, consts.RequestForwardingALPN)),
 		grpc.WithInsecure(), // it's not, we handle it in the dialer
 		grpc.WithKeepaliveParams(keepalive.ClientParameters{
@@ -286,15 +284,7 @@ func (c *Core) refreshRequestForwardingConnection(ctx context.Context, clusterAd
 		grpc.WithDefaultCallOptions(
 			grpc.MaxCallRecvMsgSize(math.MaxInt32),
 			grpc.MaxCallSendMsgSize(math.MaxInt32),
-		),
-	}
-	if c.grpcMinConnectTimeout != 0 {
-		opts = append(opts, grpc.WithConnectParams(grpc.ConnectParams{
-			MinConnectTimeout: c.grpcMinConnectTimeout,
-			Backoff:           backoff.DefaultConfig,
-		}))
-	}
-	c.rpcClientConn, err = grpc.DialContext(dctx, clusterURL.Host, opts...)
+		))
 	if err != nil {
 		cancelFunc()
 		c.logger.Error("err setting up forwarding rpc client", "error", err)