Fixup case of config only map superseeding nm map

This commit is contained in:
riccardom
2026-06-30 23:43:44 +02:00
parent b70fc4015b
commit cf8d92fbb0
2 changed files with 45 additions and 1 deletions

View File

@@ -6,6 +6,7 @@ import (
"time"
log "github.com/sirupsen/logrus"
"google.golang.org/protobuf/proto"
mgmProto "github.com/netbirdio/netbird/shared/management/proto"
)
@@ -109,7 +110,22 @@ func (m *mapStateManager) SetTarget(update *mgmProto.SyncResponse) error {
// rest of the manager (generation tracking, convergence, signaling) is unaffected
// because it already treats target as "the complete desired state, whatever it is".
func (m *mapStateManager) mergeTarget(prev, update *mgmProto.SyncResponse) *mgmProto.SyncResponse {
return update
// Plain replace unless a config-only update (no map) arrives while the pending
// target still has an unconverged map (targetGen > appliedGen). In that case graft
// the pending map (and its checks) onto the incoming config update, so the next pass
// keeps converging the map instead of settling on a nil-map target and stranding the
// remaining peer batches until another full map arrives.
if update == nil || update.GetNetworkMap() != nil || prev == nil || prev.GetNetworkMap() == nil || m.targetGen == m.appliedGen {
return update
}
merged, ok := proto.Clone(update).(*mgmProto.SyncResponse)
if !ok {
return update
}
merged.NetworkMap = prev.GetNetworkMap()
merged.Checks = prev.Checks
return merged
}
// run drives convergence until ctx is done. It is meant to run in its own goroutine.

View File

@@ -12,6 +12,34 @@ import (
mgmProto "github.com/netbirdio/netbird/shared/management/proto"
)
// a config-only update arriving while a full map is still converging must keep the
// pending map (so its remaining peer batches still apply); once converged or when the
// pending target has no map, it replaces as usual.
func TestMapStateManager_MergeTargetPreservesPendingMap(t *testing.T) {
m := newMapStateManager(nil, nil, nil)
fullMap := &mgmProto.SyncResponse{NetworkMap: &mgmProto.NetworkMap{Serial: 5}}
configOnly := &mgmProto.SyncResponse{NetbirdConfig: &mgmProto.NetbirdConfig{}}
// still converging the full map (targetGen > appliedGen): graft the map onto the
// incoming config-only update instead of dropping it
m.targetGen, m.appliedGen = 5, 4
merged := m.mergeTarget(fullMap, configOnly)
require.NotNil(t, merged.GetNetworkMap(), "pending map must be preserved")
require.EqualValues(t, 5, merged.GetNetworkMap().GetSerial())
require.NotNil(t, merged.GetNetbirdConfig(), "new config must be carried")
require.NotSame(t, configOnly, merged, "must not mutate the received update in place")
// already converged (targetGen == appliedGen): nothing pending -> plain replace
m.targetGen, m.appliedGen = 5, 5
require.Same(t, configOnly, m.mergeTarget(fullMap, configOnly))
// a full map always replaces
newFull := &mgmProto.SyncResponse{NetworkMap: &mgmProto.NetworkMap{Serial: 6}}
m.targetGen, m.appliedGen = 5, 4
require.Same(t, newFull, m.mergeTarget(fullMap, newFull))
}
// converges over the bounded passes (apply returns more until the 3rd pass),
// fires onConverged exactly once, then blocks (no further apply) until a new target.
func TestMapStateManager_ConvergesThenStops(t *testing.T) {