Skip to content

Commit

Permalink
Refactor Route IDs (#1891)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lixmal
lixmal authored May 6, 2024
1 parent 6a49351 commit 4e7c177
Show file tree
Hide file tree
Showing 25 changed files with 320 additions and 292 deletions.
17 changes: 7 additions & 10 deletions client/internal/engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ type Engine struct {
TURNs []*stun.URI

// clientRoutes is the most recent list of clientRoutes received from the Management Service
clientRoutes map[string][]*route.Route
clientRoutes route.HAMap

cancel context.CancelFunc

Expand Down Expand Up @@ -736,9 +736,9 @@ func toRoutes(protoRoutes []*mgmProto.Route) []*route.Route {
for _, protoRoute := range protoRoutes {
_, prefix, _ := route.ParseNetwork(protoRoute.Network)
convertedRoute := &route.Route{
ID: protoRoute.ID,
ID: route.ID(protoRoute.ID),
Network: prefix,
NetID: protoRoute.NetID,
NetID: route.NetID(protoRoute.NetID),
NetworkType: route.NetworkType(protoRoute.NetworkType),
Peer: protoRoute.Peer,
Metric: int(protoRoute.Metric),
Expand Down Expand Up @@ -1238,18 +1238,15 @@ func (e *Engine) newDnsServer() ([]*route.Route, dns.Server, error) {
}

// GetClientRoutes returns the current routes from the route map
func (e *Engine) GetClientRoutes() map[string][]*route.Route {
func (e *Engine) GetClientRoutes() route.HAMap {
return e.clientRoutes
}

// GetClientRoutesWithNetID returns the current routes from the route map, but the keys consist of the network ID only
func (e *Engine) GetClientRoutesWithNetID() map[string][]*route.Route {
routes := make(map[string][]*route.Route, len(e.clientRoutes))
func (e *Engine) GetClientRoutesWithNetID() map[route.NetID][]*route.Route {
routes := make(map[route.NetID][]*route.Route, len(e.clientRoutes))
for id, v := range e.clientRoutes {
if i := strings.LastIndex(id, "-"); i != -1 {
id = id[:i]
}
routes[id] = v
routes[id.NetID()] = v
}
return routes
}
Expand Down
4 changes: 2 additions & 2 deletions client/internal/engine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) {
}{}

mockRouteManager := &routemanager.MockManager{
UpdateRoutesFunc: func(updateSerial uint64, newRoutes []*route.Route) (map[string]*route.Route, map[string][]*route.Route, error) {
UpdateRoutesFunc: func(updateSerial uint64, newRoutes []*route.Route) (map[route.ID]*route.Route, route.HAMap, error) {
input.inputSerial = updateSerial
input.inputRoutes = newRoutes
return nil, nil, testCase.inputErr
Expand Down Expand Up @@ -743,7 +743,7 @@ func TestEngine_UpdateNetworkMapWithDNSUpdate(t *testing.T) {
assert.NoError(t, err, "shouldn't return error")

mockRouteManager := &routemanager.MockManager{
UpdateRoutesFunc: func(updateSerial uint64, newRoutes []*route.Route) (map[string]*route.Route, map[string][]*route.Route, error) {
UpdateRoutesFunc: func(updateSerial uint64, newRoutes []*route.Route) (map[route.ID]*route.Route, route.HAMap, error) {
return nil, nil, nil
},
}
Expand Down
16 changes: 8 additions & 8 deletions client/internal/routemanager/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type clientNetwork struct {
stop context.CancelFunc
statusRecorder *peer.Status
wgInterface *iface.WGIface
routes map[string]*route.Route
routes map[route.ID]*route.Route
routeUpdate chan routesUpdate
peerStateUpdate chan struct{}
routePeersNotifiers map[string]chan struct{}
Expand All @@ -50,7 +50,7 @@ func newClientNetworkWatcher(ctx context.Context, wgInterface *iface.WGIface, st
stop: cancel,
statusRecorder: statusRecorder,
wgInterface: wgInterface,
routes: make(map[string]*route.Route),
routes: make(map[route.ID]*route.Route),
routePeersNotifiers: make(map[string]chan struct{}),
routeUpdate: make(chan routesUpdate),
peerStateUpdate: make(chan struct{}),
Expand All @@ -59,8 +59,8 @@ func newClientNetworkWatcher(ctx context.Context, wgInterface *iface.WGIface, st
return client
}

func (c *clientNetwork) getRouterPeerStatuses() map[string]routerPeerStatus {
routePeerStatuses := make(map[string]routerPeerStatus)
func (c *clientNetwork) getRouterPeerStatuses() map[route.ID]routerPeerStatus {
routePeerStatuses := make(map[route.ID]routerPeerStatus)
for _, r := range c.routes {
peerStatus, err := c.statusRecorder.GetPeer(r.Peer)
if err != nil {
Expand Down Expand Up @@ -90,12 +90,12 @@ func (c *clientNetwork) getRouterPeerStatuses() map[string]routerPeerStatus {
// * Latency: Routes with lower latency are prioritized.
//
// It returns the ID of the selected optimal route.
func (c *clientNetwork) getBestRouteFromStatuses(routePeerStatuses map[string]routerPeerStatus) string {
chosen := ""
func (c *clientNetwork) getBestRouteFromStatuses(routePeerStatuses map[route.ID]routerPeerStatus) route.ID {
chosen := route.ID("")
chosenScore := float64(0)
currScore := float64(0)

currID := ""
currID := route.ID("")
if c.chosenRoute != nil {
currID = c.chosenRoute.ID
}
Expand Down Expand Up @@ -295,7 +295,7 @@ func (c *clientNetwork) sendUpdateToClientNetworkWatcher(update routesUpdate) {
}

func (c *clientNetwork) handleUpdate(update routesUpdate) {
updateMap := make(map[string]*route.Route)
updateMap := make(map[route.ID]*route.Route)

for _, r := range update.routes {
updateMap[r.ID] = r
Expand Down
56 changes: 28 additions & 28 deletions client/internal/routemanager/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,21 @@ func TestGetBestrouteFromStatuses(t *testing.T) {

testCases := []struct {
name string
statuses map[string]routerPeerStatus
expectedRouteID string
currentRoute string
existingRoutes map[string]*route.Route
statuses map[route.ID]routerPeerStatus
expectedRouteID route.ID
currentRoute route.ID
existingRoutes map[route.ID]*route.Route
}{
{
name: "one route",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: true,
relayed: false,
direct: true,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: route.MaxMetric,
Expand All @@ -38,14 +38,14 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
},
{
name: "one connected routes with relayed and direct",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: true,
relayed: true,
direct: true,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: route.MaxMetric,
Expand All @@ -57,14 +57,14 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
},
{
name: "one connected routes with relayed and no direct",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: true,
relayed: true,
direct: false,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: route.MaxMetric,
Expand All @@ -76,14 +76,14 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
},
{
name: "no connected peers",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: false,
relayed: false,
direct: false,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: route.MaxMetric,
Expand All @@ -95,7 +95,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
},
{
name: "multiple connected peers with different metrics",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: true,
relayed: false,
Expand All @@ -107,7 +107,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
direct: true,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: 9000,
Expand All @@ -124,7 +124,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
},
{
name: "multiple connected peers with one relayed",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: true,
relayed: false,
Expand All @@ -136,7 +136,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
direct: true,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: route.MaxMetric,
Expand All @@ -153,7 +153,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
},
{
name: "multiple connected peers with one direct",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: true,
relayed: false,
Expand All @@ -165,7 +165,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
direct: false,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: route.MaxMetric,
Expand All @@ -182,7 +182,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
},
{
name: "multiple connected peers with different latencies",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: true,
latency: 300 * time.Millisecond,
Expand All @@ -192,7 +192,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
latency: 10 * time.Millisecond,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: route.MaxMetric,
Expand All @@ -209,7 +209,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
},
{
name: "should ignore routes with latency 0",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: true,
latency: 0 * time.Millisecond,
Expand All @@ -219,7 +219,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
latency: 10 * time.Millisecond,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: route.MaxMetric,
Expand All @@ -236,7 +236,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
},
{
name: "current route with similar score and similar but slightly worse latency should not change",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: true,
relayed: false,
Expand All @@ -250,7 +250,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
latency: 10 * time.Millisecond,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: route.MaxMetric,
Expand All @@ -267,7 +267,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
},
{
name: "current route with bad score should be changed to route with better score",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: true,
relayed: false,
Expand All @@ -281,7 +281,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
latency: 10 * time.Millisecond,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: route.MaxMetric,
Expand All @@ -298,7 +298,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
},
{
name: "current chosen route doesn't exist anymore",
statuses: map[string]routerPeerStatus{
statuses: map[route.ID]routerPeerStatus{
"route1": {
connected: true,
relayed: false,
Expand All @@ -312,7 +312,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
latency: 10 * time.Millisecond,
},
},
existingRoutes: map[string]*route.Route{
existingRoutes: map[route.ID]*route.Route{
"route1": {
ID: "route1",
Metric: route.MaxMetric,
Expand Down
Loading

0 comments on commit 4e7c177

Please sign in to comment.