UpdateRoutes: drop updateServerRoutes() handling

more details at netbirdio#612 (comment)
nazarewk · Dec 8, 2022 · d1c35f3 · d1c35f3
1 parent a4b82dc
commit d1c35f3
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 103 deletions.
diff --git a/client/internal/routemanager/manager.go b/client/internal/routemanager/manager.go
@@ -2,7 +2,6 @@ package routemanager
 
 import (
 	"context"
-	"fmt"
 	"runtime"
 	"sync"
 
@@ -84,58 +83,6 @@ func (m *DefaultManager) updateClientNetworks(updateSerial uint64, networks map[
 	}
 }
 
-func (m *DefaultManager) updateServerRoutes(routesMap map[string]*route.Route) error {
-	serverRoutesToRemove := make([]string, 0)
-
-	if len(routesMap) > 0 {
-		err := m.serverRouter.firewall.RestoreOrCreateContainers()
-		if err != nil {
-			return fmt.Errorf("couldn't initialize firewall containers, got err: %v", err)
-		}
-	}
-
-	for routeID := range m.serverRoutes {
-		update, found := routesMap[routeID]
-		if !found || !update.IsEqual(m.serverRoutes[routeID]) {
-			serverRoutesToRemove = append(serverRoutesToRemove, routeID)
-			continue
-		}
-	}
-
-	for _, routeID := range serverRoutesToRemove {
-		oldRoute := m.serverRoutes[routeID]
-		err := m.removeFromServerNetwork(oldRoute)
-		if err != nil {
-			log.Errorf("unable to remove route id: %s, network %s, from server, got: %v",
-				oldRoute.ID, oldRoute.Network, err)
-		}
-		delete(m.serverRoutes, routeID)
-	}
-
-	for id, newRoute := range routesMap {
-		_, found := m.serverRoutes[id]
-		if found {
-			continue
-		}
-
-		err := m.addToServerNetwork(newRoute)
-		if err != nil {
-			log.Errorf("unable to add route %s from server, got: %v", newRoute.ID, err)
-			continue
-		}
-		m.serverRoutes[id] = newRoute
-	}
-
-	if len(m.serverRoutes) > 0 {
-		err := enableIPForwarding()
-		if err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // UpdateRoutes compares received routes with existing routes and remove, update or add them to the client and server maps
 func (m *DefaultManager) UpdateRoutes(updateSerial uint64, newRoutes []*route.Route) error {
 	select {
@@ -147,25 +94,22 @@ func (m *DefaultManager) UpdateRoutes(updateSerial uint64, newRoutes []*route.Ro
 		defer m.mux.Unlock()
 
 		newClientRoutesIDMap := make(map[string][]*route.Route)
-		newServerRoutesMap := make(map[string]*route.Route)
 		ownNetworkIDs := make(map[string]bool)
 
 		for _, newRoute := range newRoutes {
 			if newRoute.Peer == m.pubKey {
+				// only linux is supported for now
+				if runtime.GOOS != "linux" {
+					log.Warnf("received a route to manage, but agent doesn't support router mode on %s OS", runtime.GOOS)
+					continue
+				}
 				ownNetworkIDs[getHANetworkID(newRoute)] = true
 			}
 		}
 
 		for _, newRoute := range newRoutes {
 			networkID := getHANetworkID(newRoute)
-			if ownNetworkIDs[networkID] {
-				// only linux is supported for now
-				if runtime.GOOS != "linux" {
-					log.Warnf("received a route to manage, but agent doesn't support router mode on %s OS", runtime.GOOS)
-					continue
-				}
-				newServerRoutesMap[newRoute.ID] = newRoute
-			} else {
+			if !ownNetworkIDs[networkID] {
 				// if prefix is too small, lets assume is a possible default route which is not yet supported
 				// we skip this route management
 				if newRoute.Network.Bits() < 7 {
@@ -179,11 +123,6 @@ func (m *DefaultManager) UpdateRoutes(updateSerial uint64, newRoutes []*route.Ro
 
 		m.updateClientNetworks(updateSerial, newClientRoutesIDMap)
 
-		err := m.updateServerRoutes(newServerRoutesMap)
-		if err != nil {
-			return err
-		}
-
 		return nil
 	}
 }
diff --git a/client/internal/routemanager/manager_test.go b/client/internal/routemanager/manager_test.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"net/netip"
-	"runtime"
 	"testing"
 
 	"github.com/netbirdio/netbird/client/status"
@@ -28,8 +27,6 @@ func TestManagerUpdateRoutes(t *testing.T) {
 		inputInitRoutes               []*route.Route
 		inputRoutes                   []*route.Route
 		inputSerial                   uint64
-		shouldCheckServerRoutes       bool
-		serverRoutesExpected          int
 		clientNetworkWatchersExpected int
 	}{
 		{
@@ -61,7 +58,7 @@ func TestManagerUpdateRoutes(t *testing.T) {
 			clientNetworkWatchersExpected: 2,
 		},
 		{
-			name: "Should Create 2 Server Routes",
+			name: "Should Create 0 client networks",
 			inputRoutes: []*route.Route{
 				{
 					ID:          "a",
@@ -85,12 +82,10 @@ func TestManagerUpdateRoutes(t *testing.T) {
 				},
 			},
 			inputSerial:                   1,
-			shouldCheckServerRoutes:       runtime.GOOS == "linux",
-			serverRoutesExpected:          2,
 			clientNetworkWatchersExpected: 0,
 		},
 		{
-			name: "Should Create 1 Route For Client And Server",
+			name: "Should Create 1 Route For Client",
 			inputRoutes: []*route.Route{
 				{
 					ID:          "a",
@@ -114,8 +109,6 @@ func TestManagerUpdateRoutes(t *testing.T) {
 				},
 			},
 			inputSerial:                   1,
-			shouldCheckServerRoutes:       runtime.GOOS == "linux",
-			serverRoutesExpected:          1,
 			clientNetworkWatchersExpected: 1,
 		},
 		{
@@ -172,25 +165,6 @@ func TestManagerUpdateRoutes(t *testing.T) {
 			inputSerial:                   1,
 			clientNetworkWatchersExpected: 0,
 		},
-		{
-			name: "No Server Routes Should Be Added To Non Linux",
-			inputRoutes: []*route.Route{
-				{
-					ID:          "a",
-					NetID:       "routeA",
-					Peer:        localPeerKey,
-					Network:     netip.MustParsePrefix("1.2.3.4/32"),
-					NetworkType: route.IPv4Network,
-					Metric:      9999,
-					Masquerade:  false,
-					Enabled:     true,
-				},
-			},
-			inputSerial:                   1,
-			shouldCheckServerRoutes:       runtime.GOOS != "linux",
-			serverRoutesExpected:          0,
-			clientNetworkWatchersExpected: 0,
-		},
 		{
 			name: "Remove 1 Client Route",
 			inputInitRoutes: []*route.Route{
@@ -333,8 +307,6 @@ func TestManagerUpdateRoutes(t *testing.T) {
 			},
 			inputRoutes:                   []*route.Route{},
 			inputSerial:                   1,
-			shouldCheckServerRoutes:       true,
-			serverRoutesExpected:          0,
 			clientNetworkWatchersExpected: 0,
 		},
 		{
@@ -382,8 +354,6 @@ func TestManagerUpdateRoutes(t *testing.T) {
 				},
 			},
 			inputSerial:                   1,
-			shouldCheckServerRoutes:       runtime.GOOS == "linux",
-			serverRoutesExpected:          3,
 			clientNetworkWatchersExpected: 1,
 		},
 	}
@@ -411,10 +381,6 @@ func TestManagerUpdateRoutes(t *testing.T) {
 			require.NoError(t, err, "should update routes")
 
 			require.Len(t, routeManager.clientNetworks, testCase.clientNetworkWatchersExpected, "client networks size should match")
-
-			if testCase.shouldCheckServerRoutes {
-				require.Len(t, routeManager.serverRoutes, testCase.serverRoutesExpected, "server networks size should match")
-			}
 		})
 	}
 }