fix equivalence test

This commit is contained in:
pascal
2026-08-03 22:24:11 +02:00
parent 07d0440e34
commit 02ec1f5dcb
@@ -3,10 +3,14 @@
// Main-vs-branch equivalence check. For every peer of every account in a real // Main-vs-branch equivalence check. For every peer of every account in a real
// Postgres copy it computes the client-facing proto.NetworkMap twice: // Postgres copy it computes the client-facing proto.NetworkMap twice:
// //
// - legacy path: main's Account → NetworkMapComponents → Calculate → proto // - legacy path: main's Account → NetworkMapComponents → Calculate → proto
// (the frozen copy in this package) // (the frozen copy in this package)
// - new path: this branch's Account → NetworkMapData → components → // - store path: the pgsql nmdata store's NetworkMapData → components →
// Calculate → ToSyncResponse → proto // Calculate → ToSyncResponse → proto (no Account involved)
// - account path: Account → toNetworkMapData twins → components → Calculate
// → ToSyncResponse → proto (the in-memory builder, no store queries)
//
// Both new paths are checked against the legacy proto.
// //
// proto.NetworkMap is generated code identical in both trees, which is what // proto.NetworkMap is generated code identical in both trees, which is what
// makes it the one usable comparison surface — the intermediate Go types differ // makes it the one usable comparison surface — the intermediate Go types differ
@@ -47,10 +51,12 @@ import (
nbdns "github.com/netbirdio/netbird/dns" nbdns "github.com/netbirdio/netbird/dns"
"github.com/netbirdio/netbird/management/internals/controllers/network_map/controller/cache" "github.com/netbirdio/netbird/management/internals/controllers/network_map/controller/cache"
networkmap_pgsql "github.com/netbirdio/netbird/management/internals/network_map_db/pgsql"
mgmtgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc" mgmtgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc"
"github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/management/server/types"
"github.com/netbirdio/netbird/management/server/types/legacynmap" "github.com/netbirdio/netbird/management/server/types/legacynmap"
"github.com/netbirdio/netbird/shared/management/networkmap/nmdata"
"github.com/netbirdio/netbird/shared/management/proto" "github.com/netbirdio/netbird/shared/management/proto"
) )
@@ -62,7 +68,6 @@ const (
type equivStats struct { type equivStats struct {
accounts int accounts int
peersChecked int peersChecked int
skippedNilNM int
} }
func TestNetworkMapProtoEquivalence(t *testing.T) { func TestNetworkMapProtoEquivalence(t *testing.T) {
@@ -81,6 +86,10 @@ func TestNetworkMapProtoEquivalence(t *testing.T) {
require.NoError(t, err, "connect to postgres") require.NoError(t, err, "connect to postgres")
t.Cleanup(func() { testStore.Close(ctx) }) t.Cleanup(func() { testStore.Close(ctx) })
nmStore, err := networkmap_pgsql.NewPostgresqlStore(ctx, dsn)
require.NoError(t, err, "connect nmdata store")
t.Cleanup(func() { nmStore.Pool.Close() })
accountIDs := equivAccountIDs(t, dsn) accountIDs := equivAccountIDs(t, dsn)
require.NotEmpty(t, accountIDs, "no accounts selected") require.NotEmpty(t, accountIDs, "no accounts selected")
@@ -94,7 +103,7 @@ func TestNetworkMapProtoEquivalence(t *testing.T) {
continue continue
} }
checkAccount(ctx, t, account, maxPeers, stats) checkAccount(ctx, t, nmStore, account, maxPeers, stats)
account = nil account = nil
debug.FreeOSMemory() debug.FreeOSMemory()
@@ -106,19 +115,22 @@ func TestNetworkMapProtoEquivalence(t *testing.T) {
} }
} }
t.Logf("equivalence: accounts=%d peers_checked=%d skipped_nil_nm=%d — no divergence", t.Logf("equivalence: accounts=%d peers_checked=%d — no divergence",
stats.accounts, stats.peersChecked, stats.skippedNilNM) stats.accounts, stats.peersChecked)
} }
// checkAccount compares both paths for every peer of one account. Nothing is // checkAccount compares both paths for every peer of one account. Nothing is
// retained across peers, so memory stays flat within an account. // retained across peers, so memory stays flat within an account.
func checkAccount(ctx context.Context, t *testing.T, account *types.Account, maxPeers int, stats *equivStats) { func checkAccount(ctx context.Context, t *testing.T, nmStore *networkmap_pgsql.PgStore, account *types.Account, maxPeers int, stats *equivStats) {
t.Helper() t.Helper()
if len(account.Peers) == 0 { if len(account.Peers) == 0 {
return return
} }
nmData, err := nmStore.GetNetworkMapData(ctx, account.Id)
require.NoError(t, err, "account %s: nmdata store load", account.Id)
validated := make(map[string]struct{}, len(account.Peers)) validated := make(map[string]struct{}, len(account.Peers))
peerIDs := make([]string, 0, len(account.Peers)) peerIDs := make([]string, 0, len(account.Peers))
for peerID := range account.Peers { for peerID := range account.Peers {
@@ -130,6 +142,15 @@ func checkAccount(ctx context.Context, t *testing.T, account *types.Account, max
peerIDs = peerIDs[:maxPeers] peerIDs = peerIDs[:maxPeers]
} }
// Production fills ValidatedPeers via the integrated-validator wrapper; here
// every peer counts as validated, matching the legacy side's map.
nmData.ValidatedPeers = validated
// The legacy side receives no account zones (main sourced them from the
// external zones manager), so the DB-sourced zone candidates must be dropped
// to keep the comparison surface identical.
nmData.AppliedZoneCandidates = nil
nmData.PrivateServiceCandidates = nil
resourcePolicies := account.GetResourcePoliciesMap() resourcePolicies := account.GetResourcePoliciesMap()
routers := account.GetResourceRoutersMap() routers := account.GetResourceRoutersMap()
groupUsers := account.GetActiveGroupUsers() groupUsers := account.GetActiveGroupUsers()
@@ -144,18 +165,32 @@ func checkAccount(ctx context.Context, t *testing.T, account *types.Account, max
if peer == nil { if peer == nil {
continue continue
} }
dataPeer := nmData.Peers[peerID]
if dataPeer == nil {
t.Fatalf("after %d peers: account=%s peer=%s present in account store, missing in nmdata store", stats.peersChecked, account.Id, peerID)
}
// NEW PATH — this branch, through the production conversion. // STORE PATH — nmdata store through the production computation, mirroring
newNM := account.GetPeerNetworkMapFromComponents( // the controller's networkMapFromData.
components := nmData.GetPeerNetworkMapComponents(peerID, nmdata.CustomZone{})
storeNM := &types.NetworkMap{Network: components.Network}
if !components.IsEmpty() {
storeNM = types.CalculateNetworkMapFromComponents(ctx, components)
}
// A separate cache per side: sharing one would let the first path
// populate entries the second then reuses, which can mask a real diff.
storeProto := mgmtgrpc.ToSyncResponse(
ctx, nil, nil, nil, dataPeer, nil, nil, storeNM, equivDNSName, nil,
&cache.DNSConfigCache{}, nmData.AccountSettings, settings.Extra, nil, 0,
).NetworkMap
// ACCOUNT PATH — Account → toNetworkMapData twins → components.
acctNM := account.GetPeerNetworkMapFromComponents(
ctx, peerID, nbdns.CustomZone{}, nil, validated, resourcePolicies, routers, nil, groupUsers, ctx, peerID, nbdns.CustomZone{}, nil, validated, resourcePolicies, routers, nil, groupUsers,
) )
if newNM == nil { acctProto := mgmtgrpc.ToSyncResponse(
stats.skippedNilNM++ ctx, nil, nil, nil, types.TwinPeer(peer), nil, nil, acctNM, equivDNSName, nil,
continue &cache.DNSConfigCache{}, types.TwinAccountSettings(settings), settings.Extra, nil, 0,
}
newProto := mgmtgrpc.ToSyncResponse(
ctx, nil, nil, nil, peer, nil, nil, newNM, equivDNSName, nil,
&cache.DNSConfigCache{}, settings, settings.Extra, nil, 0,
).NetworkMap ).NetworkMap
// LEGACY PATH — main's frozen copy. // LEGACY PATH — main's frozen copy.
@@ -165,18 +200,20 @@ func checkAccount(ctx context.Context, t *testing.T, account *types.Account, max
if legacyNM == nil { if legacyNM == nil {
t.Fatalf("after %d peers: account=%s peer=%s legacy NetworkMap nil, new non-nil", stats.peersChecked, account.Id, peerID) t.Fatalf("after %d peers: account=%s peer=%s legacy NetworkMap nil, new non-nil", stats.peersChecked, account.Id, peerID)
} }
// A separate cache per side: sharing one would let the first path
// populate entries the second then reuses, which can mask a real diff.
legacyProto := legacynmap.ToProtoNetworkMap( legacyProto := legacynmap.ToProtoNetworkMap(
ctx, peer, legacyNM, equivDNSName, settings, nil, &cache.DNSConfigCache{}, 0, ctx, peer, legacyNM, equivDNSName, settings, nil, &cache.DNSConfigCache{}, 0,
) )
canonicalize(legacyProto) canonicalize(legacyProto)
canonicalize(newProto) canonicalize(storeProto)
canonicalize(acctProto)
stats.peersChecked++ stats.peersChecked++
if !goproto.Equal(legacyProto, newProto) { if !goproto.Equal(legacyProto, storeProto) {
t.Fatalf("after %d peers: %s", stats.peersChecked, describeDivergence(legacyProto, newProto, account.Id, peerID)) t.Fatalf("after %d peers: store path: %s", stats.peersChecked, describeDivergence(legacyProto, storeProto, account.Id, peerID))
}
if !goproto.Equal(legacyProto, acctProto) {
t.Fatalf("after %d peers: account path: %s", stats.peersChecked, describeDivergence(legacyProto, acctProto, account.Id, peerID))
} }
} }
} }