Skip to content

Commit 93c32f7

Browse files
spikecurtisspikecurtis
committed
fix: stop spamming DERP map updates for equivalent maps
1 parent f5dbc71 commit 93c32f7

File tree

5 files changed

+46
-36
lines changed

5 files changed

+46
-36
lines changed

‎coderd/workspaceagents.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,7 @@ func (api *API) _dialWorkspaceAgentTailnet(agentID uuid.UUID) (*codersdk.Workspa
904904
}
905905

906906
derpMap := api.DERPMap()
907-
if lastDERPMap == nil || tailnet.CompareDERPMaps(lastDERPMap, derpMap) {
907+
if lastDERPMap == nil || !tailnet.CompareDERPMaps(lastDERPMap, derpMap) {
908908
conn.SetDERPMap(derpMap)
909909
lastDERPMap = derpMap
910910
}

‎tailnet/configmaps.go

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ type configMaps struct {
6565
static netmap.NetworkMap
6666
peers map[uuid.UUID]*peerLifecycle
6767
addresses []netip.Prefix
68-
derpMap *proto.DERPMap
68+
derpMap *tailcfg.DERPMap
6969
logger slog.Logger
7070
blockEndpoints bool
7171

@@ -204,7 +204,7 @@ func (c *configMaps) netMapLocked() *netmap.NetworkMap {
204204
nm.Addresses = make([]netip.Prefix, len(c.addresses))
205205
copy(nm.Addresses, c.addresses)
206206

207-
nm.DERPMap = DERPMapFromProto(c.derpMap)
207+
nm.DERPMap = c.derpMap.Clone()
208208
nm.Peers = c.peerConfigLocked()
209209
nm.SelfNode.Addresses = nm.Addresses
210210
nm.SelfNode.AllowedIPs = nm.Addresses
@@ -255,15 +255,10 @@ func (c *configMaps) setBlockEndpoints(blockEndpoints bool) {
255255

256256
// setDERPMap sets the DERP map, triggering a configuration of the engine if it has changed.
257257
// c.L MUST NOT be held.
258-
func (c *configMaps) setDERPMap(derpMap *proto.DERPMap) {
258+
func (c *configMaps) setDERPMap(derpMap *tailcfg.DERPMap) {
259259
c.L.Lock()
260260
defer c.L.Unlock()
261-
eq, err := c.derpMap.Equal(derpMap)
262-
if err != nil {
263-
c.logger.Critical(context.Background(), "failed to compare DERP maps", slog.Error(err))
264-
return
265-
}
266-
if eq {
261+
if CompareDERPMaps(c.derpMap, derpMap) {
267262
return
268263
}
269264
c.derpMap = derpMap
@@ -273,8 +268,7 @@ func (c *configMaps) setDERPMap(derpMap *proto.DERPMap) {
273268

274269
// derMapLocked returns the current DERPMap. c.L must be held
275270
func (c *configMaps) derpMapLocked() *tailcfg.DERPMap {
276-
m := DERPMapFromProto(c.derpMap)
277-
return m
271+
return c.derpMap.Clone()
278272
}
279273

280274
// reconfig computes the correct wireguard config and calls the engine.Reconfig

‎tailnet/configmaps_internal_test.go

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -674,12 +674,12 @@ func TestConfigMaps_setDERPMap_different(t *testing.T) {
674674
uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public())
675675
defer uut.close()
676676

677-
derpMap := &proto.DERPMap{
678-
HomeParams: &proto.DERPMap_HomeParams{RegionScore: map[int64]float64{1: 0.025}},
679-
Regions: map[int64]*proto.DERPMap_Region{
677+
derpMap := &tailcfg.DERPMap{
678+
HomeParams: &tailcfg.DERPHomeParams{RegionScore: map[int]float64{1: 0.025}},
679+
Regions: map[int]*tailcfg.DERPRegion{
680680
1: {
681681
RegionCode: "AUH",
682-
Nodes: []*proto.DERPMap_Region_Node{
682+
Nodes: []*tailcfg.DERPNode{
683683
{Name: "AUH0"},
684684
},
685685
},
@@ -716,15 +716,24 @@ func TestConfigMaps_setDERPMap_same(t *testing.T) {
716716
defer uut.close()
717717

718718
// Given: DERP Map already set
719-
derpMap := &proto.DERPMap{
720-
HomeParams: &proto.DERPMap_HomeParams{RegionScore: map[int64]float64{1: 0.025}},
721-
Regions: map[int64]*proto.DERPMap_Region{
719+
derpMap := &tailcfg.DERPMap{
720+
HomeParams: &tailcfg.DERPHomeParams{RegionScore: map[int]float64{
721+
1: 0.025,
722+
1001: 0.111,
723+
}},
724+
Regions: map[int]*tailcfg.DERPRegion{
722725
1: {
723726
RegionCode: "AUH",
724-
Nodes: []*proto.DERPMap_Region_Node{
727+
Nodes: []*tailcfg.DERPNode{
725728
{Name: "AUH0"},
726729
},
727730
},
731+
1001: {
732+
RegionCode: "DXB",
733+
Nodes: []*tailcfg.DERPNode{
734+
{Name: "DXB0"},
735+
},
736+
},
728737
},
729738
}
730739
uut.L.Lock()
@@ -734,8 +743,27 @@ func TestConfigMaps_setDERPMap_same(t *testing.T) {
734743
// Then: we don't configure
735744
requireNeverConfigures(ctx, t, &uut.phased)
736745

737-
// When we set the same DERP map
738-
uut.setDERPMap(derpMap)
746+
// When we set the equivalent DERP map, with different ordering
747+
uut.setDERPMap(&tailcfg.DERPMap{
748+
HomeParams: &tailcfg.DERPHomeParams{RegionScore: map[int]float64{
749+
1001: 0.111,
750+
1: 0.025,
751+
}},
752+
Regions: map[int]*tailcfg.DERPRegion{
753+
1001: {
754+
RegionCode: "DXB",
755+
Nodes: []*tailcfg.DERPNode{
756+
{Name: "DXB0"},
757+
},
758+
},
759+
1: {
760+
RegionCode: "AUH",
761+
Nodes: []*tailcfg.DERPNode{
762+
{Name: "AUH0"},
763+
},
764+
},
765+
},
766+
})
739767

740768
done := make(chan struct{})
741769
go func() {

‎tailnet/conn.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ func NewConn(options *Options) (conn *Conn, err error) {
218218
magicConn.DiscoPublicKey(),
219219
)
220220
cfgMaps.setAddresses(options.Addresses)
221-
cfgMaps.setDERPMap(DERPMapToProto(options.DERPMap))
221+
cfgMaps.setDERPMap(options.DERPMap)
222222
cfgMaps.setBlockEndpoints(options.BlockEndpoints)
223223

224224
nodeUp := newNodeUpdater(
@@ -326,7 +326,7 @@ func (c *Conn) SetNodeCallback(callback func(node *Node)) {
326326

327327
// SetDERPMap updates the DERPMap of a connection.
328328
func (c *Conn) SetDERPMap(derpMap *tailcfg.DERPMap) {
329-
c.configMaps.setDERPMap(DERPMapToProto(derpMap))
329+
c.configMaps.setDERPMap(derpMap)
330330
}
331331

332332
func (c *Conn) SetDERPForceWebSockets(v bool) {

‎tailnet/proto/compare.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,3 @@ func (s *Node) Equal(o *Node) (bool, error) {
1818
}
1919
return bytes.Equal(sBytes, oBytes), nil
2020
}
21-
22-
func (s *DERPMap) Equal(o *DERPMap) (bool, error) {
23-
sBytes, err := gProto.Marshal(s)
24-
if err != nil {
25-
return false, err
26-
}
27-
oBytes, err := gProto.Marshal(o)
28-
if err != nil {
29-
return false, err
30-
}
31-
return bytes.Equal(sBytes, oBytes), nil
32-
}

0 commit comments

Comments
 (0)