Split Reset into three methods

The Reset method was difficult to understand. The reason is that it
was called in three different ways and did different things in each
case. It is easier to read when the three different modes are each
their own method.

This came up as I was looking deeper into the threading model around
policies. I think this change makes it easier to understand the code.

2 files changed, 48 insertions, 21 deletions

diff --git a/internal/pkg/table/policy.go b/internal/pkg/table/policy.go
index 3395863d..35fa8c58 100644
--- a/internal/pkg/table/policy.go
+++ b/internal/pkg/table/policy.go
@@ -3832,32 +3832,59 @@ func (r *RoutingPolicy) SetPolicyAssignment(id string, dir PolicyDirection, poli
 	return err
 }
 
-func (r *RoutingPolicy) Reset(rp *config.RoutingPolicy, ap map[string]config.ApplyPolicy) error {
+func (r *RoutingPolicy) Initialize() error {
 	r.mu.Lock()
 	defer r.mu.Unlock()
 
-	if rp != nil {
-		if err := r.reload(*rp); err != nil {
+	if err := r.reload(config.RoutingPolicy{}); err != nil {
+		log.WithFields(log.Fields{
+			"Topic": "Policy",
+		}).Errorf("failed to create routing policy: %s", err)
+		return err
+	}
+	return nil
+}
+
+func (r *RoutingPolicy) setPeerPolicy(id string, c config.ApplyPolicy) {
+	for _, dir := range []PolicyDirection{POLICY_DIRECTION_IMPORT, POLICY_DIRECTION_EXPORT} {
+		ps, def, err := r.getAssignmentFromConfig(dir, c)
+		if err != nil {
 			log.WithFields(log.Fields{
 				"Topic": "Policy",
-			}).Errorf("failed to create routing policy: %s", err)
-			return err
+				"Dir":   dir,
+			}).Errorf("failed to get policy info: %s", err)
+			continue
 		}
+		r.setDefaultPolicy(id, dir, def)
+		r.setPolicy(id, dir, ps)
+	}
+}
+
+func (r *RoutingPolicy) SetPeerPolicy(peerId string, c config.ApplyPolicy) error {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+
+	r.setPeerPolicy(peerId, c)
+	return nil
+}
+
+func (r *RoutingPolicy) Reset(rp *config.RoutingPolicy, ap map[string]config.ApplyPolicy) error {
+	if rp == nil {
+		return fmt.Errorf("routing Policy is nil in call to Reset")
+	}
+
+	r.mu.Lock()
+	defer r.mu.Unlock()
+
+	if err := r.reload(*rp); err != nil {
+		log.WithFields(log.Fields{
+			"Topic": "Policy",
+		}).Errorf("failed to create routing policy: %s", err)
+		return err
 	}
 
 	for id, c := range ap {
-		for _, dir := range []PolicyDirection{POLICY_DIRECTION_IMPORT, POLICY_DIRECTION_EXPORT} {
-			ps, def, err := r.getAssignmentFromConfig(dir, c)
-			if err != nil {
-				log.WithFields(log.Fields{
-					"Topic": "Policy",
-					"Dir":   dir,
-				}).Errorf("failed to get policy info: %s", err)
-				continue
-			}
-			r.setDefaultPolicy(id, dir, def)
-			r.setPolicy(id, dir, ps)
-		}
+		r.setPeerPolicy(id, c)
 	}
 	return nil
 }
diff --git a/pkg/server/server.go b/pkg/server/server.go
index 4ee64cf6..2312b39d 100644
--- a/pkg/server/server.go
+++ b/pkg/server/server.go
@@ -307,7 +307,7 @@ func (s *BgpServer) passConnToPeer(conn *net.TCPConn) {
 		peer.fsm.lock.RLock()
 		policy := peer.fsm.pConf.ApplyPolicy
 		peer.fsm.lock.RUnlock()
-		s.policy.Reset(nil, map[string]config.ApplyPolicy{peer.ID(): policy})
+		s.policy.SetPeerPolicy(peer.ID(), policy)
 		s.neighborMap[remoteAddr] = peer
 		peer.startFSMHandler()
 		s.broadcastPeerState(peer, bgp.BGP_FSM_ACTIVE, nil)
@@ -2111,7 +2111,7 @@ func (s *BgpServer) StartBgp(ctx context.Context, r *api.StartBgpRequest) error
 		s.globalRib = table.NewTableManager(rfs)
 		s.rsRib = table.NewTableManager(rfs)
 
-		if err := s.policy.Reset(&config.RoutingPolicy{}, map[string]config.ApplyPolicy{}); err != nil {
+		if err := s.policy.Initialize(); err != nil {
 			return err
 		}
 		s.bgpConfig.Global = *c
@@ -2812,7 +2812,7 @@ func (s *BgpServer) addNeighbor(c *config.Neighbor) error {
 	}
 	peer := newPeer(&s.bgpConfig.Global, c, rib, s.policy)
 	s.addIncoming(peer.fsm.incomingCh)
-	s.policy.Reset(nil, map[string]config.ApplyPolicy{peer.ID(): c.ApplyPolicy})
+	s.policy.SetPeerPolicy(peer.ID(), c.ApplyPolicy)
 	s.neighborMap[addr] = peer
 	if name := c.Config.PeerGroup; name != "" {
 		s.peerGroupMap[name].AddMember(*c)
@@ -2996,7 +2996,7 @@ func (s *BgpServer) updateNeighbor(c *config.Neighbor) (needsSoftResetIn bool, e
 			"Topic": "Peer",
 			"Key":   addr,
 		}).Info("Update ApplyPolicy")
-		s.policy.Reset(nil, map[string]config.ApplyPolicy{peer.ID(): c.ApplyPolicy})
+		s.policy.SetPeerPolicy(peer.ID(), c.ApplyPolicy)
 		peer.fsm.pConf.ApplyPolicy = c.ApplyPolicy
 		needsSoftResetIn = true
 	}