The Wayback Machine - https://web.archive.org/web/20240702232217/https://github.com/caddyserver/caddy/issues/3967
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

Feature request: readd reload on USR1 signal #3967

Closed
chulkilee opened this issue Jan 12, 2021 · 21 comments
Closed

Feature request: readd reload on USR1 signal #3967

chulkilee opened this issue Jan 12, 2021 · 21 comments
Labels
discussion 💬 The right solution needs to be found feature ⚙️ New feature or request

Comments

@chulkilee
Copy link

chulkilee commented Jan 12, 2021

Caddy < 2 had reload on USR1 signal (#107 / 41c4484), but it was dropped in v2 (#2242) (see also discussion)

Although there are other ways to reload the config (using --watch option, or making POST /load request) but it would be nice to have the feature back so that ops can determine when to apply the changes without installing http client.

(edit: also caddy reload is there, but it requires passing config again)

@francislavoie
Copy link
Member

You're aware you can use the caddy reload command, right?

@francislavoie francislavoie added feature ⚙️ New feature or request good first issue 🐤 Good for newcomers labels Jan 12, 2021
@chulkilee
Copy link
Author

Oh yes (will add it to the list) - but it's still simpler for process managers to send a signal instead of forking a process to run caddy reload.

@francislavoie
Copy link
Member

What process manager are you using here? With systemd, we use ExecReload

https://github.com/caddyserver/dist/blob/master/init/caddy.service#L26

@mholt
Copy link
Member

mholt commented Jan 12, 2021

Yeah; sorry, I'm not sure what the motivation is for a posix-only signal when we already have a more flexible cross-platform solution for this; one that is the same experience for the user on linux too: systemctl reload caddy

@chulkilee
Copy link
Author

I'd like to use make nomad to trigger reload when caddyfile changed via template - using change_signal

I may use consul template directly to use template.command but it requires adding consul-template inside caddy docker image and passing consul secrets down to caddy container, which is not desired.

@mholt
Copy link
Member

mholt commented Jan 12, 2021

Can you pass string arguments that way? Because I'm not sure how that's going to work otherwise. Signals aren't powerful enough. Where will Caddy get the config? Without a --config argument, Caddy will have to assume the config is at ./Caddyfile, but that's not often the case except with interactive use.

@mr-karan
Copy link

+1 for Nomad usecase. If there's a signal which can be defined to do exactly what caddy reload does, I guess that should solve the problem?

@mholt
Copy link
Member

mholt commented Feb 11, 2021

@mr-karan But with what arguments/flags? That's the question. AFAIK you can't pass arguments with signals. caddy reload without any args will try to load a Caddyfile in the current working directory, but that's seldom useful in production... I think.

@mr-karan
Copy link

@mholt Not sure if this makes sense but if we hold the config location in memory, we can just reload the existing config with which caddy was running? That's the ideal behaviour of reload IMHO, to basically re-read from the same config file with which it was started.

Prior Art: nginx -s reload and Prometheus Reload work the same way if I'm not wrong.

@mholt
Copy link
Member

mholt commented Feb 12, 2021

@mr-karan That doesn't really make sense to me. :)

if we hold the config location in memory, we can just reload the existing config with which caddy was running?

We can, but reloading the exact same config with no changes has limited utility (like, triggering a manual reload of TLS certs from disk).

That's the ideal behaviour of reload IMHO, to basically re-read from the same config file with which it was started.

This is different, now you want to re-read a file. This is typically the purpose of a config reload, but which file? So does it reload the config in memory or read it from disk?

@mr-karan
Copy link

mr-karan commented Feb 12, 2021

but reloading the exact same config with

We're not reloading the exact config. The config contents changed, the config path didn't change.

This is typically the purpose of a config reload, but which file?

same file with which the caddy server was started.

So does it reload the config in memory or read it from disk?

Config is stored on disk. We can just store the config file-path in memory (like /etc/caddy/Caddyfile) and re-read the contents from disk.

Well this is a typical behavior of hot reloading any process.. please correct if I'm wrong :)

@mholt
Copy link
Member

mholt commented Feb 12, 2021

We can just store the config file-path in memory (like /etc/caddy/Caddyfile) and re-read the contents from disk.

I guess with most servers that is easy, but with Caddy I'm not sure of a clean way to do it yet. Any way I can think of is pretty hacky. Caddy's config doesn't come from files, it comes through the API. Only the command line interface has any knowledge of files, and only at initial startup. That's why the caddy reload command doesn't attempt to "remember" any config files from before, because it's not really safe to assume that the current config running came from a file.

@mholt mholt removed the good first issue 🐤 Good for newcomers label Feb 12, 2021
@mr-karan
Copy link

because it's not really safe to assume that the current config running came from a file.

Ah you're right. I totally missed this 🤔 .

@mholt
Copy link
Member

mholt commented Feb 12, 2021

Yeah, so that's the main problem: "reload" -- reload what? Do you intend to reload a file even though the config may have changed through the API? (By the way, even configs from files get loaded via the API, the CLI just wraps that up for you.) Or do you intend to reload the currently-running config that was POSTed to the API, without changes (because where would you get the changes from)?

Hence the command line arguments; and why, with a signal, it doesn't really make sense since you can't provide args.

@mholt mholt added the discussion 💬 The right solution needs to be found label Feb 12, 2021
@chulkilee
Copy link
Author

Oh, if the server process holds only the final config, not the location of config to look up.. then yes, it has limited utility - only "reevaluating" the existing config.

However, I'm wondering we can use import to mimic the config file reload without passing config again?. For example I can use import sites-enabled/* and, update the file, and then make the reload config.

@optiz0r
Copy link

optiz0r commented Mar 2, 2021

For the nomad use case, I've extended the Caddy container with a bash signal handler that triggers a reload using the Caddyfile which was used when the container was first launched. The code is here (https://github.com/optiz0r/caddy-consul) in case it's useful to anyone else.

@chulkilee
Copy link
Author

chulkilee commented Mar 2, 2021

As there is a workaround and adding the default file location isn't trivial, I'm closing this issue. Thanks all!

@optiz0r
Copy link

optiz0r commented Mar 4, 2021

Is it worth adding the sh script workaround to the pre-built docker image, which is hardcoded to use the Caddyfile on startup? (My workaround script puts non-json text to stdout where it gets mixed up with json-formatted output from caddy, so those echos should probably be dropped).

It embeds the latest version of tini (to avoid the shell script having to bear the pid1 responsibilities), handles SIGHUP, and passes on SIGTERM, SIGINT, SIGQUIT (but could very easily pass on other signals too, e.g. USR1/2 etc).

If you're happy to accept the change, I'll raise it as a PR?

@thijsvandien
Copy link

From what I understand, the main argument against keeping/reintroducing the USR1 signal is that it's ambiguous which config to load. I'd say a good default would be doing that what a restart would've done, but without interrupting anything.

In any case, with admin off, I believe no such ambiguity exists—there is a designated config file with no dynamic changes. At the same time, there is no way to reload it on demand then. Could we at least have a signal handler for this simpler scenario?

milanaleksic added a commit to milanaleksic/caddy-cloudflare that referenced this issue Apr 3, 2022
@rawb1
Copy link

rawb1 commented Apr 10, 2022

@optiz0r your script work really well, thanks 👍
I ended up creating and publishing a docker image using your script :
https://github.com/rawb1/caddy-hcp
https://hub.docker.com/r/rawb1/caddy-hcp

For Nomad another solution is to use caddy dynamic reverse proxy with consul service SRV record so you don't need to reload caddy config but it requires more configuration to setup Consul DNS forwarding https://learn.hashicorp.com/tutorials/consul/dns-forwarding ...

@cycneuramus
Copy link

For the Nomad use case, we can make use of the change_mode parameter of the template block to achieve a clean reload without having to build custom images or manipulate signals:

template {
  data        = file("Caddyfile.tpl")
  destination = "/local/Caddyfile"
  change_mode = "script"
  change_script {
    command = "caddy"
    args    = ["reload", "--config", "/local/Caddyfile", "--adapter", "caddyfile"]
  }
}

<...>

config {
  image = caddy
  entrypoint = ["caddy", "run", "--config", "/local/Caddyfile", "--adapter", "caddyfile"]

<...>
}

(Sorry for commenting on an old issue; I just thought I might drop this hint for future troubleshooters.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion 💬 The right solution needs to be found feature ⚙️ New feature or request
8 participants