Repository navigation
fix(freebsd): make the rc.d scripts start their services - #136
Conversation
The v2.2.0 contrib/freebsd scripts built a daemon(8) command line that
could never run:
- the unquoted two-word title (-t ${desc}) split after rc.subr's eval,
so daemon(8) ran "hub" / "vantage" as the program;
- rc.subr passes ${name}_flags to $command, so smoked_flags
(default -downsample) reached daemon(8), which rejected it;
- ${name}_user made rc.subr su to smoke, so the unprivileged daemon(8)
could not create /var/run/<name>.pid, and it was also given -u smoke.
Follow the ports convention (sysutils/node_exporter,
net-mgmt/prometheus2): rc.subr runs daemon(8) as ${name}_user with no -u,
the supervisor pidfile (-P, so stop does not trigger a -r restart) lives
in a /var/run/<name>/ directory that user owns, made in start_precmd, and
the title and syslog tag are the single-word service name. Extra flags
move to smoked_args / smoke_agent_args; a leftover *_flags value is still
used, with a warning.
Also fix smoke_agent_spool="" (documented to disable the spool): the
":=" default replaced an empty value, so the agent always spooled.
smoked_args="" likewise now drops the default -downsample.
CI: the freebsd-test job now runs .github/scripts/freebsd-rc-test.sh in
its FreeBSD VM. It installs both scripts, drives start/status/restart/
stop through service(8) as the smoke account, probes smoked's API, and
checks process users, argv, the DSN-in-environment path, syslog tags,
daemon(8) restart-on-crash and pidfile cleanup.
# Conflicts: # CHANGELOG.md
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 35 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review of
|
Without -f, daemon(8) passes noclose=1 to daemon(3), so the long-lived supervisor keeps the stdout/stderr of whatever ran `service <name> start`. A caller capturing that output (the CI rc test's $(service smoked onerestart 2>&1), or any script/pipe) then blocks until the service stops. -f points the supervisor's stdio at /dev/null; -T still sends the child's output to syslog. Matches the node_exporter/prometheus ports.
|
Codex P1 ( |
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
1 participant
Problem
The
contrib/freebsdrc.d scripts shipped in v2.2.0 could not start either service. There were three independent blockers, each confirmed by driving the scripts through the official FreeBSD 14.3rc.subrwith command execution intercepted:-t ${desc}was unquoted.desc="Heliograph hub"became-t Heliographafter rc.subr'seval, andhubbecame daemon(8)'s child program.${name}_flagsis rc.subr's reservedrc_flags, which is inserted between$commandand$command_args. The hub's defaultsmoked_flags=-downsampletherefore went to/usr/sbin/daemonitself (illegal option -- d).${name}_user=smokemakes rc.subr wrap the start insu -m smoke, so daemon(8) ran assmokeand could not create/var/run/<name>.pid, while the script also passed-u smoke.There was also a fourth bug:
: ${smoke_agent_spool:="…"}replaces an empty value too, so the documentedsmoke_agent_spool=""never disabled the spool.The existing CI only ran
sh -n, which cannot catch any of these.Fix
This follows the ports convention used by
sysutils/node_exporterandnet-mgmt/prometheus2:${name}_userruns daemon(8) and the child unprivileged; there is no-u.-P) lives in/var/run/<name>/, owned by the service user and created instart_precmd. The supervisor can create it and remove it on exit, andservice stopsignals the supervisor rather than the child, which-rwould otherwise restart.-t/-T) are the single word${name}. The defaultprocname(/usr/sbin/daemon) matches thedaemon: <name>[pid]title for status/stop.${name}_args. A leftover${name}_flagsvalue from v2.2.0 is migrated to_argswith a warning, so it no longer reaches daemon(8).smoke_agent_spool=""andsmoked_args=""now take effect: they use=rather than:=.SMOKED_DSNenvironment variable, never argv; the agent always gets an explicit-spool-dir=; the spool dir is created in precmd.Emitted start line, before and after (rc.subr harness):
Real verification in CI
freebsd-testnow also runs.github/scripts/freebsd-rc-test.shinside the FreeBSD VM. It installs the binaries and rc.d scripts as the README describes, creates thesmokeuser, and checks throughservice(8):onestartbrings up/api/targetswith the demo targets, andonestatuspasses. The supervisor and the child both run assmoke, the child's argv is exact, and its output reaches syslog under the tagsmoked.smoked_flags:onerestartwith it set gives a new supervisor, the warning, and the flag on smoked.procstat -eshowsSMOKED_DSNin the child's environment andpgrep -ffinds it on no command line.onestop,onestatusfails, no process is left, and the pidfile is gone.smoke,spool.lockis owned bysmoke, argv and the syslog tag are right,smoke_agent_spool=""yields-spool-dir=, andonestopcleans up.