From 79906a016448aa7ac787d9d002d02e76bd71d1d6 Mon Sep 17 00:00:00 2001 From: David Reiss Date: Fri, 27 Sep 2024 14:40:33 -0700 Subject: [PATCH] Allow embedding dynamic config in static config --- common/config/config.go | 4 ++ common/config/loader.go | 19 +++++- common/config/loader_test.go | 34 +++++------ common/dynamicconfig/file_based_client.go | 58 ++++++++++++------- .../dynamicconfig/file_based_client_test.go | 27 +++++++++ temporal/server_impl.go | 3 +- 6 files changed, 104 insertions(+), 41 deletions(-) diff --git a/common/config/config.go b/common/config/config.go index 9f3d8b59034..f9684fdfa4a 100644 --- a/common/config/config.go +++ b/common/config/config.go @@ -65,6 +65,10 @@ type ( // DynamicConfigClient is the config for setting up the file based dynamic config client // Filepath should be relative to the root directory DynamicConfigClient *dynamicconfig.FileBasedClientConfig `yaml:"dynamicConfigClient"` + // If DynamicConfig is present, it is used as the dynamic config for the server (and + // reloaded periodically if dynamicConfigClient.pollInterval is set). This conflicts + // with dynamicConfigClient.filepath. + DynamicConfig any `yaml:"dynamicConfig"` // NamespaceDefaults is the default config for every namespace NamespaceDefaults NamespaceDefaults `yaml:"namespaceDefaults"` // ExporterConfig allows the specification of process-wide OTEL exporters diff --git a/common/config/loader.go b/common/config/loader.go index aeefb87a272..565276d8fb4 100644 --- a/common/config/loader.go +++ b/common/config/loader.go @@ -25,10 +25,12 @@ package config import ( + "errors" "fmt" stdlog "log" "os" + "go.temporal.io/server/common/dynamicconfig" "gopkg.in/validator.v2" "gopkg.in/yaml.v3" ) @@ -71,7 +73,7 @@ const ( // base.yaml // env.yaml -- environment is one of the input params ex-development // env_az.yaml -- zone is another input param -func Load(env string, configDir string, zone string, config interface{}) error { +func Load(env string, configDir string, zone string, config *Config) error { if len(env) == 0 { env = envDevelopment } @@ -103,6 +105,21 @@ func Load(env string, configDir string, zone string, config interface{}) error { } } + // Inject self-dynamic-config-client if dynamicConfig section present. + if config.DynamicConfig != nil { + if config.DynamicConfigClient == nil { + config.DynamicConfigClient = &dynamicconfig.FileBasedClientConfig{} + } + if config.DynamicConfigClient.Filepath != "" { + return errors.New("dynamicConfig can't be used with dynamicConfigClient.filepath") + } else if len(files) > 1 { + return errors.New("dynamicConfig can't be used when merging mulitple static config files") + } + + config.DynamicConfigClient.Filepath = files[0] + config.DynamicConfigClient.BelowDynamicConfigKey = true + } + return validator.Validate(config) } diff --git a/common/config/loader_test.go b/common/config/loader_test.go index bba40d731a1..604cb238078 100644 --- a/common/config/loader_test.go +++ b/common/config/loader_test.go @@ -40,15 +40,6 @@ type ( *require.Assertions suite.Suite } - - itemsConfig struct { - Item1 string `yaml:"item1"` - Item2 string `yaml:"item2"` - } - - testConfig struct { - Items itemsConfig `yaml:"items"` - } ) func TestLoaderSuite(t *testing.T) { @@ -71,11 +62,11 @@ func (s *LoaderSuite) TestBaseYaml() { for _, env := range envs { for _, zone := range zones { - var cfg testConfig + var cfg Config err = Load(env, dir, zone, &cfg) s.Nil(err) - s.Equal("hello__", cfg.Items.Item1) - s.Equal("world__", cfg.Items.Item2) + s.Equal("hello__", cfg.Log.Level) + s.Equal("world__", cfg.Log.Format) } } } @@ -106,16 +97,16 @@ func (s *LoaderSuite) TestHierarchy() { } for _, tc := range testCases { - var cfg testConfig + var cfg Config err := Load(tc.env, dir, tc.zone, &cfg) s.Nil(err) - s.Equal(tc.item1, cfg.Items.Item1) - s.Equal(tc.item2, cfg.Items.Item2) + s.Equal(tc.item1, cfg.Log.Level) + s.Equal(tc.item2, cfg.Log.Format) } } func (s *LoaderSuite) TestInvalidPath() { - var cfg testConfig + var cfg Config err := Load("prod", "", "", &cfg) s.NotNil(err) } @@ -129,7 +120,12 @@ func buildConfig(env, zone string) string { item1 := concat("hello", concat(env, zone)) item2 := concat("world", concat(env, zone)) return ` - items: - item1: ` + item1 + ` - item2: ` + item2 + log: + level: ` + item1 + ` + format: ` + item2 + ` + # make validator happy: + persistence: + defaultStore: dummy + numHistoryShards: 10 +` } diff --git a/common/dynamicconfig/file_based_client.go b/common/dynamicconfig/file_based_client.go index a7c29b4adb1..f7b36b3f948 100644 --- a/common/dynamicconfig/file_based_client.go +++ b/common/dynamicconfig/file_based_client.go @@ -63,6 +63,10 @@ type ( FileBasedClientConfig struct { Filepath string `yaml:"filepath"` PollInterval time.Duration `yaml:"pollInterval"` + // If this is false (default), then the top level of the file is expected to contain + // dynamic config settings. If it's true, then the top level should have a key + // "dynamicConfig" which should have settings under that. + BelowDynamicConfigKey bool `yaml:"belowDynamicConfigKey"` } configValueMap map[string][]ConstrainedValue @@ -94,7 +98,7 @@ type ( ) func ValidateFile(contents []byte) *LoadResult { - _, lr := loadFile(contents) + _, lr := loadFile(contents, false) return lr } @@ -153,21 +157,23 @@ func (fc *fileBasedClient) init() error { return fmt.Errorf("unable to read dynamic config: %w", err) } - go func() { - ticker := time.NewTicker(fc.config.PollInterval) - for { - select { - case <-ticker.C: - err := fc.Update() - if err != nil { - fc.logger.Error("Unable to update dynamic config.", tag.Error(err)) + if fc.config.PollInterval > 0 { + go func() { + ticker := time.NewTicker(fc.config.PollInterval) + for { + select { + case <-ticker.C: + err := fc.Update() + if err != nil { + fc.logger.Error("Unable to update dynamic config.", tag.Error(err)) + } + case <-fc.doneCh: + ticker.Stop() + return } - case <-fc.doneCh: - ticker.Stop() - return } - } - }() + }() + } return nil } @@ -189,7 +195,7 @@ func (fc *fileBasedClient) Update() error { return fmt.Errorf("dynamic config file: %s: %w", fc.config.Filepath, err) } - newValues, lr := loadFile(contents) + newValues, lr := loadFile(contents, fc.config.BelowDynamicConfigKey) for _, e := range lr.Errors { fc.logger.Warn("dynamic config error", tag.Error(e)) } @@ -221,15 +227,27 @@ func (fc *fileBasedClient) Update() error { return nil } -func loadFile(contents []byte) (configValueMap, *LoadResult) { +func loadFile(contents []byte, belowDynamicConfigKey bool) (configValueMap, *LoadResult) { lr := &LoadResult{} - var yamlValues map[string][]struct { + type valueMap map[string][]struct { Constraints map[string]any Value any } - if err := yaml.Unmarshal(contents, &yamlValues); err != nil { - return nil, lr.errorf("decode error: %w", err) + var yamlValues valueMap + + if belowDynamicConfigKey { + var topLevel struct { + DynamicConfig valueMap `yaml:"dynamicConfig"` + } + if err := yaml.Unmarshal(contents, &topLevel); err != nil { + return nil, lr.errorf("decode error: %w", err) + } + yamlValues = topLevel.DynamicConfig + } else { + if err := yaml.Unmarshal(contents, &yamlValues); err != nil { + return nil, lr.errorf("decode error: %w", err) + } } newValues := make(configValueMap, len(yamlValues)) @@ -276,7 +294,7 @@ func (fc *fileBasedClient) validateStaticConfig(config *FileBasedClientConfig) e if _, err := fc.reader.GetModTime(); err != nil { return fmt.Errorf("dynamic config: %s: %w", config.Filepath, err) } - if config.PollInterval < minPollInterval { + if config.PollInterval > 0 && config.PollInterval < minPollInterval { return fmt.Errorf("poll interval should be at least %v", minPollInterval) } return nil diff --git a/common/dynamicconfig/file_based_client_test.go b/common/dynamicconfig/file_based_client_test.go index e3b8d2036bd..8a605086f6e 100644 --- a/common/dynamicconfig/file_based_client_test.go +++ b/common/dynamicconfig/file_based_client_test.go @@ -665,3 +665,30 @@ testGetBoolPropertyKey: } s.Equal(3, found) } + +func (s *fileBasedClientSuite) TestBelowDynamicConfigKey() { + data := []byte(` +dynamicConfig: + setting1: + - value: 1000 +`) + ctrl := gomock.NewController(s.T()) + doneCh := make(chan interface{}) + reader := dynamicconfig.NewMockFileReader(ctrl) + reader.EXPECT().GetModTime().Return(time.Now(), nil).AnyTimes() + reader.EXPECT().ReadFile().Return(data, nil).AnyTimes() + + client, err := dynamicconfig.NewFileBasedClientWithReader( + reader, + &dynamicconfig.FileBasedClientConfig{ + Filepath: "dummy", + BelowDynamicConfigKey: true, + }, + log.NewNoopLogger(), + doneCh) + s.NoError(err) + defer close(doneCh) + + s.NotNil(client.GetValue(dynamicconfig.Key("setting1"))) + s.Nil(client.GetValue(dynamicconfig.Key("setting2"))) +} diff --git a/temporal/server_impl.go b/temporal/server_impl.go index 73bcdeb403d..f773c233fea 100644 --- a/temporal/server_impl.go +++ b/temporal/server_impl.go @@ -102,7 +102,8 @@ func NewServerFxImpl( func (s *ServerImpl) Start(ctx context.Context) error { s.logger.Info("Starting server for services", tag.Value(s.so.serviceNames)) - s.logger.Debug(s.so.config.String()) + // Using NewAnyTag on config (which is a fmt.Stringer) avoids marshaling unless it's needed. + s.logger.Debug("Using static config", tag.NewAnyTag("static-config", s.so.config)) if err := initSystemNamespaces( ctx,