diff --git a/internal/agentupdate/coverage_test.go b/internal/agentupdate/coverage_test.go index ad2caa320..29f538f65 100644 --- a/internal/agentupdate/coverage_test.go +++ b/internal/agentupdate/coverage_test.go @@ -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) { diff --git a/internal/agentupdate/update.go b/internal/agentupdate/update.go index 5bf080f0a..e8fdf5fb9 100644 --- a/internal/agentupdate/update.go +++ b/internal/agentupdate/update.go @@ -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 diff --git a/internal/api/unified_agent_install_script_test.go b/internal/api/unified_agent_install_script_test.go new file mode 100644 index 000000000..8597a0630 --- /dev/null +++ b/internal/api/unified_agent_install_script_test.go @@ -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) + } + } +} diff --git a/scripts/install.sh b/scripts/install.sh index dc7e27c3a..d7f033548 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -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