From 938cded4d0f7606910aa25dbacd14fcf4c535ad0 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Mon, 31 Jul 2023 11:52:51 -0400 Subject: [PATCH] scheduler: fix panic in `render_templates` destructive update check (#18100) In #18054 we introduced a new field `render_templates` in the `restart` block. Previously changes to the `restart` block were always non-destructive in the scheduler but we now need to check the new field so that we can update the template runner. The check assumed that the block was always non-nil, which causes panics in our scheduler tests. --- scheduler/util.go | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/scheduler/util.go b/scheduler/util.go index 9dc0cf76f1e0..dde0b67955d1 100644 --- a/scheduler/util.go +++ b/scheduler/util.go @@ -260,8 +260,9 @@ func tasksUpdated(jobA, jobB *structs.Job, taskGroup string) comparison { // Check if restart.render_templates is updated // this requires a destructive update for template hook to receive the new config - if a.RestartPolicy.RenderTemplates != b.RestartPolicy.RenderTemplates { - return difference("group restart render_templates", a.RestartPolicy.RenderTemplates, b.RestartPolicy.RenderTemplates) + if c := renderTemplatesUpdated(a.RestartPolicy, b.RestartPolicy, + "group restart render_templates"); c.modified { + return c } // Check each task @@ -327,9 +328,11 @@ func tasksUpdated(jobA, jobB *structs.Job, taskGroup string) comparison { } // Check if restart.render_templates is updated - if at.RestartPolicy.RenderTemplates != bt.RestartPolicy.RenderTemplates { - return difference("task restart render_templates", at.RestartPolicy.RenderTemplates, bt.RestartPolicy.RenderTemplates) + if c := renderTemplatesUpdated(at.RestartPolicy, bt.RestartPolicy, + "task restart render_templates"); c.modified { + return c } + } // none of the fields that trigger a destructive update were modified, @@ -574,6 +577,23 @@ func spreadsUpdated(jobA, jobB *structs.Job, taskGroup string) comparison { return same } +// renderTemplatesUpdated returns the difference in the RestartPolicy's +// render_templates field, if set +func renderTemplatesUpdated(a, b *structs.RestartPolicy, msg string) comparison { + + noRenderA := a == nil || !a.RenderTemplates + noRenderB := b == nil || !b.RenderTemplates + + if noRenderA && !noRenderB { + return difference(msg, false, true) + } + if !noRenderA && noRenderB { + return difference(msg, true, false) + } + + return same // both nil, or one nil and the other false +} + // setStatus is used to update the status of the evaluation func setStatus(logger log.Logger, planner Planner, eval, nextEval, spawnedBlocked *structs.Evaluation,