On Thu, Jun 19, 2025 at 12:20 PM Dumitru Ceara <dce...@redhat.com> wrote:
> On 6/18/25 2:29 PM, Ales Musil via dev wrote:
> > Add missing sync calls which wuld lead to flaky tests.
> >
> > Signed-off-by: Ales Musil <amu...@redhat.com>
> > ---
>
> Hi Ales,
>
> Thanks for the patch!
>
Hi Dumitru,
thank you for the review.
>
> > tests/ovn.at | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index b561c69aa..57260affb 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -15943,6 +15943,7 @@ hv1_uuid=$(fetch_column Chassis _uuid name=hv1)
> > hv2_uuid=$(fetch_column Chassis _uuid name=hv2)
> >
> > OVN_POPULATE_ARP
> > +check ovn-nbctl --wait=hv sync
> >
>
> Shouldn't this one be a bit lower, after "wait_for_ports_up"? Just
> after the current line we configure options on the "migrator" LSP.
> Alternatively, we could add --wait=hv to that lsp-set-options
> command.
>
Yes you are right, I've moved it below "wait_for_ports_up".
>
> > # Start with Migrator on hv1 but not hv2
> > check ovn-nbctl lsp-set-options migrator requested-chassis=hv1
> > @@ -16029,7 +16030,7 @@ check_packets
> > reset_env
> >
> > # Start port migration hv1 -> hv2: both hypervisors are now bound
> > -check ovn-nbctl lsp-set-options migrator requested-chassis=hv1,hv2
> > +check ovn-nbctl --wait=hv lsp-set-options migrator
> requested-chassis=hv1,hv2
> > wait_for_ports_up
> > OVN_WAIT_PATCH_PORT_FLOWS(["public"], ["hv1"])
> > OVN_WAIT_PATCH_PORT_FLOWS(["public"], ["hv2"])
> > @@ -16127,7 +16128,7 @@ p2=$(as main ovs-ofctl show n1 | grep
> hv2_br-phys | awk '{print int($1)}')
> > OVS_WAIT_UNTIL([test x`as main ovs-appctl fdb/show n1 | grep
> 00:00:00:00:00:ff | awk '{print $1}'` = x$p1])
> >
> > # Complete migration: destination is bound
> > -check ovn-nbctl lsp-set-options migrator requested-chassis=hv2
> > +check ovn-nbctl --wait=hv lsp-set-options migrator requested-chassis=hv2
>
> I'm confused about this one. The whole sequence of commands here is:
>
> check ovn-nbctl --wait=hv lsp-set-options migrator requested-chassis=hv2
> wait_column "$hv2_uuid" Port_Binding chassis logical_port=migrator
> wait_column "$hv2_uuid" Port_Binding requested_chassis
> logical_port=migrator
> wait_column "" Port_Binding additional_chassis logical_port=migrator
> wait_column "" Port_Binding requested_additional_chassis
> logical_port=migrator
> wait_for_ports_up
>
> check ovn-nbctl --wait=hv sync
>
> So, why do we need the first --wait=hv?
>
We don't, I have removed it in v2.
>
> > wait_column "$hv2_uuid" Port_Binding chassis logical_port=migrator
> > wait_column "$hv2_uuid" Port_Binding requested_chassis
> logical_port=migrator
> > wait_column "" Port_Binding additional_chassis logical_port=migrator
> > @@ -31102,6 +31103,7 @@ OVS_WAIT_UNTIL([
> > cr_lr0_public_ch=$(ovn-sbctl --bare --columns chassis list
> port_binding cr-lr0-public)
> > test "$cr_lr0_public_ch" = $hv3_uuid
> > ])
> > +check ovn-nbctl --wait=hv sync
> >
> > test_arp_response 000020201213 $(ip_to_hex 172 16 0 100) hv3 hv1 hv2
> > test_arp_response 000020201213 $(ip_to_hex 172 16 0 101) hv3 hv1 hv2
> > @@ -31115,6 +31117,7 @@ OVS_WAIT_UNTIL([
> > gw_router_ch=$(ovn-sbctl --bare --columns chassis list port_binding
> gw_router-public)
> > test "$gw_router_ch" = $hv1_uuid
> > ])
> > +check ovn-nbctl --wait=hv sync
> >
> > # Send ARP request for the IP which belongs to gw_router
> > test_arp_response 000030303233 $(ip_to_hex 172 16 0 200) hv1 hv2 hv3
>
> Regards,
> Dumitru
>
>
>
Thanks,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
