On 6/23/22 13:02, Mohammad Heib wrote:
> currently ovn ci only has one test case with the option
> ovn-monitor-all enabled, this patch will add one more
> execution with option ovn-monitor-all=true for each test case that
> are wrapped by OVN_FOR_EACH_NORTHD macro.
> 
> This will more or less double the number of test cases.
> It is possible to select a reduce set of test cases using -k "keywords".
> Keyword such as
>         dp-groups=yes
>         dp-groups=no
>         parallelization=yes
>         parallelization=no
>         ovn-northd
>         ovn-northd-ddlog
>         ovn_monitor_all=yes
> can be used to select a range of tests, as title is searched as well.
> 
> For instance, to run ovn-monitor-all tests, with dp-groups enabled and ddlog 
> disabled:
>         make check TESTSUITEFLAGS="-k 
> dp-groups=yes,ovn_monitor_all=yes,\!ovn-northd-ddlog"
> 
> Signed-off-by: Mohammad Heib <[email protected]>
> ---

Hi Mohammad,

>  tests/ovn-macros.at | 18 ++++++++++++++----
>  tests/ovs-macros.at |  4 +++-
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
> index 335f9158c..1858281c6 100644
> --- a/tests/ovn-macros.at
> +++ b/tests/ovn-macros.at
> @@ -323,6 +323,18 @@ ovn_az_attach() {
>          -- --may-exist add-br br-int \
>          -- set bridge br-int fail-mode=secure 
> other-config:disable-in-band=true \
>          || return 1
> +
> +    # currently this is the optimal place to add the ovn-monitor-all=true 
> option,
> +    # this can be implemented in a different way by redefining the sim-add 
> function
> +    # to add the ovn-related external-ids when we add a new simulated node 
> via sim-add.
> +    #
> +    # wait one sec to make sure that the ovn notice and update it 
> configuration
> +    # according to the new option.
> +    #

This comment needs an update, we're not sleeping anymore.

If you really want to be sure that the value is set before advancing to
the next statements you could add the "set open .
external_ids:ovn-monitor-all" to the ovs-vsctl transaction above this
block (where we set ovn-remote).

But I don't think that's needed, ovn-controller is not running at this
point, it will start only after ovs-vsctl exits, which should happen
after the DB was updated.  I think we're fine as-is so just removing the
comment should be ok.

> +    if test X$OVN_MONITOR_ALL = Xyes; then
> +        ovs-vsctl set open . external_ids:ovn-monitor-all=true
> +    fi
> +
>      start_daemon ovn-controller --enable-dummy-vif-plug || return 1
>  }
>  
> @@ -750,12 +762,10 @@ m4_define([OVN_POPULATE_ARP], 
> [AT_CHECK(ovn_populate_arp__, [0], [ignore])])
>  m4_define([OVN_FOR_EACH_NORTHD],
>    [m4_foreach([NORTHD_TYPE], [ovn-northd],
>       [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no],
> -       [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1
> -])])])])
> +       [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], 
> [m4_foreach([OVN_MONITOR_ALL], [yes, no], [$1])])])])])

I would keep the original indentation, so:

m4_define([OVN_FOR_EACH_NORTHD],
  [m4_foreach([NORTHD_TYPE], [ovn-northd],
     [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no],
       [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no],
         [m4_foreach([OVN_MONITOR_ALL], [yes, no], [$1
])])])])])

>  
>  # Some tests aren't prepared for dp groups to be enabled.
>  m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DP_GROUPS],
>    [m4_foreach([NORTHD_TYPE], [ovn-northd],
>       [m4_foreach([NORTHD_USE_DP_GROUPS], [no],
> -       [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1
> -])])])])
> +       [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], 
> [m4_foreach([OVN_MONITOR_ALL], [yes, no], [$1])])])])])

Same here.

Thanks,
Dumitru

> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> index 94dffa994..df4266d1f 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -9,7 +9,7 @@ dnl - If NORTHD_TYPE is defined, then append it to the test 
> name and
>  dnl   set it as a shell variable as well.
>  m4_rename([AT_SETUP], [OVS_AT_SETUP])
>  m4_define([AT_SETUP],
> -  [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- 
> NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- 
> dp-groups=NORTHD_USE_DP_GROUPS])[]m4_ifdef([NORTHD_USE_PARALLELIZATION], [ -- 
> parallelization=NORTHD_USE_PARALLELIZATION]))
> +  [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- 
> NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- 
> dp-groups=NORTHD_USE_DP_GROUPS])[]m4_ifdef([NORTHD_USE_PARALLELIZATION], [ -- 
> parallelization=NORTHD_USE_PARALLELIZATION])[]m4_ifdef([OVN_MONITOR_ALL], [ 
> -- ovn_monitor_all=OVN_MONITOR_ALL]))
>  m4_ifdef([NORTHD_TYPE], [[NORTHD_TYPE]=NORTHD_TYPE
>  ])dnl
>  m4_ifdef([NORTHD_USE_DP_GROUPS], [[NORTHD_USE_DP_GROUPS]=NORTHD_USE_DP_GROUPS
> @@ -18,6 +18,8 @@ m4_ifdef([NORTHD_USE_PARALLELIZATION], 
> [[NORTHD_USE_PARALLELIZATION]=NORTHD_USE_
>  ])dnl
>  m4_ifdef([NORTHD_DUMMY_NUMA], [[NORTHD_DUMMY_NUMA]=NORTHD_DUMMY_NUMA
>  ])dnl
> +m4_ifdef([OVN_MONITOR_ALL], [[OVN_MONITOR_ALL]=OVN_MONITOR_ALL
> +])dnl
>  ovs_init
>  ])
>  

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

Reply via email to