| Message ID | 1467927001-26508-1-git-send-email-lrichard@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
"dev" <dev-bounces@openvswitch.org> wrote on 07/07/2016 04:30:01 PM: > From: Lance Richardson <lrichard@redhat.com> > To: dev@openvswitch.org > Date: 07/07/2016 04:30 PM > Subject: [ovs-dev] [PATCH v2] ovn-sbctl: eliminate a spurious test > case error cause > Sent by: "dev" <dev-bounces@openvswitch.org> > > The "ovn-sbctl" test fails occasionally due to log messages > similar to these: > > jsonrpc|WARN|unix: receive error: Connection reset by peer > reconnect|WARN|unix: connection dropped (Connection reset by peer) > > Since we're already ignoring "Broken pipe" messages in this test > case, and the difference between EPIPE and ECONNRESET on send > is simply a matter of whether the peer had unconsumed data > in its receive buffer when the peer socket was closed, it should > be OK to ignore "reset by peer" logs as well. > > This same type of failure has been observed in ovs-nbctl and > ovn-vtep-controller tests, so fix it there as well. > > Signed-off-by: Lance Richardson <lrichard@redhat.com> > --- This looks simple enough, applies clean and passes unit tests, so. If it were a crash, I'd look for a test case, but since it isn't... Acked-by: Ryan Moats <rmoats@us.ibm.com>
On Mon, Jul 18, 2016 at 11:38 PM, Ryan Moats <rmoats@us.ibm.com> wrote: > "dev" <dev-bounces@openvswitch.org> wrote on 07/07/2016 04:30:01 PM: > > > From: Lance Richardson <lrichard@redhat.com> > > To: dev@openvswitch.org > > Date: 07/07/2016 04:30 PM > > Subject: [ovs-dev] [PATCH v2] ovn-sbctl: eliminate a spurious test > > case error cause > > Sent by: "dev" <dev-bounces@openvswitch.org> > > > > The "ovn-sbctl" test fails occasionally due to log messages > > similar to these: > > > > jsonrpc|WARN|unix: receive error: Connection reset by peer > > reconnect|WARN|unix: connection dropped (Connection reset by peer) > > > > Since we're already ignoring "Broken pipe" messages in this test > > case, and the difference between EPIPE and ECONNRESET on send > > is simply a matter of whether the peer had unconsumed data > > in its receive buffer when the peer socket was closed, it should > > be OK to ignore "reset by peer" logs as well. > > > > This same type of failure has been observed in ovs-nbctl and > > ovn-vtep-controller tests, so fix it there as well. > > > > Signed-off-by: Lance Richardson <lrichard@redhat.com> > > --- > > This looks simple enough, applies clean and passes unit tests, > so. If it were a crash, I'd look for a test case, but since it isn't... > > Acked-by: Ryan Moats <rmoats@us.ibm.com> Thanks, applied to master.
diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at index 4bbda73..8bf738d 100644 --- a/tests/ovn-controller-vtep.at +++ b/tests/ovn-controller-vtep.at @@ -86,8 +86,12 @@ m4_define([OVN_CONTROLLER_VTEP_STOP], # may immediately update the database. this later update may cause database # sending update back to *ctl command if *ctl has not proceeded to exit yet. # and if *ctl command exits before database calling send, the send from - # database will fail with 'Broken pipe' error. + # database will fail with 'Broken pipe' error. Also removes all "connection + # reset" warning logs by a similar rationale (either EPIPE or ECONNRESET can + # be returned on a send depending on whether the peer had unconsumed data when + # it closed the socket). AT_CHECK([check_logs "$1 +/reset by peer/d /Broken pipe/d"]) OVS_APP_EXIT_AND_WAIT([ovs-vtep]) OVS_APP_EXIT_AND_WAIT([ovn-northd]) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 0c756ed..ff362a9 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -23,8 +23,12 @@ m4_define([OVN_NBCTL_TEST_STOP], # may immediately update the database. this later update may cause database # sending update back to *ctl command if *ctl has not proceeded to exit yet. # and if *ctl command exits before database calling send, the send from - # database will fail with 'Broken pipe' error. + # database will fail with 'Broken pipe' error. Also removes all "connection + # reset" warning logs by a similar rationale (either EPIPE or ECONNRESET can + # be returned on a send depending on whether the peer had unconsumed data when + # it closed the socket). AT_CHECK([check_logs "$1 +/reset by peer/d /Broken pipe/d"]) OVS_APP_EXIT_AND_WAIT([ovsdb-server])]) diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at index 5232933..5d8fad3 100644 --- a/tests/ovn-sbctl.at +++ b/tests/ovn-sbctl.at @@ -33,8 +33,12 @@ m4_define([OVN_SBCTL_TEST_STOP], # may immediately update the database. this later update may cause database # sending update back to *ctl command if *ctl has not proceeded to exit yet. # and if *ctl command exits before database calling send, the send from - # database will fail with 'Broken pipe' error. + # database will fail with 'Broken pipe' error. Also removes all "connection + # reset" warning logs by a similar rationale (either EPIPE or ECONNRESET can + # be returned on a send depending on whether the peer had unconsumed data when + # it closed the socket). AT_CHECK([check_logs "$1 +/reset by peer/d /Broken pipe/d"]) OVS_APP_EXIT_AND_WAIT([ovn-northd]) OVS_APP_EXIT_AND_WAIT_BY_TARGET([$OVS_RUNDIR/ovnnb_db.ctl], [$OVS_RUNDIR/ovnnb_db.pid])
The "ovn-sbctl" test fails occasionally due to log messages similar to these: jsonrpc|WARN|unix: receive error: Connection reset by peer reconnect|WARN|unix: connection dropped (Connection reset by peer) Since we're already ignoring "Broken pipe" messages in this test case, and the difference between EPIPE and ECONNRESET on send is simply a matter of whether the peer had unconsumed data in its receive buffer when the peer socket was closed, it should be OK to ignore "reset by peer" logs as well. This same type of failure has been observed in ovs-nbctl and ovn-vtep-controller tests, so fix it there as well. Signed-off-by: Lance Richardson <lrichard@redhat.com> --- Changes in v2: - Duplicated fix in ovn-nbctl.at and ovn-controller-vtep.at - Changed sed match string from 'connection reset' to 'reset by peer'. tests/ovn-controller-vtep.at | 6 +++++- tests/ovn-nbctl.at | 6 +++++- tests/ovn-sbctl.at | 6 +++++- 3 files changed, 15 insertions(+), 3 deletions(-)