From 1f8d2290763122d233871d5e154451764f13afa8 Mon Sep 17 00:00:00 2001 From: Tim Beatham Date: Thu, 4 Jan 2024 21:16:33 +0000 Subject: [PATCH] 81-seperate-synchronisation-into-independent-process - nil dereference due to concurrency issues (the method shouldn't be concurrent) --- pkg/crdt/datastore.go | 8 ++++---- pkg/ctrlserver/ctrlserver.go | 2 +- pkg/mesh/config.go | 7 ++++++- pkg/sync/syncer.go | 20 +++++++++++--------- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/pkg/crdt/datastore.go b/pkg/crdt/datastore.go index 68c7571..ed4fea0 100644 --- a/pkg/crdt/datastore.go +++ b/pkg/crdt/datastore.go @@ -22,7 +22,7 @@ type Route struct { // Destination the route is advertising Destination string // Path to the destination - Path []string + Path []string } // GetDestination implements mesh.Route. @@ -316,7 +316,7 @@ func (m *TwoPhaseStoreMeshManager) AddRoutes(nodeId string, routes ...mesh.Route } } - // Only add nodes on changes. Otherwise the node will advertise new + // Only add nodes on changes. Otherwise the node will advertise new // information whenever they get new routes if changes { m.store.Put(nodeId, node) @@ -466,7 +466,7 @@ func (m *TwoPhaseStoreMeshManager) getRoutes(targetNode string) (map[string]Rout return node.Routes, nil } -// GetRoutes: Get all unique routes the target node is advertising. +// GetRoutes: Get all unique routes the target node is advertising. // on conflicts the route with the least hop count is chosen func (m *TwoPhaseStoreMeshManager) GetRoutes(targetNode string) (map[string]mesh.Route, error) { node, err := m.GetNode(targetNode) @@ -521,7 +521,7 @@ func (m *TwoPhaseStoreMeshManager) RemoveNode(nodeId string) error { return nil } -// GetConfiguration gets the WireGuard configuration to use for this +// GetConfiguration gets the WireGuard configuration to use for this // network func (m *TwoPhaseStoreMeshManager) GetConfiguration() *conf.WgConfiguration { return m.Conf diff --git a/pkg/ctrlserver/ctrlserver.go b/pkg/ctrlserver/ctrlserver.go index cdd25bb..bbe94ff 100644 --- a/pkg/ctrlserver/ctrlserver.go +++ b/pkg/ctrlserver/ctrlserver.go @@ -107,7 +107,7 @@ func NewCtrlServer(params *NewCtrlServerParams) (*MeshCtrlServer, error) { logging.Log.WriteErrorf(err.Error()) } - return nil + return err }, 1) heartbeatTimer := lib.NewTimer(func() error { diff --git a/pkg/mesh/config.go b/pkg/mesh/config.go index c89d421..d205684 100644 --- a/pkg/mesh/config.go +++ b/pkg/mesh/config.go @@ -120,7 +120,12 @@ func (m *WgMeshConfigApplyer) convertMeshNode(params convertMeshNodeParams) (*wg // getRoutes: finds the routes with the least hop distance. If more than one route exists // consistently hash to evenly spread the distribution of traffic func (m *WgMeshConfigApplyer) getRoutes(meshProvider MeshProvider) (map[string][]routeNode, error) { - mesh, _ := meshProvider.GetMesh() + mesh, err := meshProvider.GetMesh() + + if err != nil { + return nil, err + } + routes := make(map[string][]routeNode) peers := lib.Filter(lib.MapValues(mesh.GetNodes()), func(p MeshNode) bool { diff --git a/pkg/sync/syncer.go b/pkg/sync/syncer.go index f92be67..6d28da6 100644 --- a/pkg/sync/syncer.go +++ b/pkg/sync/syncer.go @@ -63,11 +63,6 @@ func (s *SyncerImpl) Sync(correspondingMesh mesh.MeshProvider) (bool, error) { } before := time.Now() - err := s.meshManager.GetRouteManager().UpdateRoutes() - - if err != nil { - logging.Log.WriteErrorf(err.Error()) - } publicKey := s.meshManager.GetPublicKey() nodeNames := correspondingMesh.GetPeers() @@ -231,12 +226,19 @@ func (s *SyncerImpl) SyncMeshes() error { if hasChanges { logging.Log.WriteInfof("updating the WireGuard configuration") err = s.meshManager.ApplyConfig() + + if err != nil { + logging.Log.WriteErrorf("failed to update config %s", err.Error()) + } + + err = s.meshManager.GetRouteManager().UpdateRoutes() + + if err != nil { + logging.Log.WriteErrorf("update routes failed %s", err.Error()) + } } - if err != nil { - logging.Log.WriteInfof("failed to update config %w", err) - } - return nil + return err } type NewSyncerParams struct {