From b7b051f90e4aa5d9684de5d8ad635d1e97785ded Mon Sep 17 00:00:00 2001 From: Bhavika Sharma Date: Tue, 3 Oct 2023 20:22:09 -0700 Subject: [PATCH] Add ENV variables to configure GRPC Keep Alive Time Signed-off-by: Bhavika Sharma --- cmpserver/server.go | 2 +- common/common.go | 23 ++++++++++-- common/common_test.go | 46 ++++++++++++++++++++++++ docs/user-guide/environment-variables.md | 1 + reposerver/server.go | 2 +- server/server.go | 2 +- util/grpc/grpc.go | 2 +- 7 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 common/common_test.go diff --git a/cmpserver/server.go b/cmpserver/server.go index bbb493f6b1d66..1d07e531394d3 100644 --- a/cmpserver/server.go +++ b/cmpserver/server.go @@ -65,7 +65,7 @@ func NewServer(initConstants plugin.CMPServerInitConstants) (*ArgoCDCMPServer, e grpc.MaxSendMsgSize(apiclient.MaxGRPCMessageSize), grpc.KeepaliveEnforcementPolicy( keepalive.EnforcementPolicy{ - MinTime: common.GRPCKeepAliveEnforcementMinimum, + MinTime: common.GetGRPCKeepAliveEnforcementMinimum(), }, ), } diff --git a/common/common.go b/common/common.go index d7c2d24738b58..5faedc2e344c4 100644 --- a/common/common.go +++ b/common/common.go @@ -258,6 +258,8 @@ const ( EnvRedisName = "ARGOCD_REDIS_NAME" // EnvRedisHaProxyName is the name of the Argo CD Redis HA proxy component, as specified by the value under the LabelKeyAppName label key. EnvRedisHaProxyName = "ARGOCD_REDIS_HAPROXY_NAME" + // EnvGRPCKeepAliveMin defines the GRPCKeepAliveEnforcementMinimum, used in the grpc.KeepaliveEnforcementPolicy. Expects a "Duration" format (e.g. 10s). + EnvGRPCKeepAliveMin = "ARGOCD_GRPC_KEEP_ALIVE_MIN" ) // Config Management Plugin related constants @@ -351,11 +353,26 @@ const ( // gRPC settings const ( - GRPCKeepAliveEnforcementMinimum = 10 * time.Second - // GRPCKeepAliveTime is 2x enforcement minimum to ensure network jitter does not introduce ENHANCE_YOUR_CALM errors - GRPCKeepAliveTime = 2 * GRPCKeepAliveEnforcementMinimum + defaultGRPCKeepAliveEnforcementMinimum = 10 * time.Second ) +func GetGRPCKeepAliveEnforcementMinimum() time.Duration { + if GRPCKeepAliveMinStr := os.Getenv(EnvGRPCKeepAliveMin); GRPCKeepAliveMinStr != "" { + GRPCKeepAliveMin, err := time.ParseDuration(GRPCKeepAliveMinStr) + if err != nil { + logrus.Warnf("invalid env var value for %s: cannot parse: %s. Default value %s will be used.", EnvGRPCKeepAliveMin, err, defaultGRPCKeepAliveEnforcementMinimum) + return defaultGRPCKeepAliveEnforcementMinimum + } + return GRPCKeepAliveMin + } + return defaultGRPCKeepAliveEnforcementMinimum +} + +func GetGRPCKeepAliveTime() time.Duration { + // GRPCKeepAliveTime is 2x enforcement minimum to ensure network jitter does not introduce ENHANCE_YOUR_CALM errors + return 2 * GetGRPCKeepAliveEnforcementMinimum() +} + // Security severity logging const ( SecurityField = "security" diff --git a/common/common_test.go b/common/common_test.go new file mode 100644 index 0000000000000..5632c1e7a78cc --- /dev/null +++ b/common/common_test.go @@ -0,0 +1,46 @@ +package common + +import ( + "fmt" + "os" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +// Test env var not set for EnvGRPCKeepAliveMin +func Test_GRPCKeepAliveMinNotSet(t *testing.T) { + grpcKeepAliveMin := GetGRPCKeepAliveEnforcementMinimum() + grpcKeepAliveExpectedMin := defaultGRPCKeepAliveEnforcementMinimum + assert.Equal(t, grpcKeepAliveExpectedMin, grpcKeepAliveMin) + + grpcKeepAliveTime := GetGRPCKeepAliveTime() + assert.Equal(t, 2*grpcKeepAliveExpectedMin, grpcKeepAliveTime) +} + +// Test valid env var set for EnvGRPCKeepAliveMin +func Test_GRPCKeepAliveMinIsSet(t *testing.T) { + numSeconds := 15 + os.Setenv(EnvGRPCKeepAliveMin, fmt.Sprintf("%ds", numSeconds)) + + grpcKeepAliveMin := GetGRPCKeepAliveEnforcementMinimum() + grpcKeepAliveExpectedMin := time.Duration(numSeconds) * time.Second + assert.Equal(t, grpcKeepAliveExpectedMin, grpcKeepAliveMin) + + grpcKeepAliveTime := GetGRPCKeepAliveTime() + assert.Equal(t, 2*grpcKeepAliveExpectedMin, grpcKeepAliveTime) +} + +// Test invalid env var set for EnvGRPCKeepAliveMin +func Test_GRPCKeepAliveMinIncorrectlySet(t *testing.T) { + numSeconds := 15 + os.Setenv(EnvGRPCKeepAliveMin, fmt.Sprintf("%d", numSeconds)) + + grpcKeepAliveMin := GetGRPCKeepAliveEnforcementMinimum() + grpcKeepAliveExpectedMin := defaultGRPCKeepAliveEnforcementMinimum + assert.Equal(t, grpcKeepAliveExpectedMin, grpcKeepAliveMin) + + grpcKeepAliveTime := GetGRPCKeepAliveTime() + assert.Equal(t, 2*grpcKeepAliveExpectedMin, grpcKeepAliveTime) +} diff --git a/docs/user-guide/environment-variables.md b/docs/user-guide/environment-variables.md index 238db85b5c718..7ef3d7296cef8 100644 --- a/docs/user-guide/environment-variables.md +++ b/docs/user-guide/environment-variables.md @@ -12,3 +12,4 @@ The following environment variables can be used with `argocd` CLI: | `ARGOCD_APPLICATION_CONTROLLER_NAME` | the Argo CD Application Controller name (default "argocd-application-controller") | | `ARGOCD_REDIS_NAME` | the Argo CD Redis name (default "argocd-redis") | | `ARGOCD_REDIS_HAPROXY_NAME` | the Argo CD Redis HA Proxy name (default "argocd-redis-ha-haproxy") | +| `ARGOCD_GRPC_KEEP_ALIVE_MIN` | defines the GRPCKeepAliveEnforcementMinimum, used in the grpc.KeepaliveEnforcementPolicy. Expects a "Duration" format (e.g. `10s`). | diff --git a/reposerver/server.go b/reposerver/server.go index 9576604751dfc..007b7136e41ed 100644 --- a/reposerver/server.go +++ b/reposerver/server.go @@ -90,7 +90,7 @@ func NewServer(metricsServer *metrics.MetricsServer, cache *reposervercache.Cach grpc.MaxSendMsgSize(apiclient.MaxGRPCMessageSize), grpc.KeepaliveEnforcementPolicy( keepalive.EnforcementPolicy{ - MinTime: common.GRPCKeepAliveEnforcementMinimum, + MinTime: common.GetGRPCKeepAliveEnforcementMinimum(), }, ), } diff --git a/server/server.go b/server/server.go index a0fc5327e985c..b4225dfdc51b8 100644 --- a/server/server.go +++ b/server/server.go @@ -731,7 +731,7 @@ func (a *ArgoCDServer) newGRPCServer() (*grpc.Server, application.AppResourceTre grpc.ConnectionTimeout(300 * time.Second), grpc.KeepaliveEnforcementPolicy( keepalive.EnforcementPolicy{ - MinTime: common.GRPCKeepAliveEnforcementMinimum, + MinTime: common.GetGRPCKeepAliveEnforcementMinimum(), }, ), } diff --git a/util/grpc/grpc.go b/util/grpc/grpc.go index 323d78398a8ce..1741727a5d604 100644 --- a/util/grpc/grpc.go +++ b/util/grpc/grpc.go @@ -88,7 +88,7 @@ func BlockingDial(ctx context.Context, network, address string, creds credential grpc.FailOnNonTempDialError(true), grpc.WithContextDialer(dialer), grpc.WithTransportCredentials(insecure.NewCredentials()), // we are handling TLS, so tell grpc not to - grpc.WithKeepaliveParams(keepalive.ClientParameters{Time: common.GRPCKeepAliveTime}), + grpc.WithKeepaliveParams(keepalive.ClientParameters{Time: common.GetGRPCKeepAliveTime()}), ) conn, err := grpc.DialContext(ctx, address, opts...) var res interface{}