Hi Xavier,
This is the only patch in the series for which I have a recommendation.
The rest all look good to me.
On 6/4/24 09:10, Xavier Simonart wrote:
In some rare cases, and despite "recent" changes to wait for cleanup
before replying to exit, ovn-controller was still running when trying
to restart it.
Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
---
tests/ovn-macros.at | 9 +++++++++
tests/ovn.at | 17 ++++++-----------
2 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 47ada5c70..71a46db8b 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -1107,6 +1107,15 @@ m4_define([OVN_CHECK_SCAPY_EDNS_CLIENT_SUBNET_SUPPORT],
AT_SKIP_IF([! echo "from scapy.layers.dns import EDNS0ClientSubnet" | python
2>&1 > /dev/null])
])
+m4_define([OVN_CONTROLLER_EXIT_RESTART],
I think this should just be called "OVN_CONTROLLER_RESTART". The goal is
to restart ovn-controller, so I think the simpler name makes sense. It's
true that we are using an "exit" command for ovn-appctl, but that is
more of an implementation detail than anything else.
+ [TMPPID=$(cat $1/ovn-controller.pid)
+ AT_CHECK([as $1 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"
+ OVS_WAIT_WHILE([kill -0 $TMPPID 2>/dev/null])
+])
+
m4_define([OFTABLE_PHY_TO_LOG], [0])
m4_define([OFTABLE_LOG_INGRESS_PIPELINE], [8])
m4_define([OFTABLE_OUTPUT_LARGE_PKT_DETECT], [37])
diff --git a/tests/ovn.at b/tests/ovn.at
index 2dd0dfd2e..8fa26c192 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -20615,7 +20615,7 @@ echo $expected | ovstest test-ovn expr-to-packets >
expected
OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected])
# Stop ovn-controller on hv2 with --restart flag
-as hv2 ovs-appctl -t ovn-controller exit --restart
+OVN_CONTROLLER_EXIT_RESTART([hv2])
# Now send the packet again. This time, it should still arrive
OVS_WAIT_UNTIL([as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"])
@@ -29316,7 +29316,7 @@ check test "$hvt2" -gt 0
# Kill ovn-controller on chassis hv3, so that it won't update nb_cfg.
# Then wait for 9 out of 10
sleep 1
-check as hv3 ovn-appctl -t ovn-controller exit --restart
+OVN_CONTROLLER_EXIT_RESTART([hv3])
wait_for_ports_up
ovn-nbctl --wait=sb sync
wait_row_count Chassis_Private 9 name!=hv3 nb_cfg=2
@@ -36252,7 +36252,7 @@ check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
check_tunnel_port hv2 br-int hv1@192.168.0.1%192.168.0.2
# Stop ovn-controller on hv1
-check as hv1 ovn-appctl -t ovn-controller exit --restart
+OVN_CONTROLLER_EXIT_RESTART([hv1])
# The tunnel should remain intact
check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
@@ -36281,7 +36281,7 @@ check_tunnel_port hv2 br-int1
hv1@192.168.0.1%192.168.0.2
check grep -q "Clearing old tunnel port \"ovn-hv1-0\" (hv1@192.168.0.1%192.168.0.2) from
bridge \"br-int\"" hv2/ovn-controller.log
# Stop ovn-controller on hv1
-check as hv1 ovn-appctl -t ovn-controller exit --restart
+OVN_CONTROLLER_EXIT_RESTART([hv1])
# The tunnel should remain intact
check_tunnel_port hv1 br-int1 hv2@192.168.0.2%192.168.0.1
@@ -36371,10 +36371,7 @@ prev_id2=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-i
# The hv2 is running we can remove the override file
rm -f ${OVN_SYSCONFDIR}/system-id-override
-check ovn-appctl -t ovn-controller exit --restart
-
-# Make sure ovn-controller stopped before restarting it
-OVS_WAIT_UNTIL([test x$(ovn-appctl -t ovn-controller debug/status) !=
"xrunning"])
+OVN_CONTROLLER_EXIT_RESTART([hv1])
# for some reason SSL ovsdb configuration overrides CLI, so
# delete ssl config from ovsdb to give CLI arguments priority
@@ -37086,9 +37083,7 @@ AT_CHECK([grep -c "NXT_CT_FLUSH_ZONE"
hv1/ovs-vswitchd.log], [0], [dnl
])
AS_BOX([Check conversion from UUID - restart])
-ovn-appctl -t ovn-controller exit --restart
-# Make sure ovn-controller stopped before restarting it
-OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" !=
"running"])
+OVN_CONTROLLER_EXIT_RESTART([hv1])
replace_with_uuid lr0
replace_with_uuid sw0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
