On 12/9/25 11:32 AM, Xavier Simonart wrote: > Hi Dumitru > Hi Xavier,
> Thanks for the patch and catching those missing cleanups. > Not sure whether we want to do this in this patch, but I think that > there are other missing cleanups: see below (adding them here > might make a backport pretty difficult). > > Thanks > Xavier > > On Mon, Dec 8, 2025 at 10:44 PM Dumitru Ceara <[email protected]> wrote: > >> Some tests (e.g., the ones in ovn-controller-vtep.at) were completely >> missed when the macro was added. Some other slipped in from in-flight >> patches. >> >> Fixes: d1f4150bbe88 ("tests: Ensure all central components stop at the end >> of the test.") >> Fixes: 5b23b9709e9d ("northd: Delete learned routes when dynamic-routing >> is false.") >> Signed-off-by: Dumitru Ceara <[email protected]> >> --- >> tests/ovn-controller-vtep.at | 7 +++++++ >> tests/ovn.at | 1 + >> tests/system-ovn.at | 9 +-------- >> 3 files changed, 9 insertions(+), 8 deletions(-) >> >> diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at >> index d410ecd284..96e93d1ba2 100644 >> --- a/tests/ovn-controller-vtep.at >> +++ b/tests/ovn-controller-vtep.at >> @@ -162,6 +162,7 @@ AT_CHECK([ovn-sbctl --columns=vtep_logical_switches >> list Chassis | cut -d ':' -f >> ]) >> >> OVN_CONTROLLER_VTEP_STOP([/Chassis for VTEP physical switch (br-vtep) >> disappears/d]) >> +OVN_CLEANUP_NORTHD >> AT_CLEANUP >> >> >> @@ -237,6 +238,7 @@ AT_CHECK_UNQUOTED([ovn-sbctl --columns=chassis list >> Port_Binding | cut -d ':' -f >> ]) >> >> OVN_CONTROLLER_VTEP_STOP([/has already been associated with logical >> port/d]) >> +OVN_CLEANUP_NORTHD >> AT_CLEANUP >> >> >> @@ -279,6 +281,7 @@ ${chassis_uuid} >> ]) >> >> OVN_CONTROLLER_VTEP_STOP([/has already been associated with logical >> datapath/d]) >> +OVN_CLEANUP_NORTHD >> AT_CLEANUP >> >> >> @@ -333,6 +336,7 @@ AT_CHECK([vtep-ctl --columns=tunnel_key list >> Logical_Switch | cut -d ':' -f2 | t >> ]) >> >> OVN_CONTROLLER_VTEP_STOP >> +OVN_CLEANUP_NORTHD >> AT_CLEANUP >> >> >> @@ -437,6 +441,7 @@ AT_CHECK([vtep-ctl --columns=MAC list >> Ucast_Macs_Remote | cut -d ':' -f2- | tr - >> ]) >> >> OVN_CONTROLLER_VTEP_STOP >> +OVN_CLEANUP_NORTHD >> AT_CLEANUP >> >> >> @@ -507,6 +512,7 @@ AT_CHECK([vtep-ctl --columns=MAC list >> Ucast_Macs_Remote | cut -d ':' -f2- | tr - >> ]) >> >> OVN_CONTROLLER_VTEP_STOP([/has already been known to be on logical >> port/d]) >> +OVN_CLEANUP_NORTHD >> AT_CLEANUP >> >> # Tests vtep module 'Mcast_Macs_Remote's. >> @@ -565,6 +571,7 @@ done | sort], [0], [dnl >> ]) >> >> OVN_CONTROLLER_VTEP_STOP >> +OVN_CLEANUP_NORTHD >> AT_CLEANUP >> > - Test "ovn-controller-vtep - hv flows" also looks wrong as calling > OVN_CLEANUP i.e. misses stopping ovn-controller-vtep > - What about test "ovn -- check ovn-northd and ovn-controller-vtep version > pinning" ? > I think it is also missing some proper exit macros. > >> >> >> diff --git a/tests/ovn.at b/tests/ovn.at >> index 66cd897e42..b3ce9cdea9 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -10379,6 +10379,7 @@ AT_CHECK([ovn-nbctl --wait=sb set >> logical_switch_port local1 tag_request=50]) >> AT_CHECK([ovn-nbctl lsp-get-tag local1], [0], [50 >> ]) >> >> +OVN_CLEANUP_NORTHD >> > AT_CLEANUP >> ]) >> > > Are there not also many other tests missing the cleanup ? > I might be wrong but I have the impression that the following tests have a > similar issue: > ---- missing all cleanups > - ovn -- enables vlan-limit=0 > - VLAN transparency, passthru=true, ND > - VLAN transparency, passthru=true > - VLAN transparency, passthru=false > - VXLAN check port/datapath key space limits > - localport doesn't suppress ARP directed to external port > - localport takes part in broadcast ARP delivery > - Disabling RARP/GARP announcements from Router options > - IPv6 switching - megaflow check for IPv6 src/dst matches > - IPv4/v6 routing to external - megaflow check for src/dst matches > - lb_force_snat_ip=router_ip select correct network for snat > > --- missing some cleanup (e.g. ovsdb-server, ovs-vswitchd in main or hv): > - dhcpv4 : 1 HV, 2 LS, 2 LSPs/LS > - check ovn-northd and ovn-controller version pinning > > --- Also missing cleanups > - trace 1 LS, 3 LSPs > - Address Set generation from Port Groups (static addressing) (line 19529) > - Address Set generation from Port Groups (dynamic addressing) (line 19569) > - ovn-nbctl duplicate addresses > - ipam to non-ipam > - ipam router ports > - can't write to a backup database server instance > - unixctl socket > - normalized lrp-add > - normalized lr-nat-add > - normalized lr-nat-del > - ACL Conntrack ID propagation > - tag allocation > Ah, I guess my search was incomplete. Thanks for spending the time to list these here. I'll work on a v2 series. Thanks for the thorough review! Regards, Dumitru >> >> diff --git a/tests/system-ovn.at b/tests/system-ovn.at >> index 33aaa4cd79..965ee3b84b 100644 >> --- a/tests/system-ovn.at >> +++ b/tests/system-ovn.at >> @@ -18678,14 +18678,7 @@ AT_CHECK([ip route del 10.10.4.1 via 20.0.0.25 >> vrf vrf-$vni]) >> >> OVN_CLEANUP_CONTROLLER([hv1], [], [], [lr-frr]) >> >> -as ovn-sb >> -OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> - >> -as ovn-nb >> -OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> - >> -as northd >> -OVS_APP_EXIT_AND_WAIT([ovn-northd]) >> +OVN_CLEANUP_NORTHD >> >> as >> OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d >> -- >> 2.51.1 >> >> There are also missing cleanup in ovn-controller.at: > - ovn-controller - Local Chassis_Template_Var updates > - ovn-controller - Requested SNAT Zone in router creation transaction > > In ovn-northd.at > - LR single line configuration > - Logical Switch ARP filtering > > in perf-northd.at > - ovn-northd basic scale test -- 200 Hypervisors, 200 Logical > Ports/Hypervisor (line 198) > - ovn-northd basic scale test -- 500 Hypervisors, 50 Logical > Ports/Hypervisor (line 206) > In ovn-ic.at > - ovn-ic -- same routes destination > > Let me know if you'd like me to take this over. > Thanks > Xavier > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
