lxd/bgp: Rework start/stop logic

This simplifies the BGP package a bit and improves on the ability to
reconfigure it.

The go-bgp ability to start/stop the daemon doesn't work reliably, so we
need to spawn a completely new listener instead.

This moves Start/Stop interactions to the main Reconfigure option and
makes sure to always feed all peers and prefixes on startup.

Signed-off-by: Stéphane Graber <[email protected]>
(cherry picked from commit 6a0d0137735c260b5872e27c7d94720e8d989228)
Signed-off-by: Nikita Mezhenskyi <[email protected]>
License: Apache-2.0

Author: stgraber

lxd/api_1.0.go

@@ -996,7 +996,7 @@ func doAPI10UpdateTriggers(d *Daemon, nodeChanged, clusterChanged map[string]str
 			return fmt.Errorf("Cannot convert BGP ASN to uint32: Upper bound exceeded")
 		}
 
-		err := s.BGP.Reconfigure(address, uint32(asn), net.ParseIP(routerid))
+		err := s.BGP.Configure(address, uint32(asn), net.ParseIP(routerid))
 		if err != nil {
 			return fmt.Errorf("Failed reconfiguring BGP: %w", err)
 		}

lxd/bgp/server.go

@@ -56,45 +56,21 @@ func NewServer() *Server {
 	return s
 }
 
-func (s *Server) setup() {
-	if s.bgp != nil {
-		return
-	}
-
-	// Spawn the BGP goroutines.
-	s.bgp = bgpServer.NewBgpServer()
-	go s.bgp.Serve()
-
-	// Insert any path that's already defined.
-	if len(s.paths) > 0 {
-		// Reset the path list.
-		paths := s.paths
-		s.paths = map[string]path{}
-
-		for _, path := range paths {
-			err := s.addPrefix(path.prefix, path.nexthop, path.owner)
-			logger.Warn("Unable to add prefix to BGP server", logger.Ctx{"prefix": path.prefix.String(), "err": err})
-		}
-	}
-}
-
 // Start sets up the BGP listener.
-func (s *Server) Start(address string, asn uint32, routerID net.IP) error {
-	// Locking.
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
-	return s.start(address, asn, routerID)
-}
-
 func (s *Server) start(address string, asn uint32, routerID net.IP) error {
 	// If routerID is nil, fill with our best guess.
 	if routerID == nil || routerID.To4() == nil {
 		return ErrBadRouterID
 	}
 
-	// Make sure we have a BGP instance.
-	s.setup()
+	// Check if already running
+	if s.bgp != nil {
+		return fmt.Errorf("BGP listener is already running")
+	}
+
+	// Spawn the BGP goroutines.
+	s.bgp = bgpServer.NewBgpServer()
+	go s.bgp.Serve()
 
 	// Get the address and port.
 	addrHost, addrPort, err := net.SplitHostPort(address)
@@ -131,8 +107,30 @@ func (s *Server) start(address string, asn uint32, routerID net.IP) error {
 		return err
 	}
 
-	// Add any existing peers.
-	for _, peer := range s.peers {
+	// Copy the path list
+	oldPaths := map[string]path{}
+	for pathUUID, path := range s.paths {
+		oldPaths[pathUUID] = path
+	}
+
+	// Add existing paths.
+	s.paths = map[string]path{}
+	for _, path := range oldPaths {
+		err := s.addPrefix(path.prefix, path.nexthop, path.owner)
+		if err != nil {
+			logger.Warn("Unable to add prefix to BGP server", logger.Ctx{"prefix": path.prefix.String(), "err": err})
+		}
+	}
+
+	// Copy the peer list.
+	oldPeers := map[string]peer{}
+	for peerUUID, peer := range s.peers {
+		oldPeers[peerUUID] = peer
+	}
+
+	// Add existing peers.
+	s.peers = map[string]peer{}
+	for _, peer := range oldPeers {
 		err := s.addPeer(peer.address, peer.asn, peer.password, peer.holdtime)
 		if err != nil {
 			return err
@@ -148,20 +146,18 @@ func (s *Server) start(address string, asn uint32, routerID net.IP) error {
 }
 
 // Stop tears down the BGP listener.
-func (s *Server) Stop() error {
-	// Locking.
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
-	return s.stop()
-}
-
 func (s *Server) stop() error {
 	// Skip if no instance.
 	if s.bgp == nil {
 		return nil
 	}
 
+	// Save the peer list.
+	oldPeers := map[string]peer{}
+	for peerUUID, peer := range s.peers {
+		oldPeers[peerUUID] = peer
+	}
+
 	// Remove all the peers (ignore failures).
 	for _, peer := range s.peers {
 		err := s.removePeer(peer.address)
@@ -170,37 +166,38 @@ func (s *Server) stop() error {
 		}
 	}
 
+	// Restore peer list.
+	s.peers = oldPeers
+
 	// Stop the listener.
 	err := s.bgp.StopBgp(context.Background(), &bgpAPI.StopBgpRequest{})
 	if err != nil {
 		return err
 	}
 
-	// Unset the address
+	// Mark the daemon as down.
 	s.address = ""
 	s.asn = 0
 	s.routerID = nil
+	s.bgp = nil
+
 	return nil
 }
 
-// Reconfigure updates the listener with a new configuration..
-func (s *Server) Reconfigure(address string, asn uint32, routerID net.IP) error {
+// Configure updates the listener with a new configuration..
+func (s *Server) Configure(address string, asn uint32, routerID net.IP) error {
 	// Locking.
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	return s.reconfigure(address, asn, routerID)
+	return s.configure(address, asn, routerID)
 }
 
-func (s *Server) reconfigure(address string, asn uint32, routerID net.IP) error {
-	// Get the old address.
+func (s *Server) configure(address string, asn uint32, routerID net.IP) error {
+	// Store current configuration for reverting.
 	oldAddress := s.address
 	oldASN := s.asn
 	oldRouterID := s.routerID
-	oldPeers := map[string]peer{}
-	for peerUUID, peer := range s.peers {
-		oldPeers[peerUUID] = peer
-	}
 
 	// Setup reverter.
 	revert := revert.New()
@@ -209,12 +206,9 @@ func (s *Server) reconfigure(address string, asn uint32, routerID net.IP) error
 	// Stop the listener.
 	err := s.stop()
 	if err != nil {
-		return err
+		return fmt.Errorf("Failed to stop current listener: %w", err)
 	}
 
-	// Restore peer list.
-	s.peers = oldPeers
-
 	// Check if we should start.
 	if address != "" && asn > 0 && routerID != nil {
 		// Restore old address on failure.
@@ -223,7 +217,7 @@ func (s *Server) reconfigure(address string, asn uint32, routerID net.IP) error
 		// Start the listener with the new address.
 		err = s.start(address, asn, routerID)
 		if err != nil {
-			return err
+			return fmt.Errorf("Failed to start new listener: %w", err)
 		}
 	}
 

lxd/daemon.go

@@ -1762,7 +1762,7 @@ func (d *Daemon) init() error {
 			return fmt.Errorf("Cannot convert BGP ASN to uint32: Upper bound exceeded")
 		}
 
-		err := d.bgp.Start(bgpAddress, uint32(bgpASN), net.ParseIP(bgpRouterID))
+		err := d.bgp.Configure(bgpAddress, uint32(bgpASN), net.ParseIP(bgpRouterID))
 		if err != nil {
 			return err
 		}