diff --git a/internal/aghnet/dhcp_unix.go b/internal/aghnet/dhcp_unix.go index 06d421e0..33a601cb 100644 --- a/internal/aghnet/dhcp_unix.go +++ b/internal/aghnet/dhcp_unix.go @@ -145,7 +145,7 @@ func discover4( ok, next, err = tryConn4(ctx, l, req, c, iface) if next { if err != nil { - l.DebugContext(ctx, "dhcpv4: trying a connection", slogutil.KeyError, err) + l.DebugContext(ctx, "trying a connection", slogutil.KeyError, err) } continue @@ -170,13 +170,13 @@ func tryConn4( ) (ok, next bool, err error) { // TODO: replicate dhclient's behavior of retrying several times with // progressively longer timeouts. - l.Log(ctx, slogutil.LevelTrace, "dhcpv4: waiting for an answer", "timeout", defaultDiscoverTime) + l.Log(ctx, slogutil.LevelTrace, "waiting for an answer", "timeout", defaultDiscoverTime) b := make([]byte, 1500) n, _, err := c.ReadFrom(b) if err != nil { if errors.Is(err, os.ErrDeadlineExceeded) { - l.DebugContext(ctx, "dhcpv4: didn't receive dhcp response") + l.DebugContext(ctx, "did not receive response") return false, false, nil } @@ -184,16 +184,16 @@ func tryConn4( return false, false, fmt.Errorf("receiving packet: %w", err) } - l.Log(ctx, slogutil.LevelTrace, "dhcpv4: received packet", "size", n) + l.Log(ctx, slogutil.LevelTrace, "received packet", "size", n) response, err := dhcpv4.FromBytes(b[:n]) if err != nil { - l.DebugContext(ctx, "dhcpv4: encoding", slogutil.KeyError, err) + l.DebugContext(ctx, "encoding", slogutil.KeyError, err) return false, true, err } - l.DebugContext(ctx, "dhcpv4: received message from server", "summary", response.Summary()) + l.DebugContext(ctx, "received message from server", "summary", response.Summary()) switch { case @@ -206,7 +206,7 @@ func tryConn4( return false, true, nil default: - l.Log(ctx, slogutil.LevelTrace, "dhcpv4: the packet is from an active dhcp server") + l.Log(ctx, slogutil.LevelTrace, "packet is from an active dhcp server") return true, false, nil } @@ -261,7 +261,7 @@ func discover6( return false, fmt.Errorf("dhcpv6: dhcpv6.NewSolicit: %w", err) } - l.DebugContext(ctx, "dhcpv6: listening to udp6", "addr", udpAddr) + l.DebugContext(ctx, "listening on udp6", "addr", udpAddr) c, err := nclient6.NewIPv6UDPConn(iface.Name, dhcpv6.DefaultClientPort) if err != nil { return false, fmt.Errorf("dhcpv6: Couldn't listen on :546: %w", err) @@ -278,7 +278,7 @@ func discover6( ok, next, err = tryConn6(ctx, l, req, c) if next { if err != nil { - l.DebugContext(ctx, "dhcpv6: trying a connection", slogutil.KeyError, err) + l.DebugContext(ctx, "trying a connection", slogutil.KeyError, err) } continue @@ -301,7 +301,7 @@ func tryConn6( ) (ok, next bool, err error) { // TODO: replicate dhclient's behavior of retrying several times with // progressively longer timeouts. - l.Log(ctx, slogutil.LevelTrace, "dhcpv6: waiting for an answer", "timeout", defaultDiscoverTime) + l.Log(ctx, slogutil.LevelTrace, "waiting for an answer", "timeout", defaultDiscoverTime) b := make([]byte, 4096) err = c.SetDeadline(time.Now().Add(defaultDiscoverTime)) @@ -312,7 +312,7 @@ func tryConn6( n, _, err := c.ReadFrom(b) if err != nil { if errors.Is(err, os.ErrDeadlineExceeded) { - l.DebugContext(ctx, "dhcpv6: didn't receive dhcp response") + l.DebugContext(ctx, "did not receive response") return false, false, nil } @@ -324,12 +324,12 @@ func tryConn6( response, err := dhcpv6.FromBytes(b[:n]) if err != nil { - l.DebugContext(ctx, "dhcpv6: encoding", slogutil.KeyError, err) + l.DebugContext(ctx, "encoding", slogutil.KeyError, err) return false, true, err } - l.DebugContext(ctx, "dhcpv6: received message from server", "summary", response.Summary()) + l.DebugContext(ctx, "received message from server", "summary", response.Summary()) cid := req.Options.ClientID() msg, err := response.GetInnerMessage() @@ -345,7 +345,7 @@ func tryConn6( rcid != nil && cid.Equal(rcid)) { - l.DebugContext(ctx, "dhcpv6: received message from server does not match our request") + l.DebugContext(ctx, "received message from server does not match our request") return false, true, nil } diff --git a/internal/aghnet/hostscontainer.go b/internal/aghnet/hostscontainer.go index b013b423..3cd3a340 100644 --- a/internal/aghnet/hostscontainer.go +++ b/internal/aghnet/hostscontainer.go @@ -109,6 +109,8 @@ func NewHostsContainer( // Close implements the [io.Closer] interface for *HostsContainer. It closes // both itself and its [aghos.FSWatcher]. Close must only be called once. +// +// TODO(s.chzhen): Implement [service.Interface]. func (hc *HostsContainer) Close() (err error) { // TODO(s.chzhen): Pass context. ctx := context.TODO() diff --git a/internal/aghnet/hostscontainer_test.go b/internal/aghnet/hostscontainer_test.go index 09813200..e541fadf 100644 --- a/internal/aghnet/hostscontainer_test.go +++ b/internal/aghnet/hostscontainer_test.go @@ -13,19 +13,12 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghtest" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/hostsfile" - "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// testTimeout is a common timeout for tests. -const testTimeout = 1 * time.Second - -// testLogger is a logger used in tests. -var testLogger = slogutil.NewDiscardLogger() - func TestNewHostsContainer(t *testing.T) { const dirname = "dir" const filename = "file1" diff --git a/internal/aghnet/net.go b/internal/aghnet/net.go index 81c51d5f..eea4bf0b 100644 --- a/internal/aghnet/net.go +++ b/internal/aghnet/net.go @@ -84,11 +84,11 @@ func GatewayIP( ) if err != nil { if code, ok := executil.ExitCodeFromError(err); ok { - l.DebugContext(ctx, "fetching gateway ip: unexpected exit code", "code", code) - } else { - l.DebugContext(ctx, "fetching gateway ip", slogutil.KeyError, err) + err = fmt.Errorf("unexpected exit code %d: %w", code, err) } + l.DebugContext(ctx, "fetching gateway ip", slogutil.KeyError, err) + return netip.Addr{} } diff --git a/internal/aghnet/net_linux.go b/internal/aghnet/net_linux.go index 5d047ff2..5523a2dd 100644 --- a/internal/aghnet/net_linux.go +++ b/internal/aghnet/net_linux.go @@ -38,7 +38,7 @@ func canBindPrivilegedPorts(ctx context.Context, l *slog.Logger) (can bool, err // warning and check admin rights. l.WarnContext( ctx, - "cannot check capability cap_net_bind_service", + "checking capability cap_net_bind_service", slogutil.KeyError, err, ) } else { diff --git a/internal/aghnet/net_test.go b/internal/aghnet/net_test.go index 06f65840..7be9c531 100644 --- a/internal/aghnet/net_test.go +++ b/internal/aghnet/net_test.go @@ -5,13 +5,21 @@ import ( "net/netip" "net/url" "testing" + "time" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" + "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" ) +// testTimeout is a common timeout for tests. +const testTimeout = 1 * time.Second + +// testLogger is a logger used in tests. +var testLogger = slogutil.NewDiscardLogger() + func TestMain(m *testing.M) { testutil.DiscardLogOutput(m) } diff --git a/internal/client/storage_test.go b/internal/client/storage_test.go index 2033e2fe..3e6e2dd3 100644 --- a/internal/client/storage_test.go +++ b/internal/client/storage_test.go @@ -576,10 +576,9 @@ func TestClientsAddExisting(t *testing.T) { // First, init a DHCP server with a single static lease. config := &dhcpd.ServerConfig{ - BaseLogger: testLogger, - Logger: testLogger, - Enabled: true, - DataDir: t.TempDir(), + Logger: testLogger, + Enabled: true, + DataDir: t.TempDir(), Conf4: dhcpd.V4ServerConf{ Enabled: true, GatewayIP: netip.MustParseAddr("1.2.3.1"), diff --git a/internal/dhcpd/config.go b/internal/dhcpd/config.go index 1916dd7f..381e7ce1 100644 --- a/internal/dhcpd/config.go +++ b/internal/dhcpd/config.go @@ -19,10 +19,6 @@ import ( // ServerConfig is the configuration for the DHCP server. The order of YAML // fields is important, since the YAML configuration file follows it. type ServerConfig struct { - // BaseLogger is used for creating loggers for dhcpv4 and dhcpv6. It must - // not be nil. - BaseLogger *slog.Logger `yaml:"-"` - // Logger is used for logging the operation of the DHCP server. It must not // be nil. Logger *slog.Logger `yaml:"-"` diff --git a/internal/dhcpd/dhcpd.go b/internal/dhcpd/dhcpd.go index ad59d814..b97cdd01 100644 --- a/internal/dhcpd/dhcpd.go +++ b/internal/dhcpd/dhcpd.go @@ -108,7 +108,6 @@ var _ Interface = (*server)(nil) func Create(ctx context.Context, conf *ServerConfig) (s *server, err error) { s = &server{ conf: &ServerConfig{ - BaseLogger: conf.BaseLogger, Logger: conf.Logger, CommandConstructor: conf.CommandConstructor, ConfModifier: conf.ConfModifier, @@ -166,7 +165,7 @@ func (s *server) setServers( conf *ServerConfig, ) (v4Enabled, v6Enabled bool, err error) { v4conf := conf.Conf4 - v4conf.Logger = s.conf.BaseLogger.With(slogutil.KeyPrefix, "dhcpv4_server") + v4conf.Logger = s.conf.Logger.With("ip_version", "4") v4conf.InterfaceName = s.conf.InterfaceName v4conf.notify = s.onNotify v4conf.Enabled = s.conf.Enabled && v4conf.RangeStart.IsValid() @@ -181,7 +180,7 @@ func (s *server) setServers( } v6conf := conf.Conf6 - v6conf.Logger = s.conf.BaseLogger.With(slogutil.KeyPrefix, "dhcpv6_server") + v6conf.Logger = s.conf.Logger.With("ip_version", "6") v6conf.InterfaceName = s.conf.InterfaceName v6conf.notify = s.onNotify v6conf.Enabled = s.conf.Enabled && len(v6conf.RangeStart) != 0 diff --git a/internal/dhcpd/http_unix_internal_test.go b/internal/dhcpd/http_unix_internal_test.go index 1760dd18..059e40ac 100644 --- a/internal/dhcpd/http_unix_internal_test.go +++ b/internal/dhcpd/http_unix_internal_test.go @@ -87,7 +87,6 @@ func TestServer_handleDHCPStatus(t *testing.T) { ctx := testutil.ContextWithTimeout(t, testTimeout) s, err := Create(ctx, &ServerConfig{ - BaseLogger: testLogger, Logger: testLogger, Enabled: true, Conf4: *defaultV4ServerConf(), @@ -184,7 +183,6 @@ func TestServer_HandleUpdateStaticLease(t *testing.T) { ctx := testutil.ContextWithTimeout(t, testTimeout) s, err := Create(ctx, &ServerConfig{ - BaseLogger: testLogger, Logger: testLogger, Enabled: true, Conf4: *defaultV4ServerConf(), @@ -275,7 +273,6 @@ func TestServer_HandleUpdateStaticLease_validation(t *testing.T) { ctx := testutil.ContextWithTimeout(t, testTimeout) s, err := Create(ctx, &ServerConfig{ - BaseLogger: testLogger, Logger: testLogger, Enabled: true, Conf4: *defaultV4ServerConf(), diff --git a/internal/dhcpd/v46_windows.go b/internal/dhcpd/v46_windows.go index 3fc462b0..a438e258 100644 --- a/internal/dhcpd/v46_windows.go +++ b/internal/dhcpd/v46_windows.go @@ -26,7 +26,7 @@ func (winServer) UpdateStaticLease(_ *dhcpsvc.Lease) (err error) { return func (winServer) FindMACbyIP(_ netip.Addr) (mac net.HardwareAddr) { return nil } func (winServer) WriteDiskConfig4(_ *V4ServerConf) {} func (winServer) WriteDiskConfig6(_ *V6ServerConf) {} -func (winServer) Start(ctx context.Context) (err error) { return nil } +func (winServer) Start(_ context.Context) (err error) { return nil } func (winServer) Stop() (err error) { return nil } func (winServer) HostByIP(_ netip.Addr) (host string) { return "" } func (winServer) IPByHost(_ string) (ip netip.Addr) { return netip.Addr{} } diff --git a/internal/home/home.go b/internal/home/home.go index 29e2f29f..48e8747f 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -315,8 +315,7 @@ func initContextClients( config.DHCP.DataDir = globalContext.getDataDir() config.DHCP.HTTPRegister = httpRegister config.DHCP.CommandConstructor = executil.SystemCommandConstructor{} - config.DHCP.BaseLogger = logger - config.DHCP.Logger = logger.With(slogutil.KeyPrefix, "dhcp_server") + config.DHCP.Logger = logger.With(slogutil.KeyPrefix, "dhcpd") config.DHCP.ConfModifier = confModifier globalContext.dhcpServer, err = dhcpd.Create(ctx, config.DHCP)