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

With "wait" consul-template spawns the child process before rendering the template #1229

Closed
aryklein opened this issue Jul 15, 2019 · 6 comments · Fixed by #1241
Closed

With "wait" consul-template spawns the child process before rendering the template #1229

aryklein opened this issue Jul 15, 2019 · 6 comments · Fixed by #1241
Assignees
Labels
Milestone

Comments

@aryklein
Copy link

aryklein commented Jul 15, 2019

Consul Template version

consul-template v0.20.0
(The same setup works fine in consul-template v0.19.5)

Configuration

/etc/consul-template.conf:

consul {
  address = "consul.query.consul:28500"
  retry {
    enabled = true
    attempts = 12
    backoff = "250ms"
    max_backoff = "1m"
  }
}
reload_signal = "SIGHUP"
kill_signal = "SIGINT"
log_level = "warn"
pid_file = "/var/run/consul-template/consul-template.pid"
exec {
  command = "/usr/local/bin/pgpool -n -D"
  kill_signal = "SIGINT"
  kill_timeout = "10s"
}
template {
  source = "/usr/local/etc/pgpool.conf.tpl"
  destination = "/usr/local/etc/pgpool.conf"
  wait {
    min = "5s"
    max = "10s"
  }
}

Command

consul-template -config /etc/consul-template.conf

Expected behavior

After the wait time, it should render the template and then spawn the child process.

Actual behavior

It spawns the child process before rendering the template (only when you use wait in the template block). It seems that it executes the command in the exec block and then waits for rendering the template.

@eikenb
Copy link
Contributor

eikenb commented Jul 16, 2019

Hey @aryklein, thanks for reporting this issue.

I think this might be a duplicate of the issue fixed with #1227. So if you want to try out a build off the master branch I think it might already be fixed. If you get a chance to try, I'd appreciate hearing back if it is fixed or not.

Thanks.

@aryklein
Copy link
Author

Hey @eikenb,

Sure thing. I was using the pre-compiled versions from here: https://releases.hashicorp.com/consul-template
Can you give me some instructions how to compile it from the master branch?

Thanks

@eikenb
Copy link
Contributor

eikenb commented Jul 18, 2019

To build the master branch you'd need a Go dev setup and then you could run make dev to build it locally. But getting that all setup takes some work.

Thinking about it I'm not 100% this is a replication of that issue though. If you have a chance to spend some time on this I think, instead of doing the work to build it yourself, it'd be better if you could try to create a standalone, simple way to replicate the issue. Something I could run locally with minimal setup.

If you don't have time for that would you mind adding your template here so I have all the parts involved. It's hard to get a good idea of what's happening without all the setup.

Thanks.

@eikenb
Copy link
Contributor

eikenb commented Jul 25, 2019

I don't think this is a duplicate of #1227 as I'm able to reproduce it with a simple test using current master (that has the fix).

This (and a better understanding of the code) also makes me think that #1209 is a case of this same issue and not related to #1227.

@eikenb
Copy link
Contributor

eikenb commented Jul 26, 2019

Most obvious fix for this is to not have it setup the wait/quiescence timers until all templates have been rendered once. The most obvious way to do that is to move the quiescence initialization code down a few lines into the existing conditional block, conditional on allTemplateRendered().

I.E. Moving this chunk...

NEXT_Q:
for _, t := range r.templates {
if _, ok := r.quiescenceMap[t.ID()]; ok {
continue NEXT_Q
}
for _, c := range r.templateConfigsFor(t) {
if *c.Wait.Enabled {
log.Printf("[DEBUG] (runner) enabling template-specific quiescence for %q", t.ID())
r.quiescenceMap[t.ID()] = newQuiescence(
r.quiescenceCh, *c.Wait.Min, *c.Wait.Max, t)
continue NEXT_Q
}
}
if *r.config.Wait.Enabled {
log.Printf("[DEBUG] (runner) enabling global quiescence for %q", t.ID())
r.quiescenceMap[t.ID()] = newQuiescence(
r.quiescenceCh, *r.config.Wait.Min, *r.config.Wait.Max, t)
continue NEXT_Q
}
}

To just after here...

if r.allTemplatesRendered() {
log.Printf("[DEBUG] (runner) all templates rendered")

@eikenb eikenb self-assigned this Jul 26, 2019
eikenb added a commit that referenced this issue Jul 26, 2019
Hold off on initializing wait/quiescence until all templates have been
rendered once. This fixes an issue with -exec firing before the template
has been rendered and would also make sure templates are rendered for
once (if that wasn't already fixed another way).

Fixes #1229
@eikenb eikenb added the bug label Jul 26, 2019
@eikenb eikenb added this to the v0.20.1 milestone Jul 26, 2019
@aryklein
Copy link
Author

@eikenb thanks for taking care of this issue 👍

eikenb added a commit that referenced this issue Jul 29, 2019
Hold off on initializing wait/quiescence until all templates have been
rendered once. This fixes an issue with -exec firing before the template
has been rendered when wait/quiescence is set, and it would also make
sure templates are rendered for once (if that wasn't already fixed
another way).

Fixes #1229
eikenb added a commit that referenced this issue Jul 29, 2019
Hold off on initializing wait/quiescence until all templates have been
rendered once. This fixes an issue with -exec firing before the template
has been rendered when wait/quiescence is set, and it would also make
sure templates are rendered for once (if that wasn't already fixed
another way).

Fixes #1229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants