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

fix: add quiescenceRun flag to avoid render loops #2010

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions manager/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,12 @@ type Runner struct {
// quiescenceMap is the map of templates to their quiescence timers.
// quiescenceCh is the channel where templates report returns from quiescence
// fires.
// quiescenceRun is the template that was triggered to render via
// its respective timer. This flag is an optimization used to avoid infinite
// rendering when multiple templates exist.
quiescenceMap map[string]*quiescence
quiescenceCh chan *template.Template
quiescenceRun *template.Template

// dedup is the deduplication manager if enabled
dedup *DedupManager
Expand Down Expand Up @@ -474,6 +478,7 @@ func (r *Runner) Start() {
// Remove the quiescence for this template from the map. This will force
// the upcoming Run call to actually evaluate and render the template.
log.Printf("[DEBUG] (runner) received template %q from quiescence", tmpl.ID())
r.quiescenceRun = tmpl
delete(r.quiescenceMap, tmpl.ID())

case c := <-childExitCh:
Expand Down Expand Up @@ -667,6 +672,9 @@ func (r *Runner) Run() error {
}
}

// Always reset quiescenceRun in case this run was triggered by a quiescence timer
r.quiescenceRun = nil

// Perform the diff and update the known dependencies.
r.diffAndUpdateDeps(runCtx.depsMap)

Expand Down Expand Up @@ -892,6 +900,13 @@ func (r *Runner) runTemplate(tmpl *template.Template, runCtx *templateRunCtx) (*
}
}

if r.quiescenceRun != nil && r.quiescenceRun != tmpl {
// During a run triggered via quiescence, mark any template not corresponding to
// the quiescence timer as ForQuiescence, signaling it was purposefully skipped.
event.ForQuiescence = true
return event, nil
}

// If quiescence is activated, start/update the timers and loop back around.
// We do not want to render the templates yet.
if q, ok := r.quiescenceMap[tmpl.ID()]; ok {
Expand Down
110 changes: 110 additions & 0 deletions manager/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,116 @@ func TestRunner_Start(t *testing.T) {
}
})

t.Run("multi-template-no-render-cycle", func(t *testing.T) {
testConsul.SetKVString(t, "multi-template-no-cycle-foo", "bar")

tmpDir := t.TempDir()
out1 := filepath.Join(tmpDir, "out1")
out2 := filepath.Join(tmpDir, "out2")

minWait := 1 * time.Second
maxWait := 10 * time.Second

c := config.DefaultConfig().Merge(&config.Config{
Wait: &config.WaitConfig{
Min: &minWait,
Max: &maxWait,
},
Consul: &config.ConsulConfig{
Address: config.String(testConsul.HTTPAddr),
},
Templates: &config.TemplateConfigs{
&config.TemplateConfig{
Contents: config.String(`{{ key "multi-template-no-cycle-foo" }}`),
Destination: config.String(out1),
},
&config.TemplateConfig{
Contents: config.String(`foobar`),
Destination: config.String(out2),
},
},
})
c.Finalize()

r, err := NewRunner(c, false)
if err != nil {
t.Fatal(err)
}

go r.Start()
defer r.Stop()

// The template with no deps will be available first
mismithhisler marked this conversation as resolved.
Show resolved Hide resolved
select {
case err := <-r.ErrCh:
t.Fatal(err)
case <-r.renderedCh:
act, err := os.ReadFile(out2)
if err != nil {
t.Fatal(err)
}
exp := "foobar"
if exp != string(act) {
t.Errorf("\nexp: %#v\nact: %#v", exp, string(act))
}
case <-time.After(2 * time.Second):
t.Fatal("timeout")
}

// The consul key will finally render
select {
case err := <-r.ErrCh:
t.Fatal(err)
case <-r.renderedCh:
act, err := os.ReadFile(out1)
if err != nil {
t.Fatal(err)
}
exp := "bar"
if exp != string(act) {
t.Errorf("\nexp: %#v\nact: %#v", exp, string(act))
}
case <-time.After(2 * time.Second):
t.Fatal("timeout")
}

// Update the value we are watching, it should update after the quiescence timer fires
testConsul.SetKVString(t, "multi-template-no-cycle-foo", "bar_again")

renderCount := 0
OUTER:
// Wait for the quiescence timer to fire and the value to actually render to disk
for {
select {
case err := <-r.ErrCh:
t.Fatal(err)
case <-r.renderedCh:
// Run() will be called a total of 3 times when the dep is updated.
// The first run will tick both the templates quiescence timers, and return
// early without sending a render event. The next 2 runs will be from both the
// templates quiescence timers firing. We don't want to move on until both of
// those take place.
renderCount++
if renderCount < 2 {
continue
mismithhisler marked this conversation as resolved.
Show resolved Hide resolved
}
break OUTER
case <-time.After(2 * time.Second):
t.Fatal("timeout")
}

}

// Once it updates, it should not render again
select {
case err := <-r.ErrCh:
t.Fatal(err)
case <-r.renderedCh:
t.Fatal("No more renders should occur, there may be a render cycle")
case <-time.After(2 * minWait):
}
})

t.Run("render_in_memory", func(t *testing.T) {
testConsul.SetKVString(t, "render-in-memory", "foo")

Expand Down
Loading