Unify gRPC flags for all servers (#6187)

We currently have two methods for registering gRPC server flags and they
both have slight inconsistencies.

This commit unifies gRPC flag registration and removes one of the two
redundant methods.

Signed-off-by: Filip Petkovski <[email protected]>

cmd/thanos/config.go

@@ -13,24 +13,23 @@ import (
 	extflag "github.com/efficientgo/tools/extkingpin"
 
 	"github.com/prometheus/common/model"
+
 	"github.com/thanos-io/thanos/pkg/extkingpin"
 )
 
 type grpcConfig struct {
-	bindAddress    string
-	gracePeriod    model.Duration
-	tlsSrvCert     string
-	tlsSrvKey      string
-	tlsSrvClientCA string
+	bindAddress      string
+	tlsSrvCert       string
+	tlsSrvKey        string
+	tlsSrvClientCA   string
+	gracePeriod      time.Duration
+	maxConnectionAge time.Duration
 }
 
 func (gc *grpcConfig) registerFlag(cmd extkingpin.FlagClause) *grpcConfig {
 	cmd.Flag("grpc-address",
 		"Listen ip:port address for gRPC endpoints (StoreAPI). Make sure this address is routable from other components.").
 		Default("0.0.0.0:10901").StringVar(&gc.bindAddress)
-	cmd.Flag("grpc-grace-period",
-		"Time to wait after an interrupt received for GRPC Server.").
-		Default("2m").SetValue(&gc.gracePeriod)
 	cmd.Flag("grpc-server-tls-cert",
 		"TLS Certificate for gRPC server, leave blank to disable TLS").
 		Default("").StringVar(&gc.tlsSrvCert)
@@ -40,6 +39,12 @@ func (gc *grpcConfig) registerFlag(cmd extkingpin.FlagClause) *grpcConfig {
 	cmd.Flag("grpc-server-tls-client-ca",
 		"TLS CA to verify clients against. If no client CA is specified, there is no client verification on server side. (tls.NoClientCert)").
 		Default("").StringVar(&gc.tlsSrvClientCA)
+	cmd.Flag("grpc-server-max-connection-age", "The grpc server max connection age. This controls how often to re-establish connections and redo TLS handshakes.").
+		Default("0s").DurationVar(&gc.maxConnectionAge)
+	cmd.Flag("grpc-grace-period",
+		"Time to wait after an interrupt received for GRPC Server.").
+		Default("2m").DurationVar(&gc.gracePeriod)
+
 	return gc
 }
 

cmd/thanos/query.go

@@ -86,7 +86,9 @@ func registerQuery(app *extkingpin.App) {
 	cmd := app.Command(comp.String(), "Query node exposing PromQL enabled Query API with data retrieved from multiple store nodes.")
 
 	httpBindAddr, httpGracePeriod, httpTLSConfig := extkingpin.RegisterHTTPFlags(cmd)
-	grpcBindAddr, grpcGracePeriod, grpcCert, grpcKey, grpcClientCA, grpcMaxConnAge := extkingpin.RegisterGRPCFlags(cmd)
+
+	var grpcServerConfig grpcConfig
+	grpcServerConfig.registerFlag(cmd)
 
 	secure := cmd.Flag("grpc-client-tls-secure", "Use TLS when talking to the gRPC server").Default("false").Bool()
 	skipVerify := cmd.Flag("grpc-client-tls-skip-verify", "Disable TLS certificate verification i.e self signed, signed by fake CA").Default("false").Bool()
@@ -281,12 +283,7 @@ func registerQuery(app *extkingpin.App) {
 			httpLogOpts,
 			grpcLogOpts,
 			tagOpts,
-			*grpcBindAddr,
-			time.Duration(*grpcGracePeriod),
-			*grpcCert,
-			*grpcKey,
-			*grpcClientCA,
-			*grpcMaxConnAge,
+			grpcServerConfig,
 			*grpcCompression,
 			*secure,
 			*skipVerify,
@@ -361,12 +358,7 @@ func runQuery(
 	httpLogOpts []logging.Option,
 	grpcLogOpts []grpc_logging.Option,
 	tagOpts []tags.Option,
-	grpcBindAddr string,
-	grpcGracePeriod time.Duration,
-	grpcCert string,
-	grpcKey string,
-	grpcClientCA string,
-	grpcMaxConnAge time.Duration,
+	grpcServerConfig grpcConfig,
 	grpcCompression string,
 	secure bool,
 	skipVerify bool,
@@ -784,7 +776,7 @@ func runQuery(
 	}
 	// Start query (proxy) gRPC StoreAPI.
 	{
-		tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), grpcCert, grpcKey, grpcClientCA)
+		tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), grpcServerConfig.tlsSrvCert, grpcServerConfig.tlsSrvKey, grpcServerConfig.tlsSrvClientCA)
 		if err != nil {
 			return errors.Wrap(err, "setup gRPC server")
 		}
@@ -822,10 +814,10 @@ func runQuery(
 			grpcserver.WithServer(metadata.RegisterMetadataServer(metadataProxy)),
 			grpcserver.WithServer(exemplars.RegisterExemplarsServer(exemplarsProxy)),
 			grpcserver.WithServer(info.RegisterInfoServer(infoSrv)),
-			grpcserver.WithListen(grpcBindAddr),
-			grpcserver.WithGracePeriod(grpcGracePeriod),
+			grpcserver.WithListen(grpcServerConfig.bindAddress),
+			grpcserver.WithGracePeriod(grpcServerConfig.gracePeriod),
+			grpcserver.WithMaxConnAge(grpcServerConfig.maxConnectionAge),
 			grpcserver.WithTLSConfig(tlsCfg),
-			grpcserver.WithMaxConnAge(grpcMaxConnAge),
 		)
 
 		g.Add(func() error {

cmd/thanos/receive.go

@@ -137,8 +137,8 @@ func runReceive(
 		logger,
 		reg,
 		tracer,
-		*conf.grpcCert != "",
-		*conf.grpcClientCA == "",
+		conf.grpcConfig.tlsSrvCert != "",
+		conf.grpcConfig.tlsSrvClientCA == "",
 		conf.rwClientCert,
 		conf.rwClientKey,
 		conf.rwClientServerCA,
@@ -300,7 +300,7 @@ func runReceive(
 
 	level.Debug(logger).Log("msg", "setting up gRPC server")
 	{
-		tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), *conf.grpcCert, *conf.grpcKey, *conf.grpcClientCA)
+		tlsCfg, err := tls.NewServerConfig(log.With(logger, "protocol", "gRPC"), conf.grpcConfig.tlsSrvCert, conf.grpcConfig.tlsSrvKey, conf.grpcConfig.tlsSrvClientCA)
 		if err != nil {
 			return errors.Wrap(err, "setup gRPC server")
 		}
@@ -343,15 +343,15 @@ func runReceive(
 			grpcserver.WithServer(store.RegisterWritableStoreServer(rw)),
 			grpcserver.WithServer(exemplars.RegisterExemplarsServer(exemplars.NewMultiTSDB(dbs.TSDBExemplars))),
 			grpcserver.WithServer(info.RegisterInfoServer(infoSrv)),
-			grpcserver.WithListen(*conf.grpcBindAddr),
-			grpcserver.WithGracePeriod(time.Duration(*conf.grpcGracePeriod)),
+			grpcserver.WithListen(conf.grpcConfig.bindAddress),
+			grpcserver.WithGracePeriod(conf.grpcConfig.gracePeriod),
+			grpcserver.WithMaxConnAge(conf.grpcConfig.maxConnectionAge),
 			grpcserver.WithTLSConfig(tlsCfg),
-			grpcserver.WithMaxConnAge(*conf.grpcMaxConnAge),
 		)
 
 		g.Add(
 			func() error {
-				level.Info(logger).Log("msg", "listening for StoreAPI and WritableStoreAPI gRPC", "address", *conf.grpcBindAddr)
+				level.Info(logger).Log("msg", "listening for StoreAPI and WritableStoreAPI gRPC", "address", conf.grpcConfig.bindAddress)
 				statusProber.Healthy()
 				return srv.ListenAndServe()
 			},
@@ -740,12 +740,7 @@ type receiveConfig struct {
 	httpGracePeriod *model.Duration
 	httpTLSConfig   *string
 
-	grpcBindAddr    *string
-	grpcGracePeriod *model.Duration
-	grpcCert        *string
-	grpcKey         *string
-	grpcClientCA    *string
-	grpcMaxConnAge  *time.Duration
+	grpcConfig grpcConfig
 
 	rwAddress          string
 	rwServerCert       string
@@ -805,7 +800,7 @@ type receiveConfig struct {
 
 func (rc *receiveConfig) registerFlag(cmd extkingpin.FlagClause) {
 	rc.httpBindAddr, rc.httpGracePeriod, rc.httpTLSConfig = extkingpin.RegisterHTTPFlags(cmd)
-	rc.grpcBindAddr, rc.grpcGracePeriod, rc.grpcCert, rc.grpcKey, rc.grpcClientCA, rc.grpcMaxConnAge = extkingpin.RegisterGRPCFlags(cmd)
+	rc.grpcConfig.registerFlag(cmd)
 	rc.storeRateLimits.RegisterFlags(cmd)
 
 	cmd.Flag("remote-write.address", "Address to listen on for remote write requests.").

cmd/thanos/rule.go

@@ -612,7 +612,8 @@ func runRule(
 	options := []grpcserver.Option{
 		grpcserver.WithServer(thanosrules.RegisterRulesServer(ruleMgr)),
 		grpcserver.WithListen(conf.grpc.bindAddress),
-		grpcserver.WithGracePeriod(time.Duration(conf.grpc.gracePeriod)),
+		grpcserver.WithGracePeriod(conf.grpc.gracePeriod),
+		grpcserver.WithGracePeriod(conf.grpc.maxConnectionAge),
 		grpcserver.WithTLSConfig(tlsCfg),
 	}
 	infoOptions := []info.ServerOptionFunc{info.WithRulesInfoFunc()}

cmd/thanos/sidecar.go

@@ -291,7 +291,8 @@ func runSidecar(
 			grpcserver.WithServer(exemplars.RegisterExemplarsServer(exemplarSrv)),
 			grpcserver.WithServer(info.RegisterInfoServer(infoSrv)),
 			grpcserver.WithListen(conf.grpc.bindAddress),
-			grpcserver.WithGracePeriod(time.Duration(conf.grpc.gracePeriod)),
+			grpcserver.WithGracePeriod(conf.grpc.gracePeriod),
+			grpcserver.WithMaxConnAge(conf.grpc.maxConnectionAge),
 			grpcserver.WithTLSConfig(tlsCfg),
 		)
 		g.Add(func() error {

cmd/thanos/store.go

@@ -456,7 +456,8 @@ func runStore(
 			grpcserver.WithServer(store.RegisterStoreServer(storeServer, logger)),
 			grpcserver.WithServer(info.RegisterInfoServer(infoSrv)),
 			grpcserver.WithListen(conf.grpcConfig.bindAddress),
-			grpcserver.WithGracePeriod(time.Duration(conf.grpcConfig.gracePeriod)),
+			grpcserver.WithGracePeriod(conf.grpcConfig.gracePeriod),
+			grpcserver.WithMaxConnAge(conf.grpcConfig.maxConnectionAge),
 			grpcserver.WithTLSConfig(tlsCfg),
 		)
 

docs/components/query.md

@@ -317,10 +317,10 @@ Flags:
                                  to other clients. Must be one of: snappy, none
       --grpc-grace-period=2m     Time to wait after an interrupt received for
                                  GRPC Server.
-      --grpc-server-max-connection-age=60m
-                                 The grpc server max connection age.
-                                 This controls how often to re-read the tls
-                                 certificates and redo the TLS handshake
+      --grpc-server-max-connection-age=0s
+                                 The grpc server max connection age. This
+                                 controls how often to re-establish connections
+                                 and redo TLS handshakes.
       --grpc-server-tls-cert=""  TLS Certificate for gRPC server, leave blank to
                                  disable TLS
       --grpc-server-tls-client-ca=""

docs/components/receive.md

@@ -221,10 +221,10 @@ Flags:
                                  from other components.
       --grpc-grace-period=2m     Time to wait after an interrupt received for
                                  GRPC Server.
-      --grpc-server-max-connection-age=60m
-                                 The grpc server max connection age.
-                                 This controls how often to re-read the tls
-                                 certificates and redo the TLS handshake
+      --grpc-server-max-connection-age=0s
+                                 The grpc server max connection age. This
+                                 controls how often to re-establish connections
+                                 and redo TLS handshakes.
       --grpc-server-tls-cert=""  TLS Certificate for gRPC server, leave blank to
                                  disable TLS
       --grpc-server-tls-client-ca=""

docs/components/rule.md

@@ -323,6 +323,10 @@ Flags:
                                  from other components.
       --grpc-grace-period=2m     Time to wait after an interrupt received for
                                  GRPC Server.
+      --grpc-server-max-connection-age=0s
+                                 The grpc server max connection age. This
+                                 controls how often to re-establish connections
+                                 and redo TLS handshakes.
       --grpc-server-tls-cert=""  TLS Certificate for gRPC server, leave blank to
                                  disable TLS
       --grpc-server-tls-client-ca=""

docs/components/sidecar.md

@@ -82,6 +82,10 @@ Flags:
                                  from other components.
       --grpc-grace-period=2m     Time to wait after an interrupt received for
                                  GRPC Server.
+      --grpc-server-max-connection-age=0s
+                                 The grpc server max connection age. This
+                                 controls how often to re-establish connections
+                                 and redo TLS handshakes.
       --grpc-server-tls-cert=""  TLS Certificate for gRPC server, leave blank to
                                  disable TLS
       --grpc-server-tls-client-ca=""

docs/components/store.md

@@ -66,6 +66,10 @@ Flags:
                                  from other components.
       --grpc-grace-period=2m     Time to wait after an interrupt received for
                                  GRPC Server.
+      --grpc-server-max-connection-age=0s
+                                 The grpc server max connection age. This
+                                 controls how often to re-establish connections
+                                 and redo TLS handshakes.
       --grpc-server-tls-cert=""  TLS Certificate for gRPC server, leave blank to
                                  disable TLS
       --grpc-server-tls-client-ca=""

pkg/extkingpin/flags.go

@@ -6,7 +6,6 @@ package extkingpin
 import (
 	"fmt"
 	"strings"
-	"time"
 
 	extflag "github.com/efficientgo/tools/extkingpin"
 	"github.com/pkg/errors"
@@ -73,32 +72,6 @@ func validateAddrs(addrs addressSlice) error {
 	return nil
 }
 
-// RegisterGRPCFlags registers flags commonly used to configure gRPC servers with.
-func RegisterGRPCFlags(cmd FlagClause) (
-	grpcBindAddr *string,
-	grpcGracePeriod *model.Duration,
-	grpcTLSSrvCert *string,
-	grpcTLSSrvKey *string,
-	grpcTLSSrvClientCA *string,
-	grpcMaxConnectionAge *time.Duration,
-) {
-	grpcBindAddr = cmd.Flag("grpc-address", "Listen ip:port address for gRPC endpoints (StoreAPI). Make sure this address is routable from other components.").
-		Default("0.0.0.0:10901").String()
-	grpcGracePeriod = ModelDuration(cmd.Flag("grpc-grace-period", "Time to wait after an interrupt received for GRPC Server.").Default("2m")) // by default it's the same as query.timeout.
-
-	grpcTLSSrvCert = cmd.Flag("grpc-server-tls-cert", "TLS Certificate for gRPC server, leave blank to disable TLS").Default("").String()
-	grpcTLSSrvKey = cmd.Flag("grpc-server-tls-key", "TLS Key for the gRPC server, leave blank to disable TLS").Default("").String()
-	grpcTLSSrvClientCA = cmd.Flag("grpc-server-tls-client-ca", "TLS CA to verify clients against. If no client CA is specified, there is no client verification on server side. (tls.NoClientCert)").Default("").String()
-	grpcMaxConnectionAge = cmd.Flag("grpc-server-max-connection-age", "The grpc server max connection age. This controls how often to re-read the tls certificates and redo the TLS handshake ").Default("60m").Duration()
-
-	return grpcBindAddr,
-		grpcGracePeriod,
-		grpcTLSSrvCert,
-		grpcTLSSrvKey,
-		grpcTLSSrvClientCA,
-		grpcMaxConnectionAge
-}
-
 // RegisterCommonObjStoreFlags register flags commonly used to configure http servers with.
 func RegisterHTTPFlags(cmd FlagClause) (httpBindAddr *string, httpGracePeriod *model.Duration, httpTLSConfig *string) {
 	httpBindAddr = cmd.Flag("http-address", "Listen host:port for HTTP endpoints.").Default("0.0.0.0:10902").String()