| Message ID | CAKgLnsHL_Wc9L2tVQOqGMLqksPXuFF4cXAztM91Ra9LOuSfx4w@mail.gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
----- Original Message ----- > From: "Darrell Ball" <dlu998@gmail.com> > To: "Lance Richardson" <lrichard@redhat.com> > Cc: "ovs dev" <dev@openvswitch.org> > Sent: Tuesday, July 12, 2016 7:05:18 PM > Subject: Re: [ovs-dev] [PATCH] ovn-controller-vtep: occasional failure in "binding 1" test case > > On Fri, Jul 8, 2016 at 7:16 AM, Lance Richardson <lrichard@redhat.com> > wrote: > > > The ovn-controller-vtep "binding 1" test case fails occasionally > > due to a race with the ovs-vtep daemon. If ovs-vtep happens to > > execute handle_physical() after "ovs-vsctl del-port p0", but before > > the test script has executed "vtep-ctl del-port br-vtep p0", the > > latter command will fail because ovs-vtep will have already deleted > > p0 from the vtep db. > > > > Accomodate this race while ensuring that p0 and p1 have been removed > > from both dbs by using the "--if-exists" option on the second delete. > > > > Signed-off-by: Lance Richardson <lrichard@redhat.com> > > --- > > tests/ovn-controller-vtep.at | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at > > index c296f0e..9543f39 100644 > > --- a/tests/ovn-controller-vtep.at > > +++ b/tests/ovn-controller-vtep.at > > @@ -225,7 +225,7 @@ AT_CHECK([sed -n 's/^.*\(|WARN|.*\)$/\1/p' > > ovn-controller-vtep.log | sed 's/([[- > > > > # deletes physical ports from vtep. > > AT_CHECK([ovs-vsctl del-port p0 -- del-port p1]) > > -AT_CHECK([vtep-ctl del-port br-vtep p0 -- del-port br-vtep p1]) > > +AT_CHECK([vtep-ctl --if-exists del-port br-vtep p0 -- --if-exists > > del-port br-vtep p1]) > > OVS_WAIT_UNTIL([test -z "`ovn-sbctl list Chassis | grep -- > > br-vtep_lswitch`"]) > > # should see empty chassis column in both binding entries. > > AT_CHECK_UNQUOTED([ovn-sbctl --columns=chassis list Port_Binding | cut -d > > ':' -f2 | tr -d ' ' | sort], [0], [dnl > > > > > Another possibility is as follows: > > The bridge database is the master here. > ovs-vtep will listen to changes and should update its database accordingly > It might be good to verify that this code path actually works. > > > diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at > index c296f0e..c978137 100644 > --- a/tests/ovn-controller-vtep.at > +++ b/tests/ovn-controller-vtep.at > @@ -225,8 +225,9 @@ AT_CHECK([sed -n 's/^.*\(|WARN|.*\)$/\1/p' > ovn-controller-vtep.log | sed 's/([[- > > # deletes physical ports from vtep. > AT_CHECK([ovs-vsctl del-port p0 -- del-port p1]) > -AT_CHECK([vtep-ctl del-port br-vtep p0 -- del-port br-vtep p1]) > OVS_WAIT_UNTIL([test -z "`ovn-sbctl list Chassis | grep -- > br-vtep_lswitch`"]) > +OVS_WAIT_UNTIL([test -z "`vtep-ctl list physical_port p0`"]) > +OVS_WAIT_UNTIL([test -z "`vtep-ctl list physical_port p1`"]) > # should see empty chassis column in both binding entries. > AT_CHECK_UNQUOTED([ovn-sbctl --columns=chassis list Port_Binding | cut -d > ':' -f2 | tr -d ' ' | sort], [0], [dnl > > > What do you think ? > Hi Darrell, That's a good idea, I'll give it a try. Thanks, Lance > > > > -- > > 2.5.5 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev > > >
> > diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at > > index c296f0e..c978137 100644 > > --- a/tests/ovn-controller-vtep.at > > +++ b/tests/ovn-controller-vtep.at > > @@ -225,8 +225,9 @@ AT_CHECK([sed -n 's/^.*\(|WARN|.*\)$/\1/p' > > ovn-controller-vtep.log | sed 's/([[- > > > > # deletes physical ports from vtep. > > AT_CHECK([ovs-vsctl del-port p0 -- del-port p1]) > > -AT_CHECK([vtep-ctl del-port br-vtep p0 -- del-port br-vtep p1]) > > OVS_WAIT_UNTIL([test -z "`ovn-sbctl list Chassis | grep -- > > br-vtep_lswitch`"]) > > +OVS_WAIT_UNTIL([test -z "`vtep-ctl list physical_port p0`"]) > > +OVS_WAIT_UNTIL([test -z "`vtep-ctl list physical_port p1`"]) > > # should see empty chassis column in both binding entries. > > AT_CHECK_UNQUOTED([ovn-sbctl --columns=chassis list Port_Binding | cut -d > > ':' -f2 | tr -d ' ' | sort], [0], [dnl > > > > > > What do you think ? > > > > Hi Darrell, > > That's a good idea, I'll give it a try. > > Thanks, > > Lance > > > > > > > -- > > > 2.5.5 > > > > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org > > > http://openvswitch.org/mailman/listinfo/dev > > > > > > Hi Darrell, This looks good in my testing. Would you like me to post a v2 with a Suggested-by:, or were you going to post your version formally? Thanks, Lance
On Wed, Jul 13, 2016 at 2:15 PM, Lance Richardson <lrichard@redhat.com> wrote: > > > diff --git a/tests/ovn-controller-vtep.at b/tests/ > ovn-controller-vtep.at > > > index c296f0e..c978137 100644 > > > --- a/tests/ovn-controller-vtep.at > > > +++ b/tests/ovn-controller-vtep.at > > > @@ -225,8 +225,9 @@ AT_CHECK([sed -n 's/^.*\(|WARN|.*\)$/\1/p' > > > ovn-controller-vtep.log | sed 's/([[- > > > > > > # deletes physical ports from vtep. > > > AT_CHECK([ovs-vsctl del-port p0 -- del-port p1]) > > > -AT_CHECK([vtep-ctl del-port br-vtep p0 -- del-port br-vtep p1]) > > > OVS_WAIT_UNTIL([test -z "`ovn-sbctl list Chassis | grep -- > > > br-vtep_lswitch`"]) > > > +OVS_WAIT_UNTIL([test -z "`vtep-ctl list physical_port p0`"]) > > > +OVS_WAIT_UNTIL([test -z "`vtep-ctl list physical_port p1`"]) > > > # should see empty chassis column in both binding entries. > > > AT_CHECK_UNQUOTED([ovn-sbctl --columns=chassis list Port_Binding | > cut -d > > > ':' -f2 | tr -d ' ' | sort], [0], [dnl > > > > > > > > > What do you think ? > > > > > > > Hi Darrell, > > > > That's a good idea, I'll give it a try. > > > > Thanks, > > > > Lance > > > > > > > > > > -- > > > > 2.5.5 > > > > > > > > _______________________________________________ > > > > dev mailing list > > > > dev@openvswitch.org > > > > http://openvswitch.org/mailman/listinfo/dev > > > > > > > > > > > Hi Darrell, > > This looks good in my testing. Would you like me to post a v2 with > a Suggested-by:, or were you going to post your version formally? > > Thanks, > > Lance > please go ahead and post V2 Thanks Darrell
diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at index c296f0e..c978137 100644 --- a/tests/ovn-controller-vtep.at +++ b/tests/ovn-controller-vtep.at @@ -225,8 +225,9 @@ AT_CHECK([sed -n 's/^.*\(|WARN|.*\)$/\1/p' ovn-controller-vtep.log | sed 's/([[- # deletes physical ports from vtep. AT_CHECK([ovs-vsctl del-port p0 -- del-port p1]) -AT_CHECK([vtep-ctl del-port br-vtep p0 -- del-port br-vtep p1]) OVS_WAIT_UNTIL([test -z "`ovn-sbctl list Chassis | grep -- br-vtep_lswitch`"]) +OVS_WAIT_UNTIL([test -z "`vtep-ctl list physical_port p0`"]) +OVS_WAIT_UNTIL([test -z "`vtep-ctl list physical_port p1`"]) # should see empty chassis column in both binding entries.