| Message ID | 20250710153345.104954-6-xsimonar@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Multiple unit/system tests fixes. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
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 <xsimonar@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
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 <mmichels@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 <xsimonar@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 > >
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
This caused ovn-controller restart to sometimes fail due with an error: "already running as pid xxx, aborting". Signed-off-by: Xavier Simonart <xsimonar@redhat.com> --- tests/ovn-controller.at | 12 +++++++++--- tests/ovn-macros.at | 5 +++-- tests/system-ovn.at | 10 ++++------ 3 files changed, 16 insertions(+), 11 deletions(-)