Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Add ENV variable to configure GRPC Keep Alive Time (#15656) #15806

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmpserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
),
}
Expand Down
23 changes: 20 additions & 3 deletions common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down
46 changes: 46 additions & 0 deletions common/common_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
1 change: 1 addition & 0 deletions docs/user-guide/environment-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`). |
2 changes: 1 addition & 1 deletion pkg/apiclient/grpcproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion reposerver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
),
}
Expand Down
2 changes: 1 addition & 1 deletion server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
),
}
Expand Down
2 changes: 1 addition & 1 deletion util/grpc/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
Loading