From 3e6f9079cbf10cb0982240093f8e016de4d713b4 Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Thu, 11 Jun 2020 10:06:38 -0700 Subject: [PATCH 01/14] Update goruntime to latest, 0.2.5 Signed-off-by: Yuki Sawa --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index f679a373..fb228646 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/gorilla/mux v1.7.4-0.20191121170500-49c01487a141 github.com/kavu/go_reuseport v1.2.0 github.com/kelseyhightower/envconfig v1.1.0 - github.com/lyft/goruntime v0.2.1 + github.com/lyft/goruntime v0.2.5 github.com/lyft/gostats v0.4.0 github.com/lyft/protoc-gen-validate v0.0.7-0.20180626203901-f9d2b11e4414 // indirect github.com/mediocregopher/radix/v3 v3.5.1 diff --git a/go.sum b/go.sum index e0747b69..229a5cff 100644 --- a/go.sum +++ b/go.sum @@ -45,6 +45,8 @@ github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQL github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/lyft/goruntime v0.2.1 h1:7DebA8oMVuoQ5TQ0j1xR/X2xRagbGrm0e2SoMdt5tRs= github.com/lyft/goruntime v0.2.1/go.mod h1:8rUh5gwIPQtyIkIXHbLN1j45HOb8cMgDhrw5GA7DF4g= +github.com/lyft/goruntime v0.2.5 h1:yRmwOXl3Zns3+Z03fDMWt5+p609rfhIErh7HYCayODg= +github.com/lyft/goruntime v0.2.5/go.mod h1:8rUh5gwIPQtyIkIXHbLN1j45HOb8cMgDhrw5GA7DF4g= github.com/lyft/gostats v0.4.0 h1:PbRWmwidTPk6Y80S6itBWDa+XVt1hGvqFM88TBJYdOo= github.com/lyft/gostats v0.4.0/go.mod h1:Tpx2xRzz4t+T2Tx0xdVgIoBdR2UMVz+dKnE3X01XSd8= github.com/lyft/protoc-gen-validate v0.0.7-0.20180626203901-f9d2b11e4414 h1:kLCSHuk3X+SI8Up26wM71id7jz77B3zCZDp01UWMVbM= From b6f862182c0cb95fe88458c431d4902e61ae5a18 Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Fri, 12 Jun 2020 17:10:51 -0700 Subject: [PATCH 02/14] Add new option for watching changes in runtime config folder directly Signed-off-by: Yuki Sawa --- README.md | 6 ++++++ src/server/server_impl.go | 23 +++++++++++++++++------ src/settings/settings.go | 1 + 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 0640cb3c..55c08d95 100644 --- a/README.md +++ b/README.md @@ -320,6 +320,12 @@ RUNTIME_IGNOREDOTFILES default:"false" **Configuration files are loaded from RUNTIME_ROOT/RUNTIME_SUBDIRECTORY/config/\*.yaml** +There are two methods for triggering a configuration reload: +1. Symlink RUNTIME_ROOT to a different directory. +2. Update the contents inside `RUNTIME_ROOT/RUNTIME_SUBDIRECTORY/config/` directly. + +The former is the default behavior. To use the latter method, set the `RUNTIME_WATCH_ROOT` environment variable to `false`. + For more information on how runtime works you can read its [README](https://github.com/lyft/goruntime). # Request Fields diff --git a/src/server/server_impl.go b/src/server/server_impl.go index da92df84..207f4e9a 100644 --- a/src/server/server_impl.go +++ b/src/server/server_impl.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/http/pprof" + "path/filepath" "sort" "os" @@ -169,12 +170,22 @@ func newServer(name string, store stats.Store, localCache *freecache.Cache, opts loaderOpts = append(loaderOpts, loader.AllowDotFiles) } - ret.runtime = loader.New( - s.RuntimePath, - s.RuntimeSubdirectory, - ret.store.Scope("runtime"), - &loader.SymlinkRefresher{RuntimePath: s.RuntimePath}, - loaderOpts...) + if s.RuntimeWatchRoot { + ret.runtime = loader.New( + s.RuntimePath, + s.RuntimeSubdirectory, + ret.store.Scope("runtime"), + &loader.SymlinkRefresher{RuntimePath: s.RuntimePath}, + loaderOpts...) + + } else { + ret.runtime = loader.New( + filepath.Join(s.RuntimePath, s.RuntimeSubdirectory), + "config", + ret.store.Scope("runtime"), + &loader.DirectoryRefresher{}, + loaderOpts...) + } // setup http router ret.router = mux.NewRouter() diff --git a/src/settings/settings.go b/src/settings/settings.go index 53ab1347..971ff60c 100644 --- a/src/settings/settings.go +++ b/src/settings/settings.go @@ -20,6 +20,7 @@ type Settings struct { RuntimePath string `envconfig:"RUNTIME_ROOT" default:"/srv/runtime_data/current"` RuntimeSubdirectory string `envconfig:"RUNTIME_SUBDIRECTORY"` RuntimeIgnoreDotFiles bool `envconfig:"RUNTIME_IGNOREDOTFILES" default:"false"` + RuntimeWatchRoot bool `envconfig:"RUNTIME_WATCH_ROOT" default:"true"` LogLevel string `envconfig:"LOG_LEVEL" default:"WARN"` RedisSocketType string `envconfig:"REDIS_SOCKET_TYPE" default:"unix"` RedisUrl string `envconfig:"REDIS_URL" default:"/var/run/nutcracker/ratelimit.sock"` From 40a3d42832355f330ecc1296e95c2e0683501d9c Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Wed, 17 Jun 2020 11:28:20 -0700 Subject: [PATCH 03/14] If runtime watch root is false, then file prefix will not start with .config so turn off that check Signed-off-by: Yuki Sawa --- src/service/ratelimit.go | 7 +++++-- src/service_cmd/runner/runner.go | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/service/ratelimit.go b/src/service/ratelimit.go index 07a8c313..0a5d7da9 100644 --- a/src/service/ratelimit.go +++ b/src/service/ratelimit.go @@ -57,6 +57,7 @@ type service struct { stats serviceStats rlStatsScope stats.Scope legacy *legacyService + runtimeWatchRoot bool } func (this *service) reloadConfig() { @@ -75,7 +76,8 @@ func (this *service) reloadConfig() { files := []config.RateLimitConfigToLoad{} snapshot := this.runtime.Snapshot() for _, key := range snapshot.Keys() { - if !strings.HasPrefix(key, "config.") { + logger.Errorf("file key: %s", key) + if this.runtimeWatchRoot && !strings.HasPrefix(key, "config.") { continue } @@ -176,7 +178,7 @@ func (this *service) GetCurrentConfig() config.RateLimitConfig { } func NewService(runtime loader.IFace, cache limiter.RateLimitCache, - configLoader config.RateLimitConfigLoader, stats stats.Scope) RateLimitServiceServer { + configLoader config.RateLimitConfigLoader, stats stats.Scope, runtimeWatchRoot bool) RateLimitServiceServer { newService := &service{ runtime: runtime, @@ -187,6 +189,7 @@ func NewService(runtime loader.IFace, cache limiter.RateLimitCache, cache: cache, stats: newServiceStats(stats), rlStatsScope: stats.Scope("rate_limit"), + runtimeWatchRoot: runtimeWatchRoot, } newService.legacy = &legacyService{ s: newService, diff --git a/src/service_cmd/runner/runner.go b/src/service_cmd/runner/runner.go index 5e43307a..85c3f899 100644 --- a/src/service_cmd/runner/runner.go +++ b/src/service_cmd/runner/runner.go @@ -60,7 +60,9 @@ func (runner *Runner) Run() { rand.New(limiter.NewLockedSource(time.Now().Unix())), s.ExpirationJitterMaxSeconds), config.NewRateLimitConfigLoaderImpl(), - srv.Scope().Scope("service")) + srv.Scope().Scope("service"), + s.RuntimeWatchRoot, + ) srv.AddDebugHttpEndpoint( "/rlconfig", From 8be09e919879921fa3c089cbd2e226acc6b01390 Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Wed, 17 Jun 2020 13:05:15 -0700 Subject: [PATCH 04/14] Fix tests Signed-off-by: Yuki Sawa --- test/service/ratelimit_legacy_test.go | 2 +- test/service/ratelimit_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/service/ratelimit_legacy_test.go b/test/service/ratelimit_legacy_test.go index ad7e6b94..71689a6a 100644 --- a/test/service/ratelimit_legacy_test.go +++ b/test/service/ratelimit_legacy_test.go @@ -224,7 +224,7 @@ func TestInitialLoadErrorLegacy(test *testing.T) { func([]config.RateLimitConfigToLoad, stats.Scope) { panic(config.RateLimitConfigError("load error")) }) - service := ratelimit.NewService(t.runtime, t.cache, t.configLoader, t.statStore) + service := ratelimit.NewService(t.runtime, t.cache, t.configLoader, t.statStore, true) request := common.NewRateLimitRequestLegacy("test-domain", [][][2]string{{{"hello", "world"}}}, 1) response, err := service.GetLegacyService().ShouldRateLimit(nil, request) diff --git a/test/service/ratelimit_test.go b/test/service/ratelimit_test.go index c51bc798..a545f4f2 100644 --- a/test/service/ratelimit_test.go +++ b/test/service/ratelimit_test.go @@ -82,7 +82,7 @@ func (this *rateLimitServiceTestSuite) setupBasicService() ratelimit.RateLimitSe this.configLoader.EXPECT().Load( []config.RateLimitConfigToLoad{{"config.basic_config", "fake_yaml"}}, gomock.Any()).Return(this.config) - return ratelimit.NewService(this.runtime, this.cache, this.configLoader, this.statStore) + return ratelimit.NewService(this.runtime, this.cache, this.configLoader, this.statStore, true) } func TestService(test *testing.T) { @@ -225,7 +225,7 @@ func TestInitialLoadError(test *testing.T) { func([]config.RateLimitConfigToLoad, stats.Scope) { panic(config.RateLimitConfigError("load error")) }) - service := ratelimit.NewService(t.runtime, t.cache, t.configLoader, t.statStore) + service := ratelimit.NewService(t.runtime, t.cache, t.configLoader, t.statStore, true) request := common.NewRateLimitRequest("test-domain", [][][2]string{{{"hello", "world"}}}, 1) response, err := service.ShouldRateLimit(nil, request) From 300d9050090eb12a1dc9217bee93b7ff0fa67c6b Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Thu, 18 Jun 2020 09:54:53 -0700 Subject: [PATCH 05/14] Remove logging msg Signed-off-by: Yuki Sawa --- src/service/ratelimit.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/service/ratelimit.go b/src/service/ratelimit.go index 0a5d7da9..1286e551 100644 --- a/src/service/ratelimit.go +++ b/src/service/ratelimit.go @@ -76,7 +76,6 @@ func (this *service) reloadConfig() { files := []config.RateLimitConfigToLoad{} snapshot := this.runtime.Snapshot() for _, key := range snapshot.Keys() { - logger.Errorf("file key: %s", key) if this.runtimeWatchRoot && !strings.HasPrefix(key, "config.") { continue } From 289c108b397d600d91e4d30d3c3aa02f214d886f Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Thu, 18 Jun 2020 09:56:28 -0700 Subject: [PATCH 06/14] Remove refs to goruntime 0.2.1 Signed-off-by: Yuki Sawa --- go.sum | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.sum b/go.sum index 229a5cff..ba1d3c07 100644 --- a/go.sum +++ b/go.sum @@ -43,8 +43,6 @@ github.com/kelseyhightower/envconfig v1.1.0 h1:4htXR8ameS6KBfrNBoqEgpg0IK2D6rozN github.com/kelseyhightower/envconfig v1.1.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg= github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= -github.com/lyft/goruntime v0.2.1 h1:7DebA8oMVuoQ5TQ0j1xR/X2xRagbGrm0e2SoMdt5tRs= -github.com/lyft/goruntime v0.2.1/go.mod h1:8rUh5gwIPQtyIkIXHbLN1j45HOb8cMgDhrw5GA7DF4g= github.com/lyft/goruntime v0.2.5 h1:yRmwOXl3Zns3+Z03fDMWt5+p609rfhIErh7HYCayODg= github.com/lyft/goruntime v0.2.5/go.mod h1:8rUh5gwIPQtyIkIXHbLN1j45HOb8cMgDhrw5GA7DF4g= github.com/lyft/gostats v0.4.0 h1:PbRWmwidTPk6Y80S6itBWDa+XVt1hGvqFM88TBJYdOo= From da21e917d16598ac69df23c504de350b1a666246 Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Thu, 25 Jun 2020 12:40:29 -0700 Subject: [PATCH 07/14] Add integration tests for config reloading Signed-off-by: Yuki Sawa --- test/integration/integration_test.go | 130 ++++++++++++++++++ .../runtime/current/ratelimit/reload.yaml | 16 +++ 2 files changed, 146 insertions(+) create mode 100644 test/integration/runtime/current/ratelimit/reload.yaml diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index 60a25257..3765b9b9 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -5,6 +5,7 @@ package integration_test import ( "bytes" "fmt" + "io" "math/rand" "net/http" "os" @@ -66,6 +67,11 @@ func TestBasicAuthConfig(t *testing.T) { t.Run("WithPerSecondRedisAuthWithLocalCache", testBasicConfigAuth("18093", "true", "1000")) } +func TestBasicReloadConfig(t *testing.T) { + t.Run("BasicWithoutWatchRoot", testBasicConfigWithoutWatchRoot("8095", "false", "0")) + t.Run("ReloadWithoutWatchRoot", testBasicConfigReload("8097", "false", "0", "false")) +} + func testBasicConfigAuthTLS(grpcPort, perSecond string, local_cache_size string) func(*testing.T) { os.Setenv("REDIS_PERSECOND_URL", "localhost:16382") os.Setenv("REDIS_URL", "localhost:16381") @@ -96,6 +102,28 @@ func testBasicConfigAuth(grpcPort, perSecond string, local_cache_size string) fu return testBasicBaseConfig(grpcPort, perSecond, local_cache_size) } +func testBasicConfigWithoutWatchRoot(grpcPort, perSecond string, local_cache_size string) func(*testing.T) { + os.Setenv("REDIS_PERSECOND_URL", "localhost:6380") + os.Setenv("REDIS_URL", "localhost:6379") + os.Setenv("REDIS_AUTH", "") + os.Setenv("REDIS_TLS", "false") + os.Setenv("REDIS_PERSECOND_AUTH", "") + os.Setenv("REDIS_PERSECOND_TLS", "false") + os.Setenv("RUNTIME_WATCH_ROOT", "false") + return testBasicBaseConfig(grpcPort, perSecond, local_cache_size) +} + +func testBasicConfigReload(grpcPort, perSecond string, local_cache_size, runtimeWatchRoot string) func(*testing.T) { + os.Setenv("REDIS_PERSECOND_URL", "localhost:6380") + os.Setenv("REDIS_URL", "localhost:6379") + os.Setenv("REDIS_AUTH", "") + os.Setenv("REDIS_TLS", "false") + os.Setenv("REDIS_PERSECOND_AUTH", "") + os.Setenv("REDIS_PERSECOND_TLS", "false") + os.Setenv("RUNTIME_WATCH_ROOT", runtimeWatchRoot) + return testConfigReload(grpcPort, perSecond, local_cache_size) +} + func getCacheKey(cacheKey string, enableLocalCache bool) string { if enableLocalCache { return cacheKey + "_local" @@ -451,3 +479,105 @@ func TestBasicConfigLegacy(t *testing.T) { assert.NoError(err) } } + +func testConfigReload(grpcPort, perSecond string, local_cache_size string) func(*testing.T) { + return func(t *testing.T) { + os.Setenv("REDIS_PERSECOND", perSecond) + os.Setenv("PORT", "8082") + os.Setenv("GRPC_PORT", grpcPort) + os.Setenv("DEBUG_PORT", "8084") + os.Setenv("RUNTIME_ROOT", "runtime/current") + os.Setenv("RUNTIME_SUBDIRECTORY", "ratelimit") + os.Setenv("REDIS_PERSECOND_SOCKET_TYPE", "tcp") + os.Setenv("REDIS_SOCKET_TYPE", "tcp") + os.Setenv("LOCAL_CACHE_SIZE_IN_BYTES", local_cache_size) + os.Setenv("USE_STATSD", "false") + + local_cache_size_val, _ := strconv.Atoi(local_cache_size) + enable_local_cache := local_cache_size_val > 0 + runner := runner.NewRunner() + + go func() { + runner.Run() + }() + + // HACK: Wait for the server to come up. Make a hook that we can wait on. + time.Sleep(1 * time.Second) + + assert := assert.New(t) + conn, err := grpc.Dial(fmt.Sprintf("localhost:%s", grpcPort), grpc.WithInsecure()) + assert.NoError(err) + defer conn.Close() + c := pb.NewRateLimitServiceClient(conn) + + response, err := c.ShouldRateLimit( + context.Background(), + common.NewRateLimitRequest("reload", [][][2]string{{{getCacheKey("block", enable_local_cache), "foo"}}}, 1)) + assert.Equal( + &pb.RateLimitResponse{ + OverallCode: pb.RateLimitResponse_OK, + Statuses: []*pb.RateLimitResponse_DescriptorStatus{{Code: pb.RateLimitResponse_OK}}}, + response) + assert.NoError(err) + + runner.GetStatsStore().Flush() + loadCount1 := runner.GetStatsStore().NewCounter("ratelimit.service.config_load_success").Value() + + // Copy a new file to config folder to test config reload functionality + in, err := os.Open("runtime/current/ratelimit/reload.yaml") + if err != nil { + panic(err) + } + defer in.Close() + out, err := os.Create("runtime/current/ratelimit/config/reload.yaml") + if err != nil { + panic(err) + } + defer out.Close() + _, err = io.Copy(out, in) + if err != nil { + panic(err) + } + err = out.Close() + if err != nil { + panic(err) + } + + // Need to wait for config reload to take place and new descriptors to be loaded. + // Shouldn't take more than 5 seconds but wait 120 at most just to be safe. + wait := 120 + reloaded := false + loadCount2 := uint64(0) + + for i := 0; i < wait; i++ { + time.Sleep(1 * time.Second) + runner.GetStatsStore().Flush() + loadCount2 = runner.GetStatsStore().NewCounter("ratelimit.service.config_load_success").Value() + + // Check that successful loads count has increased before continuing. + if loadCount2 > loadCount1 { + reloaded = true + break + } + } + + assert.True(reloaded) + assert.Greater(loadCount2, loadCount1) + + response, err = c.ShouldRateLimit( + context.Background(), + common.NewRateLimitRequest("reload", [][][2]string{{{getCacheKey("key1", enable_local_cache), "foo"}}}, 1)) + assert.Equal( + &pb.RateLimitResponse{ + OverallCode: pb.RateLimitResponse_OK, + Statuses: []*pb.RateLimitResponse_DescriptorStatus{ + newDescriptorStatus(pb.RateLimitResponse_OK, 50, pb.RateLimitResponse_RateLimit_SECOND, 49)}}, + response) + assert.NoError(err) + + err = os.Remove("runtime/current/ratelimit/config/reload.yaml") + if err != nil { + panic(err) + } + } +} \ No newline at end of file diff --git a/test/integration/runtime/current/ratelimit/reload.yaml b/test/integration/runtime/current/ratelimit/reload.yaml new file mode 100644 index 00000000..5da29e52 --- /dev/null +++ b/test/integration/runtime/current/ratelimit/reload.yaml @@ -0,0 +1,16 @@ +domain: reload +descriptors: + - key: key1 + rate_limit: + unit: second + requests_per_unit: 50 + + - key: block + rate_limit: + unit: second + requests_per_unit: 0 + + - key: one_per_minute + rate_limit: + unit: minute + requests_per_unit: 1 From 901169d9ed9d9dc22d8557f2b9a08e05c49f09ca Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Thu, 11 Jun 2020 10:06:38 -0700 Subject: [PATCH 08/14] Update goruntime to latest, 0.2.5 Signed-off-by: Yuki Sawa --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index f679a373..fb228646 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/gorilla/mux v1.7.4-0.20191121170500-49c01487a141 github.com/kavu/go_reuseport v1.2.0 github.com/kelseyhightower/envconfig v1.1.0 - github.com/lyft/goruntime v0.2.1 + github.com/lyft/goruntime v0.2.5 github.com/lyft/gostats v0.4.0 github.com/lyft/protoc-gen-validate v0.0.7-0.20180626203901-f9d2b11e4414 // indirect github.com/mediocregopher/radix/v3 v3.5.1 diff --git a/go.sum b/go.sum index e0747b69..229a5cff 100644 --- a/go.sum +++ b/go.sum @@ -45,6 +45,8 @@ github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQL github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/lyft/goruntime v0.2.1 h1:7DebA8oMVuoQ5TQ0j1xR/X2xRagbGrm0e2SoMdt5tRs= github.com/lyft/goruntime v0.2.1/go.mod h1:8rUh5gwIPQtyIkIXHbLN1j45HOb8cMgDhrw5GA7DF4g= +github.com/lyft/goruntime v0.2.5 h1:yRmwOXl3Zns3+Z03fDMWt5+p609rfhIErh7HYCayODg= +github.com/lyft/goruntime v0.2.5/go.mod h1:8rUh5gwIPQtyIkIXHbLN1j45HOb8cMgDhrw5GA7DF4g= github.com/lyft/gostats v0.4.0 h1:PbRWmwidTPk6Y80S6itBWDa+XVt1hGvqFM88TBJYdOo= github.com/lyft/gostats v0.4.0/go.mod h1:Tpx2xRzz4t+T2Tx0xdVgIoBdR2UMVz+dKnE3X01XSd8= github.com/lyft/protoc-gen-validate v0.0.7-0.20180626203901-f9d2b11e4414 h1:kLCSHuk3X+SI8Up26wM71id7jz77B3zCZDp01UWMVbM= From 4fa76c7207ffe1817f6dfa6ed9eb373db5b707ae Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Fri, 12 Jun 2020 17:10:51 -0700 Subject: [PATCH 09/14] Add new option for watching changes in runtime config folder directly Signed-off-by: Yuki Sawa --- README.md | 6 ++++++ src/server/server_impl.go | 23 +++++++++++++++++------ src/settings/settings.go | 1 + 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index eca0d002..746c5f5a 100644 --- a/README.md +++ b/README.md @@ -320,6 +320,12 @@ RUNTIME_IGNOREDOTFILES default:"false" **Configuration files are loaded from RUNTIME_ROOT/RUNTIME_SUBDIRECTORY/config/\*.yaml** +There are two methods for triggering a configuration reload: +1. Symlink RUNTIME_ROOT to a different directory. +2. Update the contents inside `RUNTIME_ROOT/RUNTIME_SUBDIRECTORY/config/` directly. + +The former is the default behavior. To use the latter method, set the `RUNTIME_WATCH_ROOT` environment variable to `false`. + For more information on how runtime works you can read its [README](https://github.com/lyft/goruntime). # Request Fields diff --git a/src/server/server_impl.go b/src/server/server_impl.go index 9a1239a9..8624f37d 100644 --- a/src/server/server_impl.go +++ b/src/server/server_impl.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/http/pprof" + "path/filepath" "sort" "os" @@ -187,12 +188,22 @@ func newServer(name string, store stats.Store, localCache *freecache.Cache, opts loaderOpts = append(loaderOpts, loader.AllowDotFiles) } - ret.runtime = loader.New( - s.RuntimePath, - s.RuntimeSubdirectory, - ret.store.Scope("runtime"), - &loader.SymlinkRefresher{RuntimePath: s.RuntimePath}, - loaderOpts...) + if s.RuntimeWatchRoot { + ret.runtime = loader.New( + s.RuntimePath, + s.RuntimeSubdirectory, + ret.store.Scope("runtime"), + &loader.SymlinkRefresher{RuntimePath: s.RuntimePath}, + loaderOpts...) + + } else { + ret.runtime = loader.New( + filepath.Join(s.RuntimePath, s.RuntimeSubdirectory), + "config", + ret.store.Scope("runtime"), + &loader.DirectoryRefresher{}, + loaderOpts...) + } // setup http router ret.router = mux.NewRouter() diff --git a/src/settings/settings.go b/src/settings/settings.go index 53ab1347..971ff60c 100644 --- a/src/settings/settings.go +++ b/src/settings/settings.go @@ -20,6 +20,7 @@ type Settings struct { RuntimePath string `envconfig:"RUNTIME_ROOT" default:"/srv/runtime_data/current"` RuntimeSubdirectory string `envconfig:"RUNTIME_SUBDIRECTORY"` RuntimeIgnoreDotFiles bool `envconfig:"RUNTIME_IGNOREDOTFILES" default:"false"` + RuntimeWatchRoot bool `envconfig:"RUNTIME_WATCH_ROOT" default:"true"` LogLevel string `envconfig:"LOG_LEVEL" default:"WARN"` RedisSocketType string `envconfig:"REDIS_SOCKET_TYPE" default:"unix"` RedisUrl string `envconfig:"REDIS_URL" default:"/var/run/nutcracker/ratelimit.sock"` From 04327ae54ba246edae3ee6bb76cc1c809ef8c6c5 Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Wed, 17 Jun 2020 11:28:20 -0700 Subject: [PATCH 10/14] If runtime watch root is false, then file prefix will not start with .config so turn off that check Signed-off-by: Yuki Sawa --- src/service/ratelimit.go | 7 +++++-- src/service_cmd/runner/runner.go | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/service/ratelimit.go b/src/service/ratelimit.go index 07a8c313..0a5d7da9 100644 --- a/src/service/ratelimit.go +++ b/src/service/ratelimit.go @@ -57,6 +57,7 @@ type service struct { stats serviceStats rlStatsScope stats.Scope legacy *legacyService + runtimeWatchRoot bool } func (this *service) reloadConfig() { @@ -75,7 +76,8 @@ func (this *service) reloadConfig() { files := []config.RateLimitConfigToLoad{} snapshot := this.runtime.Snapshot() for _, key := range snapshot.Keys() { - if !strings.HasPrefix(key, "config.") { + logger.Errorf("file key: %s", key) + if this.runtimeWatchRoot && !strings.HasPrefix(key, "config.") { continue } @@ -176,7 +178,7 @@ func (this *service) GetCurrentConfig() config.RateLimitConfig { } func NewService(runtime loader.IFace, cache limiter.RateLimitCache, - configLoader config.RateLimitConfigLoader, stats stats.Scope) RateLimitServiceServer { + configLoader config.RateLimitConfigLoader, stats stats.Scope, runtimeWatchRoot bool) RateLimitServiceServer { newService := &service{ runtime: runtime, @@ -187,6 +189,7 @@ func NewService(runtime loader.IFace, cache limiter.RateLimitCache, cache: cache, stats: newServiceStats(stats), rlStatsScope: stats.Scope("rate_limit"), + runtimeWatchRoot: runtimeWatchRoot, } newService.legacy = &legacyService{ s: newService, diff --git a/src/service_cmd/runner/runner.go b/src/service_cmd/runner/runner.go index 5e43307a..85c3f899 100644 --- a/src/service_cmd/runner/runner.go +++ b/src/service_cmd/runner/runner.go @@ -60,7 +60,9 @@ func (runner *Runner) Run() { rand.New(limiter.NewLockedSource(time.Now().Unix())), s.ExpirationJitterMaxSeconds), config.NewRateLimitConfigLoaderImpl(), - srv.Scope().Scope("service")) + srv.Scope().Scope("service"), + s.RuntimeWatchRoot, + ) srv.AddDebugHttpEndpoint( "/rlconfig", From aef49768b9798a10cf3b8d1cd0d28c054dcaa0d4 Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Wed, 17 Jun 2020 13:05:15 -0700 Subject: [PATCH 11/14] Fix tests Signed-off-by: Yuki Sawa --- test/service/ratelimit_legacy_test.go | 2 +- test/service/ratelimit_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/service/ratelimit_legacy_test.go b/test/service/ratelimit_legacy_test.go index ad7e6b94..71689a6a 100644 --- a/test/service/ratelimit_legacy_test.go +++ b/test/service/ratelimit_legacy_test.go @@ -224,7 +224,7 @@ func TestInitialLoadErrorLegacy(test *testing.T) { func([]config.RateLimitConfigToLoad, stats.Scope) { panic(config.RateLimitConfigError("load error")) }) - service := ratelimit.NewService(t.runtime, t.cache, t.configLoader, t.statStore) + service := ratelimit.NewService(t.runtime, t.cache, t.configLoader, t.statStore, true) request := common.NewRateLimitRequestLegacy("test-domain", [][][2]string{{{"hello", "world"}}}, 1) response, err := service.GetLegacyService().ShouldRateLimit(nil, request) diff --git a/test/service/ratelimit_test.go b/test/service/ratelimit_test.go index c51bc798..a545f4f2 100644 --- a/test/service/ratelimit_test.go +++ b/test/service/ratelimit_test.go @@ -82,7 +82,7 @@ func (this *rateLimitServiceTestSuite) setupBasicService() ratelimit.RateLimitSe this.configLoader.EXPECT().Load( []config.RateLimitConfigToLoad{{"config.basic_config", "fake_yaml"}}, gomock.Any()).Return(this.config) - return ratelimit.NewService(this.runtime, this.cache, this.configLoader, this.statStore) + return ratelimit.NewService(this.runtime, this.cache, this.configLoader, this.statStore, true) } func TestService(test *testing.T) { @@ -225,7 +225,7 @@ func TestInitialLoadError(test *testing.T) { func([]config.RateLimitConfigToLoad, stats.Scope) { panic(config.RateLimitConfigError("load error")) }) - service := ratelimit.NewService(t.runtime, t.cache, t.configLoader, t.statStore) + service := ratelimit.NewService(t.runtime, t.cache, t.configLoader, t.statStore, true) request := common.NewRateLimitRequest("test-domain", [][][2]string{{{"hello", "world"}}}, 1) response, err := service.ShouldRateLimit(nil, request) From a1eb8ef367eb31bc55f39d187aedfc2e97ceaec0 Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Thu, 18 Jun 2020 09:54:53 -0700 Subject: [PATCH 12/14] Remove logging msg Signed-off-by: Yuki Sawa --- src/service/ratelimit.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/service/ratelimit.go b/src/service/ratelimit.go index 0a5d7da9..1286e551 100644 --- a/src/service/ratelimit.go +++ b/src/service/ratelimit.go @@ -76,7 +76,6 @@ func (this *service) reloadConfig() { files := []config.RateLimitConfigToLoad{} snapshot := this.runtime.Snapshot() for _, key := range snapshot.Keys() { - logger.Errorf("file key: %s", key) if this.runtimeWatchRoot && !strings.HasPrefix(key, "config.") { continue } From 4c176137841d8925f3455f719b1ad08652c3b174 Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Thu, 18 Jun 2020 09:56:28 -0700 Subject: [PATCH 13/14] Remove refs to goruntime 0.2.1 Signed-off-by: Yuki Sawa --- go.sum | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.sum b/go.sum index 229a5cff..ba1d3c07 100644 --- a/go.sum +++ b/go.sum @@ -43,8 +43,6 @@ github.com/kelseyhightower/envconfig v1.1.0 h1:4htXR8ameS6KBfrNBoqEgpg0IK2D6rozN github.com/kelseyhightower/envconfig v1.1.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg= github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= -github.com/lyft/goruntime v0.2.1 h1:7DebA8oMVuoQ5TQ0j1xR/X2xRagbGrm0e2SoMdt5tRs= -github.com/lyft/goruntime v0.2.1/go.mod h1:8rUh5gwIPQtyIkIXHbLN1j45HOb8cMgDhrw5GA7DF4g= github.com/lyft/goruntime v0.2.5 h1:yRmwOXl3Zns3+Z03fDMWt5+p609rfhIErh7HYCayODg= github.com/lyft/goruntime v0.2.5/go.mod h1:8rUh5gwIPQtyIkIXHbLN1j45HOb8cMgDhrw5GA7DF4g= github.com/lyft/gostats v0.4.0 h1:PbRWmwidTPk6Y80S6itBWDa+XVt1hGvqFM88TBJYdOo= From 0df8c6b860f2a216ed69706932fa5de5c678a4ca Mon Sep 17 00:00:00 2001 From: Yuki Sawa Date: Thu, 25 Jun 2020 12:40:29 -0700 Subject: [PATCH 14/14] Add integration tests for config reloading Signed-off-by: Yuki Sawa --- test/integration/integration_test.go | 130 ++++++++++++++++++ .../runtime/current/ratelimit/reload.yaml | 16 +++ 2 files changed, 146 insertions(+) create mode 100644 test/integration/runtime/current/ratelimit/reload.yaml diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index 6e1b9efd..9c9f01ea 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -6,6 +6,7 @@ import ( "bytes" "fmt" "io/ioutil" + "io" "math/rand" "net/http" "os" @@ -67,6 +68,11 @@ func TestBasicAuthConfig(t *testing.T) { t.Run("WithPerSecondRedisAuthWithLocalCache", testBasicConfigAuth("18093", "true", "1000")) } +func TestBasicReloadConfig(t *testing.T) { + t.Run("BasicWithoutWatchRoot", testBasicConfigWithoutWatchRoot("8095", "false", "0")) + t.Run("ReloadWithoutWatchRoot", testBasicConfigReload("8097", "false", "0", "false")) +} + func testBasicConfigAuthTLS(grpcPort, perSecond string, local_cache_size string) func(*testing.T) { os.Setenv("REDIS_PERSECOND_URL", "localhost:16382") os.Setenv("REDIS_URL", "localhost:16381") @@ -97,6 +103,28 @@ func testBasicConfigAuth(grpcPort, perSecond string, local_cache_size string) fu return testBasicBaseConfig(grpcPort, perSecond, local_cache_size) } +func testBasicConfigWithoutWatchRoot(grpcPort, perSecond string, local_cache_size string) func(*testing.T) { + os.Setenv("REDIS_PERSECOND_URL", "localhost:6380") + os.Setenv("REDIS_URL", "localhost:6379") + os.Setenv("REDIS_AUTH", "") + os.Setenv("REDIS_TLS", "false") + os.Setenv("REDIS_PERSECOND_AUTH", "") + os.Setenv("REDIS_PERSECOND_TLS", "false") + os.Setenv("RUNTIME_WATCH_ROOT", "false") + return testBasicBaseConfig(grpcPort, perSecond, local_cache_size) +} + +func testBasicConfigReload(grpcPort, perSecond string, local_cache_size, runtimeWatchRoot string) func(*testing.T) { + os.Setenv("REDIS_PERSECOND_URL", "localhost:6380") + os.Setenv("REDIS_URL", "localhost:6379") + os.Setenv("REDIS_AUTH", "") + os.Setenv("REDIS_TLS", "false") + os.Setenv("REDIS_PERSECOND_AUTH", "") + os.Setenv("REDIS_PERSECOND_TLS", "false") + os.Setenv("RUNTIME_WATCH_ROOT", runtimeWatchRoot) + return testConfigReload(grpcPort, perSecond, local_cache_size) +} + func getCacheKey(cacheKey string, enableLocalCache bool) string { if enableLocalCache { return cacheKey + "_local" @@ -456,3 +484,105 @@ func TestBasicConfigLegacy(t *testing.T) { assert.NoError(err) } } + +func testConfigReload(grpcPort, perSecond string, local_cache_size string) func(*testing.T) { + return func(t *testing.T) { + os.Setenv("REDIS_PERSECOND", perSecond) + os.Setenv("PORT", "8082") + os.Setenv("GRPC_PORT", grpcPort) + os.Setenv("DEBUG_PORT", "8084") + os.Setenv("RUNTIME_ROOT", "runtime/current") + os.Setenv("RUNTIME_SUBDIRECTORY", "ratelimit") + os.Setenv("REDIS_PERSECOND_SOCKET_TYPE", "tcp") + os.Setenv("REDIS_SOCKET_TYPE", "tcp") + os.Setenv("LOCAL_CACHE_SIZE_IN_BYTES", local_cache_size) + os.Setenv("USE_STATSD", "false") + + local_cache_size_val, _ := strconv.Atoi(local_cache_size) + enable_local_cache := local_cache_size_val > 0 + runner := runner.NewRunner() + + go func() { + runner.Run() + }() + + // HACK: Wait for the server to come up. Make a hook that we can wait on. + time.Sleep(1 * time.Second) + + assert := assert.New(t) + conn, err := grpc.Dial(fmt.Sprintf("localhost:%s", grpcPort), grpc.WithInsecure()) + assert.NoError(err) + defer conn.Close() + c := pb.NewRateLimitServiceClient(conn) + + response, err := c.ShouldRateLimit( + context.Background(), + common.NewRateLimitRequest("reload", [][][2]string{{{getCacheKey("block", enable_local_cache), "foo"}}}, 1)) + assert.Equal( + &pb.RateLimitResponse{ + OverallCode: pb.RateLimitResponse_OK, + Statuses: []*pb.RateLimitResponse_DescriptorStatus{{Code: pb.RateLimitResponse_OK}}}, + response) + assert.NoError(err) + + runner.GetStatsStore().Flush() + loadCount1 := runner.GetStatsStore().NewCounter("ratelimit.service.config_load_success").Value() + + // Copy a new file to config folder to test config reload functionality + in, err := os.Open("runtime/current/ratelimit/reload.yaml") + if err != nil { + panic(err) + } + defer in.Close() + out, err := os.Create("runtime/current/ratelimit/config/reload.yaml") + if err != nil { + panic(err) + } + defer out.Close() + _, err = io.Copy(out, in) + if err != nil { + panic(err) + } + err = out.Close() + if err != nil { + panic(err) + } + + // Need to wait for config reload to take place and new descriptors to be loaded. + // Shouldn't take more than 5 seconds but wait 120 at most just to be safe. + wait := 120 + reloaded := false + loadCount2 := uint64(0) + + for i := 0; i < wait; i++ { + time.Sleep(1 * time.Second) + runner.GetStatsStore().Flush() + loadCount2 = runner.GetStatsStore().NewCounter("ratelimit.service.config_load_success").Value() + + // Check that successful loads count has increased before continuing. + if loadCount2 > loadCount1 { + reloaded = true + break + } + } + + assert.True(reloaded) + assert.Greater(loadCount2, loadCount1) + + response, err = c.ShouldRateLimit( + context.Background(), + common.NewRateLimitRequest("reload", [][][2]string{{{getCacheKey("key1", enable_local_cache), "foo"}}}, 1)) + assert.Equal( + &pb.RateLimitResponse{ + OverallCode: pb.RateLimitResponse_OK, + Statuses: []*pb.RateLimitResponse_DescriptorStatus{ + newDescriptorStatus(pb.RateLimitResponse_OK, 50, pb.RateLimitResponse_RateLimit_SECOND, 49)}}, + response) + assert.NoError(err) + + err = os.Remove("runtime/current/ratelimit/config/reload.yaml") + if err != nil { + panic(err) + } + } +} \ No newline at end of file diff --git a/test/integration/runtime/current/ratelimit/reload.yaml b/test/integration/runtime/current/ratelimit/reload.yaml new file mode 100644 index 00000000..5da29e52 --- /dev/null +++ b/test/integration/runtime/current/ratelimit/reload.yaml @@ -0,0 +1,16 @@ +domain: reload +descriptors: + - key: key1 + rate_limit: + unit: second + requests_per_unit: 50 + + - key: block + rate_limit: + unit: second + requests_per_unit: 0 + + - key: one_per_minute + rate_limit: + unit: minute + requests_per_unit: 1