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]>

Thanks,
Han
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to