rc: improve NAME_setup handling #1258
Conversation
Pull requests are happening because too many phabs fell on the floor like this. I totally understand your frustration. |
And does this still work correctly? |
|
It did appear to do the right thing but let me double check on Monday and report back. |
| if [ "$rc_arg" = 'start' -o \ | ||
| "$rc_arg" = 'restart' -o \ | ||
| "$rc_arg" = 'reload' ]; then |
There was a problem hiding this comment.
just for reference I'm thinking about making this configurable because www/caddy also has a "reloadssl" option which requires setup to be run but will be missed here because it's a hand-rolled command
There was a problem hiding this comment.
In the case of caddy it requires dealing with this extended reload concept :
www/caddy/files/caddy.in:reloadssl_cmd="caddy_execute reload --force ${caddy_flags}"
So when we are in this condition here we also check for existence of reloadssl_setup="YES" with a if ! checkyesno $_setup; then -- it requires fixing affected ports but only since the port was extended in this particular way.
I think it's fair to assume this only pertains to custom $_cmd definitions. For general service handling start, restart and reload do the right thing since they are the unmodified standard commands then and shouldn't be disabled, but it could also be added for consistency.
|
Ok this works as expected. Shall I bump the date in the manual page? |
|
Yes. I may land this on the plane or at the hotel tonight. Generally i tell people not to do that, but in this case it's fine and would help me. Also, if you could add a signed-off-by line that would be great since you are pushing anyway. It's desired, but not mandatory though. Thanks. |
fichtner
force-pushed
the
rc_setup
branch
2 times, most recently
from
db1c6de
to
50f72cc
Compare
|
@bsdimp done, all yours |
|
OK. One more sanity check and I'll commit. Too much for today, though, to get squared away after travel. |
Reload is used for service reconfiguration as well and lacks a NAME_prepend-like mechanism so it makes sense to extend the NAME_reload hook into this action. precmd may use configuration checks and blocks setup from doing its designated work (e.g. nginx). In moving the invoke of the setup script in front allows us to provide custom scripts for config file generation and fixing prior to precmd checking configuration integrity. Also introduce _run_rc_setup to separate the launcher from the main one. Let it run correctly in the case of restart_precmd and block further execution as would be the case in start due to the internal plumbing of restart being split into calling stop and start afterwards. Differential-Revsiion: https://reviews.freebsd.org/D36259 Signed-off-by: Franco Fichtner <franco@opnsense.org> Reviewed by: imp, oshogbo Pull Request: freebsd#1258
Reload is used for service reconfiguration as well and lacks a NAME_prepend-like mechanism so it makes sense to extend the NAME_reload hook into this
action.
precmd may use configuration checks and blocks setup from doing its designated work (e.g. nginx). In moving the invoke of the setup script in front allows us to provide custom scripts for config file generation and fixing prior to precmd checking configuration integrity.
Also introduce _run_rc_setup to separate the launcher from the main one. Let it run correctly in the case of restart_precmd and block further execution as
would be the case in start due to the internal plumbing of restart being split into calling stop and start afterwards.
Also see: https://reviews.freebsd.org/D36259