Page MenuHomeFreeBSD

Add rc.conf support for foo_daemon="-r".
Needs ReviewPublic

Authored by trasz on Aug 11 2016, 3:21 PM.

Details

Reviewers
koobs
Summary

Add rc.conf support for foo_daemon="-r".

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4864
Build 4927: arc lint + arc unit

Event Timeline

trasz updated this revision to Diff 19208.Aug 11 2016, 3:21 PM
trasz retitled this revision from to Add rc.conf support for foo_daemon="-r"..
trasz updated this object.
trasz edited the test plan for this revision. (Show Details)
trasz added a reviewer: koobs.Aug 11 2016, 3:26 PM
koobs edited edge metadata.Aug 11 2016, 3:28 PM

@trasz Very nice, and thanks for giving this a crack :)

koobs added a comment.Aug 11 2016, 3:36 PM

Minimally, I see two primary use cases for this:

  1. Use daemon without any supervision. Useful for non-backgroundable/detachable softwares/ports, and offers users a dont touch my processes choice.
  2. Use daemon with supervision (-r), and/or other flags. Useful for the 80/20 case of simple process supervision.

I don't mind, but I'm not entirely sure yet that foo_daemon="-bar" is the way to go vs something like foo_daemon_{enable,flags}= or as you said foo_daemon=yes|supervise, except the latter would preclude the ability for users/porters to customise args/flags to daemon.

Yeah, I think it should be split between "foo_daemon" set by individual rc scripts and "foo_restart=YES" settable in rc.conf. This way the sysadmin would have control over whether the restart behaviour is enabled or not, while the rc scripts could adjust other flags to match. In particular, the rc script should disable any internal daemonization mechanisms the software they control might have (ie /etc/rc.d/sshd could add '-D' to sshd_flags), and also change the pidfile paths, so that they point to the pidfile created by daemon(8) instead of the pidfiles created by the software; otherwise it's impossible to stop the service - the "stop" command stops the process, but daemon(8) immediately restarts it.

koobs added a comment.EditedAug 17 2016, 11:57 AM

All mostly makes sense, except foo_restart -> foo_supervise (naming things, one of two hard problems).

Also, is there really a distinction between foo_daemon being set in rc.conf versus ONLY in scripts as far as /etc/rc (framework) is concerned, or are we talking about merely a documenting thing?

If there's nothing stopping anyone (port maintainer, sysadmin, user) from adding foo_daemon to rc.conf, I wouldn't be doing anything to specifically preclude doing that in rc.

For example, whats wrong with etc/rc.conf:

foo_daemon='yes'    # daemonise/wrap this undaemonisable/foreground only thing
foo_supervise='yes' # also supervise it (restart if it dies)

Or is this just about us not liking two orthogonal looking variable names that are actually cumulative/complementary? ie: You can't _supervise unless you first/also _daemon

I believe this was solved In The Past™ by:

foo_daemon_enable
foo_daemon_supervise
trasz added a comment.Aug 17 2016, 7:30 PM

I'm talking about the intended usage, a convention. It would be possible to set both in rc.conf, but the user should make sure they know what they're doing; otherwise they would risk overwriting the variable already set by an rc script.

trasz added a comment.Aug 17 2016, 7:37 PM

One thing I'm not sure how to do at this point, and I'd like to get this done before this stuff gets committed, to make sure the infrastructure below is actually usable in the intended way, is to modify the sshd rc script to use it. Basically, if "sshd_supervise" is set to YES in rc.conf, the script would have to set "sshd_daemon" to "-r -P /var/pid/sshd-daemon.pid", set "pidfile" to "/var/run/sshd-daemon.pid", and append "-D" to "sshd_flags". Do we have any convention about changing the rc variables this way?

trasz updated this revision to Diff 19485.Aug 19 2016, 9:54 AM
trasz edited edge metadata.

Stuff.

In D7474#156797, @trasz wrote:

I'm talking about the intended usage, a convention. It would be possible to set both in rc.conf, but the user should make sure they know what they're doing; otherwise they would risk overwriting the variable already set by an rc script.

Roger that, that's fine :)

trasz updated this revision to Diff 19490.Aug 19 2016, 12:54 PM

Syslog, cron (cron depends on https://reviews.freebsd.org/D7581).

trasz updated this object.Aug 19 2016, 1:03 PM
koobs added a comment.May 7 2019, 4:56 AM

D7581 committed/closed, can this progress now?