On 11/11/20 10:23 AM, Dumitru Ceara wrote: > On 11/11/20 2:24 AM, Ben Pfaff wrote: >> Many of these could be replaced by "ovn-nbctl sync". Some weren't >> really needed at all because they were adjacent to something that itself >> called sync or otherwise used --wait. Some were more appropriately >> done with explicit waits for what was really needed. >> >> I left some "sleep"s. Some were because they were "negative" sleeps: >> they were giving time for something to happen that should *not* happen >> (in other words, if you wait for it to happen, you'll wait forever, >> unless there's a bug). Some were because I didn't know how to properly >> wait for what they were waiting for, or because I didn't understand >> the circumstances deeply enough. >> >> Signed-off-by: Ben Pfaff <[email protected]> >> --- > > As mentioned in the discussion on the V3 of this series, this also needs > the following incremental to make the "ovn -- /32 router IP address" > test less racy. > > Thanks, > Dumtiru > > diff --git a/tests/ovn.at b/tests/ovn.at > index d30f55622..e12f3b668 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -11074,6 +11074,9 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \ > OVN_POPULATE_ARP > > # Allow some time for ovn-northd and ovn-controller to catch up. > +OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up foo1` = xup]) > +OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up alice1` = xup]) > + > check ovn-nbctl --wait=hv sync > > ovn-sbctl dump-flows > sbflows >
Actually, I think something like this might need to be done for more tests in ovn.at now that the "sleep"s are removed. It's not really a problem introduced by your patch but if we apply it as is I'm afraid we make the tests racier than they already are. Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
