diff --git a/.changelog/8696.txt b/.changelog/8696.txt new file mode 100644 index 000000000..eda07dd57 --- /dev/null +++ b/.changelog/8696.txt @@ -0,0 +1,4 @@ +```release-note:bug +config: Fixed a bug where `rpc_max_conns_per_client` could not be changed by reloading the +config. +``` diff --git a/agent/agent.go b/agent/agent.go index e6b05a9ca..21e7b98a1 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -1143,7 +1143,7 @@ func newConsulConfig(runtimeCfg *config.RuntimeConfig, logger hclog.Logger) (*co // Rate limiting for RPC calls. if runtimeCfg.RPCRateLimit > 0 { - cfg.RPCRate = runtimeCfg.RPCRateLimit + cfg.RPCRateLimit = runtimeCfg.RPCRateLimit } if runtimeCfg.RPCMaxBurst > 0 { cfg.RPCMaxBurst = runtimeCfg.RPCMaxBurst diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index 0d42589e5..73dc267b1 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -21,6 +21,7 @@ import ( "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/connect" + "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/debug" "github.com/hashicorp/consul/agent/local" "github.com/hashicorp/consul/agent/structs" @@ -37,6 +38,7 @@ import ( "github.com/hashicorp/serf/serf" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/time/rate" ) func makeReadOnlyAgentACL(t *testing.T, srv *HTTPHandlers) string { @@ -1452,6 +1454,8 @@ func TestAgent_Reload(t *testing.T) { `, }) + shim := &delegateConfigReloadShim{delegate: a.delegate} + a.delegate = shim if err := a.reloadConfigInternal(cfg2); err != nil { t.Fatalf("got error %v want nil", err) } @@ -1459,13 +1463,8 @@ func TestAgent_Reload(t *testing.T) { t.Fatal("missing redis-reloaded service") } - if a.config.RPCRateLimit != 2 { - t.Fatalf("RPC rate not set correctly. Got %v. Want 2", a.config.RPCRateLimit) - } - - if a.config.RPCMaxBurst != 200 { - t.Fatalf("RPC max burst not set correctly. Got %v. Want 200", a.config.RPCMaxBurst) - } + require.Equal(t, rate.Limit(2), shim.newCfg.RPCRateLimit) + require.Equal(t, 200, shim.newCfg.RPCMaxBurst) for _, wp := range a.watchPlans { if !wp.IsStopped() { @@ -1474,6 +1473,16 @@ func TestAgent_Reload(t *testing.T) { } } +type delegateConfigReloadShim struct { + delegate + newCfg consul.ReloadableConfig +} + +func (s *delegateConfigReloadShim) ReloadConfig(cfg consul.ReloadableConfig) error { + s.newCfg = cfg + return s.delegate.ReloadConfig(cfg) +} + // TestAgent_ReloadDoesNotTriggerWatch Ensure watches not triggered after reload // see https://github.com/hashicorp/consul/issues/7446 func TestAgent_ReloadDoesNotTriggerWatch(t *testing.T) { diff --git a/agent/config/runtime.go b/agent/config/runtime.go index a1e5baa0f..c5f387218 100644 --- a/agent/config/runtime.go +++ b/agent/config/runtime.go @@ -921,8 +921,8 @@ type RuntimeConfig struct { // RPCRateLimit and RPCMaxBurst control how frequently RPC calls are allowed // to happen. In any large enough time interval, rate limiter limits the - // rate to RPCRate tokens per second, with a maximum burst size of - // RPCMaxBurst events. As a special case, if RPCRate == Inf (the infinite + // rate to RPCRateLimit tokens per second, with a maximum burst size of + // RPCMaxBurst events. As a special case, if RPCRateLimit == Inf (the infinite // rate), RPCMaxBurst is ignored. // // See https://en.wikipedia.org/wiki/Token_bucket for more about token diff --git a/agent/consul/client.go b/agent/consul/client.go index ee9147ddf..7b55dfaec 100644 --- a/agent/consul/client.go +++ b/agent/consul/client.go @@ -113,7 +113,7 @@ func NewClient(config *Config, deps Deps) (*Client, error) { tlsConfigurator: deps.TLSConfigurator, } - c.rpcLimiter.Store(rate.NewLimiter(config.RPCRate, config.RPCMaxBurst)) + c.rpcLimiter.Store(rate.NewLimiter(config.RPCRateLimit, config.RPCMaxBurst)) if err := c.initEnterprise(); err != nil { c.Shutdown() diff --git a/agent/consul/client_test.go b/agent/consul/client_test.go index 01583daca..8253c2512 100644 --- a/agent/consul/client_test.go +++ b/agent/consul/client_test.go @@ -533,7 +533,7 @@ func TestClient_RPC_RateLimit(t *testing.T) { testrpc.WaitForLeader(t, s1.RPC, "dc1") _, conf2 := testClientConfig(t) - conf2.RPCRate = 2 + conf2.RPCRateLimit = 2 conf2.RPCMaxBurst = 2 c1 := newClient(t, conf2) @@ -602,7 +602,7 @@ func TestClient_SnapshotRPC_RateLimit(t *testing.T) { testrpc.WaitForLeader(t, s1.RPC, "dc1") _, conf1 := testClientConfig(t) - conf1.RPCRate = 2 + conf1.RPCRateLimit = 2 conf1.RPCMaxBurst = 2 c1 := newClient(t, conf1) @@ -765,7 +765,7 @@ func TestClientServer_UserEvent(t *testing.T) { func TestClient_ReloadConfig(t *testing.T) { _, cfg := testClientConfig(t) - cfg.RPCRate = rate.Limit(500) + cfg.RPCRateLimit = rate.Limit(500) cfg.RPCMaxBurst = 5000 deps := newDefaultDeps(t, &Config{NodeName: "node1", Datacenter: "dc1"}) c, err := NewClient(cfg, deps) diff --git a/agent/consul/config.go b/agent/consul/config.go index dd6e7bca2..509b4cfe9 100644 --- a/agent/consul/config.go +++ b/agent/consul/config.go @@ -416,16 +416,16 @@ type Config struct { // place, and a small jitter is applied to avoid a thundering herd. RPCHoldTimeout time.Duration - // RPCRate and RPCMaxBurst control how frequently RPC calls are allowed + // RPCRateLimit and RPCMaxBurst control how frequently RPC calls are allowed // to happen. In any large enough time interval, rate limiter limits the - // rate to RPCRate tokens per second, with a maximum burst size of - // RPCMaxBurst events. As a special case, if RPCRate == Inf (the infinite + // rate to RPCRateLimit tokens per second, with a maximum burst size of + // RPCMaxBurst events. As a special case, if RPCRateLimit == Inf (the infinite // rate), RPCMaxBurst is ignored. // // See https://en.wikipedia.org/wiki/Token_bucket for more about token // buckets. - RPCRate rate.Limit - RPCMaxBurst int + RPCRateLimit rate.Limit + RPCMaxBurst int // RPCMaxConnsPerClient is the limit of how many concurrent connections are // allowed from a single source IP. @@ -582,8 +582,8 @@ func DefaultConfig() *Config { CheckOutputMaxSize: checks.DefaultBufSize, - RPCRate: rate.Inf, - RPCMaxBurst: 1000, + RPCRateLimit: rate.Inf, + RPCMaxBurst: 1000, TLSMinVersion: "tls10", diff --git a/agent/consul/rpc_test.go b/agent/consul/rpc_test.go index 64cba910e..4c283867e 100644 --- a/agent/consul/rpc_test.go +++ b/agent/consul/rpc_test.go @@ -730,7 +730,7 @@ func TestRPC_RPCMaxConnsPerClient(t *testing.T) { // Reload config with higher limit rc := ReloadableConfig{ - RPCRateLimit: s1.config.RPCRate, + RPCRateLimit: s1.config.RPCRateLimit, RPCMaxBurst: s1.config.RPCMaxBurst, RPCMaxConnsPerClient: 10, } diff --git a/agent/consul/server.go b/agent/consul/server.go index 15c47ec97..72be18c16 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -395,7 +395,7 @@ func NewServer(config *Config, flat Deps) (*Server, error) { return nil, err } - s.rpcLimiter.Store(rate.NewLimiter(config.RPCRate, config.RPCMaxBurst)) + s.rpcLimiter.Store(rate.NewLimiter(config.RPCRateLimit, config.RPCMaxBurst)) configReplicatorConfig := ReplicatorConfig{ Name: logging.ConfigEntry, diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index d31db5f3c..5fd40b06f 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -1479,7 +1479,7 @@ func TestServer_ReloadConfig(t *testing.T) { dir1, s := testServerWithConfig(t, func(c *Config) { c.Build = "1.5.0" - c.RPCRate = 500 + c.RPCRateLimit = 500 c.RPCMaxBurst = 5000 }) defer os.RemoveAll(dir1) @@ -1520,7 +1520,7 @@ func TestServer_RPC_RateLimit(t *testing.T) { t.Parallel() _, conf1 := testServerConfig(t) - conf1.RPCRate = 2 + conf1.RPCRateLimit = 2 conf1.RPCMaxBurst = 2 s1, err := newServer(t, conf1) if err != nil {