Pull request 2342: 7627-dhcp-client-hostname

Updates #7627.

Squashed commit of the following:

commit 5eb677f5b91d2664094d759b4b07c64c1889c57b
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Feb 14 17:21:35 2025 +0300

    client: fix test

commit e7c6ee81ef9aa6627538c82f150f4707df92d820
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Feb 14 17:00:00 2025 +0300

    client: imp test

commit b59ca8b3ca4d00007d5c48b96fcc630342414ae7
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Feb 14 16:53:02 2025 +0300

    client: fix dhcp

CHANGELOG.md

@@ -32,9 +32,11 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 ### Fixed
 
+- The hostnames of DHCP clients not being shown in the *Top clients* table on the dashboard ([#7627]).
 - The formatting of large numbers in the upstream table and query log ([#7590]).
 
 [#7590]: https://github.com/AdguardTeam/AdGuardHome/issues/7590
+[#7627]: https://github.com/AdguardTeam/AdGuardHome/issues/7627
 
 [go-1.23.6]: https://groups.google.com/g/golang-announce/c/xU1ZCHUZw3k
 

internal/client/client.go

@@ -178,8 +178,12 @@ func (r *Runtime) Addr() (ip netip.Addr) {
 	return r.ip
 }
 
-// clone returns a deep copy of the runtime client.
+// clone returns a deep copy of the runtime client.  If r is nil, c is nil.
 func (r *Runtime) clone() (c *Runtime) {
+	if r == nil {
+		return nil
+	}
+
 	return &Runtime{
 		ip:        r.ip,
 		whois:     r.whois.Clone(),

internal/client/storage.go

@@ -591,17 +591,21 @@ func (s *Storage) ClientRuntime(ip netip.Addr) (rc *Runtime) {
 	defer s.mu.Unlock()
 
 	rc = s.runtimeIndex.client(ip)
-	if rc != nil {
+	if !s.runtimeSourceDHCP {
 		return rc.clone()
 	}
 
-	if !s.runtimeSourceDHCP {
+	// SourceHostsFile > SourceDHCP, so return immediately if the client is from
-		return nil
+	// the hosts file.
+	if rc != nil && rc.hostsFile != nil {
+		return rc.clone()
 	}
 
+	// Otherwise, check the DHCP server and add the client information if there
+	// is any.
 	host := s.dhcp.HostByIP(ip)
 	if host == "" {
-		return nil
+		return rc.clone()
 	}
 
 	rc = s.runtimeIndex.setInfo(ip, SourceDHCP, []string{host})

internal/client/storage_test.go

@@ -353,6 +353,9 @@ func TestClientsDHCP(t *testing.T) {
 		prsCliIP   = netip.MustParseAddr("4.3.2.1")
 		prsCliMAC  = mustParseMAC("AA:AA:AA:AA:AA:AA")
 		prsCliName = "persistent.dhcp"
+
+		otherARPCliName = "other.arp"
+		otherARPCliIP   = netip.MustParseAddr("192.0.2.1")
 	)
 
 	ipToHost := map[netip.Addr]string{
@@ -372,7 +375,20 @@ func TestClientsDHCP(t *testing.T) {
 		HWAddr:   cliMAC3,
 	}}
 
-	d := &testDHCP{
+	arpCh := make(chan []arpdb.Neighbor, 1)
+	arpDB := &testARPDB{
+		onRefresh: func() (err error) { return nil },
+		onNeighbors: func() (ns []arpdb.Neighbor) {
+			select {
+			case ns = <-arpCh:
+				return ns
+			default:
+				return nil
+			}
+		},
+	}
+
+	dhcp := &testDHCP{
 		OnLeases: func() (ls []*dhcpsvc.Lease) {
 			return leases
 		},
@@ -384,22 +400,111 @@ func TestClientsDHCP(t *testing.T) {
 		},
 	}
 
+	etcHostsCh := make(chan *hostsfile.DefaultStorage, 1)
+	etcHosts := &testHostsContainer{
+		onUpd: func() (updates <-chan *hostsfile.DefaultStorage) {
+			return etcHostsCh
+		},
+	}
+
 	ctx := testutil.ContextWithTimeout(t, testTimeout)
 	storage, err := client.NewStorage(ctx, &client.StorageConfig{
 		Logger:                 slogutil.NewDiscardLogger(),
-		DHCP:              d,
+		ARPDB:                  arpDB,
+		DHCP:                   dhcp,
+		EtcHosts:               etcHosts,
 		RuntimeSourceDHCP:      true,
+		ARPClientsUpdatePeriod: testTimeout / 10,
 	})
 	require.NoError(t, err)
 
-	t.Run("find_runtime", func(t *testing.T) {
+	err = storage.Start(testutil.ContextWithTimeout(t, testTimeout))
+	require.NoError(t, err)
+
+	testutil.CleanupAndRequireSuccess(t, func() (err error) {
+		return storage.Shutdown(testutil.ContextWithTimeout(t, testTimeout))
+	})
+
+	require.True(t, t.Run("find_runtime_lower_priority", func(t *testing.T) {
+		// Add a lower-priority client.
+		ns := []arpdb.Neighbor{{
+			Name: cliName1,
+			IP:   cliIP1,
+		}}
+
+		testutil.RequireSend(t, arpCh, ns, testTimeout)
+
+		storage.ReloadARP(testutil.ContextWithTimeout(t, testTimeout))
+
 		cli1 := storage.ClientRuntime(cliIP1)
 		require.NotNil(t, cli1)
 
 		assert.True(t, compareRuntimeInfo(cli1, client.SourceDHCP, cliName1))
+
+		// Remove the matching client.
+		//
+		// TODO(a.garipov):  Consider adding ways of explicitly clearing runtime
+		// sources by source.
+		ns = []arpdb.Neighbor{{
+			Name: otherARPCliName,
+			IP:   otherARPCliIP,
+		}}
+
+		testutil.RequireSend(t, arpCh, ns, testTimeout)
+
+		storage.ReloadARP(testutil.ContextWithTimeout(t, testTimeout))
+	}))
+
+	require.True(t, t.Run("find_runtime", func(t *testing.T) {
+		cli1 := storage.ClientRuntime(cliIP1)
+		require.NotNil(t, cli1)
+
+		assert.True(t, compareRuntimeInfo(cli1, client.SourceDHCP, cliName1))
+	}))
+
+	require.True(t, t.Run("find_runtime_higher_priority", func(t *testing.T) {
+		// Add a higher-priority client.
+		s, strgErr := hostsfile.NewDefaultStorage()
+		require.NoError(t, strgErr)
+
+		s.Add(&hostsfile.Record{
+			Addr:  cliIP1,
+			Names: []string{cliName1},
 		})
 
-	t.Run("find_persistent", func(t *testing.T) {
+		testutil.RequireSend(t, etcHostsCh, s, testTimeout)
+
+		cli1 := storage.ClientRuntime(cliIP1)
+		require.NotNil(t, cli1)
+
+		require.Eventually(t, func() (ok bool) {
+			cli := storage.ClientRuntime(cliIP1)
+			if cli == nil {
+				return false
+			}
+
+			assert.True(t, compareRuntimeInfo(cli, client.SourceHostsFile, cliName1))
+
+			return true
+		}, testTimeout, testTimeout/10)
+
+		// Remove the matching client.
+		//
+		// TODO(a.garipov):  Consider adding ways of explicitly clearing runtime
+		// sources by source.
+		s, strgErr = hostsfile.NewDefaultStorage()
+		require.NoError(t, strgErr)
+
+		testutil.RequireSend(t, etcHostsCh, s, testTimeout)
+
+		require.Eventually(t, func() (ok bool) {
+			cli := storage.ClientRuntime(cliIP1)
+
+			return compareRuntimeInfo(cli, client.SourceDHCP, cliName1)
+		}, testTimeout, testTimeout/10)
+	}))
+
+	require.True(t, t.Run("find_persistent", func(t *testing.T) {
 		err = storage.Add(ctx, &client.Persistent{
 			Name: prsCliName,
 			UID:  client.MustNewUID(),
@@ -411,9 +516,9 @@ func TestClientsDHCP(t *testing.T) {
 		require.True(t, ok)
 
 		assert.Equal(t, prsCliName, prsCli.Name)
-	})
+	}))
 
-	t.Run("leases", func(t *testing.T) {
+	require.True(t, t.Run("leases", func(t *testing.T) {
 		delete(ipToHost, cliIP1)
 		storage.UpdateDHCP(ctx)
 
@@ -428,18 +533,20 @@ func TestClientsDHCP(t *testing.T) {
 			assert.Equal(t, client.SourceDHCP, src)
 			assert.Equal(t, leases[i].Hostname, host)
 		}
-	})
+	}))
 
-	t.Run("range", func(t *testing.T) {
+	require.True(t, t.Run("range", func(t *testing.T) {
 		s := 0
 		storage.RangeRuntime(func(rc *client.Runtime) (cont bool) {
+			if src, _ := rc.Info(); src == client.SourceDHCP {
 				s++
+			}
 
 			return true
 		})
 
 		assert.Equal(t, len(leases), s)
-	})
+	}))
 }
 
 func TestClientsAddExisting(t *testing.T) {