Skip to content

init: add --force to ExecReload#84

Merged
mholt merged 1 commit into
caddyserver:masterfrom
jian-lin:init-force-reload
Jun 28, 2022
Merged

init: add --force to ExecReload#84
mholt merged 1 commit into
caddyserver:masterfrom
jian-lin:init-force-reload

Conversation

@jian-lin

Copy link
Copy Markdown
Contributor

This caddy commit1 adds --force flag, which helps to reload tls
certificates from disk when the config does not change.

This patch add this flag to ExecReload so that we can get this benefit
when using systemctl reload.

This caddy commit[1] adds --force flag, which helps to reload tls
certificates from disk when the config does not change.

This patch add this flag to ExecReload so that we can get this benefit
when using systemctl reload.

[1]: caddyserver/caddy@2772ede
@mholt

mholt commented Jun 27, 2022

Copy link
Copy Markdown
Member

Hmm, is there a way to set that flag on the systemctl command? It would be unfortunate if every reload always was forced even if not necessary or intentional...

@jian-lin

jian-lin commented Jun 27, 2022

Copy link
Copy Markdown
Contributor Author

is there a way to set that flag on the systemctl command

I don't think so.

My use case is that when my acme cert is renewed, the acme service runs systemctl try-reload-or-restart caddy.service to try to let caddy use the new cert. Without --force, caddy still uses the old cert.

It would be unfortunate if every reload always was forced even if not necessary or intentional

I agree if the cost is not small. IMHO, reload will not be frequent in performance-critical case.

@francislavoie

francislavoie commented Jun 28, 2022

Copy link
Copy Markdown
Member

Why aren't you using Caddy for ACME instead? You can reload Caddy directly by running the caddy reload command yourself outside of the systemd service.

@jian-lin

Copy link
Copy Markdown
Contributor Author

Well, that's a cert for my email server. Currently, I use lego and caddy together to manage that cert. It seems that if only caddy is used, it's not easy to notify my email server when the cert is renewed.

@jian-lin

Copy link
Copy Markdown
Contributor Author

I admit it's not a common use case and I am fine if you close this pr.

@mholt

mholt commented Jun 28, 2022

Copy link
Copy Markdown
Member

I'm probably ok with merging it. I don't think it'll cause a problem for most users. Maybe none at all!

@mholt mholt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go ahead and try this. If we get (enough, or serious enough) complaints, we can revert; but I think it makes sense that if you explicitly tell the service to reload, to always reload, not just "try" to reload. If you call caddy reload directly then you can customize the flags.

@mholt mholt merged commit 979e498 into caddyserver:master Jun 28, 2022
@jian-lin jian-lin deleted the init-force-reload branch June 28, 2022 02:42
@francislavoie

Copy link
Copy Markdown
Member

FWIW, you can also override the systemd service if necessary: https://caddyserver.com/docs/running#overrides

@mholt

mholt commented Jun 28, 2022

Copy link
Copy Markdown
Member

Yeah, and maybe that's better for this. We'll find out, based on feedback we get in the future.

@jian-lin

Copy link
Copy Markdown
Contributor Author

FWIW, you can also override the systemd service if necessary

Yeah, I know that. Thanks.
I make this pr because I think the community may benefit from it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants