fix: Add ENV variable to configure GRPC Keep Alive Time (argoproj#15656…

…) (argoproj#15806) * Add ENV variables to configure GRPC Keep Alive Time Signed-off-by: Bhavika Sharma <bsharma@splunk.com> * Retrigger CI pipeline Signed-off-by: Bhavika Sharma <bsharma@splunk.com> * Resolve conflict with master Signed-off-by: Bhavika Sharma <bsharma@splunk.com> * Update docs/user-guide/environment-variables.md Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com> Signed-off-by: BhavikaSharma <BhavikaSharma@users.noreply.github.com> --------- Signed-off-by: Bhavika Sharma <bsharma@splunk.com> Signed-off-by: BhavikaSharma <BhavikaSharma@users.noreply.github.com> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
vladfr · Dec 13, 2023 · 845171d · 845171d
1 parent b6fd7d0
commit 845171d
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 (default `10s`).                                                                       |
diff --git a/pkg/apiclient/grpcproxy.go b/pkg/apiclient/grpcproxy.go
@@ -116,7 +116,7 @@ func (c *client) startGRPCProxy() (*grpc.Server, net.Listener, error) {
 		grpc.ForceServerCodec(&noopCodec{}),
 		grpc.KeepaliveEnforcementPolicy(
 			keepalive.EnforcementPolicy{
-				MinTime: common.GRPCKeepAliveEnforcementMinimum,
+				MinTime: common.GetGRPCKeepAliveEnforcementMinimum(),
 			},
 		),
 		grpc.UnknownServiceHandler(func(srv interface{}, stream grpc.ServerStream) error {

diff --git 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
@@ -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
@@ -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{}