On 10/21/26 7:20 PM, Rosemarie O'Riorden via dev wrote: > On 10/21/26 12:49 PM, Xavier Simonart wrote: >> Hi Rosemarie >> >> Thanks for the review/feedback/questions. >> >> On Wed, Oct 21, 2026 at 6:05 PM Rosemarie O'Riorden >> <[email protected] <mailto:[email protected]>> wrote: >> >> Hi Xavier, >> >> On 10/20/26 8:46 AM, Xavier Simonart via dev wrote: >> > Avoid using AT_CHECK in construction such as 'while read ... done >> < db.txt' >> > as it prevents OVS_PAUSE_TEST=1 to pause for terminal input, as it >> > consumes the next line from db.txt and bypasses the pause prompt >> > instantly. >> > >> > Also add checks for vtep related db changes. We do this after >> stopping >> > ovs-vtep because ovs-vtep is quite slow. >> > >> > Fixes: c0486bfa1074 ("tests: Detect unexpected db changes when >> test completes.") >> > Signed-off-by: Xavier Simonart <[email protected] >> <mailto:[email protected]>> >> > --- >> > tests/ovn-controller-vtep.at <http://ovn-controller-vtep.at> >> | 1 + >> > tests/ovn-macros.at <http://ovn-macros.at> | 18 ++++++ >> +++++------- >> > 2 files changed, 12 insertions(+), 7 deletions(-) >> > >> > diff --git a/tests/ovn-controller-vtep.at <http://ovn-controller- >> vtep.at> b/tests/ovn-controller-vtep.at <http://ovn-controller- >> vtep.at> >> > index aa8da62a6..5b2895676 100644 >> > --- a/tests/ovn-controller-vtep.at <http://ovn-controller-vtep.at> >> > +++ b/tests/ovn-controller-vtep.at <http://ovn-controller-vtep.at> >> > @@ -72,6 +72,7 @@ m4_define([OVN_CONTROLLER_VTEP_STOP],[ >> > test -n "$2" && as "$2" >> > #AT_CHECK([check_logs "$1"]) >> > OVS_APP_EXIT_AND_WAIT([ovs-vtep]) >> > + CHECK_UNEXPECTED_DB_CHANGES >> >> I know you said it's ok to do this after stopping ovs-vtep because >> it's >> slow, but why not put it before in case it happens to stop quicker >> sometimes? In every other STOP macro, CHECK_UNEXPECTED_DB_CHANGES is >> run >> before stopping the daemon, so I'm just curious as to why you'd >> want to >> do it the other way around for this case. >> >> Those are valid questions. The key here is that the macro checks the >> database state, expecting >> a stable system; i.e., it expects to see no - or very few - changes. >> But ovs-vtep is ** really ** very slow i.e. I saw it sometimes >> handling changes >> after several seconds. >> So, if we check the DB while ovs-vtep is still running, we would see >> db changes (done by ovs-vtep) >> and report those as unexpected changes. Tests are supposed to be >> stable once completed, which >> isn't the case when ovs-vtep is still making changes. >> Stopping ovs-vtep is the easiest way to make those tests stable >> before/ while cleanup. >> >> Another way would be to modify each vtep related test and add code to >> ensure >> ovs-vtep completed handling the changes before the clean up. But I >> found this would not only >> be quite painful (compared to the added-value) but also would make >> writing new vtep related tests more difficult. >> WDYT? >> >> If ovs-vtep happens to be fast, then there is no issue: the test would >> be stable before cleanup, and stopping it would not cause >> any issue. > > Hi Xavier, thanks for the response and clarification! > > Ok this ordering makes sense to me then. This solution definitely seems > better than changing every vtep-related test. > > Acked-by: Rosemarie O'Riorden <[email protected]> >
Hi Xavier, Rosemarie, Thanks for the patch and review! Applied to main. Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
