| Message ID | 20220530082835.2830676-1-mheib@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] OVN-CI: remove ddlog test cases. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 5/30/22 10:28, Mohammad Heib wrote: > currently there is no new changes applied to the ddlog code base > in ovn and we keep skipping ddlog test cases in our ci runs which leads > to so many skips lines printed to the ci logs and that cause us to miss > some tests cases that were skipped because of missing packages or any other reason. > > This patch will remove ddlog test cases from the OVN-CI, > which will give us more clear ci output and less execution time. > > Signed-off-by: Mohammad Heib <mheib@redhat.com> > --- Hi Mohammad, Thanks for the patch! I wonder if we should rebase https://patchwork.ozlabs.org/project/ovn/patch/20211007194649.4014965-1-blp@ovn.org/ instead. In any case, for your patch specifically, I think we can also update the comment for test "ovn -- NAT and Load Balancer flows" (and wrap the test in OVN_FOR_EACH_NORTHD()). A few other tests mention ddlog in the comments but will now never be run with ovn-northd-ddlog. We might as well update those comments too. > tests/ovn-macros.at | 11 +++-------- > tests/ovn.at | 3 --- > tests/ovs-macros.at | 4 ---- > 3 files changed, 3 insertions(+), 15 deletions(-) > > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > index c6f0f6251..df3f35554 100644 > --- a/tests/ovn-macros.at > +++ b/tests/ovn-macros.at > @@ -748,22 +748,17 @@ m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) > # Defines a versions of a test with all combinations of northd and > # datapath groups. > m4_define([OVN_FOR_EACH_NORTHD], > - [m4_foreach([NORTHD_TYPE], [ovn-northd, ovn-northd-ddlog], > + [m4_foreach([NORTHD_TYPE], [ovn-northd], > [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], > [m4_foreach([NORTHD_USE_PARALLELIZATION], [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, ovn-northd-ddlog], > + [m4_foreach([NORTHD_TYPE], [ovn-northd], > [m4_foreach([NORTHD_USE_DP_GROUPS], [no], > [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1 > ])])])]) > > # Some tests aren't prepared for ddlog to be enabled. > -m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DDLOG], > - [m4_foreach([NORTHD_TYPE], [ovn-northd], > - [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], > - [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1 > -])])])]) > - > +m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DDLOG], [OVN_FOR_EACH_NORTHD($@)]) As far as I can tell there are only two places where this macro is used. I think we can just remove it and update the two callers. Thanks, Dumitru > diff --git a/tests/ovn.at b/tests/ovn.at > index 4461b840e..1dcd82762 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -22993,9 +22993,6 @@ AT_CLEANUP > OVN_FOR_EACH_NORTHD([ > AT_SETUP([interconnection]) > > -dnl This test has problems with ovn-northd-ddlog. > -AT_SKIP_IF([test NORTHD_TYPE = ovn-northd-ddlog && test "$RUN_ANYWAY" != yes]) > - > ovn_init_ic_db > n_az=5 > n_ts=5 > diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at > index 0482b7f5b..dc3f34dc9 100644 > --- a/tests/ovs-macros.at > +++ b/tests/ovs-macros.at > @@ -7,7 +7,6 @@ dnl Make AT_SETUP automatically do some things for us: > dnl - Run the ovs_init() shell function as the first step in every test. > dnl - If NORTHD_TYPE is defined, then append it to the test name and > 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])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])[]m4_ifdef([NORTHD_USE_PARALLELIZATION], [ -- parallelization=NORTHD_USE_PARALLELIZATION])) > @@ -19,9 +18,6 @@ m4_ifdef([NORTHD_USE_PARALLELIZATION], [[NORTHD_USE_PARALLELIZATION]=NORTHD_USE_ > ])dnl > m4_ifdef([NORTHD_DUMMY_NUMA], [[NORTHD_DUMMY_NUMA]=NORTHD_DUMMY_NUMA > ])dnl > -m4_if(NORTHD_TYPE, [ovn-northd-ddlog], [dnl > -AT_SKIP_IF([test $TEST_DDLOG = no]) > -])dnl > ovs_init > ]) >
Hi Dumitru, thank you for reviewing this patch. On 5/30/22 18:50, Dumitru Ceara wrote: > On 5/30/22 10:28, Mohammad Heib wrote: >> currently there is no new changes applied to the ddlog code base >> in ovn and we keep skipping ddlog test cases in our ci runs which leads >> to so many skips lines printed to the ci logs and that cause us to miss >> some tests cases that were skipped because of missing packages or any other reason. >> >> This patch will remove ddlog test cases from the OVN-CI, >> which will give us more clear ci output and less execution time. >> >> Signed-off-by: Mohammad Heib<mheib@redhat.com> >> --- > Hi Mohammad, > > Thanks for the patch! > > I wonder if we should rebase > https://patchwork.ozlabs.org/project/ovn/patch/20211007194649.4014965-1-blp@ovn.org/ > instead. > > In any case, for your patch specifically, I think we can also update the > comment for test "ovn -- NAT and Load Balancer flows" (and wrap the test > in OVN_FOR_EACH_NORTHD()). i updated the code and removed the ddlog related code from almost all the unit tests, still have two places that mentioning ddlog i will remove in a future patch. v2. <https://patchwork.ozlabs.org/project/ovn/patch/20220609122755.961288-1-mheib@redhat.com/> thanks > > A few other tests mention ddlog in the comments but will now never be > run with ovn-northd-ddlog. We might as well update those comments too. > >> tests/ovn-macros.at | 11 +++-------- >> tests/ovn.at | 3 --- >> tests/ovs-macros.at | 4 ---- >> 3 files changed, 3 insertions(+), 15 deletions(-) >> >> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at >> index c6f0f6251..df3f35554 100644 >> --- a/tests/ovn-macros.at >> +++ b/tests/ovn-macros.at >> @@ -748,22 +748,17 @@ m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) >> # Defines a versions of a test with all combinations of northd and >> # datapath groups. >> m4_define([OVN_FOR_EACH_NORTHD], >> - [m4_foreach([NORTHD_TYPE], [ovn-northd, ovn-northd-ddlog], >> + [m4_foreach([NORTHD_TYPE], [ovn-northd], >> [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], >> [m4_foreach([NORTHD_USE_PARALLELIZATION], [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, ovn-northd-ddlog], >> + [m4_foreach([NORTHD_TYPE], [ovn-northd], >> [m4_foreach([NORTHD_USE_DP_GROUPS], [no], >> [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1 >> ])])])]) >> >> # Some tests aren't prepared for ddlog to be enabled. >> -m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DDLOG], >> - [m4_foreach([NORTHD_TYPE], [ovn-northd], >> - [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], >> - [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1 >> -])])])]) >> - >> +m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DDLOG], [OVN_FOR_EACH_NORTHD($@)]) > As far as I can tell there are only two places where this macro is used. > I think we can just remove it and update the two callers. > > Thanks, > Dumitru > >> diff --git a/tests/ovn.at b/tests/ovn.at >> index 4461b840e..1dcd82762 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -22993,9 +22993,6 @@ AT_CLEANUP >> OVN_FOR_EACH_NORTHD([ >> AT_SETUP([interconnection]) >> >> -dnl This test has problems with ovn-northd-ddlog. >> -AT_SKIP_IF([test NORTHD_TYPE = ovn-northd-ddlog && test "$RUN_ANYWAY" != yes]) >> - >> ovn_init_ic_db >> n_az=5 >> n_ts=5 >> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at >> index 0482b7f5b..dc3f34dc9 100644 >> --- a/tests/ovs-macros.at >> +++ b/tests/ovs-macros.at >> @@ -7,7 +7,6 @@ dnl Make AT_SETUP automatically do some things for us: >> dnl - Run the ovs_init() shell function as the first step in every test. >> dnl - If NORTHD_TYPE is defined, then append it to the test name and >> 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])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])[]m4_ifdef([NORTHD_USE_PARALLELIZATION], [ -- parallelization=NORTHD_USE_PARALLELIZATION])) >> @@ -19,9 +18,6 @@ m4_ifdef([NORTHD_USE_PARALLELIZATION], [[NORTHD_USE_PARALLELIZATION]=NORTHD_USE_ >> ])dnl >> m4_ifdef([NORTHD_DUMMY_NUMA], [[NORTHD_DUMMY_NUMA]=NORTHD_DUMMY_NUMA >> ])dnl >> -m4_if(NORTHD_TYPE, [ovn-northd-ddlog], [dnl >> -AT_SKIP_IF([test $TEST_DDLOG = no]) >> -])dnl >> ovs_init >> ]) >>
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at index c6f0f6251..df3f35554 100644 --- a/tests/ovn-macros.at +++ b/tests/ovn-macros.at @@ -748,22 +748,17 @@ m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) # Defines a versions of a test with all combinations of northd and # datapath groups. m4_define([OVN_FOR_EACH_NORTHD], - [m4_foreach([NORTHD_TYPE], [ovn-northd, ovn-northd-ddlog], + [m4_foreach([NORTHD_TYPE], [ovn-northd], [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], [m4_foreach([NORTHD_USE_PARALLELIZATION], [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, ovn-northd-ddlog], + [m4_foreach([NORTHD_TYPE], [ovn-northd], [m4_foreach([NORTHD_USE_DP_GROUPS], [no], [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1 ])])])]) # Some tests aren't prepared for ddlog to be enabled. -m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DDLOG], - [m4_foreach([NORTHD_TYPE], [ovn-northd], - [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], - [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1 -])])])]) - +m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DDLOG], [OVN_FOR_EACH_NORTHD($@)]) diff --git a/tests/ovn.at b/tests/ovn.at index 4461b840e..1dcd82762 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -22993,9 +22993,6 @@ AT_CLEANUP OVN_FOR_EACH_NORTHD([ AT_SETUP([interconnection]) -dnl This test has problems with ovn-northd-ddlog. -AT_SKIP_IF([test NORTHD_TYPE = ovn-northd-ddlog && test "$RUN_ANYWAY" != yes]) - ovn_init_ic_db n_az=5 n_ts=5 diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at index 0482b7f5b..dc3f34dc9 100644 --- a/tests/ovs-macros.at +++ b/tests/ovs-macros.at @@ -7,7 +7,6 @@ dnl Make AT_SETUP automatically do some things for us: dnl - Run the ovs_init() shell function as the first step in every test. dnl - If NORTHD_TYPE is defined, then append it to the test name and 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])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])[]m4_ifdef([NORTHD_USE_PARALLELIZATION], [ -- parallelization=NORTHD_USE_PARALLELIZATION])) @@ -19,9 +18,6 @@ m4_ifdef([NORTHD_USE_PARALLELIZATION], [[NORTHD_USE_PARALLELIZATION]=NORTHD_USE_ ])dnl m4_ifdef([NORTHD_DUMMY_NUMA], [[NORTHD_DUMMY_NUMA]=NORTHD_DUMMY_NUMA ])dnl -m4_if(NORTHD_TYPE, [ovn-northd-ddlog], [dnl -AT_SKIP_IF([test $TEST_DDLOG = no]) -])dnl ovs_init ])
currently there is no new changes applied to the ddlog code base in ovn and we keep skipping ddlog test cases in our ci runs which leads to so many skips lines printed to the ci logs and that cause us to miss some tests cases that were skipped because of missing packages or any other reason. This patch will remove ddlog test cases from the OVN-CI, which will give us more clear ci output and less execution time. Signed-off-by: Mohammad Heib <mheib@redhat.com> --- tests/ovn-macros.at | 11 +++-------- tests/ovn.at | 3 --- tests/ovs-macros.at | 4 ---- 3 files changed, 3 insertions(+), 15 deletions(-)