Skip to content

Commit

Permalink
Allow embedding dynamic config in static config
Browse files Browse the repository at this point in the history
  • Loading branch information
dnr committed Sep 27, 2024
1 parent e125ed5 commit 79906a0
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 41 deletions.
4 changes: 4 additions & 0 deletions common/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 18 additions & 1 deletion common/config/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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")

Check failure on line 114 in common/config/loader.go

View workflow job for this annotation

GitHub Actions / lint

do not define dynamic errors, use wrapped static errors instead: "errors.New(\"dynamicConfig can't be used with dynamicConfigClient.filepath\")" (err113)
} else if len(files) > 1 {
return errors.New("dynamicConfig can't be used when merging mulitple static config files")

Check failure on line 116 in common/config/loader.go

View workflow job for this annotation

GitHub Actions / lint

do not define dynamic errors, use wrapped static errors instead: "errors.New(\"dynamicConfig can't be used when merging mulitple static config files\")" (err113)
}

config.DynamicConfigClient.Filepath = files[0]
config.DynamicConfigClient.BelowDynamicConfigKey = true
}

return validator.Validate(config)
}

Expand Down
34 changes: 15 additions & 19 deletions common/config/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
}
}
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
`
}
58 changes: 38 additions & 20 deletions common/dynamicconfig/file_based_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -94,7 +98,7 @@ type (
)

func ValidateFile(contents []byte) *LoadResult {
_, lr := loadFile(contents)
_, lr := loadFile(contents, false)
return lr
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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))
}
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions common/dynamicconfig/file_based_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")))
}
3 changes: 2 additions & 1 deletion temporal/server_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 79906a0

Please sign in to comment.