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

Config Merger only reads configuration on startup; should be more often #517

Open
chases2 opened this issue Jun 23, 2021 · 0 comments
Open
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@chases2
Copy link
Collaborator

chases2 commented Jun 23, 2021

The Config Merger is a controller that takes multiple configurations and merges them together.
It requires a list of configurations to merge together.

list, err := merger.ParseAndCheck(file)

It then periodically reads those configs and merges them together. Changes in the config are picked up in this loop, but changes to the config list are not.

updateOnce := func(ctx context.Context) {
start := time.Now()
ctx, cancel := context.WithTimeout(ctx, 10*time.Minute)
defer cancel()
log.Info("Starting MergeAndUpdate")
err := merger.MergeAndUpdate(ctx, client, list, opt.skipValidate, opt.confirm)
cycle.Set(int64(time.Since(start).Seconds()), "config_merger")
if err != nil {
log.WithError(err).Error("Update failed")
errors.Add(1, "config_merger")
return
}
successes.Add(1, "config_merger")
log.Info("Update successful")
}

This has caused issues when the config is changed, and Config Merger silently ignores those changes until it's rebooted. See GoogleCloudPlatform/oss-test-infra#919

Additional logic to determine if the config list has changed and to either log this event or do less work if it hasn't changed would also be nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant