| Message ID | 20210414100939.15161-1-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] tests: Run tests with Datapath Groups enabled/disabled. | expand |
Acked-by: Mark Michelson <mmichels@redhat.com> On 4/14/21 6:09 AM, Dumitru Ceara wrote: > This doubles the size of the test matrix but should help protecting > against regressions. > > This change is inspired from Ilya's work below but instead of selectively > running tests with Datapath Groups if an external variable is set, it > always runs both sets of tests expanding the OVN_FOR_EACH_NORTHD > implementation: > https://github.com/igsilya/ovn/commit/a5eef74cc374f04bb6a12545ceb504108e724135 > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > Note: > - If accepted this should probably be pushed after the following > patch is accepted: > http://patchwork.ozlabs.org/project/ovn/patch/20210412124009.20468-1-dceara@redhat.com/ This patch is merged. > - It might turn out during review that Ilya's original approach is > preferable. > - A sample CI run for reference wrt. additional runtime can be found > here (the failure is due to a known flaky test): > https://github.com/dceara/ovn/actions/runs/747763765 > --- > tests/ovn-macros.at | 10 ++++++++++ > tests/ovn-northd.at | 10 ++++++++++ > tests/ovs-macros.at | 4 +++- > 3 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > index 25f3dbe34..3f432cef5 100644 > --- a/tests/ovn-macros.at > +++ b/tests/ovn-macros.at > @@ -232,6 +232,10 @@ ovn_start () { > --ic-nb-db=unix:"$ovs_base"/ovn-ic-nb/ovn-ic-nb.sock \ > --ic-sb-db=unix:"$ovs_base"/ovn-ic-sb/ovn-ic-sb.sock > fi > + > + if test X$NORTHD_USE_DP_GROUPS = Xyes; then > + ovn-nbctl set NB_Global . options:use_logical_dp_groups=true > + fi > } > > # Interconnection networks. > @@ -493,9 +497,15 @@ m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) > > m4_define([OVN_FOR_EACH_NORTHD], [dnl > m4_pushdef([NORTHD_TYPE], [ovn-northd])dnl > +m4_pushdef([NORTHD_USE_DP_GROUPS], [yes])dnl > +$1 > +m4_popdef([NORTHD_USE_DP_GROUPS])dnl > $1 > m4_popdef([NORTHD_TYPE])dnl > m4_pushdef([NORTHD_TYPE], [ovn-northd-ddlog])dnl > +m4_pushdef([NORTHD_USE_DP_GROUPS], [yes])dnl > +$1 > +m4_popdef([NORTHD_USE_DP_GROUPS])dnl > $1 > m4_popdef([NORTHD_TYPE])dnl > ]) > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index eef360802..5548ae9fd 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -736,6 +736,11 @@ AT_CLEANUP > > OVN_FOR_EACH_NORTHD([ > AT_SETUP([ovn -- northbound database reconnection]) > + > +dnl This test doesn't take into account flows that are shared between > +dnl datapaths when datapath groups are enabled. > +AT_SKIP_IF([test NORTHD_USE_DP_GROUPS = yes]) > + > ovn_start --no-backup-northd > > # Check that ovn-northd is active, by verifying that it creates and > @@ -771,6 +776,11 @@ AT_CLEANUP > > OVN_FOR_EACH_NORTHD([ > AT_SETUP([ovn -- southbound database reconnection]) > + > +dnl This test doesn't take into account flows that are shared between > +dnl datapaths when datapath groups are enabled. > +AT_SKIP_IF([test NORTHD_USE_DP_GROUPS = yes]) > + > ovn_start --no-backup-northd > > # Check that ovn-northd is active, by verifying that it creates and > diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at > index cd6d7488a..634c205a8 100644 > --- a/tests/ovs-macros.at > +++ b/tests/ovs-macros.at > @@ -10,9 +10,11 @@ dnl set it as a shell variable as well. > dnl - Skip the test if it's for ovn-northd-ddlog but it didn't get built. > m4_rename([AT_SETUP], [OVS_AT_SETUP]) > m4_define([AT_SETUP], > - [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])) > + [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])) > m4_ifdef([NORTHD_TYPE], [[NORTHD_TYPE]=NORTHD_TYPE > ])dnl > +m4_ifdef([NORTHD_USE_DP_GROUPS], [[NORTHD_USE_DP_GROUPS]=NORTHD_USE_DP_GROUPS > +])dnl > m4_if(NORTHD_TYPE, [ovn-northd-ddlog], [dnl > AT_SKIP_IF([test $TEST_DDLOG = no]) > ])dnl >
On Tue, Apr 20, 2021 at 2:47 PM Mark Michelson <mmichels@redhat.com> wrote: > > Acked-by: Mark Michelson <mmichels@redhat.com> > Thanks Dumitru (and Mark). I applied the patch to the main branch. Numan > On 4/14/21 6:09 AM, Dumitru Ceara wrote: > > This doubles the size of the test matrix but should help protecting > > against regressions. > > > > This change is inspired from Ilya's work below but instead of selectively > > running tests with Datapath Groups if an external variable is set, it > > always runs both sets of tests expanding the OVN_FOR_EACH_NORTHD > > implementation: > > https://github.com/igsilya/ovn/commit/a5eef74cc374f04bb6a12545ceb504108e724135 > > > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > --- > > Note: > > - If accepted this should probably be pushed after the following > > patch is accepted: > > http://patchwork.ozlabs.org/project/ovn/patch/20210412124009.20468-1-dceara@redhat.com/ > > This patch is merged. > > > - It might turn out during review that Ilya's original approach is > > preferable. > > - A sample CI run for reference wrt. additional runtime can be found > > here (the failure is due to a known flaky test): > > https://github.com/dceara/ovn/actions/runs/747763765 > > --- > > tests/ovn-macros.at | 10 ++++++++++ > > tests/ovn-northd.at | 10 ++++++++++ > > tests/ovs-macros.at | 4 +++- > > 3 files changed, 23 insertions(+), 1 deletion(-) > > > > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > > index 25f3dbe34..3f432cef5 100644 > > --- a/tests/ovn-macros.at > > +++ b/tests/ovn-macros.at > > @@ -232,6 +232,10 @@ ovn_start () { > > --ic-nb-db=unix:"$ovs_base"/ovn-ic-nb/ovn-ic-nb.sock \ > > --ic-sb-db=unix:"$ovs_base"/ovn-ic-sb/ovn-ic-sb.sock > > fi > > + > > + if test X$NORTHD_USE_DP_GROUPS = Xyes; then > > + ovn-nbctl set NB_Global . options:use_logical_dp_groups=true > > + fi > > } > > > > # Interconnection networks. > > @@ -493,9 +497,15 @@ m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) > > > > m4_define([OVN_FOR_EACH_NORTHD], [dnl > > m4_pushdef([NORTHD_TYPE], [ovn-northd])dnl > > +m4_pushdef([NORTHD_USE_DP_GROUPS], [yes])dnl > > +$1 > > +m4_popdef([NORTHD_USE_DP_GROUPS])dnl > > $1 > > m4_popdef([NORTHD_TYPE])dnl > > m4_pushdef([NORTHD_TYPE], [ovn-northd-ddlog])dnl > > +m4_pushdef([NORTHD_USE_DP_GROUPS], [yes])dnl > > +$1 > > +m4_popdef([NORTHD_USE_DP_GROUPS])dnl > > $1 > > m4_popdef([NORTHD_TYPE])dnl > > ]) > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index eef360802..5548ae9fd 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -736,6 +736,11 @@ AT_CLEANUP > > > > OVN_FOR_EACH_NORTHD([ > > AT_SETUP([ovn -- northbound database reconnection]) > > + > > +dnl This test doesn't take into account flows that are shared between > > +dnl datapaths when datapath groups are enabled. > > +AT_SKIP_IF([test NORTHD_USE_DP_GROUPS = yes]) > > + > > ovn_start --no-backup-northd > > > > # Check that ovn-northd is active, by verifying that it creates and > > @@ -771,6 +776,11 @@ AT_CLEANUP > > > > OVN_FOR_EACH_NORTHD([ > > AT_SETUP([ovn -- southbound database reconnection]) > > + > > +dnl This test doesn't take into account flows that are shared between > > +dnl datapaths when datapath groups are enabled. > > +AT_SKIP_IF([test NORTHD_USE_DP_GROUPS = yes]) > > + > > ovn_start --no-backup-northd > > > > # Check that ovn-northd is active, by verifying that it creates and > > diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at > > index cd6d7488a..634c205a8 100644 > > --- a/tests/ovs-macros.at > > +++ b/tests/ovs-macros.at > > @@ -10,9 +10,11 @@ dnl set it as a shell variable as well. > > dnl - Skip the test if it's for ovn-northd-ddlog but it didn't get built. > > m4_rename([AT_SETUP], [OVS_AT_SETUP]) > > m4_define([AT_SETUP], > > - [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])) > > + [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])) > > m4_ifdef([NORTHD_TYPE], [[NORTHD_TYPE]=NORTHD_TYPE > > ])dnl > > +m4_ifdef([NORTHD_USE_DP_GROUPS], [[NORTHD_USE_DP_GROUPS]=NORTHD_USE_DP_GROUPS > > +])dnl > > m4_if(NORTHD_TYPE, [ovn-northd-ddlog], [dnl > > AT_SKIP_IF([test $TEST_DDLOG = no]) > > ])dnl > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at index 25f3dbe34..3f432cef5 100644 --- a/tests/ovn-macros.at +++ b/tests/ovn-macros.at @@ -232,6 +232,10 @@ ovn_start () { --ic-nb-db=unix:"$ovs_base"/ovn-ic-nb/ovn-ic-nb.sock \ --ic-sb-db=unix:"$ovs_base"/ovn-ic-sb/ovn-ic-sb.sock fi + + if test X$NORTHD_USE_DP_GROUPS = Xyes; then + ovn-nbctl set NB_Global . options:use_logical_dp_groups=true + fi } # Interconnection networks. @@ -493,9 +497,15 @@ m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) m4_define([OVN_FOR_EACH_NORTHD], [dnl m4_pushdef([NORTHD_TYPE], [ovn-northd])dnl +m4_pushdef([NORTHD_USE_DP_GROUPS], [yes])dnl +$1 +m4_popdef([NORTHD_USE_DP_GROUPS])dnl $1 m4_popdef([NORTHD_TYPE])dnl m4_pushdef([NORTHD_TYPE], [ovn-northd-ddlog])dnl +m4_pushdef([NORTHD_USE_DP_GROUPS], [yes])dnl +$1 +m4_popdef([NORTHD_USE_DP_GROUPS])dnl $1 m4_popdef([NORTHD_TYPE])dnl ]) diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index eef360802..5548ae9fd 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -736,6 +736,11 @@ AT_CLEANUP OVN_FOR_EACH_NORTHD([ AT_SETUP([ovn -- northbound database reconnection]) + +dnl This test doesn't take into account flows that are shared between +dnl datapaths when datapath groups are enabled. +AT_SKIP_IF([test NORTHD_USE_DP_GROUPS = yes]) + ovn_start --no-backup-northd # Check that ovn-northd is active, by verifying that it creates and @@ -771,6 +776,11 @@ AT_CLEANUP OVN_FOR_EACH_NORTHD([ AT_SETUP([ovn -- southbound database reconnection]) + +dnl This test doesn't take into account flows that are shared between +dnl datapaths when datapath groups are enabled. +AT_SKIP_IF([test NORTHD_USE_DP_GROUPS = yes]) + ovn_start --no-backup-northd # Check that ovn-northd is active, by verifying that it creates and diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at index cd6d7488a..634c205a8 100644 --- a/tests/ovs-macros.at +++ b/tests/ovs-macros.at @@ -10,9 +10,11 @@ dnl set it as a shell variable as well. dnl - Skip the test if it's for ovn-northd-ddlog but it didn't get built. m4_rename([AT_SETUP], [OVS_AT_SETUP]) m4_define([AT_SETUP], - [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])) + [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])) m4_ifdef([NORTHD_TYPE], [[NORTHD_TYPE]=NORTHD_TYPE ])dnl +m4_ifdef([NORTHD_USE_DP_GROUPS], [[NORTHD_USE_DP_GROUPS]=NORTHD_USE_DP_GROUPS +])dnl m4_if(NORTHD_TYPE, [ovn-northd-ddlog], [dnl AT_SKIP_IF([test $TEST_DDLOG = no]) ])dnl
This doubles the size of the test matrix but should help protecting against regressions. This change is inspired from Ilya's work below but instead of selectively running tests with Datapath Groups if an external variable is set, it always runs both sets of tests expanding the OVN_FOR_EACH_NORTHD implementation: https://github.com/igsilya/ovn/commit/a5eef74cc374f04bb6a12545ceb504108e724135 Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- Note: - If accepted this should probably be pushed after the following patch is accepted: http://patchwork.ozlabs.org/project/ovn/patch/20210412124009.20468-1-dceara@redhat.com/ - It might turn out during review that Ilya's original approach is preferable. - A sample CI run for reference wrt. additional runtime can be found here (the failure is due to a known flaky test): https://github.com/dceara/ovn/actions/runs/747763765 --- tests/ovn-macros.at | 10 ++++++++++ tests/ovn-northd.at | 10 ++++++++++ tests/ovs-macros.at | 4 +++- 3 files changed, 23 insertions(+), 1 deletion(-)