From e83591c056d43a7a612f936b609d0a79fbe6cef1 Mon Sep 17 00:00:00 2001 From: Tim Pepper Date: Thu, 18 Aug 2016 15:37:10 -0700 Subject: [PATCH 1/4] testutil: send status from go routine The senders block on writing the results channel until a reader reads the channel or a shutdown closes all the channels. This SSNTP server call into client handlers such as DisconnectNotify(), ConnectNotify(), etc. are not in goroutines, so the client implementation of those MUST NOT block. Signed-off-by: Tim Pepper --- testutil/agent.go | 32 ++++++++++++++++---------------- testutil/controller.go | 10 +++++----- testutil/server.go | 14 +++++++------- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/testutil/agent.go b/testutil/agent.go index 220cb04cb..e3b970746 100644 --- a/testutil/agent.go +++ b/testutil/agent.go @@ -310,14 +310,14 @@ func closeClientChans(client *SsntpTestClient) { func (client *SsntpTestClient) ConnectNotify() { var result Result - client.SendResultAndDelEventChan(ssntp.NodeConnected, result) + go client.SendResultAndDelEventChan(ssntp.NodeConnected, result) } // DisconnectNotify implements the SSNTP client ConnectNotify callback for SsntpTestClient func (client *SsntpTestClient) DisconnectNotify() { var result Result - client.SendResultAndDelEventChan(ssntp.NodeDisconnected, result) + go client.SendResultAndDelEventChan(ssntp.NodeDisconnected, result) } // StatusNotify implements the SSNTP client StatusNotify callback for SsntpTestClient @@ -345,7 +345,7 @@ func (client *SsntpTestClient) handleStart(payload []byte) Result { if client.StartFail == true { result.Err = errors.New(client.StartFailReason.String()) client.sendStartFailure(cmd.Start.InstanceUUID, client.StartFailReason) - client.SendResultAndDelErrorChan(ssntp.StartFailure, result) + go client.SendResultAndDelErrorChan(ssntp.StartFailure, result) return result } @@ -376,7 +376,7 @@ func (client *SsntpTestClient) handleStop(payload []byte) Result { if client.StopFail == true { result.Err = errors.New(client.StopFailReason.String()) client.sendStopFailure(cmd.Stop.InstanceUUID, client.StopFailReason) - client.SendResultAndDelErrorChan(ssntp.StopFailure, result) + go client.SendResultAndDelErrorChan(ssntp.StopFailure, result) return result } @@ -405,7 +405,7 @@ func (client *SsntpTestClient) handleRestart(payload []byte) Result { if client.RestartFail == true { result.Err = errors.New(client.RestartFailReason.String()) client.sendRestartFailure(cmd.Restart.InstanceUUID, client.RestartFailReason) - client.SendResultAndDelErrorChan(ssntp.RestartFailure, result) + go client.SendResultAndDelErrorChan(ssntp.RestartFailure, result) return result } @@ -434,7 +434,7 @@ func (client *SsntpTestClient) handleDelete(payload []byte) Result { if client.DeleteFail == true { result.Err = errors.New(client.DeleteFailReason.String()) client.sendDeleteFailure(cmd.Delete.InstanceUUID, client.DeleteFailReason) - client.SendResultAndDelErrorChan(ssntp.DeleteFailure, result) + go client.SendResultAndDelErrorChan(ssntp.DeleteFailure, result) return result } @@ -489,7 +489,7 @@ func (client *SsntpTestClient) CommandNotify(command ssntp.Command, frame *ssntp fmt.Fprintf(os.Stderr, "client %s unhandled command %s\n", client.Role.String(), command.String()) } - client.SendResultAndDelCmdChan(command, result) + go client.SendResultAndDelCmdChan(command, result) } // EventNotify is an SSNTP callback stub for SsntpTestClient @@ -515,7 +515,7 @@ func (client *SsntpTestClient) EventNotify(event ssntp.Event, frame *ssntp.Frame fmt.Fprintf(os.Stderr, "client %s unhandled event: %s\n", client.Role.String(), event.String()) } - client.SendResultAndDelEventChan(event, result) + go client.SendResultAndDelEventChan(event, result) } // ErrorNotify is an SSNTP callback stub for SsntpTestClient @@ -538,7 +538,7 @@ func (client *SsntpTestClient) SendStatsCmd() { } } - client.SendResultAndDelCmdChan(ssntp.STATS, result) + go client.SendResultAndDelCmdChan(ssntp.STATS, result) } // SendStatus pushes an ssntp status frame from the SsntpTestClient with @@ -558,7 +558,7 @@ func (client *SsntpTestClient) SendStatus(memTotal int, memAvail int) { } } - client.SendResultAndDelCmdChan(ssntp.STATS, result) + go client.SendResultAndDelCmdChan(ssntp.STATS, result) } // SendTrace allows an SsntpTestClient to push an ssntp.TraceReport event frame @@ -590,7 +590,7 @@ func (client *SsntpTestClient) SendTrace() { } } - client.SendResultAndDelEventChan(ssntp.TraceReport, result) + go client.SendResultAndDelEventChan(ssntp.TraceReport, result) } // SendDeleteEvent allows an SsntpTestClient to push an ssntp.InstanceDeleted event frame @@ -615,7 +615,7 @@ func (client *SsntpTestClient) SendDeleteEvent(uuid string) { } } - client.SendResultAndDelEventChan(ssntp.InstanceDeleted, result) + go client.SendResultAndDelEventChan(ssntp.InstanceDeleted, result) } // SendTenantAddedEvent allows an SsntpTestClient to push an ssntp.TenantAdded event frame @@ -627,7 +627,7 @@ func (client *SsntpTestClient) SendTenantAddedEvent() { result.Err = err } - client.SendResultAndDelEventChan(ssntp.TenantAdded, result) + go client.SendResultAndDelEventChan(ssntp.TenantAdded, result) } // SendTenantRemovedEvent allows an SsntpTestClient to push an ssntp.TenantRemoved event frame @@ -639,7 +639,7 @@ func (client *SsntpTestClient) SendTenantRemovedEvent() { result.Err = err } - client.SendResultAndDelEventChan(ssntp.TenantRemoved, result) + go client.SendResultAndDelEventChan(ssntp.TenantRemoved, result) } // SendPublicIPAssignedEvent allows an SsntpTestClient to push an ssntp.PublicIPAssigned event frame @@ -651,7 +651,7 @@ func (client *SsntpTestClient) SendPublicIPAssignedEvent() { result.Err = err } - client.SendResultAndDelEventChan(ssntp.PublicIPAssigned, result) + go client.SendResultAndDelEventChan(ssntp.PublicIPAssigned, result) } // SendConcentratorAddedEvent allows an SsntpTestClient to push an ssntp.ConcentratorInstanceAdded event frame @@ -680,7 +680,7 @@ func (client *SsntpTestClient) SendConcentratorAddedEvent(instanceUUID string, t } } - client.SendResultAndDelEventChan(ssntp.ConcentratorInstanceAdded, result) + go client.SendResultAndDelEventChan(ssntp.ConcentratorInstanceAdded, result) } func (client *SsntpTestClient) sendStartFailure(instanceUUID string, reason payloads.StartFailureReason) { diff --git a/testutil/controller.go b/testutil/controller.go index 25c9e2e81..f6365b73d 100644 --- a/testutil/controller.go +++ b/testutil/controller.go @@ -238,14 +238,14 @@ func closeControllerChans(ctl *SsntpTestController) { func (ctl *SsntpTestController) ConnectNotify() { var result Result - ctl.SendResultAndDelEventChan(ssntp.NodeConnected, result) + go ctl.SendResultAndDelEventChan(ssntp.NodeConnected, result) } // DisconnectNotify implements the SSNTP client DisconnectNotify callback for SsntpTestController func (ctl *SsntpTestController) DisconnectNotify() { var result Result - ctl.SendResultAndDelEventChan(ssntp.NodeDisconnected, result) + go ctl.SendResultAndDelEventChan(ssntp.NodeDisconnected, result) } // StatusNotify implements the SSNTP client StatusNotify callback for SsntpTestController @@ -279,7 +279,7 @@ func (ctl *SsntpTestController) CommandNotify(command ssntp.Command, frame *ssnt fmt.Fprintf(os.Stderr, "controller unhandled command: %s\n", command.String()) } - ctl.SendResultAndDelCmdChan(command, result) + go ctl.SendResultAndDelCmdChan(command, result) } // EventNotify implements the SSNTP client EventNotify callback for SsntpTestController @@ -323,7 +323,7 @@ func (ctl *SsntpTestController) EventNotify(event ssntp.Event, frame *ssntp.Fram fmt.Fprintf(os.Stderr, "controller unhandled event: %s\n", event.String()) } - ctl.SendResultAndDelEventChan(event, result) + go ctl.SendResultAndDelEventChan(event, result) } // ErrorNotify implements the SSNTP client ErrorNotify callback for SsntpTestController @@ -347,5 +347,5 @@ func (ctl *SsntpTestController) ErrorNotify(error ssntp.Error, frame *ssntp.Fram fmt.Fprintf(os.Stderr, "controller unhandled error %s\n", error.String()) } - ctl.SendResultAndDelErrorChan(error, result) + go ctl.SendResultAndDelErrorChan(error, result) } diff --git a/testutil/server.go b/testutil/server.go index b1b6e60a6..7aec64a1a 100644 --- a/testutil/server.go +++ b/testutil/server.go @@ -267,7 +267,7 @@ func (server *SsntpTestServer) ConnectNotify(uuid string, role ssntp.Role) { server.netClients = append(server.netClients, uuid) } - server.SendResultAndDelEventChan(ssntp.NodeConnected, result) + go server.SendResultAndDelEventChan(ssntp.NodeConnected, result) } // DisconnectNotify implements an SSNTP DisconnectNotify callback for SsntpTestServer @@ -296,7 +296,7 @@ func (server *SsntpTestServer) DisconnectNotify(uuid string, role ssntp.Role) { server.netClientsLock.Unlock() } - server.SendResultAndDelEventChan(ssntp.NodeDisconnected, result) + go server.SendResultAndDelEventChan(ssntp.NodeDisconnected, result) } // StatusNotify is an SSNTP callback stub for SsntpTestServer @@ -310,7 +310,7 @@ func (server *SsntpTestServer) StatusNotify(uuid string, status ssntp.Status, fr fmt.Fprintf(os.Stderr, "server unhandled status frame from node %s\n", uuid) } - server.SendResultAndDelStatusChan(status, result) + go server.SendResultAndDelStatusChan(status, result) } // CommandNotify implements an SSNTP CommandNotify callback for SsntpTestServer @@ -395,7 +395,7 @@ func (server *SsntpTestServer) CommandNotify(uuid string, command ssntp.Command, fmt.Fprintf(os.Stderr, "server unhandled command %s\n", command.String()) } - server.SendResultAndDelCmdChan(command, result) + go server.SendResultAndDelCmdChan(command, result) } // EventNotify implements an SSNTP EventNotify callback for SsntpTestServer @@ -431,7 +431,7 @@ func (server *SsntpTestServer) EventNotify(uuid string, event ssntp.Event, frame fmt.Fprintf(os.Stderr, "server unhandled event %s\n", event.String()) } - server.SendResultAndDelEventChan(event, result) + go server.SendResultAndDelEventChan(event, result) } func getConcentratorUUID(event ssntp.Event, payload []byte) (string, error) { @@ -479,7 +479,7 @@ func (server *SsntpTestServer) EventForward(uuid string, event ssntp.Event, fram result.Err = err } - server.SendResultAndDelEventChan(event, result) + go server.SendResultAndDelEventChan(event, result) return dest } @@ -519,7 +519,7 @@ func (server *SsntpTestServer) ErrorNotify(uuid string, error ssntp.Error, frame fmt.Fprintf(os.Stderr, "server unhandled error %s\n", error.String()) } - server.SendResultAndDelErrorChan(error, result) + go server.SendResultAndDelErrorChan(error, result) } func (server *SsntpTestServer) handleStart(payload []byte) (dest ssntp.ForwardDestination) { From b2ea19bb6834eb06316a932e35a699f1bf213e94 Mon Sep 17 00:00:00 2001 From: Tim Pepper Date: Thu, 18 Aug 2016 15:31:46 -0700 Subject: [PATCH 2/4] testutil: goroutine server test shutdown The ssntp shutdown will trigger all clients to send a disconnect event with the testutil code doing a blocking write to the results channel. But there is there is no reader of that notification unless the shutdown is in a goroutine or the shutdown finishes. If shutdown blocks for any reason, the channel readers never run. Signed-off-by: Tim Pepper --- testutil/client_server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testutil/client_server_test.go b/testutil/client_server_test.go index e785b49c0..cfdf684f7 100644 --- a/testutil/client_server_test.go +++ b/testutil/client_server_test.go @@ -451,7 +451,7 @@ func stopServer() error { netAgentCh := netAgent.AddEventChan(ssntp.NodeDisconnected) agentCh := agent.AddEventChan(ssntp.NodeDisconnected) - server.Shutdown() + go server.Shutdown() _, err := controller.GetEventChanResult(controllerCh, ssntp.NodeDisconnected) if err != nil { From 4fb588958da0edd34b8b09a13ced24e4734511a2 Mon Sep 17 00:00:00 2001 From: Tim Pepper Date: Fri, 19 Aug 2016 10:40:49 -0700 Subject: [PATCH 3/4] testutil: use consistent timeout for results channels These timeouts "should never trigger", but if they do we get a test failure sooner than waiting for go test's very long timeout. The 25 second number isn't special. And it's known to possibly be too short for example in the case of TestReconnects(), where the ssntp client incurs a series of delays that are 0..5, 0..10, 0..20, 0..40 seconds. If the server is slow in restarting it _might_ take 4 seconds to restart in travis. But worst case we hit client delays of 1,1,1,40 seconds. In which case despite everything "working correctly" the test case will timeout for lack of NodeConnected result after 25s, when it would be coming at 43s. Signed-off-by: Tim Pepper --- testutil/agent.go | 8 ++++---- testutil/controller.go | 6 +++--- testutil/server.go | 8 ++++---- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/testutil/agent.go b/testutil/agent.go index e3b970746..8329a106c 100644 --- a/testutil/agent.go +++ b/testutil/agent.go @@ -120,7 +120,7 @@ func (client *SsntpTestClient) GetCmdChanResult(c *chan Result, cmd ssntp.Comman if result.Err != nil { err = fmt.Errorf("Client error sending %s command: %s\n", cmd, result.Err) } - case <-time.After(5 * time.Second): + case <-time.After(25 * time.Second): err = fmt.Errorf("Timeout waiting for client %s command result\n", cmd) } @@ -159,7 +159,7 @@ func (client *SsntpTestClient) GetEventChanResult(c *chan Result, evt ssntp.Even if result.Err != nil { err = fmt.Errorf("Client error sending %s event: %s\n", evt, result.Err) } - case <-time.After(20 * time.Second): + case <-time.After(25 * time.Second): err = fmt.Errorf("Timeout waiting for client %s event result\n", evt) } @@ -198,7 +198,7 @@ func (client *SsntpTestClient) GetErrorChanResult(c *chan Result, error ssntp.Er if result.Err != nil { err = fmt.Errorf("Client error sending %s error: %s\n", error, result.Err) } - case <-time.After(20 * time.Second): + case <-time.After(25 * time.Second): err = fmt.Errorf("Timeout waiting for client %s error result\n", error) } @@ -237,7 +237,7 @@ func (client *SsntpTestClient) GetStatusChanResult(c *chan Result, status ssntp. if result.Err != nil { err = fmt.Errorf("Client error sending %s status: %s\n", status, result.Err) } - case <-time.After(5 * time.Second): + case <-time.After(25 * time.Second): err = fmt.Errorf("Timeout waiting for client %s status result\n", status) } diff --git a/testutil/controller.go b/testutil/controller.go index f6365b73d..9ab909e51 100644 --- a/testutil/controller.go +++ b/testutil/controller.go @@ -98,7 +98,7 @@ func (ctl *SsntpTestController) GetCmdChanResult(c *chan Result, cmd ssntp.Comma if result.Err != nil { err = fmt.Errorf("Controller error sending %s command: %s\n", cmd, result.Err) } - case <-time.After(5 * time.Second): + case <-time.After(25 * time.Second): err = fmt.Errorf("Timeout waiting for controller %s command result\n", cmd) } @@ -137,7 +137,7 @@ func (ctl *SsntpTestController) GetEventChanResult(c *chan Result, evt ssntp.Eve if result.Err != nil { err = fmt.Errorf("Controller error sending %s event: %s\n", evt, result.Err) } - case <-time.After(20 * time.Second): + case <-time.After(25 * time.Second): err = fmt.Errorf("Timeout waiting for controller %s event result\n", evt) } @@ -176,7 +176,7 @@ func (ctl *SsntpTestController) GetErrorChanResult(c *chan Result, error ssntp.E if result.Err != nil { err = fmt.Errorf("Controller error sending %s error: %s\n", error, result.Err) } - case <-time.After(20 * time.Second): + case <-time.After(25 * time.Second): err = fmt.Errorf("Timeout waiting for controller %s error result\n", error) } diff --git a/testutil/server.go b/testutil/server.go index 7aec64a1a..4dc219810 100644 --- a/testutil/server.go +++ b/testutil/server.go @@ -65,7 +65,7 @@ func (server *SsntpTestServer) GetCmdChanResult(c *chan Result, cmd ssntp.Comman if result.Err != nil { err = fmt.Errorf("Server error on %s command: %s\n", cmd, result.Err) } - case <-time.After(5 * time.Second): + case <-time.After(25 * time.Second): err = fmt.Errorf("Timeout waiting for server %s command result\n", cmd) } @@ -104,7 +104,7 @@ func (server *SsntpTestServer) GetEventChanResult(c *chan Result, evt ssntp.Even if result.Err != nil { err = fmt.Errorf("Server error handling %s event: %s\n", evt, result.Err) } - case <-time.After(20 * time.Second): + case <-time.After(25 * time.Second): err = fmt.Errorf("Timeout waiting for server %s event result\n", evt) } @@ -143,7 +143,7 @@ func (server *SsntpTestServer) GetErrorChanResult(c *chan Result, error ssntp.Er if result.Err != nil { err = fmt.Errorf("Server error handling %s error: %s\n", error, result.Err) } - case <-time.After(20 * time.Second): + case <-time.After(25 * time.Second): err = fmt.Errorf("Timeout waiting for server %s error result\n", error) } @@ -182,7 +182,7 @@ func (server *SsntpTestServer) GetStatusChanResult(c *chan Result, status ssntp. if result.Err != nil { err = fmt.Errorf("Server error handling %s status: %s\n", status, result.Err) } - case <-time.After(5 * time.Second): + case <-time.After(25 * time.Second): err = fmt.Errorf("Timeout waiting for server %s status result\n", status) } From 97711a54974fdae68e23249100a569d01519e19e Mon Sep 17 00:00:00 2001 From: Tim Pepper Date: Fri, 19 Aug 2016 10:49:15 -0700 Subject: [PATCH 4/4] testutil: don't dup result chan write for NodeConnected/Disconnected The NodeConnected/Disconnected() helpers are a little odd in how they're disjoint from all the other events which flow through EventNotify(). They event shouldn't be processed in both places. Insure that by returning from EventNotify() in the case those are the event being processed. Signed-off-by: Tim Pepper --- testutil/agent.go | 6 ++++++ testutil/controller.go | 8 ++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/testutil/agent.go b/testutil/agent.go index 8329a106c..b2c2efd4f 100644 --- a/testutil/agent.go +++ b/testutil/agent.go @@ -497,6 +497,12 @@ func (client *SsntpTestClient) EventNotify(event ssntp.Event, frame *ssntp.Frame var result Result switch event { + case ssntp.NodeConnected: + //handled by ConnectNotify() + return + case ssntp.NodeDisconnected: + //handled by DisconnectNotify() + return case ssntp.TenantAdded: var tenantAddedEvent payloads.EventTenantAdded diff --git a/testutil/controller.go b/testutil/controller.go index 9ab909e51..3a544c12f 100644 --- a/testutil/controller.go +++ b/testutil/controller.go @@ -287,10 +287,14 @@ func (ctl *SsntpTestController) EventNotify(event ssntp.Event, frame *ssntp.Fram var result Result switch event { - // case ssntp.NodeConnected: handled by ConnectNotify() - // case ssntp.NodeDisconnected: handled by DisconnectNotify() // case ssntp.TenantAdded: does not reach controller // case ssntp.TenantRemoved: does not reach controller + case ssntp.NodeConnected: + //handled by ConnectNotify() + return + case ssntp.NodeDisconnected: + //handled by DisconnectNotify() + return case ssntp.PublicIPAssigned: var publicIPAssignedEvent payloads.EventPublicIPAssigned