From bac73a0368dd308fe9f3f3b1b9b04fe0a66ad6be Mon Sep 17 00:00:00 2001 From: Filip Petkovski Date: Mon, 6 Mar 2023 08:30:07 +0100 Subject: [PATCH] Unify gRPC flags for all servers 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 --- cmd/thanos/config.go | 21 +++++++++++++-------- cmd/thanos/query.go | 26 +++++++++----------------- cmd/thanos/receive.go | 23 +++++++++-------------- cmd/thanos/rule.go | 3 ++- cmd/thanos/sidecar.go | 3 ++- cmd/thanos/store.go | 3 ++- docs/components/query.md | 8 ++++---- docs/components/receive.md | 8 ++++---- docs/components/rule.md | 4 ++++ docs/components/sidecar.md | 4 ++++ docs/components/store.md | 4 ++++ pkg/extkingpin/flags.go | 27 --------------------------- 12 files changed, 57 insertions(+), 77 deletions(-) diff --git a/cmd/thanos/config.go b/cmd/thanos/config.go index f6280dd1bd..5e054b4a9f 100644 --- a/cmd/thanos/config.go +++ b/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 } diff --git a/cmd/thanos/query.go b/cmd/thanos/query.go index 47bedf234a..5275fef55d 100644 --- a/cmd/thanos/query.go +++ b/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 { diff --git a/cmd/thanos/receive.go b/cmd/thanos/receive.go index 055f143f2d..3c3b4da214 100644 --- a/cmd/thanos/receive.go +++ b/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."). diff --git a/cmd/thanos/rule.go b/cmd/thanos/rule.go index 9e05ffa42e..6edf95416c 100644 --- a/cmd/thanos/rule.go +++ b/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()} diff --git a/cmd/thanos/sidecar.go b/cmd/thanos/sidecar.go index 1f629f1af7..8fd399232e 100644 --- a/cmd/thanos/sidecar.go +++ b/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 { diff --git a/cmd/thanos/store.go b/cmd/thanos/store.go index 81aadcd72d..cb6659b921 100644 --- a/cmd/thanos/store.go +++ b/cmd/thanos/store.go @@ -453,7 +453,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), ) diff --git a/docs/components/query.md b/docs/components/query.md index e68a8f68a1..27f9edc7fb 100644 --- a/docs/components/query.md +++ b/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="" diff --git a/docs/components/receive.md b/docs/components/receive.md index fec5822ea0..8a081a9d47 100644 --- a/docs/components/receive.md +++ b/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="" diff --git a/docs/components/rule.md b/docs/components/rule.md index 3736d6bb2c..a871a6880a 100644 --- a/docs/components/rule.md +++ b/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="" diff --git a/docs/components/sidecar.md b/docs/components/sidecar.md index 3e265c9db8..7ac113536a 100644 --- a/docs/components/sidecar.md +++ b/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="" diff --git a/docs/components/store.md b/docs/components/store.md index 015d66e898..02deabfd51 100644 --- a/docs/components/store.md +++ b/docs/components/store.md @@ -63,6 +63,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="" diff --git a/pkg/extkingpin/flags.go b/pkg/extkingpin/flags.go index 59aaccf8b8..cee8c5df76 100644 --- a/pkg/extkingpin/flags.go +++ b/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()