| Message ID | 20240718194037.3196243-1-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovn-northd.at: Fix flaky VXLAN mode disabling 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 | success | github build: passed |
On Thu, Jul 18, 2024 at 9:40 PM Ilya Maximets <i.maximets@ovn.org> wrote: > This tests constructs a very large operation that frequently takes more > than a second on busy CI systems. As a result ovn-nbctl emits a warning > for a long poll interval and fails the test. > > Fix that by splitting a single large command into a series of smaller > ones - 100 switches at a time. This makes the test a bit slower, but > much more reliable. > > Fixes: 7e99500e60bf ("northd: Add support for disabling vxlan mode.") > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > Hi Ilya, thank you for addressing this, however there is one patch already pending addressing the same issue https://patchwork.ozlabs.org/project/ovn/patch/20240715091707.83910-1-amusil@redhat.com/ . > tests/ovn-northd.at | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index a389d1988..d7551f69a 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -2882,12 +2882,16 @@ ovn-sbctl \ > --id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="vxlan" \ > -- --id=@c create chassis name=hv1 encaps=@e > > -cmd="ovn-nbctl --wait=sb" > +cmd="" > for i in {1..4097..1}; do > cmd="${cmd} -- ls-add lsw-${i}" > + if test $(($i % 100)) -eq 0; then > + check ovn-nbctl $cmd > + cmd="" > + fi > done > > -check $cmd > +check ovn-nbctl --wait=sb $cmd > > check_row_count nb:Logical_Switch 4097 > wait_row_count sb:Datapath_Binding 4095 > -- > 2.45.2 > > Thanks, Ales
On 7/19/24 07:04, Ales Musil wrote: > > > On Thu, Jul 18, 2024 at 9:40 PM Ilya Maximets <i.maximets@ovn.org <mailto:i.maximets@ovn.org>> wrote: > > This tests constructs a very large operation that frequently takes more > than a second on busy CI systems. As a result ovn-nbctl emits a warning > for a long poll interval and fails the test. > > Fix that by splitting a single large command into a series of smaller > ones - 100 switches at a time. This makes the test a bit slower, but > much more reliable. > > Fixes: 7e99500e60bf ("northd: Add support for disabling vxlan mode.") > Signed-off-by: Ilya Maximets <i.maximets@ovn.org <mailto:i.maximets@ovn.org>> > --- > > > Hi Ilya, > > thank you for addressing this, however there is one patch already pending > addressing the same issue https://patchwork.ozlabs.org/project/ovn/patch/20240715091707.83910-1-amusil@redhat.com/ . Ah, thanks for the pointer! I did look through patchwork before sending, but somehow missed this one. OTOH, I'm not a big fan of hiding the executed commands from the testsuite log, which will happen if we replace check with eval. Also, constructing command lines that long is generally not a good idea since the maximum command line length is system-dependent and can be as low as 4096 characters. Best regards, Ilya Maximets. > > > tests/ovn-northd.at <http://ovn-northd.at> | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/tests/ovn-northd.at <http://ovn-northd.at> b/tests/ovn-northd.at <http://ovn-northd.at> > index a389d1988..d7551f69a 100644 > --- a/tests/ovn-northd.at <http://ovn-northd.at> > +++ b/tests/ovn-northd.at <http://ovn-northd.at> > @@ -2882,12 +2882,16 @@ ovn-sbctl \ > --id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="vxlan" \ > -- --id=@c create chassis name=hv1 encaps=@e > > -cmd="ovn-nbctl --wait=sb" > +cmd="" > for i in {1..4097..1}; do > cmd="${cmd} -- ls-add lsw-${i}" > + if test $(($i % 100)) -eq 0; then > + check ovn-nbctl $cmd > + cmd="" > + fi > done > > -check $cmd > +check ovn-nbctl --wait=sb $cmd > > check_row_count nb:Logical_Switch 4097 > wait_row_count sb:Datapath_Binding 4095 > -- > 2.45.2 > > > Thanks, > Ales
On Fri, Jul 19, 2024 at 6:01 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 7/19/24 07:04, Ales Musil wrote: > > > > > > On Thu, Jul 18, 2024 at 9:40 PM Ilya Maximets <i.maximets@ovn.org <mailto:i.maximets@ovn.org>> wrote: > > > > This tests constructs a very large operation that frequently takes more > > than a second on busy CI systems. As a result ovn-nbctl emits a warning > > for a long poll interval and fails the test. > > > > Fix that by splitting a single large command into a series of smaller > > ones - 100 switches at a time. This makes the test a bit slower, but > > much more reliable. > > > > Fixes: 7e99500e60bf ("northd: Add support for disabling vxlan mode.") > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org <mailto:i.maximets@ovn.org>> > > --- > > > > > > Hi Ilya, > > > > thank you for addressing this, however there is one patch already pending > > addressing the same issue https://patchwork.ozlabs.org/project/ovn/patch/20240715091707.83910-1-amusil@redhat.com/ . > > Ah, thanks for the pointer! I did look through patchwork before > sending, but somehow missed this one. > > OTOH, I'm not a big fan of hiding the executed commands from the > testsuite log, which will happen if we replace check with eval. > Also, constructing command lines that long is generally not a good > idea since the maximum command line length is system-dependent > and can be as low as 4096 characters. > > Best regards, Ilya Maximets. Thanks Ilya and Ales for the patches to address this issue. @Ales - I went ahead and applied this patch for the reasons stated by Ilya. Thanks Numan > > > > > > > tests/ovn-northd.at <http://ovn-northd.at> | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/tests/ovn-northd.at <http://ovn-northd.at> b/tests/ovn-northd.at <http://ovn-northd.at> > > index a389d1988..d7551f69a 100644 > > --- a/tests/ovn-northd.at <http://ovn-northd.at> > > +++ b/tests/ovn-northd.at <http://ovn-northd.at> > > @@ -2882,12 +2882,16 @@ ovn-sbctl \ > > --id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="vxlan" \ > > -- --id=@c create chassis name=hv1 encaps=@e > > > > -cmd="ovn-nbctl --wait=sb" > > +cmd="" > > for i in {1..4097..1}; do > > cmd="${cmd} -- ls-add lsw-${i}" > > + if test $(($i % 100)) -eq 0; then > > + check ovn-nbctl $cmd > > + cmd="" > > + fi > > done > > > > -check $cmd > > +check ovn-nbctl --wait=sb $cmd > > > > check_row_count nb:Logical_Switch 4097 > > wait_row_count sb:Datapath_Binding 4095 > > -- > > 2.45.2 > > > > > > Thanks, > > Ales > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index a389d1988..d7551f69a 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -2882,12 +2882,16 @@ ovn-sbctl \ --id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="vxlan" \ -- --id=@c create chassis name=hv1 encaps=@e -cmd="ovn-nbctl --wait=sb" +cmd="" for i in {1..4097..1}; do cmd="${cmd} -- ls-add lsw-${i}" + if test $(($i % 100)) -eq 0; then + check ovn-nbctl $cmd + cmd="" + fi done -check $cmd +check ovn-nbctl --wait=sb $cmd check_row_count nb:Logical_Switch 4097 wait_row_count sb:Datapath_Binding 4095
This tests constructs a very large operation that frequently takes more than a second on busy CI systems. As a result ovn-nbctl emits a warning for a long poll interval and fails the test. Fix that by splitting a single large command into a series of smaller ones - 100 switches at a time. This makes the test a bit slower, but much more reliable. Fixes: 7e99500e60bf ("northd: Add support for disabling vxlan mode.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- tests/ovn-northd.at | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)