On 9/7/23 20:52, Han Zhou wrote: > On Thu, Sep 7, 2023 at 7:48 AM Ales Musil <[email protected]> wrote: >> >> Single test needs to run 4 times to check all >> the permutations, this is taking a lot of CI time >> and job space. >> >> Remove the parallel permutation and leave parallelization >> enabled for all tests, as this use case is more complex. >> Only exception are 10 tests in ovn-northd.at that still run >> parallelization permutations. This should be enough to cover >> the code with parallelization disabled. >> >> This allows us to greatly reduce the number of test cases >> (almost by half) and we can also remove 6 jobs from the CI >> pipeline. The time reduction is very noticeable going down >> from ~30 min to ~20 min. >> >> Signed-off-by: Ales Musil <[email protected]> >> --- >> .github/workflows/test.yml | 28 +++++++++++----------------- >> tests/ovn-macros.at | 15 +++++++++++---- >> tests/ovn-northd.at | 20 ++++++++++---------- >> 3 files changed, 32 insertions(+), 31 deletions(-) >> >> diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml >> index fe2a14c40..5c5ce6ed1 100644 >> --- a/.github/workflows/test.yml >> +++ b/.github/workflows/test.yml >> @@ -102,29 +102,23 @@ jobs: >> cfg: >> - { compiler: gcc, opts: --disable-ssl } >> - { compiler: clang, opts: --disable-ssl } >> - - { compiler: gcc, testsuite: test, test_range: "-500" } >> - - { compiler: gcc, testsuite: test, test_range: "501-1000" } >> - - { compiler: gcc, testsuite: test, test_range: "1001-" } >> + - { compiler: gcc, testsuite: test, test_range: "-300" } >> + - { compiler: gcc, testsuite: test, test_range: "301-600" } >> + - { compiler: gcc, testsuite: test, test_range: "601-" } >> - { 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-900" } >> - - { compiler: clang, testsuite: test, sanitizers: sanitizers, > test_range: "901-1200" } >> - - { compiler: clang, testsuite: test, sanitizers: sanitizers, > test_range: "1201-" } >> - - { compiler: gcc, testsuite: test, libs: -ljemalloc, > test_range: "-500" } >> - - { compiler: gcc, testsuite: test, libs: -ljemalloc, > test_range: "501-1000" } >> - - { compiler: gcc, testsuite: test, libs: -ljemalloc, > test_range: "1001-" } >> + - { compiler: clang, testsuite: test, sanitizers: sanitizers, > test_range: "601-" } >> + - { 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: system-test-dpdk, dpdk: dpdk, > test_range: "-100" } >> - - { compiler: gcc, testsuite: system-test-dpdk, dpdk: dpdk, > test_range: "101-200" } >> - - { compiler: gcc, testsuite: system-test-dpdk, dpdk: dpdk, > test_range: "201-" } >> + - { compiler: gcc, testsuite: system-test-dpdk, dpdk: dpdk, > test_range: "101-" } >> - { compiler: gcc, testsuite: system-test-userspace, test_range: > "-100" } >> - - { compiler: gcc, testsuite: system-test-userspace, test_range: > "101-200" } >> - - { compiler: gcc, testsuite: system-test-userspace, test_range: > "201-" } >> + - { compiler: gcc, testsuite: system-test-userspace, test_range: > "101-" } >> - { compiler: gcc, testsuite: system-test, test_range: "-100" } >> - - { compiler: gcc, testsuite: system-test, test_range: "101-200" > } >> - - { compiler: gcc, testsuite: system-test, test_range: "201-" } >> + - { compiler: gcc, testsuite: system-test, test_range: "101-" } >> - { compiler: clang, testsuite: system-test, sanitizers: > sanitizers, test_range: "-100" } >> - - { compiler: clang, testsuite: system-test, sanitizers: > sanitizers, test_range: "101-200" } >> - - { compiler: clang, testsuite: system-test, sanitizers: > sanitizers, test_range: "201-" } >> + - { compiler: clang, testsuite: system-test, sanitizers: > sanitizers, test_range: "101-" } >> - { arch: x86, compiler: gcc, opts: --disable-ssl } >> >> steps: >> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at >> index 13d5dc3d4..7c0abdece 100644 >> --- a/tests/ovn-macros.at >> +++ b/tests/ovn-macros.at >> @@ -887,21 +887,28 @@ OVS_END_SHELL_HELPERS >> m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], > [ignore])]) >> >> # Defines versions of the test with all combinations of northd, >> -# parallelization on/off and conditional monitoring on/off. >> +# parallelization enabled and conditional monitoring on/off. >> m4_define([OVN_FOR_EACH_NORTHD], >> [m4_foreach([NORTHD_TYPE], [ovn-northd], >> - [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], >> + [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes], >> [m4_foreach([OVN_MONITOR_ALL], [yes, no], [$1 >> ])])])]) >> >> # Defines versions of the test with all combinations of northd and >> -# parallelization on/off. To be used when the ovn-controller > configuration >> +# parallelization enabled. To be used when the ovn-controller > configuration >> # is not relevant. >> m4_define([OVN_FOR_EACH_NORTHD_NO_HV], >> [m4_foreach([NORTHD_TYPE], [ovn-northd], >> - [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1 >> + [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes], [$1 >> ])])]) >> >> +# Defines versions of the test with all combinations of northd and >> +# parallelization on/off. To be used when the ovn-controller > configuration >> +# is not relevant and we want to test parallelization permutations. >> +m4_define([OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION], >> + [m4_foreach([NORTHD_TYPE], [ovn-northd], >> + [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1 >> +])])]) >> >> # OVN_NBCTL(NBCTL_COMMAND) adds NBCTL_COMMAND to list of commands to be > run by RUN_OVN_NBCTL(). >> m4_define([OVN_NBCTL], [ >> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >> index 23dbe111f..c26561550 100644 >> --- a/tests/ovn-northd.at >> +++ b/tests/ovn-northd.at >> @@ -3869,7 +3869,7 @@ wait_row_count nb:Logical_Switch_Port 1 up=false > name=lsp1 >> AT_CLEANUP >> ]) >> >> -OVN_FOR_EACH_NORTHD_NO_HV([ >> +OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([ >> AT_SETUP([Load Balancers and lb_force_snat_ip for Gateway Routers]) >> ovn_start >> >> @@ -4408,7 +4408,7 @@ AT_CHECK([ovn-sbctl dump-flows sw0 | grep "ls_in_lb > " | sed 's/table=../table=?? >> AT_CLEANUP >> ]) >> >> -OVN_FOR_EACH_NORTHD_NO_HV([ >> +OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([ >> AT_SETUP([ovn -- ACL label usage]) >> ovn_start >> >> @@ -4501,7 +4501,7 @@ AT_CHECK([grep "ls_out_stateful" sw0flows | sed > 's/table=../table=??/' | sort], >> AT_CLEANUP >> ]) >> >> -OVN_FOR_EACH_NORTHD_NO_HV([ >> +OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([ >> AT_SETUP([ovn -- ct.inv usage]) >> ovn_start >> >> @@ -5000,7 +5000,7 @@ check_lflows 0 >> AT_CLEANUP >> ]) >> >> -OVN_FOR_EACH_NORTHD_NO_HV([ >> +OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([ >> AT_SETUP([ovn -- ARP flows for unreachable addresses - NAT and LB]) >> ovn_start >> >> @@ -5206,7 +5206,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | grep > "192.168.4.100" | grep -v clone >> AT_CLEANUP >> ]) >> >> -OVN_FOR_EACH_NORTHD_NO_HV([ >> +OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([ >> AT_SETUP([ovn -- LR NAT flows]) >> ovn_start >> >> @@ -6000,7 +6000,7 @@ AT_CHECK([ovn-sbctl dump-flows lr0 | grep > "lr_in_dnat" | sort], [0], [dnl >> AT_CLEANUP >> ]) >> >> -OVN_FOR_EACH_NORTHD_NO_HV([ >> +OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([ >> AT_SETUP([Load Balancer SB duplicates]) >> ovn_start >> >> @@ -6651,7 +6651,7 @@ ct_dnat /* assuming no un-dnat entry, so no change > */ /* default (use --ct to cu >> AT_CLEANUP >> ]) >> >> -OVN_FOR_EACH_NORTHD_NO_HV([ >> +OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([ >> AT_SETUP([route tables -- flows]) >> AT_KEYWORDS([route-tables-flows]) >> ovn_start >> @@ -7078,7 +7078,7 @@ AT_CHECK([grep -e 'lr_in_ip_routing ' lrflows | > grep -e 'igmp' -e 'mld' | sed >> AT_CLEANUP >> ]) >> >> -OVN_FOR_EACH_NORTHD_NO_HV([ >> +OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([ >> AT_SETUP([ACLs after lb]) >> AT_KEYWORDS([acl]) >> ovn_start >> @@ -7255,7 +7255,7 @@ AT_CHECK([grep -e "ls_in_stateful" lsflows | sed > 's/table=../table=??/' | sort], >> AT_CLEANUP >> ]) >> >> -OVN_FOR_EACH_NORTHD_NO_HV([ >> +OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([ >> AT_SETUP([ovn-northd -- lr multiple gw ports NAT]) >> AT_KEYWORDS([multiple-l3dgw-ports]) >> ovn_start >> @@ -7494,7 +7494,7 @@ AT_CHECK([cat lrflows | grep -e > lr_in_lookup_neighbor -e lr_in_learn_neighbor | >> AT_CLEANUP >> ]) >> >> -OVN_FOR_EACH_NORTHD_NO_HV([ >> +OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([ >> AT_SETUP([LS default ACL drop]) >> AT_KEYWORDS([acl]) >> >> -- >> 2.41.0 >> > > Thanks Ales! I am good with this approach, but I'd like to hear agreement > from other maintainers before merging it. > > Acked-by: Han Zhou <[email protected]> >
+1, I went ahead and applied this change to main. Thanks, Han and Ales! Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
