server: make public API clearer

fixes osrg#1331

Signed-off-by: Wataru Ishida <ishida.wataru@lab.ntt.co.jp>

api/grpc_server.go

@@ -2059,7 +2059,7 @@ func (s *Server) StartServer(ctx context.Context, arg *StartServerRequest) (*Sta
 }
 
 func (s *Server) StopServer(ctx context.Context, arg *StopServerRequest) (*StopServerResponse, error) {
-	return &StopServerResponse{}, s.bgpServer.Stop()
+	return &StopServerResponse{}, s.bgpServer.StopServer()
 }
 
 func (s *Server) GetRibInfo(ctx context.Context, arg *GetRibInfoRequest) (*GetRibInfoResponse, error) {

server/server.go

@@ -103,6 +103,13 @@ type BgpServer struct {
 	zclient     *zebraClient
 	bmpManager  *bmpClientManager
 	mrtManager  *mrtManager
+	endCh       chan chan error
+}
+
+func New(c *config.Global) (*BgpServer, error) {
+	s := NewBgpServer()
+	go s.Serve()
+	return s, s.Start(c)
 }
 
 func NewBgpServer() *BgpServer {
@@ -113,6 +120,7 @@ func NewBgpServer() *BgpServer {
 		roaManager:  roaManager,
 		mgmtCh:      make(chan *mgmtOp, 1),
 		watcherMap:  make(map[WatchEventType][]*Watcher),
+		endCh:       make(chan chan error),
 	}
 	s.bmpManager = newBmpClientManager(s)
 	s.mrtManager = newMrtManager(s)
@@ -274,6 +282,9 @@ func (server *BgpServer) Serve() {
 			handleFsmMsg(e.(*FsmMsg))
 		case e := <-server.fsmStateCh:
 			handleFsmMsg(e)
+		case ch := <-server.endCh:
+			ch <- server.stopServer()
+			return
 		}
 	}
 }
@@ -1287,22 +1298,32 @@ func (s *BgpServer) DeleteVrf(name string) error {
 	}, true)
 }
 
-func (s *BgpServer) Stop() error {
-	return s.mgmtOperation(func() error {
-		for k, _ := range s.neighborMap {
-			if err := s.deleteNeighbor(&config.Neighbor{Config: config.NeighborConfig{
-				NeighborAddress: k}}, bgp.BGP_ERROR_CEASE, bgp.BGP_ERROR_SUB_PEER_DECONFIGURED); err != nil {
-				return err
-			}
-		}
-		for _, l := range s.listeners {
-			l.Close()
+func (s *BgpServer) stopServer() error {
+	for k, _ := range s.neighborMap {
+		if err := s.deleteNeighbor(&config.Neighbor{Config: config.NeighborConfig{
+			NeighborAddress: k}}, bgp.BGP_ERROR_CEASE, bgp.BGP_ERROR_SUB_PEER_DECONFIGURED); err != nil {
+			return err
 		}
-		s.bgpConfig.Global = config.Global{}
-		return nil
+	}
+	for _, l := range s.listeners {
+		l.Close()
+	}
+	s.bgpConfig.Global = config.Global{}
+	return nil
+}
+
+func (s *BgpServer) StopServer() error {
+	return s.mgmtOperation(func() error {
+		return s.stopServer()
 	}, true)
 }
 
+func (s *BgpServer) Stop() error {
+	ch := make(chan error)
+	s.endCh <- ch
+	return <-ch
+}
+
 func (s *BgpServer) softResetIn(addr string, family bgp.RouteFamily) error {
 	peers, err := s.addrToPeers(addr)
 	if err != nil {

server/server_test.go

@@ -29,15 +29,18 @@ import (
 
 func TestModPolicyAssign(t *testing.T) {
 	assert := assert.New(t)
-	s := NewBgpServer()
-	go s.Serve()
-	s.Start(&config.Global{
+	s, err := New(&config.Global{
 		Config: config.GlobalConfig{
 			As:       1,
 			RouterId: "1.1.1.1",
+			Port:     -1,
 		},
 	})
-	err := s.AddPolicy(&table.Policy{Name: "p1"}, false)
+	defer func() {
+		assert.Nil(s.Stop())
+	}()
+	assert.Nil(err)
+	err = s.AddPolicy(&table.Policy{Name: "p1"}, false)
 	assert.Nil(err)
 
 	err = s.AddPolicy(&table.Policy{Name: "p2"}, false)
@@ -60,15 +63,18 @@ func TestModPolicyAssign(t *testing.T) {
 
 func TestMonitor(test *testing.T) {
 	assert := assert.New(test)
-	s := NewBgpServer()
-	go s.Serve()
-	s.Start(&config.Global{
+	log.SetLevel(log.DebugLevel)
+	s, err := New(&config.Global{
 		Config: config.GlobalConfig{
 			As:       1,
 			RouterId: "1.1.1.1",
 			Port:     10179,
 		},
 	})
+	defer func() {
+		assert.Nil(s.Stop())
+	}()
+	assert.Nil(err)
 	n := &config.Neighbor{
 		Config: config.NeighborConfig{
 			NeighborAddress: "127.0.0.1",
@@ -83,15 +89,17 @@ func TestMonitor(test *testing.T) {
 	if err := s.AddNeighbor(n); err != nil {
 		log.Fatal(err)
 	}
-	t := NewBgpServer()
-	go t.Serve()
-	t.Start(&config.Global{
+	t, err := New(&config.Global{
 		Config: config.GlobalConfig{
 			As:       2,
 			RouterId: "2.2.2.2",
 			Port:     -1,
 		},
 	})
+	defer func() {
+		assert.Nil(t.Stop())
+	}()
+	assert.Nil(err)
 	m := &config.Neighbor{
 		Config: config.NeighborConfig{
 			NeighborAddress: "127.0.0.1",
@@ -149,15 +157,16 @@ func TestMonitor(test *testing.T) {
 
 func TestNumGoroutineWithAddDeleteNeighbor(t *testing.T) {
 	assert := assert.New(t)
-	s := NewBgpServer()
-	go s.Serve()
-	err := s.Start(&config.Global{
+	s, err := New(&config.Global{
 		Config: config.GlobalConfig{
 			As:       1,
 			RouterId: "1.1.1.1",
 			Port:     -1,
 		},
 	})
+	defer func() {
+		assert.Nil(s.Stop())
+	}()
 	assert.Nil(err)
 
 	num := runtime.NumGoroutine()