On 4/21/26 12:14 PM, Mairtin O'Loingsigh via dev wrote: > Tests for ovn-ic-nbctl commands and simple transit switch > configurations. > > Reported-at: https://issues.redhat.com/browse/FDP-2878 > Signed-off-by: Mairtin O'Loingsigh <[email protected]> > ---
Hi Mairtin, Thanks for the patch! I think I'd actually squash all 3 patches in this series into a single one in the next revision. Tests should be in the same patch where we add functionality (in case we need to revert stuff in the future). And the schema change is minimal so it could also go in there IMO. > tests/multinode.at | 1 - > tests/ovn-ic-nbctl.at | 26 ++++++++++++++++++++++ > tests/ovn-ic.at | 50 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 76 insertions(+), 1 deletion(-) > > diff --git a/tests/multinode.at b/tests/multinode.at > index d07660797..5f72bc237 100644 > --- a/tests/multinode.at > +++ b/tests/multinode.at > @@ -2453,7 +2453,6 @@ for i in 1 2; do > check m_as $chassis ovn-nbctl lsp-add-router-port ts ts-tr tr-ts > > check m_as $chassis ovn-nbctl lrp-add tr tr-ts 00:00:00:00:10:00 > 10.100.200.1/24 10:200::1/64 > - check m_as $chassis ovn-nbctl set logical_router tr > options:requested-tnl-key=20 This is removed because we already had ovn-ic setting it, right? OTOH, does it belong in this series? > check m_as $chassis ovn-nbctl lrp-set-gateway-chassis tr-ts $chassis > > # Add TS pods, with the same tunnel keys on both sides > diff --git a/tests/ovn-ic-nbctl.at b/tests/ovn-ic-nbctl.at > index 4c5269784..8309a009c 100644 > --- a/tests/ovn-ic-nbctl.at > +++ b/tests/ovn-ic-nbctl.at > @@ -61,6 +61,32 @@ AT_CHECK([ovn-ic-nbctl ts-del ts2], [1], [], > > AT_CHECK([ovn-ic-nbctl --if-exists ts-del ts2]) > > +AT_CHECK([ovn-ic-nbctl --may-exist ts-add ts0]) > +AT_CHECK([ovn-ic-nbctl ts-list | uuidfilt], [0], [dnl > +<0> (ts0) > +]) > + > +AT_CHECK([ovn-ic-nbctl tsp-add], [1], [], > + [ovn-ic-nbctl: 'tsp-add' command requires at least 2 arguments > +]) > + > +AT_CHECK([ovn-ic-nbctl tsp-add ts0], [1], [], > + [ovn-ic-nbctl: 'tsp-add' command requires at least 2 arguments > +]) > + > +AT_CHECK([ovn-ic-nbctl tsp-add ts0 ts0-p0 chassis=chassis]) > + > +AT_CHECK([ovn-ic-nbctl --may-exist tsp-add ts0 ts0-p0 chassis=chassis]) > +AT_CHECK([ovn-ic-nbctl tsp-set-addr ts0-p0 "00:11:22:11:22:33 > 192.168.10.10/24"]) > +#sleep 6000 Nit: probably a leftover. > +AT_CHECK([ovn-ic-nbctl tsp-set-addr ts0-p1 "00:11:22:11:22:34 > 192.168.10.11/24"], [1], [], > + [ovn-ic-nbctl: ts0-p1: switch port name not found > +]) > + > +AT_CHECK([ovn-ic-nbctl tsp-del], [1], [], > + [ovn-ic-nbctl: 'tsp-del' command requires at least 1 arguments > +]) > + > OVN_IC_NBCTL_TEST_STOP > AT_CLEANUP > > diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at > index 0fa7c4f29..58dfa3f06 100644 > --- a/tests/ovn-ic.at > +++ b/tests/ovn-ic.at > @@ -164,6 +164,56 @@ AT_CHECK([ovn-ic-sbctl show | grep -A2 lsp1], [0], [dnl > address: [["00:00:00:00:00:01 10.0.0.1/24"]] > ]) > > +# remove transit switch and check if port_binding is deleted > +check ovn-ic-nbctl --wait=sb ts-del ts1 > +check_row_count ic-sb:Port_Binding 0 logical_port=lsp1 > +for i in 1 2; do > + az=az$i > + ovn_as $az > + OVN_CLEANUP_SBOX(gw-$az) > + OVN_CLEANUP_AZ([$az]) > +done > +OVN_CLEANUP_IC > +AT_CLEANUP > +]) > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn-ic -- transit port-bindings deletion upon TS deletion]) It feels a bit incomplete to not test everything that's supposed to be added in this series, from the "Reported-at" issue description: https://redhat.atlassian.net/browse/FDP-2878 "Add ovn-ic-nbctl helpers for adding transit switch ports, this should allow addition for type "router" and "" (VIFs). The router port handling is already in ovn-ic, but it's based on picking up from NB database instead of getting the definition from IC NB. The vif support isn't there at all. Mainly the VIF support has a big advantage of synchronizing the tunnel_keys for given port so it doesn't have to be done manually. There also needs to be a change to the current logic for "router" TSP, if the port represents connection between TS and TR that ports needs to be still synchronized, just local everywhere instead of local in one AZ and remote in the rest." e.g., I don't see any tsp-add calls in the tests for non-router ports. > + > +ovn_init_ic_db > +net_add n1 > + > +# 1 GW per AZ > +for i in 1 2; do > + az=az$i > + ovn_start $az > + sim_add gw-$az > + as gw-$az > + check ovs-vsctl add-br br-phys > + ovn_az_attach $az n1 br-phys 192.168.1.$i > + check ovs-vsctl set open . external-ids:ovn-is-interconn=true > +done > + > +ovn_as az1 > + > +# create transit switch and connect to LR > +check ovn-ic-nbctl --wait=sb ts-add ts1 > +check ovn-nbctl lr-add lr1 > +check ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:01 10.0.0.1/24 > +check ovn-nbctl lrp-set-gateway-chassis lrp1 gw-az1 > + > +check ovn-ic-nbctl tsp-add ts1 lsp1 type=router chassis=gw-az1 option=lrp1 > + > +wait_row_count Datapath_Binding 1 external_ids:interconn-ts=ts1 > + > +# Sync ic-sb DB to see the TS changes. > +check ovn-ic-nbctl --wait=sb sync > + > +AT_CHECK([ovn-ic-sbctl show | grep -A2 lsp1], [0], [dnl > + port lsp1 > + transit switch: ts1 > + address: [["00:00:00:00:00:01 10.0.0.1/24"]] > +]) > + > # remove transit switch and check if port_binding is deleted > check ovn-ic-nbctl --wait=sb ts-del ts1 > check_row_count ic-sb:Port_Binding 0 logical_port=lsp1 Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
