dhcpsvc: use slog

internal/dhcpsvc/config.go

@@ -2,11 +2,12 @@ package dhcpsvc
 
 import (
 	"fmt"
-	"slices"
+	"log/slog"
 	"time"
 
+	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/mapsutil"
 	"github.com/AdguardTeam/golibs/netutil"
-	"golang.org/x/exp/maps"
 )
 
 // Config is the configuration for the DHCP service.
@@ -15,6 +16,9 @@ type Config struct {
 	// interface identified by its name.
 	Interfaces map[string]*InterfaceConfig
 
+	// Logger will be used to log the DHCP events.
+	Logger *slog.Logger
+
 	// LocalDomainName is the top-level domain name to use for resolving DHCP
 	// clients' hostnames.
 	LocalDomainName string
@@ -38,36 +42,44 @@ type InterfaceConfig struct {
 }
 
 // Validate returns an error in conf if any.
+//
+// TODO(e.burkov):  Unexport and rewrite the test.
 func (conf *Config) Validate() (err error) {
 	switch {
 	case conf == nil:
 		return errNilConfig
 	case !conf.Enabled:
 		return nil
-	case conf.ICMPTimeout < 0:
-		return newMustErr("icmp timeout", "be non-negative", conf.ICMPTimeout)
+	}
+
+	var errs []error
+	if conf.ICMPTimeout < 0 {
+		err = newMustErr("icmp timeout", "be non-negative", conf.ICMPTimeout)
+		errs = append(errs, err)
 	}
 
 	err = netutil.ValidateDomainName(conf.LocalDomainName)
 	if err != nil {
 		// Don't wrap the error since it's informative enough as is.
-		return err
+		errs = append(errs, err)
 	}
 
 	if len(conf.Interfaces) == 0 {
-		return errNoInterfaces
-	}
+		errs = append(errs, errNoInterfaces)
 
-	ifaces := maps.Keys(conf.Interfaces)
-	slices.Sort(ifaces)
+		return errors.Join(errs...)
+	}
 
-	for _, iface := range ifaces {
-		if err = conf.Interfaces[iface].validate(); err != nil {
-			return fmt.Errorf("interface %q: %w", iface, err)
+	mapsutil.SortedRange(conf.Interfaces, func(iface string, ic *InterfaceConfig) (ok bool) {
+		err = ic.validate()
+		if err != nil {
+			errs = append(errs, fmt.Errorf("interface %q: %w", iface, err))
 		}
-	}
 
-	return nil
+		return true
+	})
+
+	return errors.Join(errs...)
 }
 
 // validate returns an error in ic, if any.

internal/dhcpsvc/config_test.go

@@ -23,7 +23,8 @@ func TestConfig_Validate(t *testing.T) {
 	}, {
 		name: "empty",
 		conf: &dhcpsvc.Config{
-			Enabled: true,
+			Enabled:    true,
+			Interfaces: testInterfaceConf,
 		},
 		wantErrMsg: `bad domain name "": domain name is empty`,
 	}, {

internal/dhcpsvc/dhcpsvc.go

@@ -11,6 +11,14 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/next/agh"
 )
 
+const (
+	// keyInterface is the key for logging the network interface name.
+	keyInterface = "iface"
+
+	// keyFamily is the key for logging the handled address family.
+	keyFamily = "family"
+)
+
 // Interface is a DHCP service.
 //
 // TODO(e.burkov):  Separate HostByIP, MACByIP, IPByHost into a separate

internal/dhcpsvc/interface.go

@@ -2,6 +2,7 @@ package dhcpsvc
 
 import (
 	"fmt"
+	"log/slog"
 	"slices"
 	"time"
 )
@@ -11,6 +12,9 @@ import (
 //
 // TODO(e.burkov):  Add other methods as [DHCPServer] evolves.
 type netInterface struct {
+	// logger logs the events related to the network interface.
+	logger *slog.Logger
+
 	// name is the name of the network interface.
 	name string
 

internal/dhcpsvc/server.go

@@ -1,6 +1,7 @@
 package dhcpsvc
 
 import (
+	"context"
 	"fmt"
 	"net"
 	"net/netip"
@@ -10,6 +11,7 @@ import (
 	"time"
 
 	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"golang.org/x/exp/maps"
 )
 
@@ -43,8 +45,12 @@ type DHCPServer struct {
 // error if the given configuration can't be used.
 //
 // TODO(e.burkov):  Use.
-func New(conf *Config) (srv *DHCPServer, err error) {
+func New(ctx context.Context, conf *Config) (srv *DHCPServer, err error) {
+	l := conf.Logger.With(slogutil.KeyPrefix, "dhcpsvc")
+
 	if !conf.Enabled {
+		l.DebugContext(ctx, "disabled")
+
 		// TODO(e.burkov):  Perhaps return [Empty]?
 		return nil, nil
 	}
@@ -59,22 +65,28 @@ func New(conf *Config) (srv *DHCPServer, err error) {
 	var i4 *netInterfaceV4
 	var i6 *netInterfaceV6
 
+	var errs []error
+
 	for _, ifaceName := range ifaceNames {
 		iface := conf.Interfaces[ifaceName]
 
-		i4, err = newNetInterfaceV4(ifaceName, iface.IPv4)
+		i4, err = newNetInterfaceV4(ctx, l, ifaceName, iface.IPv4)
 		if err != nil {
-			return nil, fmt.Errorf("interface %q: ipv4: %w", ifaceName, err)
+			errs = append(errs, fmt.Errorf("interface %q: ipv4: %w", ifaceName, err))
 		} else if i4 != nil {
 			ifaces4 = append(ifaces4, i4)
 		}
 
-		i6 = newNetInterfaceV6(ifaceName, iface.IPv6)
+		i6 = newNetInterfaceV6(ctx, l, ifaceName, iface.IPv6)
 		if i6 != nil {
 			ifaces6 = append(ifaces6, i6)
 		}
 	}
 
+	if err = errors.Join(errs...); err != nil {
+		return nil, err
+	}
+
 	enabled := &atomic.Bool{}
 	enabled.Store(conf.Enabled)
 

internal/dhcpsvc/server_test.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -16,6 +17,12 @@ import (
 // testLocalTLD is a common local TLD for tests.
 const testLocalTLD = "local"
 
+// testTimeout is a common timeout for tests and contexts.
+const testTimeout time.Duration = 10 * time.Second
+
+// discardLog is a logger to discard test output.
+var discardLog = slogutil.NewDiscardLogger()
+
 // testInterfaceConf is a common set of interface configurations for tests.
 var testInterfaceConf = map[string]*dhcpsvc.InterfaceConfig{
 	"eth0": {
@@ -103,6 +110,7 @@ func TestNew(t *testing.T) {
 	}{{
 		conf: &dhcpsvc.Config{
 			Enabled:         true,
+			Logger:          discardLog,
 			LocalDomainName: testLocalTLD,
 			Interfaces: map[string]*dhcpsvc.InterfaceConfig{
 				"eth0": {
@@ -116,6 +124,7 @@ func TestNew(t *testing.T) {
 	}, {
 		conf: &dhcpsvc.Config{
 			Enabled:         true,
+			Logger:          discardLog,
 			LocalDomainName: testLocalTLD,
 			Interfaces: map[string]*dhcpsvc.InterfaceConfig{
 				"eth0": {
@@ -129,6 +138,7 @@ func TestNew(t *testing.T) {
 	}, {
 		conf: &dhcpsvc.Config{
 			Enabled:         true,
+			Logger:          discardLog,
 			LocalDomainName: testLocalTLD,
 			Interfaces: map[string]*dhcpsvc.InterfaceConfig{
 				"eth0": {
@@ -143,6 +153,7 @@ func TestNew(t *testing.T) {
 	}, {
 		conf: &dhcpsvc.Config{
 			Enabled:         true,
+			Logger:          discardLog,
 			LocalDomainName: testLocalTLD,
 			Interfaces: map[string]*dhcpsvc.InterfaceConfig{
 				"eth0": {
@@ -156,17 +167,22 @@ func TestNew(t *testing.T) {
 			`range start 127.0.0.1 is not within 192.168.0.1/24`,
 	}}
 
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			_, err := dhcpsvc.New(tc.conf)
+			_, err := dhcpsvc.New(ctx, tc.conf)
 			testutil.AssertErrorMsg(t, tc.wantErrMsg, err)
 		})
 	}
 }
 
 func TestDHCPServer_AddLease(t *testing.T) {
-	srv, err := dhcpsvc.New(&dhcpsvc.Config{
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
+	srv, err := dhcpsvc.New(ctx, &dhcpsvc.Config{
 		Enabled:         true,
+		Logger:          discardLog,
 		LocalDomainName: testLocalTLD,
 		Interfaces:      testInterfaceConf,
 	})
@@ -267,8 +283,11 @@ func TestDHCPServer_AddLease(t *testing.T) {
 }
 
 func TestDHCPServer_index(t *testing.T) {
-	srv, err := dhcpsvc.New(&dhcpsvc.Config{
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
+	srv, err := dhcpsvc.New(ctx, &dhcpsvc.Config{
 		Enabled:         true,
+		Logger:          discardLog,
 		LocalDomainName: testLocalTLD,
 		Interfaces:      testInterfaceConf,
 	})
@@ -342,8 +361,11 @@ func TestDHCPServer_index(t *testing.T) {
 }
 
 func TestDHCPServer_UpdateStaticLease(t *testing.T) {
-	srv, err := dhcpsvc.New(&dhcpsvc.Config{
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
+	srv, err := dhcpsvc.New(ctx, &dhcpsvc.Config{
 		Enabled:         true,
+		Logger:          discardLog,
 		LocalDomainName: testLocalTLD,
 		Interfaces:      testInterfaceConf,
 	})
@@ -462,8 +484,11 @@ func TestDHCPServer_UpdateStaticLease(t *testing.T) {
 }
 
 func TestDHCPServer_RemoveLease(t *testing.T) {
-	srv, err := dhcpsvc.New(&dhcpsvc.Config{
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
+	srv, err := dhcpsvc.New(ctx, &dhcpsvc.Config{
 		Enabled:         true,
+		Logger:          discardLog,
 		LocalDomainName: testLocalTLD,
 		Interfaces:      testInterfaceConf,
 	})
@@ -554,8 +579,11 @@ func TestDHCPServer_RemoveLease(t *testing.T) {
 }
 
 func TestDHCPServer_Reset(t *testing.T) {
-	srv, err := dhcpsvc.New(&dhcpsvc.Config{
+	ctx := testutil.ContextWithTimeout(t, testTimeout)
+
+	srv, err := dhcpsvc.New(ctx, &dhcpsvc.Config{
 		Enabled:         true,
+		Logger:          discardLog,
 		LocalDomainName: testLocalTLD,
 		Interfaces:      testInterfaceConf,
 	})