change: drop subnet-router full update, use policy change
Don't send a full update when a subnet router goes up or down; the gated policy change already recomputes peers and is a smaller payload. Updates #3293
This commit is contained in:
parent
cffdb77c8b
commit
5228cb1a40
3 changed files with 97 additions and 18 deletions
|
|
@ -456,29 +456,19 @@ func NodeRemoved(id types.NodeID) Change {
|
|||
return PeersRemoved(id)
|
||||
}
|
||||
|
||||
// NodeOnlineFor returns a [Change] for when a node comes online.
|
||||
// If the node is a subnet router, a full update is sent instead of a patch.
|
||||
// NodeOnlineFor returns the [Change] for a node coming online: a lightweight
|
||||
// [NodeOnline] peer patch. Subnet routers, relay targets, and via targets get
|
||||
// their full peer recompute from the gated [PolicyChange] that State.Connect
|
||||
// emits, so no full update is needed here.
|
||||
func NodeOnlineFor(node types.NodeView) Change {
|
||||
if node.IsSubnetRouter() {
|
||||
c := FullUpdate()
|
||||
c.Reason = "subnet router online"
|
||||
|
||||
return c
|
||||
}
|
||||
|
||||
return NodeOnline(node.ID())
|
||||
}
|
||||
|
||||
// NodeOfflineFor returns a [Change] for when a node goes offline.
|
||||
// If the node is a subnet router, a full update is sent instead of a patch.
|
||||
// NodeOfflineFor returns the [Change] for a node going offline: a lightweight
|
||||
// [NodeOffline] peer patch. As with [NodeOnlineFor], subnet routers and other
|
||||
// recompute-forcing nodes rely on the gated [PolicyChange] from State.Disconnect
|
||||
// for the peer recompute, so no full update is needed here.
|
||||
func NodeOfflineFor(node types.NodeView) Change {
|
||||
if node.IsSubnetRouter() {
|
||||
c := FullUpdate()
|
||||
c.Reason = "subnet router offline"
|
||||
|
||||
return c
|
||||
}
|
||||
|
||||
return NodeOffline(node.ID())
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,11 +1,13 @@
|
|||
package change
|
||||
|
||||
import (
|
||||
"net/netip"
|
||||
"reflect"
|
||||
"testing"
|
||||
|
||||
"github.com/juanfont/headscale/hscontrol/types"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"tailscale.com/tailcfg"
|
||||
)
|
||||
|
||||
|
|
@ -612,3 +614,42 @@ func TestUniqueNodeIDs(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNodeOnlineOfflineForSubnetRouter(t *testing.T) {
|
||||
route := netip.MustParsePrefix("10.0.0.0/24")
|
||||
router := types.Node{
|
||||
ID: 1,
|
||||
Hostinfo: &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{route}},
|
||||
ApprovedRoutes: []netip.Prefix{route},
|
||||
}
|
||||
view := router.View()
|
||||
require.True(t, view.IsSubnetRouter(), "test node must be a subnet router")
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
got Change
|
||||
wantOnline bool
|
||||
}{
|
||||
{name: "online", got: NodeOnlineFor(view), wantOnline: true},
|
||||
{name: "offline", got: NodeOfflineFor(view), wantOnline: false},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// A subnet router's online/offline transition rides the lightweight
|
||||
// peer patch, not a full update: the gated PolicyChange that
|
||||
// State.Connect/Disconnect emit owns the netmap recompute.
|
||||
assert.False(t, tt.got.IsFull(),
|
||||
"subnet router online/offline must be a peer patch, not a full update")
|
||||
|
||||
require.NotEmpty(t, tt.got.PeerPatches,
|
||||
"expected an online/offline peer patch")
|
||||
|
||||
patch := tt.got.PeerPatches[0]
|
||||
assert.Equal(t, view.ID().NodeID(), patch.NodeID)
|
||||
|
||||
require.NotNil(t, patch.Online)
|
||||
assert.Equal(t, tt.wantOnline, *patch.Online)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue