Hi Mark
Thanks for the review and comments. I'll send v2.
Thanks
Xavier
On Mon, Jul 21, 2025 at 10:49 PM Mark Michelson <mmich...@redhat.com> wrote:
> Would it be worthwhile to provide an optional parameter to
> OVN_CONTROLLER_EXIT() that makes it so that it does not pass the
> `--restart` flag when exiting? This way, you could use
> OVN_CONTROLLER_EXIT() everywhere.
>
> On 7/10/25 11:33 AM, Xavier Simonart via dev wrote:
> > This caused ovn-controller restart to sometimes fail due with an error:
> > "already running as pid xxx, aborting".
> >
> > Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
> > ---
> > tests/ovn-controller.at | 12 +++++++++---
> > tests/ovn-macros.at | 5 +++--
> > tests/system-ovn.at | 10 ++++------
> > 3 files changed, 16 insertions(+), 11 deletions(-)
> >
> > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> > index 6007a7454..28189b049 100644
> > --- a/tests/ovn-controller.at
> > +++ b/tests/ovn-controller.at
> > @@ -3729,12 +3729,14 @@ start_controller() {
> > }
> >
> > # Stop ovn-controller, without any argument it will cleanup resources.
> > +TMPPID=$(cat hv1/ovn-controller.pid)
> > check ovn-appctl -t ovn-controller exit
> > check grep -q "Exiting ovn-controller, resource cleanup: True"
> hv1/ovn-controller.log
> > +OVS_WAIT_WHILE([kill -0 $TMPPID 2>/dev/null])>
> > # Stop ovn-controller without cleanup.
> > start_controller
> > -check ovn-appctl -t ovn-controller exit --restart
> > +OVN_CONTROLLER_EXIT([hv1])
> > check grep -q "Exiting ovn-controller, resource cleanup: False
> (--restart)" hv1/ovn-controller.log
> >
> > # Set cleanup on exit.
> > @@ -3742,12 +3744,14 @@ check ovs-vsctl set open .
> external-ids:ovn-cleanup-on_exit=true
> >
> > # Stop ovn-controller, without any argument it will cleanup resources.
> > start_controller
> > +TMPPID=$(cat hv1/ovn-controller.pid)
> > check ovn-appctl -t ovn-controller exit
> > check grep -q "Exiting ovn-controller, resource cleanup: True"
> hv1/ovn-controller.log
> > +OVS_WAIT_WHILE([kill -0 $TMPPID 2>/dev/null])
> >
> > # Stop ovn-controller, --restart flag has priority over external-ids,
> no cleanup.
> > start_controller
> > -check ovn-appctl -t ovn-controller exit --restart
> > +OVN_CONTROLLER_EXIT([hv1])
> > check grep -q "Exiting ovn-controller, resource cleanup: False
> (--restart)" hv1/ovn-controller.log
> >
> > # Set cleanup on exit to false.
> > @@ -3755,12 +3759,14 @@ check ovs-vsctl set open .
> external-ids:ovn-cleanup-on-exit=false
> >
> > # Stop ovn-controller, without any argument it won't cleanup due to
> the external-ids.
> > start_controller
> > +TMPPID=$(cat hv1/ovn-controller.pid)
> > check ovn-appctl -t ovn-controller exit
> > check grep -q "Exiting ovn-controller, resource cleanup: False
> (--restart)" hv1/ovn-controller.log
> > +OVS_WAIT_WHILE([kill -0 $TMPPID 2>/dev/null])
> >
> > # Stop ovn-controller without cleanup.
> > start_controller
> > -check ovn-appctl -t ovn-controller exit --restart
> > +OVN_CONTROLLER_EXIT([hv1])
> > check grep -q "Exiting ovn-controller, resource cleanup: False
> (--restart)" hv1/ovn-controller.log
> >
> > # Start the controller so the test cleanup routing doesn't get stuck.
> > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
> > index 13cff5c38..20755942b 100644
> > --- a/tests/ovn-macros.at
> > +++ b/tests/ovn-macros.at
> > @@ -1460,8 +1460,9 @@
> m4_define([OVN_CHECK_SCAPY_EDNS_CLIENT_SUBNET_SUPPORT],
> > ])
> >
> > m4_define([OVN_CONTROLLER_EXIT],
> > - [TMPPID=$(cat $1/ovn-controller.pid)
> > - AT_CHECK([as $1 ovn-appctl -t ovn-controller exit --restart])
> > + [TMPPID=$(cat ./$1/ovn-controller.pid)
> > + AT_CHECK([as $1
> > +check ovn-appctl -t ovn-controller exit --restart])
> > # Make sure ovn-controller stopped so that a future restart will
> not fail.
> > # Checking debug/status is running is not enough as there might be
> a small race condition.
> > echo "Waiting for pid $TMPPID"
> > diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> > index c2b70d3a1..d0888badf 100644
> > --- a/tests/system-ovn.at
> > +++ b/tests/system-ovn.at
> > @@ -16266,9 +16266,8 @@ blackhole 198.51.100.0/24 proto ovn metric 1000
> > 233.252.0.0/24 via 192.168.10.10 dev lo onlink
> > 233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink])
> >
> > -# Stoping with --restart will not touch the routes.
> > -check ovn-appctl -t ovn-controller exit --restart
> > -OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" !=
> "running"])
> > +# Stopping with --restart will not touch the routes.
> > +OVN_CONTROLLER_EXIT([])
> > OVN_ROUTE_EQUAL([ovnvrf1338], [dnl
> > blackhole 192.0.2.1 proto ovn metric 1000
> > blackhole 192.0.2.2 proto ovn metric 100
> > @@ -16610,9 +16609,8 @@ blackhole 198.51.100.0/24 proto ovn metric 1000
> > 233.252.0.0/24 via 192.168.10.10 dev lo onlink
> > 233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink])
> >
> > -# Stoping with --restart will not touch the routes.
> > -check ovn-appctl -t ovn-controller exit --restart
> > -OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" !=
> "running"])
> > +# Stopping with --restart will not touch the routes.
> > +OVN_CONTROLLER_EXIT([])
> > OVN_ROUTE_EQUAL([ovnvrf1337], [dnl
> > blackhole 192.0.2.1 proto ovn metric 100
> > blackhole 192.0.2.2 proto ovn metric 100
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
