Pull request: 3457 fix service reload

Merge in DNS/adguard-home from 3457-darwin-svc-reload to master

Closes #3457.

Squashed commit of the following:

commit e3d6fbccf8373194360b6480e2d702ccd0ec7107
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Wed Aug 18 00:52:39 2021 +0300

    aghos: imp docs

commit 220d37ebc1e0c2e9ba37a34650bff1480bd2fcf6
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Tue Aug 17 15:45:52 2021 +0300

    all: fix ps
This commit is contained in:
Eugene Burkov 2021-08-18 13:20:56 +03:00
parent d2c9052dde
commit 550b1798a1
9 changed files with 220 additions and 53 deletions

View File

@ -65,6 +65,7 @@ and this project adheres to
### Fixed ### Fixed
- `reload` service action on macOS and FreeBSD ([#3457]).
- Inaccurate using of service actions in the installation script ([#3450]). - Inaccurate using of service actions in the installation script ([#3450]).
- Client ID checking ([#3437]). - Client ID checking ([#3437]).
- Discovering other DHCP servers on `darwin` and `freebsd` ([#3417]). - Discovering other DHCP servers on `darwin` and `freebsd` ([#3417]).
@ -128,6 +129,7 @@ and this project adheres to
[#3435]: https://github.com/AdguardTeam/AdGuardHome/issues/3435 [#3435]: https://github.com/AdguardTeam/AdGuardHome/issues/3435
[#3437]: https://github.com/AdguardTeam/AdGuardHome/issues/3437 [#3437]: https://github.com/AdguardTeam/AdGuardHome/issues/3437
[#3450]: https://github.com/AdguardTeam/AdGuardHome/issues/3450 [#3450]: https://github.com/AdguardTeam/AdGuardHome/issues/3450
[#3457]: https://github.com/AdguardTeam/AdGuardHome/issues/3457

View File

@ -13,8 +13,6 @@ import (
"sync" "sync"
"time" "time"
"github.com/AdguardTeam/AdGuardHome/internal/aghio"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/log"
) )
@ -115,12 +113,6 @@ func (sr *systemResolvers) getAddrs() (addrs []string, err error) {
return nil, fmt.Errorf("getting the command's stdout pipe: %w", err) return nil, fmt.Errorf("getting the command's stdout pipe: %w", err)
} }
var stdoutLimited io.Reader
stdoutLimited, err = aghio.LimitReader(stdout, aghos.MaxCmdOutputSize)
if err != nil {
return nil, fmt.Errorf("limiting stdout reader: %w", err)
}
go writeExit(stdin) go writeExit(stdin)
err = cmd.Start() err = cmd.Start()
@ -128,7 +120,7 @@ func (sr *systemResolvers) getAddrs() (addrs []string, err error) {
return nil, fmt.Errorf("start command executing: %w", err) return nil, fmt.Errorf("start command executing: %w", err)
} }
s := bufio.NewScanner(stdoutLimited) s := bufio.NewScanner(stdout)
addrs = scanAddrs(s) addrs = scanAddrs(s)
err = cmd.Wait() err = cmd.Wait()

View File

@ -4,10 +4,16 @@
package aghos package aghos
import ( import (
"bufio"
"fmt" "fmt"
"io"
"os/exec" "os/exec"
"path"
"runtime" "runtime"
"syscall" "strconv"
"strings"
"github.com/AdguardTeam/golibs/log"
) )
// UnsupportedError is returned by functions and methods when a particular // UnsupportedError is returned by functions and methods when a particular
@ -43,11 +49,6 @@ func HaveAdminRights() (bool, error) {
return haveAdminRights() return haveAdminRights()
} }
// SendProcessSignal sends signal to a process.
func SendProcessSignal(pid int, sig syscall.Signal) error {
return sendProcessSignal(pid, sig)
}
// MaxCmdOutputSize is the maximum length of performed shell command output. // MaxCmdOutputSize is the maximum length of performed shell command output.
const MaxCmdOutputSize = 2 * 1024 const MaxCmdOutputSize = 2 * 1024
@ -65,6 +66,91 @@ func RunCommand(command string, arguments ...string) (int, string, error) {
return cmd.ProcessState.ExitCode(), string(out), nil return cmd.ProcessState.ExitCode(), string(out), nil
} }
// PIDByCommand searches for process named command and returns its PID ignoring
// the PIDs from except. If no processes found, the error returned.
func PIDByCommand(command string, except ...int) (pid int, err error) {
// Don't use -C flag here since it's a feature of linux's ps
// implementation. Use POSIX-compatible flags instead.
//
// See https://github.com/AdguardTeam/AdGuardHome/issues/3457.
cmd := exec.Command("ps", "-A", "-o", "pid=", "-o", "comm=")
var stdout io.ReadCloser
if stdout, err = cmd.StdoutPipe(); err != nil {
return 0, fmt.Errorf("getting the command's stdout pipe: %w", err)
}
if err = cmd.Start(); err != nil {
return 0, fmt.Errorf("start command executing: %w", err)
}
var instNum int
pid, instNum, err = parsePSOutput(stdout, command, except...)
if err != nil {
return 0, err
}
if err = cmd.Wait(); err != nil {
return 0, fmt.Errorf("executing the command: %w", err)
}
switch instNum {
case 0:
// TODO(e.burkov): Use constant error.
return 0, fmt.Errorf("no %s instances found", command)
case 1:
// Go on.
default:
log.Info("warning: %d %s instances found", instNum, command)
}
if code := cmd.ProcessState.ExitCode(); code != 0 {
return 0, fmt.Errorf("ps finished with code %d", code)
}
return pid, nil
}
// parsePSOutput scans the output of ps searching the largest PID of the process
// associated with cmdName ignoring PIDs from ignore. Valid r's line shoud be
// like:
//
// 123 ./example-cmd
// 1230 some/base/path/example-cmd
// 3210 example-cmd
//
func parsePSOutput(r io.Reader, cmdName string, ignore ...int) (largest, instNum int, err error) {
s := bufio.NewScanner(r)
ScanLoop:
for s.Scan() {
fields := strings.Fields(s.Text())
if len(fields) != 2 || path.Base(fields[1]) != cmdName {
continue
}
cur, aerr := strconv.Atoi(fields[0])
if aerr != nil || cur < 0 {
continue
}
for _, pid := range ignore {
if cur == pid {
continue ScanLoop
}
}
instNum++
if cur > largest {
largest = cur
}
}
if err = s.Err(); err != nil {
return 0, 0, fmt.Errorf("scanning stdout: %w", err)
}
return largest, instNum, nil
}
// IsOpenWrt returns true if host OS is OpenWrt. // IsOpenWrt returns true if host OS is OpenWrt.
func IsOpenWrt() (ok bool) { func IsOpenWrt() (ok bool) {
return isOpenWrt() return isOpenWrt()

View File

@ -20,10 +20,6 @@ func haveAdminRights() (bool, error) {
return os.Getuid() == 0, nil return os.Getuid() == 0, nil
} }
func sendProcessSignal(pid int, sig syscall.Signal) error {
return syscall.Kill(pid, sig)
}
func isOpenWrt() (ok bool) { func isOpenWrt() (ok bool) {
return false return false
} }

View File

@ -20,10 +20,6 @@ func haveAdminRights() (bool, error) {
return os.Getuid() == 0, nil return os.Getuid() == 0, nil
} }
func sendProcessSignal(pid int, sig syscall.Signal) error {
return syscall.Kill(pid, sig)
}
func isOpenWrt() (ok bool) { func isOpenWrt() (ok bool) {
return false return false
} }

View File

@ -25,10 +25,6 @@ func haveAdminRights() (bool, error) {
return os.Getuid() == 0, nil return os.Getuid() == 0, nil
} }
func sendProcessSignal(pid int, sig syscall.Signal) error {
return syscall.Kill(pid, sig)
}
func isOpenWrt() (ok bool) { func isOpenWrt() (ok bool) {
var err error var err error
ok, err = FileWalker(func(r io.Reader) (_ []string, cont bool, err error) { ok, err = FileWalker(func(r io.Reader) (_ []string, cont bool, err error) {

98
internal/aghos/os_test.go Normal file
View File

@ -0,0 +1,98 @@
package aghos
import (
"bytes"
"testing"
"github.com/AdguardTeam/AdGuardHome/internal/aghio"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestLargestLabeled(t *testing.T) {
const (
comm = `command-name`
nl = "\n"
)
testCases := []struct {
name string
data []byte
wantPID int
wantInstNum int
}{{
name: "success",
data: []byte(nl +
` 123 not-a-` + comm + nl +
` 321 ` + comm + nl,
),
wantPID: 321,
wantInstNum: 1,
}, {
name: "several",
data: []byte(nl +
`1 ` + comm + nl +
`5 /` + comm + nl +
`2 /some/path/` + comm + nl +
`4 ./` + comm + nl +
`3 ` + comm + nl +
`10 .` + comm + nl,
),
wantPID: 5,
wantInstNum: 5,
}, {
name: "no_any",
data: []byte(nl +
`1 ` + `not-a-` + comm + nl +
`2 ` + `not-a-` + comm + nl +
`3 ` + `not-a-` + comm + nl,
),
wantPID: 0,
wantInstNum: 0,
}, {
name: "weird_input",
data: []byte(nl +
`abc ` + comm + nl +
`-1 ` + comm + nl,
),
wantPID: 0,
wantInstNum: 0,
}}
for _, tc := range testCases {
r := bytes.NewReader(tc.data)
t.Run(tc.name, func(t *testing.T) {
pid, instNum, err := parsePSOutput(r, comm)
require.NoError(t, err)
assert.Equal(t, tc.wantPID, pid)
assert.Equal(t, tc.wantInstNum, instNum)
})
}
t.Run("scanner_fail", func(t *testing.T) {
lr, err := aghio.LimitReader(bytes.NewReader([]byte{1, 2, 3}), 0)
require.NoError(t, err)
target := &aghio.LimitReachedError{}
_, _, err = parsePSOutput(lr, "")
require.ErrorAs(t, err, &target)
assert.EqualValues(t, 0, target.Limit)
})
t.Run("ignore", func(t *testing.T) {
r := bytes.NewReader([]byte(nl +
`1 ` + comm + nl +
`2 ` + comm + nl +
`3` + comm + nl,
))
pid, instances, err := parsePSOutput(r, comm, 1, 3)
require.NoError(t, err)
assert.Equal(t, 2, pid)
assert.Equal(t, 1, instances)
})
}

View File

@ -4,8 +4,6 @@
package aghos package aghos
import ( import (
"syscall"
"golang.org/x/sys/windows" "golang.org/x/sys/windows"
) )
@ -34,10 +32,6 @@ func haveAdminRights() (bool, error) {
return true, nil return true, nil
} }
func sendProcessSignal(pid int, sig syscall.Signal) error {
return Unsupported("kill")
}
func isOpenWrt() (ok bool) { func isOpenWrt() (ok bool) {
return false return false
} }

View File

@ -97,41 +97,48 @@ func sendSigReload() {
} }
pidfile := fmt.Sprintf("/var/run/%s.pid", serviceName) pidfile := fmt.Sprintf("/var/run/%s.pid", serviceName)
var pid int
data, err := os.ReadFile(pidfile) data, err := os.ReadFile(pidfile)
if errors.Is(err, os.ErrNotExist) { if errors.Is(err, os.ErrNotExist) {
var code int if pid, err = aghos.PIDByCommand(serviceName, os.Getpid()); err != nil {
var psdata string log.Error("finding AdGuardHome process: %s", err)
code, psdata, err = aghos.RunCommand("ps", "-C", serviceName, "-o", "pid=")
if err != nil || code != 0 {
log.Error("finding AdGuardHome process: code: %d, error: %s", code, err)
return return
} }
data = []byte(psdata)
} else if err != nil { } else if err != nil {
log.Error("reading pid file %s: %s", pidfile, err) log.Error("reading pid file %s: %s", pidfile, err)
return return
} else {
parts := strings.SplitN(string(data), "\n", 2)
if len(parts) == 0 {
log.Error("can't read pid file %s: bad value", pidfile)
return
}
if pid, err = strconv.Atoi(strings.TrimSpace(parts[0])); err != nil {
log.Error("can't read pid file %s: %s", pidfile, err)
return
}
} }
parts := strings.SplitN(string(data), "\n", 2) var proc *os.Process
if len(parts) == 0 { if proc, err = os.FindProcess(pid); err != nil {
log.Error("Can't read PID file %s: bad value", pidfile) log.Error("can't send signal to pid %d: %s", pid, err)
return return
} }
pid, err := strconv.Atoi(strings.TrimSpace(parts[0])) if err = proc.Signal(syscall.SIGHUP); err != nil {
if err != nil { log.Error("Can't send signal to pid %d: %s", pid, err)
log.Error("Can't read PID file %s: %s", pidfile, err)
return return
} }
err = aghos.SendProcessSignal(pid, syscall.SIGHUP)
if err != nil { log.Debug("sent signal to PID %d", pid)
log.Error("Can't send signal to PID %d: %s", pid, err)
return
}
log.Debug("Sent signal to PID %d", pid)
} }
// handleServiceControlAction one of the possible control actions: // handleServiceControlAction one of the possible control actions:
@ -541,6 +548,6 @@ name="{{.Name}}"
{{.Name}}_user="root" {{.Name}}_user="root"
pidfile="/var/run/${name}.pid" pidfile="/var/run/${name}.pid"
command="/usr/sbin/daemon" command="/usr/sbin/daemon"
command_args="-P ${pidfile} -f -r {{.WorkingDirectory}}/{{.Name}}" command_args="-p ${pidfile} -f -r {{.WorkingDirectory}}/{{.Name}}"
run_rc_command "$1" run_rc_command "$1"
` `