Hi Jacob Thanks for the review.
On Thu, Aug 27, 2026 at 10:12 PM Jacob Tanenbaum <[email protected]> wrote: > > > On Thu, Aug 27, 2026 at 11:38 AM Xavier Simonart via dev < > [email protected]> wrote: > >> Some multinode test [1] writes chassis-mac-mapping, but does not >> clean it up on exit. This would cause some other tests, not expecting >> the chassis-mac-mapping, to fail if executed afterwards. >> For instance, executing [0] after [1] fails. >> [0] HA: Check for missing garp on leader when BFD goes back up. >> [1] redirect-bridged to non-gw destination switch port. >> >> Signed-off-by: Xavier Simonart <[email protected]> >> --- >> tests/multinode.at | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/tests/multinode.at b/tests/multinode.at >> index 91f220abd..3df2921ee 100644 >> --- a/tests/multinode.at >> +++ b/tests/multinode.at >> @@ -5275,6 +5275,9 @@ check multinode_nbctl lrp-set-gateway-chassis >> ro-public ovn-gw-1 >> check multinode_nbctl lrp-set-redirect-type ro-public bridged >> >> m_as ovn-gw-1 ovs-vsctl set open . >> external-ids:ovn-bridge-mappings=public:br-ex >> + >> +on_exit 'm_as ovn-chassis-1 ovs-vsctl remove open . external-ids >> ovn-chassis-mac-mappings' >> +on_exit 'm_as ovn-gw-1 ovs-vsctl remove open . external-ids >> ovn-chassis-mac-mappings' >> > > Would this be better placedd in cleanup_multinode_resources_by_nodes in > tests/multinode-macros.at? That way, any other tests added in the future > will have these external-ids cleaned up without needing to add the cleanup > code. I see that other tests in this file have cleanup code in them so this > approch is fine. > I do not have a strong opinion on this, but I would not add it to the macro. If we were doing it in the macro, ideally, we would then need to remove other external_ids as well (but not all of them e.g. ovn-encap-type), or run something similar to multinode_setup_controller. We would need to run this in cleanup macros for all nodes, in most cases for no reason (only few tests set external_ids), and it would take time/resources. So I feel each test which sets some external_ids should clean it itself. As you noted, other tests already clean up what they set (such as dynamic-routing-port-mapping). So I would leave the patch as it is now. > > >> m_as ovn-chassis-1 ovs-vsctl set open . >> external-ids:ovn-chassis-mac-mappings="public:aa:bb:cc:dd:01:01" >> m_as ovn-gw-1 ovs-vsctl set open . >> external-ids:ovn-chassis-mac-mappings="public:aa:bb:cc:dd:02:01" >> >> -- >> 2.47.1 >> > Thanks Xavier > >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
