server: fix wait time before active conneciton

also avoid unittest race.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

3 files changed, 10 insertions, 56 deletions

diff --git a/pkg/server/fsm.go b/pkg/server/fsm.go
index 67ab0cfc..319f1eb6 100644
--- a/pkg/server/fsm.go
+++ b/pkg/server/fsm.go
@@ -35,6 +35,10 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
+const (
+	minConnectRetryInterval = 10
+)
+
 type peerDownReason int
 
 const (
@@ -509,13 +513,13 @@ func (h *fsmHandler) connectLoop(ctx context.Context, wg *sync.WaitGroup) {
 	defer wg.Done()
 	fsm := h.fsm
 
-	tick, addr, port, password, ttl, ttlMin, localAddress := func() (int, string, int, string, uint8, uint8, string) {
+	retry, addr, port, password, ttl, ttlMin, localAddress := func() (int, string, int, string, uint8, uint8, string) {
 		fsm.lock.RLock()
 		defer fsm.lock.RUnlock()
 
 		tick := int(fsm.pConf.Timers.Config.ConnectRetry)
-		if tick < minConnectRetry {
-			tick = minConnectRetry
+		if tick < minConnectRetryInterval {
+			tick = minConnectRetryInterval
 		}
 
 		addr := fsm.pConf.State.NeighborAddress
@@ -539,6 +543,7 @@ func (h *fsmHandler) connectLoop(ctx context.Context, wg *sync.WaitGroup) {
 		return tick, addr, port, password, ttl, ttlMin, fsm.pConf.Transport.Config.LocalAddress
 	}()
 
+	tick := minConnectRetryInterval
 	for {
 		r := rand.New(rand.NewSource(time.Now().UnixNano()))
 		timer := time.NewTimer(time.Duration(r.Intn(tick)+tick) * time.Second)
@@ -603,6 +608,7 @@ func (h *fsmHandler) connectLoop(ctx context.Context, wg *sync.WaitGroup) {
 				}).Debugf("failed to connect: %s", err)
 			}
 		}
+		tick = retry
 	}
 }
 
diff --git a/pkg/server/peer.go b/pkg/server/peer.go
index 8d41428c..b4a7b23c 100644
--- a/pkg/server/peer.go
+++ b/pkg/server/peer.go
@@ -32,8 +32,6 @@ const (
 	flopThreshold = time.Second * 30
 )
 
-var minConnectRetry = 10
-
 type peerGroup struct {
 	Conf             *config.PeerGroup
 	members          map[string]config.Neighbor
diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go
index 804842bd..54a2dd00 100644
--- a/pkg/server/server_test.go
+++ b/pkg/server/server_test.go
@@ -171,8 +171,6 @@ func TestListPolicyAssignment(t *testing.T) {
 }
 
 func TestMonitor(test *testing.T) {
-	minConnectRetry = 5
-
 	assert := assert.New(test)
 	s := NewBgpServer()
 	go s.Serve()
@@ -222,11 +220,6 @@ func TestMonitor(test *testing.T) {
 				RemotePort: 10179,
 			},
 		},
-		Timers: config.Timers{
-			Config: config.TimersConfig{
-				ConnectRetry: 5,
-			},
-		},
 	}
 	ch := make(chan struct{})
 	go t.MonitorPeer(context.Background(), &api.MonitorPeerRequest{}, func(peer *api.Peer) {
@@ -532,8 +525,6 @@ func TestFilterpathWithRejectPolicy(t *testing.T) {
 }
 
 func TestPeerGroup(test *testing.T) {
-	minConnectRetry = 5
-
 	assert := assert.New(test)
 	log.SetLevel(log.DebugLevel)
 	s := NewBgpServer()
@@ -553,11 +544,6 @@ func TestPeerGroup(test *testing.T) {
 			PeerAs:        2,
 			PeerGroupName: "g",
 		},
-		Timers: config.Timers{
-			Config: config.TimersConfig{
-				ConnectRetry: 5,
-			},
-		},
 	}
 	err = s.addPeerGroup(g)
 	assert.Nil(err)
@@ -572,11 +558,6 @@ func TestPeerGroup(test *testing.T) {
 				PassiveMode: true,
 			},
 		},
-		Timers: config.Timers{
-			Config: config.TimersConfig{
-				ConnectRetry: 5,
-			},
-		},
 	}
 	configured := map[string]interface{}{
 		"config": map[string]interface{}{
@@ -615,11 +596,6 @@ func TestPeerGroup(test *testing.T) {
 				RemotePort: 10179,
 			},
 		},
-		Timers: config.Timers{
-			Config: config.TimersConfig{
-				ConnectRetry: 5,
-			},
-		},
 	}
 	ch := make(chan struct{})
 	go t.MonitorPeer(context.Background(), &api.MonitorPeerRequest{}, func(peer *api.Peer) {
@@ -633,8 +609,6 @@ func TestPeerGroup(test *testing.T) {
 }
 
 func TestDynamicNeighbor(t *testing.T) {
-	minConnectRetry = 5
-
 	assert := assert.New(t)
 	log.SetLevel(log.DebugLevel)
 	s1 := NewBgpServer()
@@ -654,11 +628,6 @@ func TestDynamicNeighbor(t *testing.T) {
 			PeerAs:        2,
 			PeerGroupName: "g",
 		},
-		Timers: config.Timers{
-			Config: config.TimersConfig{
-				ConnectRetry: 5,
-			},
-		},
 	}
 	err = s1.addPeerGroup(g)
 	assert.Nil(err)
@@ -694,11 +663,6 @@ func TestDynamicNeighbor(t *testing.T) {
 				RemotePort: 10179,
 			},
 		},
-		Timers: config.Timers{
-			Config: config.TimersConfig{
-				ConnectRetry: 5,
-			},
-		},
 	}
 	ch := make(chan struct{})
 	go s2.MonitorPeer(context.Background(), &api.MonitorPeerRequest{}, func(peer *api.Peer) {
@@ -712,8 +676,6 @@ func TestDynamicNeighbor(t *testing.T) {
 }
 
 func TestGracefulRestartTimerExpired(t *testing.T) {
-	minConnectRetry = 5
-
 	assert := assert.New(t)
 	s1 := NewBgpServer()
 	go s1.Serve()
@@ -740,7 +702,7 @@ func TestGracefulRestartTimerExpired(t *testing.T) {
 		GracefulRestart: config.GracefulRestart{
 			Config: config.GracefulRestartConfig{
 				Enabled:     true,
-				RestartTime: 5,
+				RestartTime: minConnectRetryInterval,
 			},
 		},
 	}
@@ -775,11 +737,6 @@ func TestGracefulRestartTimerExpired(t *testing.T) {
 				RestartTime: 1,
 			},
 		},
-		Timers: config.Timers{
-			Config: config.TimersConfig{
-				ConnectRetry: 5,
-			},
-		},
 	}
 	ch := make(chan struct{})
 	go s2.MonitorPeer(context.Background(), &api.MonitorPeerRequest{}, func(peer *api.Peer) {
@@ -893,11 +850,6 @@ func peerServers(t *testing.T, ctx context.Context, servers []*BgpServer, famili
 						RemotePort: uint16(peer.bgpConfig.Global.Config.Port),
 					},
 				},
-				Timers: config.Timers{
-					Config: config.TimersConfig{
-						ConnectRetry: 5,
-					},
-				},
 			}
 
 			// first server to get neighbor config is passive to hopefully make handshake faster
@@ -957,8 +909,6 @@ func addVrf(t *testing.T, s *BgpServer, vrfName, rdStr string, id uint32) {
 }
 
 func TestDoNotReactToDuplicateRTCMemberships(t *testing.T) {
-	minConnectRetry = 5
-
 	ctx := context.Background()
 	log.SetLevel(log.DebugLevel)