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]>
> OVS_APP_EXIT_AND_WAIT([ovn-controller-vtep])
> OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])])
> diff --git a/tests/ovn-macros.at <http://ovn-macros.at> b/tests/
ovn-macros.at <http://ovn-macros.at>
> index e0f70bcde..aa2e4453b 100644
> --- a/tests/ovn-macros.at <http://ovn-macros.at>
> +++ b/tests/ovn-macros.at <http://ovn-macros.at>
> @@ -189,14 +189,18 @@ m4_define([CHECK_UNEXPECTED_DB_CHANGES],[
> # Second pass: if there is any db change, rerun check and
ignore small number of changes.
> if [[ "$diff" -ne 0 ]]; then
> sleep 1
> - while read before_count db; do
> - after_count=$(wc -l < "$db")
> - diff=$((after_count - before_count))
> - if [[ "$diff" -ge "50" ]]; then
> - printf 'Got %d changes in %s\n' "$diff" "$db"
> - AT_CHECK([test "$diff" -lt "50"])
> + db_list=$(cut -d' ' -f2- db.txt)
> + for db in $db_list; do
> + before_count=$(grep " $db\$" db.txt | cut -d' ' -f1)
> + if test -n "$before_count"; then
> + after_count=$(wc -l < "$db")
> + diff=$((after_count - before_count))
> + if [[ "$diff" -ge "50" ]]; then
> + printf 'Got %d changes in %s\n' "$diff" "$db"
> + AT_CHECK([test "$diff" -lt "50"])
> + fi
> fi
> - done < db.txt
> + done
> fi
> ])
>
Thanks
Xavier
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
