| Message ID | 20210715233451.2427987-1-hzhou@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovn-sbctl.at: Fix timing problem of count-flows test. | 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 | fail | github build: failed |
On Thu, Jul 15, 2021 at 04:34:51PM -0700, Han Zhou wrote: > Fixes: 895e02ec0be6("ovn-sbctl.c Add logical flows count numbers") > Signed-off-by: Han Zhou <hzhou@ovn.org> > --- > tests/ovn-sbctl.at | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at > index 16f5dabcc..dabf62d2b 100644 > --- a/tests/ovn-sbctl.at > +++ b/tests/ovn-sbctl.at > @@ -200,7 +200,7 @@ Total number of logical flows = 0 > ]) > > # create some logical flows > -check ovn-nbctl ls-add count-test > +check ovn-nbctl --wait=sb ls-add count-test > > OVS_WAIT_UNTIL([total_lflows=`count_entries`; test $total_lflows -ne 0]) I'm surprised that the above change is needed, since OVS_WAIT_UNTIL should wait until the condition is true. > @@ -219,7 +219,7 @@ $ingress_lflows > ]) > > # add another datapath > -check ovn-nbctl ls-add count-test2 > +check ovn-nbctl --wait=sb ls-add count-test2 > > # check total logical flows in 2 datapathes > AT_CHECK_UNQUOTED([ovn-sbctl count-flows | grep "flows =" | awk 'NF>1{print $NF}'], [0], [dnl The one above makes sense to me.
On Thu, Jul 15, 2021 at 4:57 PM Ben Pfaff <blp@ovn.org> wrote: > On Thu, Jul 15, 2021 at 04:34:51PM -0700, Han Zhou wrote: > > Fixes: 895e02ec0be6("ovn-sbctl.c Add logical flows count numbers") > > Signed-off-by: Han Zhou <hzhou@ovn.org> > > --- > > tests/ovn-sbctl.at | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at > > index 16f5dabcc..dabf62d2b 100644 > > --- a/tests/ovn-sbctl.at > > +++ b/tests/ovn-sbctl.at > > @@ -200,7 +200,7 @@ Total number of logical flows = 0 > > ]) > > > > # create some logical flows > > -check ovn-nbctl ls-add count-test > > +check ovn-nbctl --wait=sb ls-add count-test > > > > OVS_WAIT_UNTIL([total_lflows=`count_entries`; test $total_lflows -ne 0]) > > I'm surprised that the above change is needed, since OVS_WAIT_UNTIL > should wait until the condition is true. Yes you are right. This is not needed, but I think it’s not harmful either. Do you want me to send a v2 with this removed? The second place is where this test occasionally fails. > > > @@ -219,7 +219,7 @@ $ingress_lflows > > ]) > > > > # add another datapath > > -check ovn-nbctl ls-add count-test2 > > +check ovn-nbctl --wait=sb ls-add count-test2 > > > > # check total logical flows in 2 datapathes > > AT_CHECK_UNQUOTED([ovn-sbctl count-flows | grep "flows =" | awk > 'NF>1{print $NF}'], [0], [dnl > > The one above makes sense to me. > > >
On Thu, Jul 15, 2021 at 06:10:40PM -0700, Han Zhou wrote: > On Thu, Jul 15, 2021 at 4:57 PM Ben Pfaff <blp@ovn.org> wrote: > > > On Thu, Jul 15, 2021 at 04:34:51PM -0700, Han Zhou wrote: > > > Fixes: 895e02ec0be6("ovn-sbctl.c Add logical flows count numbers") > > > Signed-off-by: Han Zhou <hzhou@ovn.org> > > > --- > > > tests/ovn-sbctl.at | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at > > > index 16f5dabcc..dabf62d2b 100644 > > > --- a/tests/ovn-sbctl.at > > > +++ b/tests/ovn-sbctl.at > > > @@ -200,7 +200,7 @@ Total number of logical flows = 0 > > > ]) > > > > > > # create some logical flows > > > -check ovn-nbctl ls-add count-test > > > +check ovn-nbctl --wait=sb ls-add count-test > > > > > > OVS_WAIT_UNTIL([total_lflows=`count_entries`; test $total_lflows -ne 0]) > > > > I'm surprised that the above change is needed, since OVS_WAIT_UNTIL > > should wait until the condition is true. > > > Yes you are right. This is not needed, but I think it’s not harmful either. > Do you want me to send a v2 with this removed? I don't think you need to send a v2, but I'd prefer to not check in that part of the change. > The second place is where this test occasionally fails. That one makes sense! For that part: Acked-by: Ben Pfaff <blp@ovn.org>
On Mon, Jul 19, 2021 at 11:22 AM Ben Pfaff <blp@ovn.org> wrote: > > On Thu, Jul 15, 2021 at 06:10:40PM -0700, Han Zhou wrote: > > On Thu, Jul 15, 2021 at 4:57 PM Ben Pfaff <blp@ovn.org> wrote: > > > > > On Thu, Jul 15, 2021 at 04:34:51PM -0700, Han Zhou wrote: > > > > Fixes: 895e02ec0be6("ovn-sbctl.c Add logical flows count numbers") > > > > Signed-off-by: Han Zhou <hzhou@ovn.org> > > > > --- > > > > tests/ovn-sbctl.at | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at > > > > index 16f5dabcc..dabf62d2b 100644 > > > > --- a/tests/ovn-sbctl.at > > > > +++ b/tests/ovn-sbctl.at > > > > @@ -200,7 +200,7 @@ Total number of logical flows = 0 > > > > ]) > > > > > > > > # create some logical flows > > > > -check ovn-nbctl ls-add count-test > > > > +check ovn-nbctl --wait=sb ls-add count-test > > > > > > > > OVS_WAIT_UNTIL([total_lflows=`count_entries`; test $total_lflows -ne 0]) > > > > > > I'm surprised that the above change is needed, since OVS_WAIT_UNTIL > > > should wait until the condition is true. > > > > > > Yes you are right. This is not needed, but I think it’s not harmful either. > > Do you want me to send a v2 with this removed? > > I don't think you need to send a v2, but I'd prefer to not check in that > part of the change. > > > The second place is where this test occasionally fails. > > That one makes sense! For that part: > Acked-by: Ben Pfaff <blp@ovn.org> > Thanks Ben. I applied with the first change removed.
diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at index 16f5dabcc..dabf62d2b 100644 --- a/tests/ovn-sbctl.at +++ b/tests/ovn-sbctl.at @@ -200,7 +200,7 @@ Total number of logical flows = 0 ]) # create some logical flows -check ovn-nbctl ls-add count-test +check ovn-nbctl --wait=sb ls-add count-test OVS_WAIT_UNTIL([total_lflows=`count_entries`; test $total_lflows -ne 0]) @@ -219,7 +219,7 @@ $ingress_lflows ]) # add another datapath -check ovn-nbctl ls-add count-test2 +check ovn-nbctl --wait=sb ls-add count-test2 # check total logical flows in 2 datapathes AT_CHECK_UNQUOTED([ovn-sbctl count-flows | grep "flows =" | awk 'NF>1{print $NF}'], [0], [dnl
Fixes: 895e02ec0be6("ovn-sbctl.c Add logical flows count numbers") Signed-off-by: Han Zhou <hzhou@ovn.org> --- tests/ovn-sbctl.at | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)