ci: Remove '--recheck' in CI.

If we want to catch new failures faster we have a better chance if CI
doesn't auto-retry (once).

There are some tests that are still "unstable" and fail every now and
then.  In order to reduce the number of false negatives keep the
--recheck for them.  To achieve that we use a new macro, TAG_UNSTABLE,
to tag all these tests.  The list of "unstable" tests is compiled based
on the following discussion:
https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405465.html

In order to avoid new GitHub actions jobs, we re-purpose the last job of
each target type to also run the unstable tests.  These jobs were
already running less tests than others so the additional run time should
not be an issue.

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Ales Musil <amusil@redhat.com>

.ci/ci.sh

@@ -101,7 +101,7 @@ function run_tests() {
         && \
         ARCH=$ARCH CC=$CC LIBS=$LIBS OPTS=$OPTS TESTSUITE=$TESTSUITE \
         TEST_RANGE=$TEST_RANGE SANITIZERS=$SANITIZERS DPDK=$DPDK \
-        ./.ci/linux-build.sh
+        RECHECK=$RECHECK UNSTABLE=$UNSTABLE ./.ci/linux-build.sh
     "
 }
 

.ci/linux-build.sh

@@ -9,6 +9,7 @@ COMMON_CFLAGS=""
 OVN_CFLAGS=""
 OPTS="$OPTS --enable-Werror"
 JOBS=${JOBS:-"-j4"}
+RECHECK=${RECHECK:-"no"}
 
 function install_dpdk()
 {
@@ -99,34 +100,80 @@ function configure_clang()
     COMMON_CFLAGS="${COMMON_CFLAGS} -Wno-error=unused-command-line-argument"
 }
 
+function run_tests()
+{
+    if ! make distcheck CFLAGS="${COMMON_CFLAGS} ${OVN_CFLAGS}" $JOBS \
+        TESTSUITEFLAGS="$JOBS $TEST_RANGE" RECHECK=$RECHECK \
+        SKIP_UNSTABLE=$SKIP_UNSTABLE
+    then
+        # testsuite.log is necessary for debugging.
+        cat */_build/sub/tests/testsuite.log
+        return 1
+    fi
+}
+
 function execute_tests()
 {
     # 'distcheck' will reconfigure with required options.
     # Now we only need to prepare the Makefile without sparse-wrapped CC.
     configure_ovn
 
     export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
-    if ! make distcheck CFLAGS="${COMMON_CFLAGS} ${OVN_CFLAGS}" $JOBS \
-        TESTSUITEFLAGS="$JOBS $TEST_RANGE" RECHECK=yes
-    then
-        # testsuite.log is necessary for debugging.
-        cat */_build/sub/tests/testsuite.log
+
+    local stable_rc=0
+    local unstable_rc=0
+
+    if ! SKIP_UNSTABLE=yes run_tests; then
+        stable_rc=1
+    fi
+
+    if [ "$UNSTABLE" ]; then
+        if ! SKIP_UNSTABLE=no TEST_RANGE="-k unstable" RECHECK=yes \
+                run_tests; then
+            unstable_rc=1
+        fi
+    fi
+
+    if [[ $stable_rc -ne 0 ]] || [[ $unstable_rc -ne 0 ]]; then
         exit 1
     fi
 }
 
+function run_system_tests()
+{
+    local type=$1
+    local log_file=$2
+
+    if ! sudo make $JOBS $type TESTSUITEFLAGS="$TEST_RANGE" \
+            RECHECK=$RECHECK SKIP_UNSTABLE=$SKIP_UNSTABLE; then
+        # $log_file is necessary for debugging.
+        cat tests/$log_file
+        return 1
+    fi
+}
+
 function execute_system_tests()
 {
-      type=$1
-      log_file=$2
-
-      configure_ovn $OPTS
-      make $JOBS || { cat config.log; exit 1; }
-      if ! sudo make $JOBS $type TESTSUITEFLAGS="$TEST_RANGE" RECHECK=yes; then
-          # $log_file is necessary for debugging.
-          cat tests/$log_file
-          exit 1
-      fi
+    configure_ovn $OPTS
+    make $JOBS || { cat config.log; exit 1; }
+
+    local stable_rc=0
+    local unstable_rc=0
+
+    if ! SKIP_UNSTABLE=yes run_system_tests $@; then
+        stable_rc=1
+    fi
+
+    if [ "$UNSTABLE" ]; then
+        if ! SKIP_UNSTABLE=no TEST_RANGE="-k unstable" RECHECK=yes \
+                run_system_tests $@; then
+            unstable_rc=1
+        fi
+    fi
+
+    if [[ $stable_rc -ne 0 ]] || [[ $unstable_rc -ne 0 ]]; then
+        exit 1
+    fi
 }
 
 configure_$CC

.github/workflows/test.yml

@@ -92,6 +92,7 @@ jobs:
       TESTSUITE:   ${{ matrix.cfg.testsuite }}
       TEST_RANGE:  ${{ matrix.cfg.test_range }}
       SANITIZERS:  ${{ matrix.cfg.sanitizers }}
+      UNSTABLE:    ${{ matrix.cfg.unstable }}
 
     name: linux ${{ join(matrix.cfg.*, ' ') }}
     runs-on: ubuntu-22.04
@@ -104,21 +105,21 @@ jobs:
         - { compiler: clang, opts: --disable-ssl }
         - { compiler: gcc, testsuite: test, test_range: "-300" }
         - { compiler: gcc, testsuite: test, test_range: "301-600" }
-        - { compiler: gcc, testsuite: test, test_range: "601-" }
+        - { compiler: gcc, testsuite: test, test_range: "601-", unstable: unstable }
         - { compiler: clang, testsuite: test, sanitizers: sanitizers, test_range: "-300" }
         - { compiler: clang, testsuite: test, sanitizers: sanitizers, test_range: "301-600" }
-        - { compiler: clang, testsuite: test, sanitizers: sanitizers, test_range: "601-" }
+        - { compiler: clang, testsuite: test, sanitizers: sanitizers, test_range: "601-", unstable: unstable }
         - { compiler: gcc, testsuite: test, libs: -ljemalloc, test_range: "-300" }
         - { compiler: gcc, testsuite: test, libs: -ljemalloc, test_range: "301-600" }
-        - { compiler: gcc, testsuite: test, libs: -ljemalloc, test_range: "601-" }
+        - { compiler: gcc, testsuite: test, libs: -ljemalloc, test_range: "601-", unstable: unstable }
         - { compiler: gcc, testsuite: system-test-dpdk, dpdk: dpdk, test_range: "-100" }
-        - { compiler: gcc, testsuite: system-test-dpdk, dpdk: dpdk, test_range: "101-" }
+        - { compiler: gcc, testsuite: system-test-dpdk, dpdk: dpdk, test_range: "101-", unstable: unstable }
         - { compiler: gcc, testsuite: system-test-userspace, test_range: "-100" }
-        - { compiler: gcc, testsuite: system-test-userspace, test_range: "101-" }
+        - { compiler: gcc, testsuite: system-test-userspace, test_range: "101-", unstable: unstable }
         - { compiler: gcc, testsuite: system-test, test_range: "-100" }
-        - { compiler: gcc, testsuite: system-test, test_range: "101-" }
+        - { compiler: gcc, testsuite: system-test, test_range: "101-", unstable: unstable }
         - { compiler: clang, testsuite: system-test, sanitizers: sanitizers, test_range: "-100" }
-        - { compiler: clang, testsuite: system-test, sanitizers: sanitizers, test_range: "101-" }
+        - { compiler: clang, testsuite: system-test, sanitizers: sanitizers, test_range: "101-", unstable: unstable }
         - { arch: x86, compiler: gcc, opts: --disable-ssl }
 
     steps:

tests/ovn-macros.at

@@ -971,3 +971,8 @@ m4_define([RUN_OVN_NBCTL], [
     check ovn-nbctl ${command}
     unset command
 ])
+
+m4_define([TAG_UNSTABLE], [
+    AT_KEYWORDS([unstable])
+    AT_SKIP_IF([test X"$SKIP_UNSTABLE" = Xyes])
+])

tests/ovn-northd.at

@@ -4616,6 +4616,7 @@ AT_CLEANUP
 
 OVN_FOR_EACH_NORTHD_NO_HV([
 AT_SETUP([northd ssl file change])
+TAG_UNSTABLE
 AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
 PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
 AT_SKIP_IF([expr "$PKIDIR" : ".*[[ 	'\"

tests/ovn-performance.at

@@ -225,6 +225,7 @@ m4_define([OVN_CONTROLLER_EXPECT_HIT_COND],[
 ])
 
 AT_SETUP([ovn-controller incremental processing])
+TAG_UNSTABLE
 # Check which operations the trigger full logical flow processing.
 #
 # Create and destroy logical routers, switches, ports, address sets and ACLs

tests/ovn.at

@@ -6084,6 +6084,7 @@ AT_CLEANUP
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([1 HV, 2 LSs, 1 lport/LS, 1 LR])
 AT_KEYWORDS([router-admin-state])
+TAG_UNSTABLE
 ovn_start
 
 # Logical network:
@@ -7892,6 +7893,7 @@ AT_CLEANUP
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([policy-based routing: 1 HVs, 2 LSs, 1 lport/LS, 1 LR])
 AT_KEYWORDS([policy-based-routing])
+TAG_UNSTABLE
 ovn_start
 
 # Logical network:
@@ -8064,6 +8066,7 @@ AT_CLEANUP
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([policy-based routing IPv6: 1 HVs, 3 LSs, 1 lport/LS, 1 LR])
 AT_KEYWORDS([policy-based-routing])
+TAG_UNSTABLE
 ovn_start
 
 # Logical network:
@@ -14395,6 +14398,7 @@ AT_CLEANUP
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([options:multiple requested-chassis for logical port])
 AT_KEYWORDS([multi-chassis])
+TAG_UNSTABLE
 ovn_start
 
 net_add n1
@@ -14526,6 +14530,7 @@ AT_CLEANUP
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([options:multiple requested-chassis for logical port: change chassis role])
 AT_KEYWORDS([multi-chassis])
+TAG_UNSTABLE
 ovn_start
 
 net_add n1
@@ -14576,6 +14581,7 @@ AT_CLEANUP
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([options:multiple requested-chassis for logical port: unclaimed behavior])
 AT_KEYWORDS([multi-chassis])
+TAG_UNSTABLE
 ovn_start
 
 net_add n1
@@ -16161,6 +16167,7 @@ AT_CLEANUP
 
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([tug-of-war between two chassis for the same port])
+TAG_UNSTABLE
 ovn_start
 
 ovn-nbctl ls-add ls0
@@ -29565,6 +29572,7 @@ AT_CLEANUP
 
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([nb_cfg timestamp])
+TAG_UNSTABLE
 ovn_start
 
 check ovn-nbctl ls-add s2
@@ -29666,6 +29674,7 @@ AT_CLEANUP
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([ARP replies for SNAT external ips])
 AT_KEYWORDS([slowtest])
+TAG_UNSTABLE
 ovn_start
 
 net_add n1
@@ -30027,6 +30036,7 @@ AT_CLEANUP
 # It is to cover a corner case when flows are re-processed in the I-P
 # iteration, combined with the scenario of conflicting ACLs.
 AT_SETUP([conflict ACLs with address set])
+TAG_UNSTABLE
 ovn_start
 
 ovn-nbctl ls-add ls1
@@ -30222,6 +30232,7 @@ AT_CLEANUP
 #
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([multi-vtep SB Chassis encap updates])
+TAG_UNSTABLE
 ovn_start
 
 net_add n1
@@ -32402,6 +32413,7 @@ AT_CLEANUP
 # - 2 for expanding the port group @pg1 to the 2 locally bound lports.
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([ACL with Port Group conjunction flow efficiency])
+TAG_UNSTABLE
 ovn_start
 
 net_add n1
@@ -34693,6 +34705,7 @@ AT_CLEANUP
 
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([recomputes])
+TAG_UNSTABLE
 ovn_start
 
 net_add n1
@@ -35127,6 +35140,7 @@ MULTIPLE_OVS_INT([])
 
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([Check default openflow flows])
+TAG_UNSTABLE
 ovn_start
 
 # Check that every table has a default (i.e: priority=0) flow.

tests/system-ovn.at

@@ -8906,6 +8906,7 @@ AT_CLEANUP
 
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([LB - ICMP related traffic])
+TAG_UNSTABLE
 
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_NAT()
@@ -11403,6 +11404,7 @@ AT_CLEANUP
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([Tiered ACLs])
 AT_KEYWORDS([acl])
+TAG_UNSTABLE
 
 ovn_start
 OVS_TRAFFIC_VSWITCHD_START()