all: imp code

internal/agh/agh.go

@@ -11,6 +11,10 @@ import (
 	"github.com/AdguardTeam/golibs/testutil/fakeos/fakeexec"
 )
 
+// DefaultOutputLimit is the default limit of bytes for commands' standard
+// output and standard error.
+const DefaultOutputLimit = 512
+
 // ConfigModifier defines an interface for updating the global configuration.
 type ConfigModifier interface {
 	// Apply applies changes to the global configuration.
@@ -27,11 +31,8 @@ var _ ConfigModifier = EmptyConfigModifier{}
 // Apply implements the [ConfigModifier] for EmptyConfigModifier.
 func (em EmptyConfigModifier) Apply(ctx context.Context) {}
 
-// TODO(s.chzhen): !! Is there another way?
-//
-// TODO(s.chzhen): !! Docs, naming.
-//
-// TODO(s.chzhen):  Move to aghtest once the import cycle is resolved.
+// exitErr implements [executil.ExitCodeError] for tests to simulate non-zero
+// process exit codes.
 type exitErr struct {
 	code osutil.ExitCode
 }
@@ -39,25 +40,37 @@ type exitErr struct {
 // type check
 var _ executil.ExitCodeError = exitErr{}
 
+// Error implements the [executil.ExitCodeError] for exitErr.
 func (e exitErr) Error() (s string) {
 	return fmt.Sprintf("exit code %d", e.code)
 }
 
+// ExitCode implements the [executil.ExitCodeError] for exitErr.
 func (e exitErr) ExitCode() (code osutil.ExitCode) {
 	return e.code
 }
 
+// ExternalCommand is a fake command used by [NewMultipleCommandConstructor].
 type ExternalCommand struct {
-	Err  error
-	Cmd  string
-	Out  string
+	// Err is the error returned, if non-nil.
+	Err error
+
+	// Cmd contains the command path and arguments.
+	Cmd string
+
+	// Out is written to stdout if non-empty.
+	Out string
+
+	// Code is returned as the exit code if non-zero.
 	Code int
 }
 
+// keyCommand builds a key for a command lookup.
 func keyCommand(path string, args []string) (k string) {
 	return path + " " + strings.Join(args, " ")
 }
 
+// parseCommand splits a command string into the executable path and args.
 func parseCommand(s string) (path string, args []string) {
 	f := strings.Fields(s)
 	if len(f) == 0 {
@@ -68,7 +81,11 @@ func parseCommand(s string) (path string, args []string) {
 }
 
 // NewMultipleCommandConstructor is a helper function that returns a mock
-// [executil.CommandConstructor] for tests.
+// [executil.CommandConstructor] for tests that supports multiple commands.
+//
+// TODO(s.chzhen):  Use this.
+//
+// TODO(s.chzhen):  Move to aghtest once the import cycle is resolved.
 func NewMultipleCommandConstructor(cmds ...ExternalCommand) (cs executil.CommandConstructor) {
 	table := make(map[string]ExternalCommand, len(cmds))
 	for _, ec := range cmds {

internal/aghos/os.go

@@ -93,6 +93,11 @@ func RunCommand(
 }
 
 // psArgs holds the default ps arguments to avoid per-call slice allocations.
+//
+// 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.
 var psArgs = []string{"-A", "-o", "pid=", "-o", "comm="}
 
 // PIDByCommand searches for process named command and returns its PID ignoring
@@ -108,10 +113,6 @@ func PIDByCommand(
 
 	l.DebugContext(ctx, "executing", "cmd", psCmd, "args", psArgs)
 
-	// 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.
 	stdoutBuf := bytes.Buffer{}
 	err = executil.Run(
 		ctx,

internal/arpdb/arpdb.go

@@ -17,6 +17,7 @@ import (
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/osutil"
 	"github.com/AdguardTeam/golibs/osutil/executil"
+	"github.com/AdguardTeam/golibs/service"
 )
 
 // Variables and functions to substitute in tests.
@@ -28,8 +29,8 @@ var (
 // Interface stores and refreshes the network neighborhood reported by ARP
 // (Address Resolution Protocol).
 type Interface interface {
-	// Refresh updates the stored data.  It must be safe for concurrent use.
-	Refresh() (err error)
+	// Refresher updates the stored data.  It must be safe for concurrent use.
+	service.Refresher
 
 	// Neighbors returnes the last set of data reported by ARP.  Both the method
 	// and it's result must be safe for concurrent use.
@@ -49,7 +50,7 @@ var _ Interface = Empty{}
 
 // Refresh implements the [Interface] interface for EmptyARPContainer.  It does
 // nothing and always returns nil error.
-func (Empty) Refresh() (err error) { return nil }
+func (Empty) Refresh(_ context.Context) (err error) { return nil }
 
 // Neighbors implements the [Interface] interface for EmptyARPContainer.  It
 // always returns nil.
@@ -174,13 +175,12 @@ type cmdARPDB struct {
 var _ Interface = (*cmdARPDB)(nil)
 
 // Refresh implements the [Interface] interface for *cmdARPDB.
-func (arp *cmdARPDB) Refresh() (err error) {
+func (arp *cmdARPDB) Refresh(ctx context.Context) (err error) {
 	defer func() { err = errors.Annotate(err, "cmd arpdb: %w") }()
 
 	var stdout bytes.Buffer
 	err = executil.Run(
-		// TODO(s.chzhen):  Pass context.
-		context.TODO(),
+		ctx,
 		arp.cmdCons,
 		&executil.CommandConfig{
 			Path:   arp.cmd,
@@ -237,11 +237,11 @@ func newARPDBs(arps ...Interface) (arp *arpdbs) {
 var _ Interface = (*arpdbs)(nil)
 
 // Refresh implements the [Interface] interface for *arpdbs.
-func (arp *arpdbs) Refresh() (err error) {
+func (arp *arpdbs) Refresh(ctx context.Context) (err error) {
 	var errs []error
 
 	for _, a := range arp.arps {
-		err = a.Refresh()
+		err = a.Refresh(ctx)
 		if err != nil {
 			errs = append(errs, err)
 

internal/arpdb/arpdb_internal_test.go

@@ -1,12 +1,14 @@
 package arpdb
 
 import (
+	"context"
 	"io/fs"
 	"net"
 	"net/netip"
 	"os"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/agh"
 	"github.com/AdguardTeam/golibs/errors"
@@ -16,6 +18,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// testTimeout is a common timeout for tests.
+const testTimeout = 1 * time.Second
+
 // testdata is the filesystem containing data for testing the package.
 var testdata fs.FS = os.DirFS("./testdata")
 
@@ -33,7 +38,7 @@ func Test_New(t *testing.T) {
 
 // TestARPDB is the mock implementation of [Interface] to use in tests.
 type TestARPDB struct {
-	OnRefresh   func() (err error)
+	OnRefresh   func(ctx context.Context) (err error)
 	OnNeighbors func() (ns []Neighbor)
 }
 
@@ -41,8 +46,8 @@ type TestARPDB struct {
 var _ Interface = (*TestARPDB)(nil)
 
 // Refresh implements the [Interface] interface for *TestARPDB.
-func (arp *TestARPDB) Refresh() (err error) {
-	return arp.OnRefresh()
+func (arp *TestARPDB) Refresh(ctx context.Context) (err error) {
+	return arp.OnRefresh(ctx)
 }
 
 // Neighbors implements the [Interface] interface for *TestARPDB.
@@ -60,13 +65,17 @@ func Test_NewARPDBs(t *testing.T) {
 	}
 
 	succDB := &TestARPDB{
-		OnRefresh: func() (err error) { succRefrCount++; return nil },
+		OnRefresh: func(_ context.Context) (err error) { succRefrCount++; return nil },
 		OnNeighbors: func() (ns []Neighbor) {
 			return []Neighbor{{Name: "abc", IP: knownIP, MAC: knownMAC}}
 		},
 	}
 	failDB := &TestARPDB{
-		OnRefresh:   func() (err error) { failRefrCount++; return errors.Error("refresh failed") },
+		OnRefresh: func(_ context.Context) (err error) {
+			failRefrCount++
+
+			return errors.Error("refresh failed")
+		},
 		OnNeighbors: func() (ns []Neighbor) { return nil },
 	}
 
@@ -74,7 +83,7 @@ func Test_NewARPDBs(t *testing.T) {
 		t.Cleanup(clnp)
 
 		a := newARPDBs(succDB, failDB)
-		err := a.Refresh()
+		err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		require.NoError(t, err)
 
 		assert.Equal(t, 1, succRefrCount)
@@ -86,7 +95,7 @@ func Test_NewARPDBs(t *testing.T) {
 		t.Cleanup(clnp)
 
 		a := newARPDBs(failDB, succDB)
-		err := a.Refresh()
+		err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		require.NoError(t, err)
 
 		assert.Equal(t, 1, succRefrCount)
@@ -100,7 +109,7 @@ func Test_NewARPDBs(t *testing.T) {
 		wantMsg := "each arpdb failed: refresh failed\nrefresh failed"
 
 		a := newARPDBs(failDB, failDB)
-		err := a.Refresh()
+		err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		require.Error(t, err)
 
 		testutil.AssertErrorMsg(t, wantMsg, err)
@@ -114,7 +123,7 @@ func Test_NewARPDBs(t *testing.T) {
 
 		shouldFail := false
 		unstableDB := &TestARPDB{
-			OnRefresh: func() (err error) {
+			OnRefresh: func(_ context.Context) (err error) {
 				if shouldFail {
 					err = errors.Error("unstable failed")
 				}
@@ -133,21 +142,21 @@ func Test_NewARPDBs(t *testing.T) {
 		a := newARPDBs(unstableDB, succDB)
 
 		// Unstable ARPDB should refresh successfully.
-		err := a.Refresh()
+		err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		require.NoError(t, err)
 
 		assert.Zero(t, succRefrCount)
 		assert.NotEmpty(t, a.Neighbors())
 
 		// Unstable ARPDB should fail and the succDB should be used.
-		err = a.Refresh()
+		err = a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		require.NoError(t, err)
 
 		assert.Equal(t, 1, succRefrCount)
 		assert.NotEmpty(t, a.Neighbors())
 
 		// Unstable ARPDB should refresh successfully again.
-		err = a.Refresh()
+		err = a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		require.NoError(t, err)
 
 		assert.Equal(t, 1, succRefrCount)
@@ -156,7 +165,7 @@ func Test_NewARPDBs(t *testing.T) {
 
 	t.Run("empty", func(t *testing.T) {
 		a := newARPDBs()
-		require.NoError(t, a.Refresh())
+		require.NoError(t, a.Refresh(testutil.ContextWithTimeout(t, testTimeout)))
 
 		assert.Empty(t, a.Neighbors())
 	})
@@ -176,7 +185,7 @@ func TestCmdARPDB_arpa(t *testing.T) {
 	t.Run("arp_a", func(t *testing.T) {
 		a.cmdCons = agh.NewCommandConstructor("cmd", 0, arpAOutput, nil)
 
-		err := a.Refresh()
+		err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		require.NoError(t, err)
 
 		assert.Equal(t, wantNeighs, a.Neighbors())
@@ -185,14 +194,14 @@ func TestCmdARPDB_arpa(t *testing.T) {
 	t.Run("runcmd_error", func(t *testing.T) {
 		a.cmdCons = agh.NewCommandConstructor("cmd", 0, "", errors.Error("can't run"))
 
-		err := a.Refresh()
+		err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		testutil.AssertErrorMsg(t, "cmd arpdb: running command: running: can't run", err)
 	})
 
 	t.Run("bad_code", func(t *testing.T) {
 		a.cmdCons = agh.NewCommandConstructor("cmd", 1, "", nil)
 
-		err := a.Refresh()
+		err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		testutil.AssertErrorMsg(
 			t,
 			"cmd arpdb: running command: unexpected exit code 1",
@@ -203,7 +212,7 @@ func TestCmdARPDB_arpa(t *testing.T) {
 	t.Run("empty", func(t *testing.T) {
 		a.cmdCons = agh.NewCommandConstructor("cmd", 0, "", nil)
 
-		err := a.Refresh()
+		err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		require.NoError(t, err)
 
 		assert.Empty(t, a.Neighbors())
@@ -216,7 +225,7 @@ func TestEmptyARPDB(t *testing.T) {
 	t.Run("refresh", func(t *testing.T) {
 		var err error
 		require.NotPanics(t, func() {
-			err = a.Refresh()
+			err = a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		})
 
 		assert.NoError(t, err)

internal/arpdb/arpdb_linux.go

@@ -4,6 +4,7 @@ package arpdb
 
 import (
 	"bufio"
+	"context"
 	"fmt"
 	"io/fs"
 	"log/slog"
@@ -76,7 +77,7 @@ type fsysARPDB struct {
 var _ Interface = (*fsysARPDB)(nil)
 
 // Refresh implements the [Interface] interface for *fsysARPDB.
-func (arp *fsysARPDB) Refresh() (err error) {
+func (arp *fsysARPDB) Refresh(_ context.Context) (err error) {
 	var f fs.File
 	f, err = arp.fsys.Open(arp.filename)
 	if err != nil {

internal/arpdb/arpdb_linux_internal_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/AdguardTeam/AdGuardHome/internal/agh"
 	"github.com/AdguardTeam/golibs/logutil/slogutil"
+	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -55,7 +56,7 @@ func TestFSysARPDB(t *testing.T) {
 		filename: "proc_net_arp",
 	}
 
-	err := a.Refresh()
+	err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 	require.NoError(t, err)
 
 	ns := a.Neighbors()
@@ -76,7 +77,7 @@ func TestCmdARPDB_linux(t *testing.T) {
 			},
 		}
 
-		err := a.Refresh()
+		err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		require.NoError(t, err)
 
 		assert.Equal(t, wantNeighs, a.Neighbors())
@@ -94,7 +95,7 @@ func TestCmdARPDB_linux(t *testing.T) {
 				ns: make([]Neighbor, 0),
 			},
 		}
-		err := a.Refresh()
+		err := a.Refresh(testutil.ContextWithTimeout(t, testTimeout))
 		require.NoError(t, err)
 
 		assert.Equal(t, wantNeighs, a.Neighbors())

internal/client/storage.go

@@ -249,7 +249,7 @@ func (s *Storage) addFromSystemARP(ctx context.Context) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	if err := s.arpDB.Refresh(); err != nil {
+	if err := s.arpDB.Refresh(ctx); err != nil {
 		s.arpDB = arpdb.Empty{}
 		s.logger.ErrorContext(ctx, "refreshing arp container", slogutil.KeyError, err)
 

internal/client/storage_test.go

@@ -1,6 +1,7 @@
 package client_test
 
 import (
+	"context"
 	"net"
 	"net/netip"
 	"runtime"
@@ -18,6 +19,7 @@ import (
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/hostsfile"
 	"github.com/AdguardTeam/golibs/logutil/slogutil"
+	"github.com/AdguardTeam/golibs/service"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/AdguardTeam/golibs/testutil/faketime"
 	"github.com/AdguardTeam/golibs/timeutil"
@@ -63,8 +65,8 @@ func (c *testHostsContainer) Upd() (updates <-chan *hostsfile.DefaultStorage) {
 // Interface stores and refreshes the network neighborhood reported by ARP
 // (Address Resolution Protocol).
 type Interface interface {
-	// Refresh updates the stored data.  It must be safe for concurrent use.
-	Refresh() (err error)
+	// Refresher updates the stored data.  It must be safe for concurrent use.
+	service.Refresher
 
 	// Neighbors returnes the last set of data reported by ARP.  Both the method
 	// and it's result must be safe for concurrent use.
@@ -73,7 +75,7 @@ type Interface interface {
 
 // testARPDB is a mock implementation of the [arpdb.Interface].
 type testARPDB struct {
-	onRefresh   func() (err error)
+	onRefresh   func(ctx context.Context) (err error)
 	onNeighbors func() (ns []arpdb.Neighbor)
 }
 
@@ -81,8 +83,8 @@ type testARPDB struct {
 var _ arpdb.Interface = (*testARPDB)(nil)
 
 // Refresh implements the [arpdb.Interface] interface for *testARP.
-func (c *testARPDB) Refresh() (err error) {
-	return c.onRefresh()
+func (c *testARPDB) Refresh(ctx context.Context) (err error) {
+	return c.onRefresh(ctx)
 }
 
 // Neighbors implements the [arpdb.Interface] interface for *testARP.
@@ -218,7 +220,7 @@ func TestStorage_Add_arp(t *testing.T) {
 	)
 
 	a := &testARPDB{
-		onRefresh: func() (err error) { return nil },
+		onRefresh: func(_ context.Context) (err error) { return nil },
 		onNeighbors: func() (ns []arpdb.Neighbor) {
 			mu.Lock()
 			defer mu.Unlock()
@@ -392,7 +394,7 @@ func TestClientsDHCP(t *testing.T) {
 
 	arpCh := make(chan []arpdb.Neighbor, 1)
 	arpDB := &testARPDB{
-		onRefresh: func() (err error) { return nil },
+		onRefresh: func(_ context.Context) (err error) { return nil },
 		onNeighbors: func() (ns []arpdb.Neighbor) {
 			select {
 			case ns = <-arpCh:

internal/home/controlinstall.go

@@ -1,7 +1,6 @@
 package home
 
 import (
-	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -256,13 +255,12 @@ func checkDNSStubListener(ctx context.Context, l *slog.Logger) (ok bool) {
 	for _, cmd := range cmds {
 		l.DebugContext(ctx, "executing", "cmd", cmd.Key, "args", cmd.Value)
 
-		err := executil.Run(
+		err := executil.RunWithPeek(
 			ctx,
 			executil.SystemCommandConstructor{},
-			&executil.CommandConfig{
-				Path: cmd.Key,
-				Args: cmd.Value,
-			},
+			agh.DefaultOutputLimit,
+			cmd.Key,
+			cmd.Value...,
 		)
 		if err != nil {
 			l.InfoContext(ctx, "execution failed", "cmd", cmd.Key, slogutil.KeyError, err)
@@ -308,23 +306,16 @@ func disableDNSStubListener(ctx context.Context, l *slog.Logger) (err error) {
 		systemctlCmd = "systemctl"
 	)
 
-	var (
-		systemctlArgs   = []string{"reload-or-restart", "systemd-resolved"}
-		systemctlStdout bytes.Buffer
-		systemctlStderr bytes.Buffer
-	)
+	systemctlArgs := []string{"reload-or-restart", "systemd-resolved"}
 
 	l.DebugContext(ctx, "executing", "cmd", systemctlCmd, "args", systemctlArgs)
 
-	err = executil.Run(
+	err = executil.RunWithPeek(
 		ctx,
 		executil.SystemCommandConstructor{},
-		&executil.CommandConfig{
-			Path:   systemctlCmd,
-			Args:   systemctlArgs,
-			Stdout: &systemctlStdout,
-			Stderr: &systemctlStderr,
-		},
+		agh.DefaultOutputLimit,
+		systemctlCmd,
+		systemctlArgs...,
 	)
 	if err != nil {
 		return fmt.Errorf("executing cmd: %w", err)

internal/home/service_linux.go

@@ -156,9 +156,7 @@ var _ service.Service = (*systemdService)(nil)
 
 // Status implements the [service.Service] interface for *systemdService.
 func (s *systemdService) Status() (status service.Status, err error) {
-	const (
-		systemctlCmd = "systemctl"
-	)
+	const systemctlCmd = "systemctl"
 
 	var (
 		systemctlArgs   = []string{"show", s.unitName}