Hi Ales Thanks for the patch!
On Thu, Nov 14, 2024 at 1:53 PM Ales Musil <[email protected]> wrote: > Ensure the chassis for remote port binding is not cleared when it's > set explicitly by CMS. The requested-chassis is still preferred > method to direct changes to SB DB, this option remains for > backward compatibility. > As a side note, ovn-ic also sets the chassis for remote pb. Hence, without this patch, we had ovn-ic fighting to set pb->chassis while northd fights to clear it (e.g. run "grep -c chassis ./az1/ovn-sb/ovn-sb.db" in tests such as "duplicate NB route adv/learn", and you will see chassis being set and cleared 100s of times w/o the patch. This patch fixes it! > > Fixes: 0688d7e834b9 ("northd: Allow multichassis port to be bound on remote > chassis.") > Signed-off-by: Ales Musil <[email protected]> > --- > northd/northd.c | 5 ++++- > tests/ovn.at | 34 ++++++++++++++++++++++++++++++++++ > 2 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 64b2e3859..aed1d425f 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -3209,8 +3209,11 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn > *ovnsb_txn, > "is-remote", false)) { > sbrec_port_binding_set_chassis(op->sb, > > op->sb->requested_chassis); > - } else { > + smap_add(&options, "is-remote-nb-bound", "true"); > + } else if (smap_get_bool(&op->sb->options, > + "is-remote-nb-bound", false)) { > sbrec_port_binding_set_chassis(op->sb, NULL); > + smap_add(&options, "is-remote-nb-bound", "false"); > } > } else if (op->sb->chassis && > smap_get_bool(&op->sb->chassis->other_config, > diff --git a/tests/ovn.at b/tests/ovn.at > index 92e1ffadc..865feab2c 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -39686,3 +39686,37 @@ OVN_CLEANUP([hv1]) > > AT_CLEANUP > ]) > + > +# ovn-kubernetes used to explicitly set chassis, > +# test if we don't accidentally remove it. > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([Remote port with explicit SB chassis]) > +ovn_start > + > +net_add n1 > + > +sim_add hv1 > +as hv1 > +check ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.11 > + > +check ovn-sbctl chassis-add hv2 geneve 192.168.0.12 \ > + -- set chassis hv2 other_config:is-remote=true > + > +check ovn-nbctl ls-add ls > +check ovn-nbctl lsp-add ls lsp > +check ovn-nbctl lsp-set-type lsp remote > + > It's missing --wait=sb or the following fetch_column Port_Binding might fail(e.g. if northd is slow) > +remote_chassis=$(fetch_column Chassis _uuid name=hv2) > +lsp_uuid=$(fetch_column Port_Binding _uuid logical_port=lsp) > + > +check ovn-sbctl set Port_Binding $lsp_uuid chassis=$remote_chassis > + > +check ovn-nbctl --wait=hv sync > + > +check_column $remote_chassis Port_Binding chassis logical_port=lsp > + > +OVN_CLEANUP([hv1]) > + > +AT_CLEANUP > +]) > -- > 2.47.0 > > Other than the missing --wait=sb, looks good to me, thanks. Acked-by: Xavier Simonart <[email protected]> Thanks Xavier > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
