Fix FreeBSD agent restart handling

Refs #1457
This commit is contained in:
rcourtman 2026-05-24 21:51:11 +01:00
parent 0dca8a0375
commit 8600706da3
4 changed files with 168 additions and 17 deletions

View file

@ -555,8 +555,13 @@ func TestGetUpdatedFromVersion(t *testing.T) {
func TestPerformUpdateWrapper(t *testing.T) {
t.Run("ExecPathError", func(t *testing.T) {
origExec := osExecutableFn
t.Cleanup(func() { osExecutableFn = origExec })
origArgs := osArgsFn
t.Cleanup(func() {
osExecutableFn = origExec
osArgsFn = origArgs
})
osExecutableFn = func() (string, error) { return "", errors.New("fail") }
osArgsFn = func() []string { return nil }
u := newUpdaterForTest("http://example")
if err := u.performUpdate(context.Background()); err == nil {
@ -591,6 +596,37 @@ func TestPerformUpdateWrapper(t *testing.T) {
t.Fatalf("expected update success, got %v", err)
}
})
t.Run("FallbackToAbsoluteArgv0", func(t *testing.T) {
data := testBinary()
check := checksum(data)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("X-Checksum-Sha256", check)
_, _ = w.Write(data)
}))
t.Cleanup(server.Close)
_, execPath := writeTempExec(t)
u := newUpdaterForTest(server.URL)
u.client = server.Client()
origExec := osExecutableFn
origArgs := osArgsFn
origRestart := restartProcessFn
t.Cleanup(func() {
osExecutableFn = origExec
osArgsFn = origArgs
restartProcessFn = origRestart
})
osExecutableFn = func() (string, error) { return "", errors.New("no such file or directory") }
osArgsFn = func() []string { return []string{execPath} }
restartProcessFn = func(string) error { return nil }
if err := u.performUpdate(context.Background()); err != nil {
t.Fatalf("expected update success via argv[0] fallback, got %v", err)
}
})
}
func TestPerformUpdateInvalidRequest(t *testing.T) {

View file

@ -44,6 +44,7 @@ var (
qnapPersistentPathFn = qnapPersistentPath
restartProcessFn = restartProcess
osExecutableFn = os.Executable
osArgsFn = func() []string { return os.Args }
evalSymlinksFn = filepath.EvalSymlinks
createTempFn = os.CreateTemp
chmodFn = os.Chmod
@ -346,13 +347,35 @@ func qnapPersistentPath(agentName string) string {
// performUpdate downloads and installs the new agent binary.
func (u *Updater) performUpdate(ctx context.Context) error {
execPath, err := osExecutableFn()
execPath, err := resolveExecutablePath()
if err != nil {
return fmt.Errorf("failed to get executable path: %w", err)
return err
}
return u.performUpdateWithExecPath(ctx, execPath)
}
func resolveExecutablePath() (string, error) {
execPath, err := osExecutableFn()
if err == nil && strings.TrimSpace(execPath) != "" {
return execPath, nil
}
args := osArgsFn()
if len(args) > 0 {
candidate := strings.TrimSpace(args[0])
if filepath.IsAbs(candidate) {
if info, statErr := os.Stat(candidate); statErr == nil && !info.IsDir() {
return candidate, nil
}
}
}
if err == nil {
err = errors.New("empty executable path")
}
return "", fmt.Errorf("failed to get executable path: %w", err)
}
func (u *Updater) performUpdateWithExecPath(ctx context.Context, execPath string) error {
// Build download URL

View file

@ -0,0 +1,37 @@
package api
import (
"os"
"path/filepath"
"strings"
"testing"
)
func TestUnifiedInstallScriptFreeBSDRcDUsesSupervisorPidfile(t *testing.T) {
scriptPath := filepath.Join("..", "..", "scripts", "install.sh")
data, err := os.ReadFile(scriptPath)
if err != nil {
t.Fatalf("read install script: %v", err)
}
script := string(data)
if strings.Contains(script, "/usr/sbin/daemon -r -p ${pidfile}") {
t.Fatal("FreeBSD rc.d templates must not use the child pidfile as the service pidfile with daemon -r")
}
wantDaemon := "/usr/sbin/daemon -r -P ${supervisor_pidfile} -p ${child_pidfile} -f ${command} ${command_args}"
if got := strings.Count(script, wantDaemon); got != 2 {
t.Fatalf("expected both FreeBSD rc.d templates to use supervisor and child pidfiles, got %d", got)
}
for _, required := range []string{
`supervisor_pidfile="/var/run/${name}.pid"`,
`child_pidfile="/var/run/${name}.child.pid"`,
`kill "${supervisor_pid}"`,
`kill "${child_pid}"`,
} {
if !strings.Contains(script, required) {
t.Fatalf("install script missing %q", required)
}
}
}

View file

@ -1900,7 +1900,8 @@ EOF
name="pulse_agent"
rcvar="pulse_agent_enable"
pidfile="/var/run/${name}.pid"
supervisor_pidfile="/var/run/${name}.pid"
child_pidfile="/var/run/${name}.child.pid"
command="RUNTIME_BINARY_PLACEHOLDER"
command_args="EXEC_ARGS_PLACEHOLDER"
@ -1912,18 +1913,38 @@ status_cmd="${name}_status"
pulse_agent_start()
{
if checkyesno ${rcvar}; then
if [ -f "${supervisor_pidfile}" ] && kill -0 "$(cat "${supervisor_pidfile}")" 2>/dev/null; then
echo "${name} is already running as supervisor pid $(cat "${supervisor_pidfile}")."
return 0
fi
echo "Starting ${name}."
SSL_CERT_FILE_PLACEHOLDER
/usr/sbin/daemon -r -p ${pidfile} -f ${command} ${command_args}
rm -f "${supervisor_pidfile}" "${child_pidfile}"
/usr/sbin/daemon -r -P ${supervisor_pidfile} -p ${child_pidfile} -f ${command} ${command_args}
fi
}
pulse_agent_stop()
{
if [ -f ${pidfile} ]; then
if [ -f "${supervisor_pidfile}" ]; then
supervisor_pid=$(cat "${supervisor_pidfile}")
echo "Stopping ${name}."
kill $(cat ${pidfile}) 2>/dev/null
rm -f ${pidfile}
supervisor_parent=$(ps -o ppid= -p "${supervisor_pid}" 2>/dev/null | tr -d '[:space:]')
if [ -n "${supervisor_parent}" ] && [ "${supervisor_parent}" != "1" ]; then
parent_comm=$(ps -o comm= -p "${supervisor_parent}" 2>/dev/null | tr -d '[:space:]')
case "${parent_comm}" in
daemon|*/daemon) kill "${supervisor_parent}" 2>/dev/null || true ;;
esac
fi
kill "${supervisor_pid}" 2>/dev/null || true
sleep 1
if [ -f "${child_pidfile}" ]; then
child_pid=$(cat "${child_pidfile}")
if kill -0 "${child_pid}" 2>/dev/null; then
kill "${child_pid}" 2>/dev/null || true
fi
fi
rm -f "${supervisor_pidfile}" "${child_pidfile}"
else
echo "${name} is not running."
fi
@ -1931,8 +1952,10 @@ pulse_agent_stop()
pulse_agent_status()
{
if [ -f ${pidfile} ] && kill -0 $(cat ${pidfile}) 2>/dev/null; then
echo "${name} is running as pid $(cat ${pidfile})."
if [ -f "${supervisor_pidfile}" ] && kill -0 "$(cat "${supervisor_pidfile}")" 2>/dev/null; then
echo "${name} is running as supervisor pid $(cat "${supervisor_pidfile}")."
elif [ -f "${child_pidfile}" ] && kill -0 "$(cat "${child_pidfile}")" 2>/dev/null; then
echo "${name} child is running without a supervisor as pid $(cat "${child_pidfile}")."
else
echo "${name} is not running."
return 1
@ -2192,6 +2215,15 @@ if [[ "$OS" == "freebsd" ]] || [[ -f /etc/rc.subr ]]; then
RCSCRIPT="/usr/local/etc/rc.d/${AGENT_NAME}"
log_info "Configuring FreeBSD rc.d service at $RCSCRIPT..."
if [[ -x "$RCSCRIPT" ]]; then
log_info "Stopping existing ${AGENT_NAME} service..."
"$RCSCRIPT" stop 2>/dev/null || true
sleep 1
fi
if command -v pkill >/dev/null 2>&1; then
pkill -f "${INSTALL_DIR}/${BINARY_NAME}" 2>/dev/null || true
fi
# Build command line args
build_exec_args
@ -2207,7 +2239,8 @@ if [[ "$OS" == "freebsd" ]] || [[ -f /etc/rc.subr ]]; then
name="pulse_agent"
rcvar="pulse_agent_enable"
pidfile="/var/run/${name}.pid"
supervisor_pidfile="/var/run/${name}.pid"
child_pidfile="/var/run/${name}.child.pid"
# These placeholders are replaced by sed below
command="INSTALL_DIR_PLACEHOLDER/BINARY_NAME_PLACEHOLDER"
@ -2220,18 +2253,38 @@ status_cmd="${name}_status"
pulse_agent_start()
{
if checkyesno ${rcvar}; then
if [ -f "${supervisor_pidfile}" ] && kill -0 "$(cat "${supervisor_pidfile}")" 2>/dev/null; then
echo "${name} is already running as supervisor pid $(cat "${supervisor_pidfile}")."
return 0
fi
echo "Starting ${name}."
SSL_CERT_FILE_PLACEHOLDER
/usr/sbin/daemon -r -p ${pidfile} -f ${command} ${command_args}
rm -f "${supervisor_pidfile}" "${child_pidfile}"
/usr/sbin/daemon -r -P ${supervisor_pidfile} -p ${child_pidfile} -f ${command} ${command_args}
fi
}
pulse_agent_stop()
{
if [ -f ${pidfile} ]; then
if [ -f "${supervisor_pidfile}" ]; then
supervisor_pid=$(cat "${supervisor_pidfile}")
echo "Stopping ${name}."
kill $(cat ${pidfile}) 2>/dev/null
rm -f ${pidfile}
supervisor_parent=$(ps -o ppid= -p "${supervisor_pid}" 2>/dev/null | tr -d '[:space:]')
if [ -n "${supervisor_parent}" ] && [ "${supervisor_parent}" != "1" ]; then
parent_comm=$(ps -o comm= -p "${supervisor_parent}" 2>/dev/null | tr -d '[:space:]')
case "${parent_comm}" in
daemon|*/daemon) kill "${supervisor_parent}" 2>/dev/null || true ;;
esac
fi
kill "${supervisor_pid}" 2>/dev/null || true
sleep 1
if [ -f "${child_pidfile}" ]; then
child_pid=$(cat "${child_pidfile}")
if kill -0 "${child_pid}" 2>/dev/null; then
kill "${child_pid}" 2>/dev/null || true
fi
fi
rm -f "${supervisor_pidfile}" "${child_pidfile}"
else
echo "${name} is not running."
fi
@ -2239,8 +2292,10 @@ pulse_agent_stop()
pulse_agent_status()
{
if [ -f ${pidfile} ] && kill -0 $(cat ${pidfile}) 2>/dev/null; then
echo "${name} is running as pid $(cat ${pidfile})."
if [ -f "${supervisor_pidfile}" ] && kill -0 "$(cat "${supervisor_pidfile}")" 2>/dev/null; then
echo "${name} is running as supervisor pid $(cat "${supervisor_pidfile}")."
elif [ -f "${child_pidfile}" ] && kill -0 "$(cat "${child_pidfile}")" 2>/dev/null; then
echo "${name} child is running without a supervisor as pid $(cat "${child_pidfile}")."
else
echo "${name} is not running."
return 1